analyzer: Stop over-reporting 'unused' parameters

Currently analyzer is reporting optional parameters of private
functions, methods, and constructors as unused in some cases where the
function (or enclosing class for a constructor) is generic. In order to
support unused_element as a no-false-positives rule, this CL changes
the behavior to _always_ consider such parameters as used, and never
report them as unused.

The real fix is harder; I think it requires something like
ParameterMembers to be created during inference, but that is a pretty
sweeping change, and requiring TypeProviders to be passed around a lot
where they are not now.

Also move the `_hasPragmaVmEntryPoint` check to much later, as it is an
exceedingly uncommon annotation.

Bug: https://github.com/dart-lang/sdk/issues/47839
Change-Id: Ie2bc0dfbe2e4527676b1cd15045e67fb5f0f1719
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221992
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

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

@@ -439,15 +439,12 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
     if (element.isSynthetic) {
       return true;
     }
-    if (_hasPragmaVmEntryPoint(element)) {
-      return true;
-    }
     if (element is LocalVariableElement ||
         element is FunctionElement && !element.isStatic) {
       // local variable or function
     } else if (element is ParameterElement) {
       var enclosingElement = element.enclosingElement;
-      // Only report unused parameters of methods or functions.
+      // Only report unused parameters of constructors, methods, and functions.
       if (enclosingElement is! ConstructorElement &&
           enclosingElement is! FunctionElement &&
           enclosingElement is! MethodElement) {
@@ -457,7 +454,22 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
       if (!element.isOptional) {
         return true;
       }
+      if (enclosingElement is ConstructorElement &&
+          enclosingElement.enclosingElement.typeParameters.isNotEmpty) {
+        // There is an issue matching arguments of instance creation
+        // expressions for generic classes with parameters, so for now,
+        // consider every parameter of a constructor of a generic class
+        // "used". See https://github.com/dart-lang/sdk/issues/47839.
+        return true;
+      }
       if (enclosingElement is ExecutableElement) {
+        if (enclosingElement.typeParameters.isNotEmpty) {
+          // There is an issue matching arguments of generic function
+          // invocations with parameters, so for now, consider every parameter
+          // of a generic function "used". See
+          // https://github.com/dart-lang/sdk/issues/47839.
+          return true;
+        }
         if (_isPubliclyAccessible(enclosingElement)) {
           return true;
         }
@@ -470,6 +482,9 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
         return true;
       }
     }
+    if (_hasPragmaVmEntryPoint(element)) {
+      return true;
+    }
     return _usedElements.elements.contains(element);
   }
 

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analyzer/src/dart/error/hint_codes.dart';
+import 'package:test/expect.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../dart/resolution/context_collection_resolution.dart';
@@ -1699,6 +1700,37 @@ main() {
 
 @reflectiveTest
 class UnusedElementWithNullSafetyTest extends PubPackageResolutionTest {
+  test_optionalParameter_isUsed_genericConstructor() async {
+    await assertNoErrorsInCode('''
+class C<T> {
+  C._([int? x]);
+}
+void foo() {
+  C._(7);
+}
+''');
+  }
+
+  test_optionalParameter_isUsed_genericFunction() async {
+    await assertNoErrorsInCode('''
+void _f<T>([int? x]) {}
+void foo() {
+  _f(7);
+}
+''');
+  }
+
+  test_optionalParameter_isUsed_genericMethod() async {
+    await assertNoErrorsInCode('''
+class C {
+  void _m<T>([int? x]) {}
+}
+void foo() {
+  C()._m(7);
+}
+''');
+  }
+
   test_optionalParameter_isUsed_overrideRequiredNamed() async {
     await assertNoErrorsInCode(r'''
 class A {
@@ -1711,6 +1743,49 @@ f() => A()._m(a: 0);
 ''');
   }
 
+  @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/47839')
+  test_optionalParameter_notUsed_genericConstructor() async {
+    // TODO(srawlins): Change to assertErrorsInCode when this is fixed.
+    addTestFile('''
+class C<T> {
+  C._([int? x]);
+}
+void foo() {
+  C._();
+}
+''');
+    await resolveTestFile();
+    expect(result.errors, isNotEmpty);
+  }
+
+  @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/47839')
+  test_optionalParameter_notUsed_genericFunction() async {
+    // TODO(srawlins): Change to assertErrorsInCode when this is fixed.
+    addTestFile('''
+void _f<T>([int? x]) {}
+void foo() {
+  _f();
+}
+''');
+    await resolveTestFile();
+    expect(result.errors, isNotEmpty);
+  }
+
+  @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/47839')
+  test_optionalParameter_notUsed_genericMethod() async {
+    // TODO(srawlins): Change to assertErrorsInCode when this is fixed.
+    addTestFile('''
+class C {
+  void _m<T>([int? x]) {}
+}
+void foo() {
+  C()._m();
+}
+''');
+    await resolveTestFile();
+    expect(result.errors, isNotEmpty);
+  }
+
   test_typeAlias_interfaceType_isUsed_typeName_isExpression() async {
     await assertNoErrorsInCode(r'''
 typedef _A = List<int>;