avoid_shadowing

example/all.yaml

@@ -31,6 +31,7 @@ linter:
     - avoid_returning_null_for_void
     - avoid_returning_this
     - avoid_setters_without_getters
+    - avoid_shadowing
     - avoid_single_cascade_in_expression_statements
     - avoid_slow_async_io
     - avoid_types_as_parameter_names

lib/src/rules.dart

@@ -33,6 +33,7 @@ import 'package:linter/src/rules/avoid_returning_null_for_void.dart';
 import 'package:linter/src/rules/avoid_returning_this.dart';
 import 'package:linter/src/rules/avoid_setters_without_getters.dart';
 import 'package:linter/src/rules/avoid_single_cascade_in_expression_statements.dart';
+import 'package:linter/src/rules/avoid_shadowing.dart';
 import 'package:linter/src/rules/avoid_slow_async_io.dart';
 import 'package:linter/src/rules/avoid_types_as_parameter_names.dart';
 import 'package:linter/src/rules/avoid_types_on_closure_parameters.dart';
@@ -166,6 +167,7 @@ void registerLintRules() {
     ..register(new AvoidReturningThis())
     ..register(new AvoidSettersWithoutGetters())
     ..register(new AvoidSingleCascadeInExpressionStatements())
+    ..register(new AvoidShadowing())
     ..register(new AvoidSlowAsyncIo())
     ..register(new AvoidTypesAsParameterNames())
     ..register(new AvoidUnusedConstructorParameters())

lib/src/rules/avoid_shadowing.dart

@@ -0,0 +1,218 @@
+// Copyright (c) 2018, 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.
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:linter/src/analyzer.dart';
+
+const desc = r'Do not use a name already visible.';
+
+const details = r'''
+
+**DO** Do not use a name already visible as getter or variable.
+
+**BAD:**
+```
+var k = null;
+
+class A {
+  var a;
+}
+
+class B extends A {
+  get b => null;
+
+  m() {
+    var a; // LINT
+    var b; // LINT
+    var k; // LINT
+  }
+}
+```
+
+**GOOD:**
+```
+var k = null;
+
+class A {
+  var a;
+}
+
+class B extends A {
+  get b => null;
+
+  m() {
+    var c; // OK
+    var d; // OK
+    var e; // OK
+  }
+}
+```
+''';
+
+class AvoidShadowing extends LintRule implements NodeLintRule {
+  AvoidShadowing()
+      : super(
+            name: 'avoid_shadowing',
+            description: desc,
+            details: details,
+            group: Group.errors);
+
+  @override
+  void registerNodeProcessors(NodeLintRegistry registry) {
+    final visitor = new _Visitor(this);
+    registry.addVariableDeclarationStatement(this, visitor);
+  }
+}
+
+class _Visitor extends SimpleAstVisitor {
+  final LintRule rule;
+
+  _Visitor(this.rule);
+
+  @override
+  visitVariableDeclarationStatement(VariableDeclarationStatement node) {
+    final variables = node.variables.variables.toList();
+    final library = variables.first.declaredElement.library;
+
+    // exclude pattern : var name = this.name;
+    variables.removeWhere((variable) {
+      final initializer = variable.initializer;
+      return initializer is PropertyAccess &&
+          initializer.propertyName.name == variable.name.name &&
+          (initializer.target is ThisExpression ||
+              initializer.target is SuperExpression);
+    });
+
+    // exclude pattern : same name as current getter name
+    variables.removeWhere(_hasSameNameAsItsGetter);
+
+    // exit on empty variables to check
+    if (variables.isEmpty) return;
+
+    bool skipInstanceMembers = false;
+    AstNode current = node;
+    while (current != null) {
+      if (current is ClassDeclaration) {
+        _checkClass(current, variables, onlyStatics: skipInstanceMembers);
+      } else if (current is ConstructorDeclaration) {
+        _checkParameters(current.parameters, variables);
+        if (current.factoryKeyword != null) {
+          skipInstanceMembers = true;
+        }
+      } else if (current is MethodDeclaration) {
+        _checkParameters(current.parameters, variables);
+        if (current.isStatic) {
+          skipInstanceMembers = true;
+        }
+      } else if (current is FunctionExpression) {
+        _checkParameters(current.parameters, variables);
+      } else if (current is FunctionDeclaration) {
+        _checkParameters(current.functionExpression.parameters, variables);
+      } else if (current is FunctionDeclarationStatement) {
+        _checkParameters(
+            current.functionDeclaration.functionExpression.parameters,
+            variables);
+      }
+
+      if (current.parent is Block) {
+        _checkParentBlock(current, variables);
+      }
+
+      current = current.parent;
+    }
+    _checkLibrary(library, variables);
+  }
+
+  void _checkClass(
+    ClassDeclaration clazz,
+    List<VariableDeclaration> variables, {
+    bool onlyStatics,
+  }) {
+    for (final variable in variables) {
+      final name = variable.name.name;
+      final getter = clazz.declaredElement
+          .lookUpGetter(name, clazz.declaredElement.library);
+      if (getter != null && (!onlyStatics || getter.isStatic))
+        rule.reportLint(variable);
+    }
+  }
+
+  void _checkLibrary(
+      LibraryElement library, List<VariableDeclaration> variables) {
+    final topLevelVariableNames = library.units
+        .expand((u) => u.topLevelVariables)
+        .map((e) => e.name)
+        .toList();
+    for (final variable in variables) {
+      final name = variable.name.name;
+      if (topLevelVariableNames.contains(name)) {
+        rule.reportLint(variable);
+      }
+    }
+  }
+
+  void _checkParameters(
+      FormalParameterList parameters, List<VariableDeclaration> variables) {
+    if (parameters == null) return;
+
+    final parameterNames =
+        parameters.parameterElements.map((e) => e.name).toList();
+
+    for (final variable in variables) {
+      final name = variable.name.name;
+      if (parameterNames.contains(name)) {
+        rule.reportLint(variable);
+      }
+    }
+  }
+
+  void _checkParentBlock(AstNode node, List<VariableDeclaration> variables) {
+    final block = node.parent as Block;
+    final names = <String>[];
+    for (final statement in block.statements.takeWhile((n) => n != node)) {
+      if (statement is VariableDeclarationStatement) {
+        names.addAll(statement.variables.variables.map((e) => e.name.name));
+      }
+    }
+    for (final variable in variables) {
+      final name = variable.name.name;
+      if (names.contains(name)) {
+        rule.reportLint(variable);
+      }
+    }
+  }
+
+  /// excludes pattern:
+  ///
+  /// ```dart
+  /// get name {
+  ///   var name;
+  ///   ...
+  /// }
+  /// ```
+  bool _hasSameNameAsItsGetter(VariableDeclaration node) {
+    final name = node.name.name;
+    VariableDeclarationList variableList = node.parent;
+    final variableDeclarationStatement = variableList.parent;
+
+    var current = variableDeclarationStatement.parent;
+    if (current is! Block) return false;
+
+    current = current.parent;
+    if (current is! BlockFunctionBody) return false;
+
+    current = current.parent;
+    if (current is MethodDeclaration && current.isGetter)
+      return current.name.name == name;
+    if (current is FunctionExpression) {
+      final parent = current.parent;
+      if (parent is FunctionDeclaration && parent.isGetter)
+        return parent.name.name == name;
+    }
+
+    return false;
+  }
+}

test/rules/avoid_shadowing.dart

@@ -0,0 +1,115 @@
+// Copyright (c) 2018, 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.
+
+// test w/ `pub run test -N avoid_shadowing`
+
+var top = null;
+
+m() {
+  f() {
+    var top; // LINT
+    var a; // OK
+  }
+
+  var a; // OK
+  var b; // OK
+  var top; // LINT
+
+  g() {
+    var a; // LINT
+    var c; // OK
+  }
+}
+
+class A {
+  var a;
+  get b => null;
+
+  ma() {
+    var a; // LINT
+    var b; // LINT
+    var top; // LINT
+  }
+
+  static sm() {
+    var a; // OK
+    var b; // OK
+    var c; // OK
+  }
+}
+
+class B extends A {
+  mb() {
+    var a; // LINT
+    var b; // LINT
+    var c; // OK
+  }
+}
+
+class C extends Object with A {
+  mc() {
+    var a; // LINT
+    var b; // LINT
+    var c; // OK
+  }
+}
+
+// special case for factory and static
+class D {
+  var a;
+  get b => null;
+  static get c => null;
+
+  static sm() {
+    var a; // OK
+    var b; // OK
+    var c; // LINT
+  }
+
+  factory D() {
+    var a; // OK
+    var b; // OK
+    var c; // LINT
+  }
+}
+
+// 2 shadowing
+class E {
+  int x;
+  m(int x) {
+    int x; // LINT
+  }
+}
+
+// exclude pattern : var x = this.x;
+class F {
+  int x;
+  m() {
+    var x = this.x; // OK
+  }
+}
+
+// exclude pattern : var x = super.x;
+class F2 extends F {
+  m() {
+    var x = super.x; // OK
+  }
+}
+
+// exclude pattern : same name as current getter name in class
+class G {
+  var i;
+  get g {
+    var g = ''; // OK
+    var i = ''; // LINT
+    return g;
+  }
+}
+
+// exclude pattern : same name as current getter name in top level
+get g {
+  var g = ''; // OK
+  var top = ''; // LINT
+  return g;
+}