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>
diff --git a/pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart b/pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
index 0d64b24..9984e82 100644
--- a/pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
+++ b/pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
@@ -439,15 +439,12 @@
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 @@
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 @@
return true;
}
}
+ if (_hasPragmaVmEntryPoint(element)) {
+ return true;
+ }
return _usedElements.elements.contains(element);
}
diff --git a/pkg/analyzer/test/src/diagnostics/unused_element_test.dart b/pkg/analyzer/test/src/diagnostics/unused_element_test.dart
index b2827a4..c377ca0 100644
--- a/pkg/analyzer/test/src/diagnostics/unused_element_test.dart
+++ b/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 @@
@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 @@
''');
}
+ @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>;
