Prevent dangerous re-cloning inside loop

Author: muglug

src/analyzer/expr/binop/assignment_analyzer.rs

@@ -182,6 +182,41 @@ pub(crate) fn analyze(
     if let (Some(var_id), Some(existing_var_type), Bop::Eq(None)) =
         (&var_id, &existing_var_type, binop)
     {
+        if context.inside_loop && !context.inside_assignment_op {
+            if let Some(assign_value) = assign_value {
+                if let aast::Expr_::Clone(cloned_expr) = &assign_value.2 {
+                    if let aast::Expr_::Lvar(cloned_var) = &cloned_expr.2 {
+                        if cloned_var.name() == var_id {
+                            let mut origin_node_ids = vec![];
+
+                            for parent_node in &existing_var_type.parent_nodes {
+                                origin_node_ids.extend(
+                                    analysis_data.data_flow_graph.get_origin_node_ids(
+                                        &parent_node.id,
+                                        &[],
+                                        false,
+                                    ),
+                                );
+                            }
+
+                            if origin_node_ids.len() > 1 {
+                                analysis_data.maybe_add_issue(
+                                    Issue::new(
+                                        IssueKind::CloneInsideLoop,
+                                        format!("Overwriting an object {} outside the loop with a clone likely not intended", var_id),
+                                        statements_analyzer.get_hpos(pos),
+                                        &context.function_context.calling_functionlike_id,
+                                    ),
+                                    statements_analyzer.get_config(),
+                                    statements_analyzer.get_file_path_actual(),
+                                )
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
         if context.inside_loop
             && !context.inside_assignment_op
             && context.for_loop_init_bounds.0 > 0
@@ -197,6 +232,14 @@ pub(crate) fn analyze(
                 ));
             }
 
+            if let Some(assign_value) = assign_value {
+                if let aast::Expr_::Clone(cloned_expr) = &assign_value.2 {
+                    if let aast::Expr_::Lvar(cloned_var) = &cloned_expr.2 {
+                        if cloned_var.name() == var_id {}
+                    }
+                }
+            }
+
             origin_node_ids.retain(|id| {
                 if let Some(node) = analysis_data.data_flow_graph.get_node(id) {
                     match (&id, &node.kind) {

src/code_info/issue.rs

@@ -18,6 +18,7 @@ pub enum IssueKind {
     BannedFunction,
     ExtendFinalClass,
     CannotInferGenericParam,
+    CloneInsideLoop,
     CustomIssue(Box<String>),
     DuplicateEnumValue,
     EmptyBlock,

tests/inference/Loop/Foreach/cloneAssignmentInLoop/input.hack

@@ -0,0 +1,11 @@
+final class A {
+    public function __construct(public int $num) {}
+}
+
+function foo(A $a, vec<int> $nums): void {
+    foreach ($nums as $num) {
+    	$a = clone $a;
+        $a->num += $num;
+        echo $a->num;
+    }
+}

tests/inference/Loop/Foreach/cloneAssignmentInLoop/output.txt

@@ -0,0 +1 @@
+ERROR: CloneInsideLoop - input.hack:7:6 - Overwriting an object $a outside the loop with a clone likely not intended