Add linter rule for missing in intents (chapel-lang#26115)

Adds a `chplcheck` rule to inform users that formals that initialize a
field should have an in intent. Doing this can reduce the number of
copies made and improve performance.

This PR takes the following approach to implement this
1. Identify all field in an aggregate type
2. For each initializer, identify all formals that have the default
intent
3. If a formal is assigned to a field before `init this`, warn

Note that this only handles the cases where the left hand side of `=`
can be scope resolved, so something like `this.field = myFormal` will
not trigger the warning yet

Future work: As the resolver comes online and is threaded into
`chplcheck`, this lint rule should make use of the logic in
`InitResolver` to distinguish between initialization and assignment.

[Reviewed by @DanilaFe]

test/chplcheck/MissingInIntent.chpl

@@ -0,0 +1,38 @@
+module MissingInIntent {
+
+  record myData {
+    var x: int;
+    proc init() {
+      x = 0;
+    }
+    proc init(in d: int) {
+      x = d;
+    }
+  }
+
+
+  record R {
+    var x: int;
+    var y: myData;
+    proc init(newX: int) {
+      x = newX;
+      y = new myData();
+    }
+    proc init(newY: myData) {
+      x = 0;
+      y = newY;
+    }
+    @chplcheck.ignore("UnusedFormal")
+    proc init(newY: myData, dummy: int) {
+      x = newY.x;
+      y = new myData();
+    }
+    @chplcheck.ignore("UnusedFormal")
+    proc init(newY: myData, dummy: int, dummy2: int) {
+      init this;
+      x = 0;
+      y = newY;
+    }
+  }
+
+}

test/chplcheck/MissingInIntent.good

@@ -0,0 +1,3 @@
+MissingInIntent.chpl:17: node violates rule MissingInIntent
+MissingInIntent.chpl:21: node violates rule MissingInIntent
+[Success matching fixit for MissingInIntent]

test/chplcheck/MissingInIntent.good-fixit

@@ -0,0 +1,38 @@
+module MissingInIntent {
+
+  record myData {
+    var x: int;
+    proc init() {
+      x = 0;
+    }
+    proc init(in d: int) {
+      x = d;
+    }
+  }
+
+
+  record R {
+    var x: int;
+    var y: myData;
+    proc init(in newX: int) {
+      x = newX;
+      y = new myData();
+    }
+    proc init(in newY: myData) {
+      x = 0;
+      y = newY;
+    }
+    @chplcheck.ignore("UnusedFormal")
+    proc init(newY: myData, dummy: int) {
+      x = newY.x;
+      y = new myData();
+    }
+    @chplcheck.ignore("UnusedFormal")
+    proc init(newY: myData, dummy: int, dummy2: int) {
+      init this;
+      x = 0;
+      y = newY;
+    }
+  }
+
+}

test/chplcheck/activerules.good

@@ -11,6 +11,7 @@ Active rules:
   IncorrectIndentation         Warn for inconsistent or missing indentation
   MethodsAfterFields           Warn for classes or records that mix field and method definitions.
   MisleadingIndentation        Warn for single-statement blocks that look like they might be multi-statement blocks.
+  MissingInIntent              Warn for formals used to initialize fields that are missing an 'in' intent.
   PascalCaseClasses            Warn for classes that are not 'PascalCase'.
   PascalCaseModules            Warn for modules that are not 'PascalCase'.
   SimpleDomainAsRange          Warn for simple domains in loops that can be ranges.

test/chplcheck/allrules.good

@@ -13,6 +13,7 @@ Available rules (default rules marked with *):
   * IncorrectIndentation         Warn for inconsistent or missing indentation
   * MethodsAfterFields           Warn for classes or records that mix field and method definitions.
   * MisleadingIndentation        Warn for single-statement blocks that look like they might be multi-statement blocks.
+  * MissingInIntent              Warn for formals used to initialize fields that are missing an 'in' intent.
     NestedCoforalls              Warn for nested 'coforall' loops, which could lead to performance hits.
   * PascalCaseClasses            Warn for classes that are not 'PascalCase'.
   * PascalCaseModules            Warn for modules that are not 'PascalCase'.

tools/chplcheck/src/rules.py

@@ -939,3 +939,74 @@ def unwrap_intermediate_block(node: AstNode) -> Optional[AstNode]:
             prev_depth = depth
             prev = child
             prev_line = line
+
+    @driver.advanced_rule
+    def MissingInIntent(_, root: chapel.AstNode):
+        """
+        Warn for formals used to initialize fields that are missing an 'in' intent.
+        """
+        if isinstance(root, chapel.Comment):
+            return
+
+        for agg, _ in chapel.each_matching(root, chapel.AggregateDecl):
+            assert isinstance(agg, chapel.AggregateDecl)
+            fields: Dict[str, chapel.Variable] = {}
+            inits: List[chapel.Function] = []
+            for nd in agg:
+                if isinstance(nd, chapel.Variable) and nd.is_field():
+                    fields[nd.unique_id()] = nd
+                if isinstance(nd, chapel.Function) and nd.name() in (
+                    "init",
+                    "init=",
+                ):
+                    inits.append(nd)
+
+            for init in inits:
+                formals: Dict[str, chapel.Formal] = {}
+                for f, _ in chapel.each_matching(init, chapel.Formal):
+                    assert isinstance(f, chapel.Formal)
+                    if f.intent() != "<default-intent>":
+                        continue
+                    formals[f.unique_id()] = f
+
+                for stmt in chapel.preorder(init):
+                    if isinstance(stmt, chapel.Init):
+                        break
+
+                    if not isinstance(stmt, chapel.OpCall):
+                        continue
+
+                    if stmt.op() != "=":
+                        continue
+
+                    lhs = stmt.actual(0)
+                    lhs_is_field = False
+                    if isinstance(lhs, (chapel.Identifier, chapel.Dot)):
+                        to = lhs.to_node()
+                        if to and to.unique_id() in fields:
+                            lhs_is_field = True
+                    rhs = stmt.actual(1)
+                    rhs_is_formal = None
+                    if isinstance(rhs, chapel.Identifier):
+                        to = rhs.to_node()
+                        if to and to.unique_id() in formals:
+                            rhs_is_formal = to
+
+                    if lhs_is_field and rhs_is_formal:
+                        yield AdvancedRuleResult(rhs_is_formal)
+
+    @driver.fixit(MissingInIntent)
+    def FixMissingInIntent(context: Context, result: AdvancedRuleResult):
+        """
+        Add the 'in' intent to the formal parameter.
+        """
+        assert isinstance(result.node, chapel.Formal)
+        lines = chapel.get_file_lines(context, result.node)
+
+        fixit = Fixit.build(
+            Edit.build(
+                result.node.location(),
+                "in " + range_to_text(result.node.location(), lines),
+            )
+        )
+        return [fixit]