Report type parameters whose name is a type in scope
See https://github.com/dart-lang/sdk/issues/59517 for background and discussion.
Change-Id: I6d02d07cdd95e8b2b647cf3f7f87a9f27a1c48b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381841
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
index e97b962..589e5ec 100644
--- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
+++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -1985,7 +1985,11 @@
notes: |-
Theoretically we could replace some uses with `is` checks, but it's probably
uncommon enough to not be worth the cost.
-LintCode.avoid_types_as_parameter_names:
+LintCode.avoid_types_as_parameter_names_type_parameter:
+ status: noFix
+ notes: |-
+ The type variable needs to be renamed, but that's a refactoring.
+LintCode.avoid_types_as_parameter_names_formal_parameter:
status: hasFix
LintCode.avoid_types_on_closure_parameters:
status: hasFix
diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
index fb6fc2b..9a3c7e5 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
+++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
@@ -311,7 +311,9 @@
// diagnostics and should also be available as an assist.
ReplaceCascadeWithDot.new,
],
- LinterLintCode.avoid_types_as_parameter_names: [ConvertToOnType.new],
+ LinterLintCode.avoid_types_as_parameter_names_formal_parameter: [
+ ConvertToOnType.new,
+ ],
LinterLintCode.avoid_types_on_closure_parameters: [
ReplaceWithIdentifier.new,
RemoveTypeAnnotation.other,
diff --git a/pkg/analyzer/lib/src/fine/library_manifest.dart b/pkg/analyzer/lib/src/fine/library_manifest.dart
index a91c166..ffc9c62 100644
--- a/pkg/analyzer/lib/src/fine/library_manifest.dart
+++ b/pkg/analyzer/lib/src/fine/library_manifest.dart
@@ -708,9 +708,9 @@
/// Returns either the existing item from [declaredItems], or builds a new one.
Item _getOrBuildElementItem<
- Element extends ElementImpl2,
+ E extends ElementImpl2,
Item extends ManifestItem
- >(Element element, Item Function() build) {
+ >(E element, Item Function() build) {
// We assume that when matching elements against the structure of
// the item, we put into [itemMap] only the type of the item that
// corresponds the type of the element.
diff --git a/pkg/analyzer/test/generated/error_suppression_test.dart b/pkg/analyzer/test/generated/error_suppression_test.dart
index 5af8bac..cacac24 100644
--- a/pkg/analyzer/test/generated/error_suppression_test.dart
+++ b/pkg/analyzer/test/generated/error_suppression_test.dart
@@ -234,7 +234,13 @@
// ignore: type=wrong
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''',
- [error(LinterLintCode.avoid_types_as_parameter_names, 34, 3)],
+ [
+ error(
+ LinterLintCode.avoid_types_as_parameter_names_formal_parameter,
+ 34,
+ 3,
+ ),
+ ],
);
}
diff --git a/pkg/linter/lib/src/lint_codes.g.dart b/pkg/linter/lib/src/lint_codes.g.dart
index 9a5799c..18709ea 100644
--- a/pkg/linter/lib/src/lint_codes.g.dart
+++ b/pkg/linter/lib/src/lint_codes.g.dart
@@ -360,13 +360,25 @@
hasPublishedDocs: true,
);
- static const LintCode avoid_types_as_parameter_names = LinterLintCode(
+ static const LintCode
+ avoid_types_as_parameter_names_formal_parameter = LinterLintCode(
LintNames.avoid_types_as_parameter_names,
"The parameter name '{0}' matches a visible type name.",
correctionMessage:
"Try adding a name for the parameter or changing the parameter name to "
"not match an existing type.",
hasPublishedDocs: true,
+ uniqueName: 'avoid_types_as_parameter_names_formal_parameter',
+ );
+
+ static const LintCode
+ avoid_types_as_parameter_names_type_parameter = LinterLintCode(
+ LintNames.avoid_types_as_parameter_names,
+ "The type parameter name '{0}' matches a visible type name.",
+ correctionMessage:
+ "Try changing the type parameter name to not match an existing type.",
+ hasPublishedDocs: true,
+ uniqueName: 'avoid_types_as_parameter_names_type_parameter',
);
static const LintCode avoid_types_on_closure_parameters = LinterLintCode(
diff --git a/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart b/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart
index f8fa24f..567c788 100644
--- a/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart
+++ b/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart
@@ -19,7 +19,10 @@
: super(name: LintNames.avoid_types_as_parameter_names, description: _desc);
@override
- LintCode get lintCode => LinterLintCode.avoid_types_as_parameter_names;
+ List<LintCode> get lintCodes => [
+ LinterLintCode.avoid_types_as_parameter_names_type_parameter,
+ LinterLintCode.avoid_types_as_parameter_names_formal_parameter,
+ ];
@override
void registerNodeProcessors(
@@ -29,6 +32,7 @@
var visitor = _Visitor(this, context);
registry.addFormalParameterList(this, visitor);
registry.addCatchClause(this, visitor);
+ registry.addTypeParameterList(this, visitor);
}
}
@@ -42,7 +46,12 @@
void visitCatchClause(CatchClause node) {
var parameter = node.exceptionParameter;
if (parameter != null && _isTypeName(node, parameter.name)) {
- rule.reportAtNode(parameter, arguments: [parameter.name.lexeme]);
+ rule.reportAtNode(
+ parameter,
+ arguments: [parameter.name.lexeme],
+ errorCode:
+ LinterLintCode.avoid_types_as_parameter_names_formal_parameter,
+ );
}
}
@@ -56,12 +65,62 @@
declaredElement.hasImplicitType &&
name != null &&
_isTypeName(node, name)) {
- rule.reportAtToken(name, arguments: [name.lexeme]);
+ rule.reportAtToken(
+ name,
+ arguments: [name.lexeme],
+ errorCode:
+ LinterLintCode.avoid_types_as_parameter_names_formal_parameter,
+ );
}
}
}
- bool _isTypeName(AstNode scope, Token name) {
+ @override
+ void visitTypeParameterList(TypeParameterList node) {
+ for (var typeParameter in node.typeParameters) {
+ var declaredElement = typeParameter.declaredFragment?.element;
+ var name = typeParameter.name;
+ var scope = node.parent;
+ var isShadowedByTypeParameter = false;
+ // Step out into enclosing scope where this type parameter isn't
+ // itself in scope (the type parameter doesn't shadow itself).
+ while (scope != null) {
+ if (scope is ClassDeclaration ||
+ scope is FunctionDeclaration ||
+ (scope is FunctionExpression &&
+ scope.parent is! FunctionDeclaration) ||
+ scope is GenericFunctionType ||
+ scope is MethodDeclaration) {
+ if (scope is MethodDeclaration) {
+ isShadowedByTypeParameter = true;
+ }
+ scope = scope.parent;
+ break;
+ }
+ scope = scope.parent;
+ }
+ if (declaredElement != null &&
+ scope != null &&
+ _isTypeName(
+ scope,
+ name,
+ isShadowedByMethodTypeParameter: isShadowedByTypeParameter,
+ )) {
+ rule.reportAtToken(
+ name,
+ arguments: [name.lexeme],
+ errorCode:
+ LinterLintCode.avoid_types_as_parameter_names_type_parameter,
+ );
+ }
+ }
+ }
+
+ bool _isTypeName(
+ AstNode scope,
+ Token name, {
+ bool isShadowedByMethodTypeParameter = false,
+ }) {
var result = resolveNameInScope(
name.lexeme,
scope,
@@ -69,6 +128,13 @@
);
if (result.isRequestedName) {
var element = result.element;
+ if (isShadowedByMethodTypeParameter && element is TypeParameterElement) {
+ // A type parameter of a static method can only shadow another type
+ // parameter when the latter is a type parameter of the enclosing
+ // declaration (class, mixin, mixin class, enum, extension, or
+ // extension type). We do not lint this case.
+ return false;
+ }
return element is ClassElement ||
element is ExtensionTypeElement ||
element is TypeAliasElement ||
diff --git a/pkg/linter/messages.yaml b/pkg/linter/messages.yaml
index 362e7b7..48ff5e4 100644
--- a/pkg/linter/messages.yaml
+++ b/pkg/linter/messages.yaml
@@ -2347,11 +2347,17 @@
return getThingFromDatabase(key: myThing.thingTypeKey);
}
```
- avoid_types_as_parameter_names:
+ avoid_types_as_parameter_names_formal_parameter:
+ sharedName: avoid_types_as_parameter_names
problemMessage: "The parameter name '{0}' matches a visible type name."
correctionMessage: "Try adding a name for the parameter or changing the parameter name to not match an existing type."
+ hasPublishedDocs: true
+ avoid_types_as_parameter_names_type_parameter:
+ sharedName: avoid_types_as_parameter_names
+ problemMessage: "The type parameter name '{0}' matches a visible type name."
state:
stable: "2.0"
+ correctionMessage: "Try changing the type parameter name to not match an existing type."
categories: [unintentional]
hasPublishedDocs: true
documentation: |-
@@ -2368,6 +2374,11 @@
shadow the existing type, which can lead to bugs that are difficult to
diagnose.
+ The analyzer also produces this diagnostic when the name of a type
+ parameter in a type parameter list is the same as a type whose name is
+ in scope. It is again recommended that the type parameter is renamed
+ such that the error-prone shadowing is avoided.
+
#### Example
The following code produces this diagnostic because the function `f` has a
diff --git a/pkg/linter/test/rules/avoid_types_as_parameter_names_test.dart b/pkg/linter/test/rules/avoid_types_as_parameter_names_test.dart
index f254176..3ee23a7 100644
--- a/pkg/linter/test/rules/avoid_types_as_parameter_names_test.dart
+++ b/pkg/linter/test/rules/avoid_types_as_parameter_names_test.dart
@@ -30,6 +30,22 @@
);
}
+ test_classTypeParameter_instanceMethodTypeParameter() async {
+ await assertNoDiagnostics(r'''
+class A<X> {
+ X instanceMethod<X>(X x) => x;
+}
+''');
+ }
+
+ test_classTypeParameter_staticMethodTypeParameter() async {
+ await assertNoDiagnostics(r'''
+class A<X> {
+ static X staticMethod<X>(X x) => x;
+}
+''');
+ }
+
test_extensionType() async {
await assertDiagnostics(
r'''
@@ -201,6 +217,37 @@
''');
}
+ test_typeParameter_class() async {
+ await assertDiagnostics(
+ r'''
+class A<Object> {}
+''',
+ [lint(8, 6)],
+ );
+ }
+
+ test_typeParameter_function() async {
+ await assertDiagnostics(
+ r'''
+void f<int>() {}
+''',
+ [lint(7, 3)],
+ );
+ }
+
+ test_typeParameter_instanceMethod_class() async {
+ await assertDiagnostics(
+ r'''
+class A<X> {
+ C instanceMethod<C>(C c) => c;
+}
+
+class C {}
+''',
+ [lint(32, 1)],
+ );
+ }
+
test_typeParameter_wildcard() async {
await assertDiagnostics(
r'''
