diff --git a/test/chplcheck/MissingInIntent.chpl b/test/chplcheck/MissingInIntent.chpl new file mode 100644 index 00000000000..6cd250ad8d5 --- /dev/null +++ b/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; + } + } + +} diff --git a/test/chplcheck/MissingInIntent.good b/test/chplcheck/MissingInIntent.good new file mode 100644 index 00000000000..cef545ba742 --- /dev/null +++ b/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] diff --git a/test/chplcheck/MissingInIntent.good-fixit b/test/chplcheck/MissingInIntent.good-fixit new file mode 100644 index 00000000000..dfc75105905 --- /dev/null +++ b/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; + } + } + +} diff --git a/test/chplcheck/activerules.good b/test/chplcheck/activerules.good index 5646f7be158..5a31b7dff42 100644 --- a/test/chplcheck/activerules.good +++ b/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. diff --git a/test/chplcheck/allrules.good b/test/chplcheck/allrules.good index 421c6c169ac..076b6de5194 100644 --- a/test/chplcheck/allrules.good +++ b/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'. diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py index 05cccac07bd..d9427b73cc0 100644 --- a/tools/chplcheck/src/rules.py +++ b/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() != "": + 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]