Add a hint for final FieldFormalParameter and SuperFormalParameter

Fixes #48699

Change-Id: Ieb16648a52884588e406931f830e4655b120cb0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239640
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: asashour

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -1417,6 +1417,8 @@ HintCode.UNIGNORABLE_IGNORE:
   status: needsEvaluation
 HintCode.UNNECESSARY_CAST:
   status: hasFix
+HintCode.UNNECESSARY_FINAL:
+  status: needsEvaluation
 HintCode.UNNECESSARY_IGNORE:
   status: needsEvaluation
 HintCode.UNNECESSARY_IMPORT:

pkg/analyzer/lib/error/error.dart

@@ -655,6 +655,7 @@ const List<ErrorCode> errorCodeValues = [
   HintCode.UNDEFINED_SHOWN_NAME,
   HintCode.UNIGNORABLE_IGNORE,
   HintCode.UNNECESSARY_CAST,
+  HintCode.UNNECESSARY_FINAL,
   HintCode.UNNECESSARY_IGNORE,
   HintCode.UNNECESSARY_IMPORT,
   HintCode.UNNECESSARY_NO_SUCH_METHOD,

pkg/analyzer/lib/src/dart/error/hint_codes.g.dart

@@ -3777,6 +3777,16 @@ class HintCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  /**
+   * No parameters.
+   */
+  static const HintCode UNNECESSARY_FINAL = HintCode(
+    'UNNECESSARY_FINAL',
+    "The keyword 'final' isn't necessary because the parameter is implicitly "
+        "'final'.",
+    correctionMessage: "Try removing the 'final'.",
+  );
+
   /**
    * Parameters:
    * 0: the name of the diagnostic being ignored

pkg/analyzer/lib/src/error/best_practices_verifier.dart

@@ -487,6 +487,12 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  @override
+  void visitFieldFormalParameter(FieldFormalParameter node) {
+    _checkFinalParameter(node, node.keyword);
+    super.visitFieldFormalParameter(node);
+  }
+
   @override
   void visitFormalParameterList(FormalParameterList node) {
     _checkRequiredParameter(node);
@@ -775,6 +781,12 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     super.visitSuperConstructorInvocation(node);
   }
 
+  @override
+  void visitSuperFormalParameter(SuperFormalParameter node) {
+    _checkFinalParameter(node, node.keyword);
+    super.visitSuperFormalParameter(node);
+  }
+
   @override
   void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
     _deprecatedVerifier.pushInDeprecatedMetadata(node.metadata);
@@ -856,6 +868,15 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
     return false;
   }
 
+  void _checkFinalParameter(FormalParameter node, Token? keyword) {
+    if (node.isFinal) {
+      _errorReporter.reportErrorForToken(
+        HintCode.UNNECESSARY_FINAL,
+        keyword!,
+      );
+    }
+  }
+
   void _checkForAssignmentOfDoNotStore(Expression? expression) {
     var expressionMap = _getSubExpressionsMarkedDoNotStore(expression);
     for (var entry in expressionMap.entries) {

pkg/analyzer/messages.yaml

@@ -20414,6 +20414,11 @@ HintCode:
         }
       }
       ```
+  UNNECESSARY_FINAL:
+    problemMessage: The keyword 'final' isn't necessary because the parameter is implicitly 'final'.
+    correctionMessage: Try removing the 'final'.
+    hasPublishedDocs: false
+    comment: No parameters.
   UNNECESSARY_IGNORE:
     problemMessage: "The diagnostic '{0}' isn't produced at this location so it doesn't need to be ignored."
     correctionMessage: Try removing the name from the list, or removing the whole comment if this is the only name in the list.

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -751,6 +751,7 @@ import 'undefined_super_operator_test.dart' as undefined_super_operator;
 import 'undefined_super_setter_test.dart' as undefined_super_setter;
 import 'unignorable_ignore_test.dart' as unignorable_ignore;
 import 'unnecessary_cast_test.dart' as unnecessary_cast;
+import 'unnecessary_final_test.dart' as unnecessary_final;
 import 'unnecessary_ignore_test.dart' as unnecessary_ignore;
 import 'unnecessary_import_test.dart' as unnecessary_import;
 import 'unnecessary_no_such_method_test.dart' as unnecessary_no_such_method;
@@ -1294,6 +1295,7 @@ main() {
     unignorable_ignore.main();
     unnecessary_import.main();
     unnecessary_cast.main();
+    unnecessary_final.main();
     unnecessary_ignore.main();
     unnecessary_no_such_method.main();
     unnecessary_non_null_assertion.main();

pkg/analyzer/test/src/diagnostics/unnecessary_final_test.dart

@@ -0,0 +1,62 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/src/error/codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(UnnecessaryFinalTest);
+  });
+}
+
+@reflectiveTest
+class UnnecessaryFinalTest extends PubPackageResolutionTest {
+  test_final() async {
+    await assertNoErrorsInCode('''
+class C {
+  C(final int value);
+}
+''');
+  }
+
+  test_positional() async {
+    await assertErrorsInCode('''
+class C {
+  C([final this.value = 0]);
+  int value;
+}
+''', [
+      error(HintCode.UNNECESSARY_FINAL, 15, 5),
+    ]);
+  }
+
+  test_super() async {
+    await assertErrorsInCode('''
+class A {
+  A(this.value);
+  int value;
+}
+
+class B extends A {
+  B(final super.value);
+}
+''', [
+      error(HintCode.UNNECESSARY_FINAL, 67, 5),
+    ]);
+  }
+
+  test_this() async {
+    await assertErrorsInCode('''
+class C {
+  C(final this.value);
+  int value;
+}
+''', [
+      error(HintCode.UNNECESSARY_FINAL, 14, 5),
+    ]);
+  }
+}

pkg/nnbd_migration/test/fix_aggregator_test.dart

@@ -1413,6 +1413,7 @@ class C {
 
   Future<void>
       test_parameter_field_formal_addExplicitType_declared_with_final() async {
+    verifyNoTestUnitErrors = false;
     await analyze('''
 class C {
   int x;