From 3ea7130e4e4b7f339bfac6e18791bbc0e54d455d Mon Sep 17 00:00:00 2001 From: Robert Nystrom Date: Tue, 24 Feb 2026 08:15:22 -0800 Subject: [PATCH] Restrict "prefer_initializing_formals" to avoid false positives. When an explicit initializer or assignment is turned into an initializing formal, it's possible for that to change the semantics if there are other references to the same parameter. For example: ```dart // Before: class C { int? x; C(int? x) { print(this.x); this.x = x; } } // After: class C { int? x; C(this.x) { print(this.x); } } ``` This prints "null" before and the argument value after. The fix might even lead to invalid code: ```dart // Before: class C { int? x; int? y; C(int? x) : y = (x = 2) { this.x = x; } } // After: class C { int? x; int? y; C(this.x) : y = (x = 2); } ``` This becomes a compile error because `x` is final when it refers to an initializing formal in the initializer list. It's also not enough to look for secondary writes inside the constructor: ```dart // Before: class C { int? x; Function()? closure; C(int? x) { closure = () { print(x); }; this.x = x; } } // After: class C { int? x; Function()? closure; C(this.x) { closure = () { print(x); }; } } // Given: main() { var c = C(1); c.x = 2; c.closure!(); } ``` This prints "1" before and "2" after. I think the safest thing is to be conservative and not show the lint if there are any other references to the parameter anywhere in the constructor, even reads. Fix #58607. Change-Id: Ib976cd5bfc4bf44439ecd4175090dbabfe1cde16 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482985 Commit-Queue: Paul Berry Commit-Queue: Bob Nystrom Reviewed-by: Paul Berry Reviewed-by: Samuel Rawlins Reviewed-by: Brian Wilkerson Auto-Submit: Bob Nystrom --- .../analysis/analysis_context_collection.dart | 2 - .../rules/prefer_initializing_formals.dart | 28 ++++++++- .../prefer_initializing_formals_test.dart | 59 ++++++++++++++++++- 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart b/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart index 11d0ef68e41..80187063419 100644 --- a/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart +++ b/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart @@ -85,8 +85,6 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection { } scheduler.start(); } - // TODO(scheglov): https://github.com/dart-lang/linter/issues/3134 - // ignore: prefer_initializing_formals this.scheduler = scheduler; _throwIfAnyNotAbsoluteNormalizedPath(includedPaths); diff --git a/pkg/linter/lib/src/rules/prefer_initializing_formals.dart b/pkg/linter/lib/src/rules/prefer_initializing_formals.dart index 55a31a234a8..233d8179f16 100644 --- a/pkg/linter/lib/src/rules/prefer_initializing_formals.dart +++ b/pkg/linter/lib/src/rules/prefer_initializing_formals.dart @@ -120,8 +120,6 @@ class _ConstructorChecker { } _nodesToLintByField.forEach((field, nodes) { - if (nodes.length > 1) return; - for (var lintNode in nodes) { _rule.reportAtNode(lintNode, arguments: [field.name!]); } @@ -168,10 +166,36 @@ class _ConstructorChecker { return; } + // There can't be any other references to the parameter. If there are, it's + // possible removing the initializer/assignment and moving it up to be an + // initializing formal could be a semantic change. + var visitor = _ReferenceCounter(parameter); + // Visit the initializers and body directly so that we ignore references in + // the doc comment. + _constructor.initializers.accept(visitor); + _constructor.body.accept(visitor); + if (visitor.count > 1) return; + _nodesToLintByField.putIfAbsent(field, () => []).add(node); } } +/// Counts references in the visited AST to a given parameter. +class _ReferenceCounter extends RecursiveAstVisitor { + final FormalParameterElement parameterElement; + + int count = 0; + + _ReferenceCounter(this.parameterElement); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (node.element == parameterElement) { + count++; + } + } +} + class _Visitor extends SimpleAstVisitor { final AnalysisRule _rule; final RuleContext _context; diff --git a/pkg/linter/test/rules/prefer_initializing_formals_test.dart b/pkg/linter/test/rules/prefer_initializing_formals_test.dart index 0b6f5c71938..23960ffbe01 100644 --- a/pkg/linter/test/rules/prefer_initializing_formals_test.dart +++ b/pkg/linter/test/rules/prefer_initializing_formals_test.dart @@ -55,7 +55,7 @@ class A { } class C extends A { int? c, d; - C(int c, int d) : super(c, d) { + C(int c, int d) : super(1, 2) { this.c = c; this.d = d; } @@ -78,6 +78,61 @@ class C { '''); } + test_assignedInBody_multipleReference_body() async { + await assertNoDiagnostics(r''' +class C { + num x = 0; + C(num x) { + print(x); + this.x = x; + } +} +'''); + } + + test_assignedInBody_multipleReference_closure() async { + await assertNoDiagnostics(r''' +class C { + int? x; + Function()? closure; + C(int? x) { + closure = () { + print(x); + }; + this.x = x; + } +} +'''); + } + + test_assignedInBody_multipleReference_docComment() async { + await assertDiagnostics( + r''' +class C { + num x = 0; + + /// References to [x] in this doc comment like [x] and [x] are ignored. + C(num x) { + this.x = x; + } +} +''', + [lint(115, 10)], + ); + } + + test_assignedInBody_multipleReference_initializer() async { + await assertNoDiagnostics(r''' +class C { + num x = 0; + num y = 0; + C(num x) : y = x { + this.x = x; + } +} +'''); + } + test_assignedInBody_namedParameters() async { await assertDiagnostics( r''' @@ -198,7 +253,7 @@ class C extends A { C(int c, int d) : this.c = c, this.d = d, - super(c, d); + super(1, 2); } ''', [lint(103, 10), lint(123, 10)],