diff --git a/lib/src/util/unrelated_types_visitor.dart b/lib/src/util/unrelated_types_visitor.dart index 662f8d9e8..54e960d52 100644 --- a/lib/src/util/unrelated_types_visitor.dart +++ b/lib/src/util/unrelated_types_visitor.dart @@ -9,21 +9,38 @@ import 'package:analyzer/dart/element/type.dart'; import 'package:linter/src/analyzer.dart'; import 'package:linter/src/util/dart_type_utilities.dart'; +typedef _InterfaceTypePredicate = bool Function(InterfaceType type); + +/// Returns a predicate which returns whether a given [InterfaceTypeDefinition] +/// is equal to [definition]. _InterfaceTypePredicate _buildImplementsDefinitionPredicate( InterfaceTypeDefinition definition) => (InterfaceType interface) => interface.name == definition.name && interface.element.library.name == definition.library; +/// Returns all implemented interfaces of [type]. +/// +/// This flattens all of the super-interfaces of [type] into one list. List _findImplementedInterfaces(InterfaceType type, - {List acc = const []}) => - acc.contains(type) - ? acc + {List accumulator = const []}) => + accumulator.contains(type) + ? accumulator : type.interfaces.fold( [type], (List acc, InterfaceType e) => new List.from(acc) - ..addAll(_findImplementedInterfaces(e, acc: acc))); + ..addAll(_findImplementedInterfaces(e, accumulator: acc))); +/// Returns the first type argument on [definition], as implemented by [type]. +/// +/// In the simplest case, [type] is the same class as [definition]. For +/// example, given the definition `List` and the type `List`, +/// this function returns the DartType for `int`. +/// +/// In a more complicated case, we must traverse [type]'s interfaces to find +/// [definition]. For example, given the definition `Set` and the type `A` +/// where `A implements B` and `B implements Set, C`, +/// this function returns the DartType for `String`. DartType _findIterableTypeArgument( InterfaceTypeDefinition definition, InterfaceType type, {List accumulator = const []}) { @@ -56,20 +73,22 @@ bool _isParameterizedMethodInvocation( node.methodName.name == methodName && node.argumentList.arguments.length == 1; -typedef bool _InterfaceTypePredicate(InterfaceType type); - /// Base class for visitor used in rules where we want to lint about invoking -/// methods on generic classes where the parameter is unrelated to the parameter -/// type of the class. Extending this visitor is as simple as knowing the method, -/// class and library that uniquely define the target, i.e. implement only -/// [definition] and [methodName]. +/// methods on generic classes where the type of the singular argument is +/// unrelated to the singular type argument of the class. Extending this +/// visitor is as simple as knowing the method, class and library that uniquely +/// define the target, i.e. implement only [definition] and [methodName]. abstract class UnrelatedTypesProcessors extends SimpleAstVisitor { final LintRule rule; UnrelatedTypesProcessors(this.rule); + /// The type definition which this [UnrelatedTypesProcessors] is concerned + /// with. InterfaceTypeDefinition get definition; + /// The name of the method which this [UnrelatedTypesProcessors] is concerned + /// with. String get methodName; @override @@ -78,28 +97,39 @@ abstract class UnrelatedTypesProcessors extends SimpleAstVisitor { return; } - DartType type; + // At this point, we know that [node] is an invocation of a method which + // has the same name as the method that this UnrelatedTypesProcessors] is + // concerned with, and that the method has a single parameter. + // + // We've completed the "cheap" checks, and must now continue with the + // arduous task of determining whether the method target implements + // [definition]. + + DartType targetType; if (node.target != null) { - type = node.target.staticType; + targetType = node.target.staticType; } else { - var classDeclaration = + final classDeclaration = node.thisOrAncestorOfType(); if (classDeclaration == null) { - type = null; + targetType = null; } else if (classDeclaration is ClassDeclaration) { - type = resolutionMap + targetType = resolutionMap .elementDeclaredByClassDeclaration(classDeclaration) ?.type; } else if (classDeclaration is MixinDeclaration) { - type = resolutionMap + targetType = resolutionMap .elementDeclaredByMixinDeclaration(classDeclaration) ?.type; } } Expression argument = node.argumentList.arguments.first; - if (type is InterfaceType && - DartTypeUtilities.unrelatedTypes( - argument.staticType, _findIterableTypeArgument(definition, type))) { + + // Finally, determine whether the type of the argument is related to the + // type of the method target. + if (targetType is InterfaceType && + DartTypeUtilities.unrelatedTypes(argument.staticType, + _findIterableTypeArgument(definition, targetType))) { rule.reportLint(node); } } diff --git a/test/rules/iterable_contains_unrelated_type.dart b/test/rules/iterable_contains_unrelated_type.dart index f2a008404..d3ca30351 100644 --- a/test/rules/iterable_contains_unrelated_type.dart +++ b/test/rules/iterable_contains_unrelated_type.dart @@ -26,7 +26,7 @@ void someFunction4() { void someFunction4_1() { List list; - if(list.contains(null)) print('someFucntion4_1'); + if (list.contains(null)) print('someFucntion4_1'); } void someFunction5_1() { @@ -97,7 +97,8 @@ void someFunction12() { } void bug_267(list) { - if (list.contains('1')) print('someFunction'); // https://github.com/dart-lang/linter/issues/267 + if (list.contains('1')) // https://github.com/dart-lang/linter/issues/267 + print('someFunction'); } abstract class ClassBase {} @@ -133,22 +134,32 @@ bool takesIterable2(Iterable iterable) => iterable.contains('a'); // OK bool takesIterable3(Iterable iterable) => iterable.contains('a'); // OK abstract class A implements Iterable {} + abstract class B extends A {} + bool takesB(B b) => b.contains('a'); // LINT abstract class A1 implements Iterable {} + abstract class B1 extends A1 {} + bool takesB1(B1 b) => b.contains('a'); // OK abstract class A3 implements Iterable {} + abstract class B3 extends A3 {} + bool takesB3(B3 b) => b.contains('a'); // OK abstract class A2 implements Iterable {} + abstract class B2 extends A2 {} + bool takesB2(B2 b) => b.contains('a'); // OK -abstract class SomeIterable implements Iterable {} +abstract class AddlInterface {} + +abstract class SomeIterable implements Iterable, AddlInterface {} abstract class MyClass implements SomeIterable { bool badMethod(String thing) => this.contains(thing); // LINT