diff --git a/bin/linter.dart b/bin/linter.dart index cf83e7988..e72da7e91 100644 --- a/bin/linter.dart +++ b/bin/linter.dart @@ -57,6 +57,7 @@ void runLinter(List args, LinterOptions initialLintOptions) { help: 'Print results in a format suitable for parsing.', defaultsTo: false, negatable: false) + ..addFlag('strong', help: 'Use strong-mode analyzer.') ..addOption('config', abbr: 'c', help: 'Use configuration from this file.') ..addOption('dart-sdk', help: 'Custom path to a Dart SDK.') ..addOption('rules', @@ -120,6 +121,9 @@ void runLinter(List args, LinterOptions initialLintOptions) { lintOptions.dartSdkPath = customSdk; } + var strongMode = options['strong']; + if (strongMode != null) lintOptions.strongMode = strongMode; + var customPackageRoot = options['package-root']; if (customPackageRoot != null) { lintOptions.packageRootPath = customPackageRoot; diff --git a/lib/src/analysis.dart b/lib/src/analysis.dart index e43440648..5ef8d5b34 100644 --- a/lib/src/analysis.dart +++ b/lib/src/analysis.dart @@ -45,6 +45,7 @@ void printAndFail(String message, {int exitCode: 15}) { AnalysisOptions _buildAnalyzerOptions(DriverOptions options) { AnalysisOptionsImpl analysisOptions = new AnalysisOptionsImpl(); analysisOptions.cacheSize = options.cacheSize; + analysisOptions.strongMode = options.strongMode; analysisOptions.hint = false; analysisOptions.lint = options.enableLints; analysisOptions.generateSdkErrors = options.showSdkWarnings; @@ -222,6 +223,9 @@ class DriverOptions { /// The path to the dart SDK. String dartSdkPath; + /// Whether to use Dart's Strong Mode analyzer. + bool strongMode = true; + /// Whether to show lint warnings. bool enableLints = true; diff --git a/lib/src/rules.dart b/lib/src/rules.dart index d5d5ba888..fc2033240 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -30,6 +30,7 @@ import 'package:linter/src/rules/iterable_contains_unrelated_type.dart'; import 'package:linter/src/rules/library_names.dart'; import 'package:linter/src/rules/library_prefixes.dart'; import 'package:linter/src/rules/non_constant_identifier_names.dart'; +import 'package:linter/src/rules/no_implicit_dynamic.dart'; import 'package:linter/src/rules/one_member_abstracts.dart'; import 'package:linter/src/rules/overridden_fields.dart'; import 'package:linter/src/rules/package_api_docs.dart'; @@ -53,6 +54,7 @@ import 'package:linter/src/rules/unrelated_type_equality_checks.dart'; final Registry ruleRegistry = new Registry() ..register(new AlwaysDeclareReturnTypes()) ..register(new AlwaysSpecifyTypes()) + ..register(new NoImplicitDynamic()) ..register(new AnnotateOverrides()) ..register(new AvoidAs()) ..register(new AvoidEmptyElse()) diff --git a/lib/src/rules/no_implicit_dynamic.dart b/lib/src/rules/no_implicit_dynamic.dart new file mode 100644 index 000000000..0430a0039 --- /dev/null +++ b/lib/src/rules/no_implicit_dynamic.dart @@ -0,0 +1,202 @@ +// Copyright (c) 2015, 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. + +library linter.src.rules.annotate_types; + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:linter/src/linter.dart'; +import 'package:linter/src/util.dart'; + +const desc = 'Implicit use of dynamic.'; + +const details = ''' +**AVOID** using "implicitly dynamic" values. + +Untyped / dynamic invocations may fail or be slower at runtime, but dynamic +types often creep up unintentionally. Explicitly mark variables or return types +as `dynamic` (instead of `var`) to express your intent unequivocally. + +Note: this works best with the --strong command-line flag and after disabling +both `always_specify_types` and `always_declare_return_types` lints. + +**GOOD:** +```dart +String trim(String s) => s.trim(); + +main() { + var s = trim(' a ').toUpperCase(); + + dynamic x; + x = ... ; + x.reallyNotSureThisExists(); +} +``` + +**BAD:** +```dart +trim(s) => s.trim(); + +main() { + var s = trim(1).toUpperCase(); + + var x; + x = ... ; + x.reallyNotSureThisExists(); +} +``` +'''; + +class NoImplicitDynamic extends LintRule { + NoImplicitDynamic() + : super( + name: 'no_implicit_dynamic', + description: desc, + details: details, + group: Group.style); + + @override + AstVisitor getVisitor() => new Visitor(this); +} + +// TODO(ochafik): Handle implicit return types of method declarations (vs. overrides). +class Visitor extends SimpleAstVisitor { + final LintRule rule; + Visitor(this.rule); + + Element _getBestElement(Expression node) { + if (node is SimpleIdentifier) return node.bestElement; + if (node is PrefixedIdentifier) return node.bestElement; + if (node is PropertyAccess) return node.propertyName.bestElement; + return null; + } + + bool _isImplicitDynamic(Expression node) { + if (node == null) return false; + while (node is ParenthesizedExpression) { + node = node.expression; + } + + if (node is AsExpression || node is Literal) return false; + var t = node.bestType; + if (!t.isDynamic && !t.isObject) return false; + + var e = _getBestElement(node); + if (e is PropertyAccessorElement) e = e.variable; + if (e is VariableElement) return e.hasImplicitType; + + if (node is ConditionalExpression) { + return !node.thenExpression.bestType.isDynamic || + !node.elseExpression.bestType.isDynamic; + } + if (node is MethodInvocation) { + return node.methodName.bestElement?.hasImplicitReturnType != false; + } + + return true; + } + + void _checkTarget(Expression target, [token]) { + if (_isImplicitDynamic(target)) { + // Avoid double taxation (if `x` is dynamic, only lint `x.y.z` once). + Expression subTarget; + if (target is PropertyAccess) subTarget = target.realTarget; + else if (target is MethodInvocation) subTarget = target.realTarget; + else if (target is IndexExpression) subTarget = target.realTarget; + else if (target is PrefixedIdentifier) subTarget = target.prefix; + + if (_isImplicitDynamic(subTarget)) return; + + _reportNodeOrToken(target, token); + } + } + + _reportNodeOrToken(AstNode node, token) { + if (token != null) { + rule.reportLintForToken(token); + } else { + rule.reportLint(node); + } + } + + @override + visitPrefixedIdentifier(PrefixedIdentifier node) { + if (_isObjectProperty(node.identifier)) return; + _checkTarget(node.prefix, node.period); + } + + @override + visitPropertyAccess(PropertyAccess node) { + if (_isObjectProperty(node.propertyName)) return; + _checkTarget(node.realTarget, node.operator); + } + + bool _isObjectProperty(SimpleIdentifier node) { + var name = node.name; + return name == 'runtimeType' || name == 'hashCode'; + } + + @override + visitIndexExpression(IndexExpression node) { + _checkTarget(node.realTarget, node.leftBracket); + } + + @override + visitAssignmentExpression(AssignmentExpression node) { + var rhs = node.rightHandSide; + _checkAssignment(rhs, + rhs.bestParameterElement ?? _getBestElement(node.leftHandSide)); + } + + @override + visitMethodInvocation(MethodInvocation node) { + var methodName = node.methodName; + _checkMethodInvocation(node.realTarget, methodName.bestElement, methodName.name, node.argumentList.arguments, node.operator); + } + + _checkMethodInvocation(Expression target, ExecutableElement methodElement, String methodName, List arguments, token) { + for (var arg in arguments) { + _checkAssignment(arg, arg.bestParameterElement); + } + + if (methodElement != null) return; + + if (methodName == 'toString' && arguments.isEmpty || + methodName == 'noSuchMethod' && arguments.size == 1) { + return; + } + _checkTarget(target, token); + } + + @override + visitBinaryExpression(BinaryExpression node) { + _checkMethodInvocation(node.leftOperand, node.bestElement, node.operator.toString(), [node.rightOperand], node.operator); + } + + _checkAssignment(Expression arg, Element toElement) { + if (!_isImplicitDynamic(arg)) return; + + if (toElement == null) return; + + if (_isDynamicOrObject(toElement.type)) return; + + rule.reportLint(arg); + } + + _isDynamicOrObject(DartType t) => t.isDynamic || t.isObject; + + @override + void visitConditionalExpression(ConditionalExpression node) { + _checkTarget(node.condition); + } + + @override + visitDeclaredIdentifier(DeclaredIdentifier node) { + if (node.type == null && node.identifier.bestType.isDynamic && node.element.type.isDynamic) { + rule.reportLintForToken(node.keyword); + } + } +} diff --git a/test/rules/no_implicit_dynamic.dart b/test/rules/no_implicit_dynamic.dart new file mode 100644 index 000000000..3b5fbf29e --- /dev/null +++ b/test/rules/no_implicit_dynamic.dart @@ -0,0 +1,132 @@ +takeInt(int i) {} +takeDynamic(dynamic i) {} +takeObject(Object i) {} + +conditionals(implicitBool, bool explicitBool) { + explicitBool ? 1 : 2; + + implicitBool //LINT + ? 1 : 2; + + takeInt(explicitBool ? 1 : "2"); //LINT +} + +methodCalls() { + var implicitDynamic; + dynamic explicitDynamic; + + takeDynamic(1); + takeObject(1); + takeInt(1); + + takeDynamic(implicitDynamic); + takeObject(implicitDynamic); + takeInt(implicitDynamic); //LINT + + takeDynamic(explicitDynamic); + takeObject(explicitDynamic); + takeInt(explicitDynamic); +} + +class Foo { + var implicitDynamic; + dynamic explicitDynamic; + int i; +} + +assignments() { + Foo newFoo() => new Foo(); + int i; + var f = newFoo(); + + // Exercice prefixed identifiers path: + f.i = f.i; + f.i = f.implicitDynamic; //LINT + f.i = f.explicitDynamic; + + // Exercice property access path: + f.i = newFoo().i; + f.i = newFoo().implicitDynamic; //LINT + f.i = newFoo().explicitDynamic; + + i = f.i; + i = f.implicitDynamic; //LINT + i = f.explicitDynamic; +} + +vars(implicitDynamic, dynamic explicitDynamic) { + implicitDynamic.foo; //LINT + implicitDynamic?.foo; //LINT + implicitDynamic.foo(); //LINT + implicitDynamic?.foo(); //LINT + implicitDynamic['foo']; //LINT + implicitDynamic.toString(); + implicitDynamic.runtimeType; + implicitDynamic.hashCode; + + (implicitDynamic as dynamic).foo; + + explicitDynamic.foo; + explicitDynamic.foo(); + explicitDynamic['foo']; + explicitDynamic.toString(); + explicitDynamic.runtimeType; + explicitDynamic.hashCode; +} + +operators(implicitDynamic, dynamic explicitDynamic) { + implicitDynamic + 1; //LINT + implicitDynamic * 1; //LINT + + explicitDynamic + 1; + explicitDynamic + null; + + // int.operator+ expects an int parameter: + 1 + implicitDynamic; //LINT + 1 + explicitDynamic; +} + +cascades() { + var implicitDynamic; + implicitDynamic + ..foo //LINT + ..foo() //LINT + ..['foo'] //LINT + ..toString() + ..runtimeType + ..hashCode; +} + +dynamicMethods() { + trim(s) => + s.trim(); //LINT + var s = trim(1) + .toUpperCase(); //LINT + + implicit() {} + implicit() + .x //LINT + .y; + + dynamic explicit() {} + explicit().x; +} + +chaining() { + var x; + // Only report the first implicit dynamic in a chain: + x + .y //LINT + .z + .w; + + dynamic y; + // Calling an explicit dynamic is okay... + y.z; + y.z(); + y['z']; + // ... but returns an implicit dynamic. + y.z.w; //LINT + y.z().w; //LINT + y['z'].w; //LINT +}