[DAS] Fixes add const for final variables and lint

[email protected]

Fixes #49294

Change-Id: I75dd2c0063ea5d3a6ebbf4afb734d2cb5d98c1d2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392860
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Auto-Submit: Felipe Morschel <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>

Author: FMorschel

pkg/analysis_server/lib/src/services/correction/dart/add_const.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/utilities/extensions/ast.dart';
 import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ -13,6 +14,7 @@ import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_dart
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:analyzer_plugin/utilities/range_factory.dart';
+import 'package:linter/src/lint_codes.dart';
 
 class AddConst extends ResolvedCorrectionProducer {
   AddConst({required super.context});
@@ -97,9 +99,12 @@ class AddConst extends ResolvedCorrectionProducer {
         DartFileEditBuilderImpl builder, Expression targetNode) {
       var edits = builder.fileEdit.edits;
       var child = targetNode.parent;
-      while (child is Expression || child is ArgumentList) {
+      while (child is Expression ||
+          child is ArgumentList ||
+          child is VariableDeclaration ||
+          child is VariableDeclarationList) {
         if (edits.any((element) =>
-            element.replacement == 'const ' &&
+            element.replacement.startsWith('const') &&
             element.offset == child!.offset)) {
           return true;
         }
@@ -137,13 +142,77 @@ class AddConst extends ResolvedCorrectionProducer {
     if (targetNode is ConstructorName) {
       targetNode = targetNode.parent;
     }
-    if (targetNode is InstanceCreationExpression) {
-      if (targetNode.keyword == null) {
+    if (targetNode case InstanceCreationExpression(:var parent, :var keyword)) {
+      var constDeclarations =
+          getCodeStyleOptions(unitResult.file).preferConstDeclarations;
+
+      if (parent is VariableDeclaration && constDeclarations) {
+        if (parent.parent
+            case VariableDeclarationList(:var finalKeyword?, :var variables)
+            when _declarationListIsFullyConst(variables)) {
+          await builder.addDartFileEdit(file, (builder) {
+            builder.addSimpleReplacement(range.token(finalKeyword), 'const');
+          });
+          return;
+        }
+      }
+      if (keyword == null) {
         await insertAtOffset(targetNode);
         return;
       }
     }
   }
+
+  /// Considers if the given [variables] to be declared with `const` if all of
+  /// them are contained in the list of errors that suggest using `const` (
+  /// `prefer_const_constructors` triggers).
+  ///
+  /// This is used to determine if a `const` keyword should be added to a
+  /// variable declaration list. E.g.:
+  ///
+  /// ```dart
+  /// final a = 1, b = 2;
+  /// ```
+  ///
+  /// Would then be transformed to:
+  ///
+  /// ```dart
+  /// const a = 1, b = 2;
+  /// ```
+  ///
+  /// If not all of the variables are contained in the list of errors that
+  /// suggest using `const` then the `const` keyword should not be added. E.g.:
+  ///
+  /// ```dart
+  /// class C {
+  ///   const C();
+  /// }
+  /// final c = C(), d = Future.value(1);
+  /// ```
+  ///
+  /// Would be transformed to:
+  ///
+  /// ```dart
+  /// final c = const C(), d = Future.value(1);
+  /// ```
+  ///
+  /// If other diagnostics are to be fixed with this CorrectionProducer the
+  /// inner test for `prefer_const_constructors` will need to be amended.
+  bool _declarationListIsFullyConst(NodeList<VariableDeclaration> variables) {
+    var errors = [
+      ...unitResult.errors.where(
+        (error) => error.errorCode == LinterLintCode.prefer_const_constructors,
+      ),
+    ];
+    var errorsRanges = errors.map(range.error);
+    var variablesRanges = variables.map((v) {
+      var initializer = v.initializer;
+      if (initializer == null) return range.node(v);
+      return range.node(initializer);
+    });
+    // If each of the variable ranges is contained in the list of error ranges.
+    return variablesRanges.every(errorsRanges.contains);
+  }
 }
 
 class _ConstRangeFinder extends RecursiveAstVisitor<void> {

pkg/analysis_server/test/src/services/correction/fix/add_const_test.dart

@@ -425,6 +425,86 @@ class AddConst_PreferConstConstructorsBulkTest extends BulkFixProcessorTest {
   @override
   String get lintCode => LintNames.prefer_const_constructors;
 
+  Future<void> test_final() async {
+    writeTestPackageConfig(meta: true);
+    createAnalysisOptionsFile(lints: [
+      LintNames.prefer_const_constructors,
+      LintNames.prefer_const_declarations,
+    ]);
+    await resolveTestCode(r'''
+class C {
+  const C([C? c]);
+}
+final c = C(C());
+''');
+    await assertHasFix(r'''
+class C {
+  const C([C? c]);
+}
+const c = C(C());
+''');
+  }
+
+  Future<void> test_final_variableDeclarationList_const() async {
+    writeTestPackageConfig(meta: true);
+    createAnalysisOptionsFile(lints: [
+      LintNames.prefer_const_constructors,
+      LintNames.prefer_const_declarations,
+    ]);
+    await resolveTestCode(r'''
+class C {
+  const C([C? c]);
+}
+final c1 = C(C()), c2 = C();
+''');
+    await assertHasFix(r'''
+class C {
+  const C([C? c]);
+}
+const c1 = C(C()), c2 = C();
+''');
+  }
+
+  Future<void> test_final_variableDeclarationList_nonConst_first() async {
+    writeTestPackageConfig(meta: true);
+    createAnalysisOptionsFile(lints: [
+      LintNames.prefer_const_constructors,
+      LintNames.prefer_const_declarations,
+    ]);
+    await resolveTestCode(r'''
+class C {
+  const C([C? c]);
+}
+final f = Future.value(7), c = C(C());
+''');
+    await assertHasFix(r'''
+class C {
+  const C([C? c]);
+}
+final f = Future.value(7), c = const C(C());
+''');
+  }
+
+  Future<void> test_final_variableDeclarationList_nonConst_last() async {
+    writeTestPackageConfig(meta: true);
+    createAnalysisOptionsFile(lints: [
+      LintNames.prefer_const_constructors,
+      LintNames.prefer_const_declarations,
+    ]);
+    await resolveTestCode(r'''
+class C {
+  const C([C? c]);
+}
+final c = C(C()), f = Future.value(7);
+''');
+    await assertHasFix(r'''
+class C {
+  const C([C? c]);
+}
+final c = const C(C()), f = Future.value(7);
+''');
+  }
+
   Future<void> test_noKeyword() async {
     writeTestPackageConfig(meta: true);
     await resolveTestCode(r'''
@@ -539,6 +619,31 @@ class AddConst_PreferConstConstructorsTest extends FixProcessorLintTest {
   @override
   String get lintCode => LintNames.prefer_const_constructors;
 
+  Future<void> test_final_variable() async {
+    createAnalysisOptionsFile(lints: [
+      LintNames.prefer_const_constructors,
+      LintNames.prefer_const_declarations,
+    ]);
+    await resolveTestCode('''
+class C {
+  const C();
+}
+void f() {
+  final c = C();
+  print(c);
+}
+''');
+    await assertHasFix('''
+class C {
+  const C();
+}
+void f() {
+  const c = C();
+  print(c);
+}
+''');
+  }
+
   Future<void> test_new() async {
     await resolveTestCode('''
 class C {

pkg/analyzer/lib/dart/analysis/code_style_options.dart

@@ -19,6 +19,9 @@ abstract class CodeStyleOptions {
   /// Whether local variables should be `final` whenever possible.
   bool get makeLocalsFinal;
 
+  /// Whether `prefer_const_declarations` is enabled.
+  bool get preferConstDeclarations;
+
   /// Whether `prefer_int_literals` is enabled.
   bool get preferIntLiterals;
 

pkg/analyzer/lib/src/analysis_options/code_style_options.dart

@@ -25,6 +25,10 @@ class CodeStyleOptionsImpl implements CodeStyleOptions {
   @override
   bool get makeLocalsFinal => _isLintEnabled('prefer_final_locals');
 
+  @override
+  bool get preferConstDeclarations =>
+      _isLintEnabled('prefer_const_declarations');
+
   @override
   bool get preferIntLiterals => _isLintEnabled('prefer_int_literals');