Lint rule: Do not override fields.

lib/src/rules/overriden_field.dart

+// Copyright (c) 2016, 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.overriden_field;
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/generated/resolver.dart';
+import 'package:linter/src/linter.dart';
+
+const desc = r'Do not override fields.';
+
+const details = r'''
+
+**DO** Do not override fields.
+
+**BAD:**
+```
+class Base {
+  Object field = 'lorem';
+
+  Object something = 'change';
+}
+
+class Bad1 extends Base {
+  @override
+  final field = 'ipsum'; // LINT
+}
+
+class Bad2 extends Base {
+  @override
+  Object something = 'done'; // LINT
+}
+```
+
+**GOOD:**
+```
+class Base {
+  Object field = 'lorem';
+
+  Object something = 'change';
+}
+
+class Ok extends Base {
+  Object newField; // OK
+
+  final Object newFinal = 'ignore'; // OK
+}
+```
+
+''';
+
+class OverridenField extends LintRule {
+  OverridenField()
+      : super(
+            name: 'overriden_field',
+            description: desc,
+            details: details,
+            group: Group.style);
+
+  @override
+  AstVisitor getVisitor() => new _Visitor(this);
+}
+
+class _Visitor extends SimpleAstVisitor {
+  InheritanceManager _manager;
+
+  final LintRule rule;
+  _Visitor(this.rule);
+
+  @override
+  visitCompilationUnit(CompilationUnit node) {
+    LibraryElement library = node?.element?.library;
+    _manager = library == null ? null : new InheritanceManager(library);
+  }
+
+  @override
+  visitFieldDeclaration(FieldDeclaration node) {
+    void reportIfFinalFieldIsOverriden(VariableDeclaration variable) {
+      ExecutableElement member = _getOverriddenMember(variable.element);
+      final parentClass = member.enclosingElement as ClassElement;

[Brian Wilkerson, 2016/04/11 18:13:35]

I believe that member can be null if the variable is marked as overriding
something but doesn't:

class Super {}

class Sub extends Super {
  @override
  int y;
}

[Alexei Diaz, 2016/04/11 18:28:53]

Fixed

+      parentClass.fields.forEach((FieldElement field) {
+        if (field.name == node.fields.variables.first.name.name &&

[Brian Wilkerson, 2016/04/11 18:13:35]

"node.fields.variables.first"? Shouldn't that be "variable"? Otherwise I think
something like the following is broken:

class Super {
  int x, y;
}

class Sub extends Super {
  @override
  int y;
}

[Alexei Diaz, 2016/04/11 18:28:53]

Hopefully is fixed now, PTAL.

+            field.getter != null &&
+            field.setter != null) {
+          rule.reportLint(variable.name);
+        }
+      });
+    }
+
+    node.fields.variables
+        .where((v) => v?.element != null && v.element.isOverride)

[Brian Wilkerson, 2016/04/11 18:13:35]

Do you only want to catch cases where the field is marked as overriding an
inherited field? What about

class Super {
  int x;
}

class Sub extends Super {
  int x;
}

[Alexei Diaz, 2016/04/11 18:28:53]

I think
https://github.com/dart-lang/linter/blob/master/lib/src/rules/annotate_overri...
takes care of that case. Would it be better to add this verification there?

[Brian Wilkerson, 2016/04/11 18:55:31]

It's possible for clients to enable this lint rule without enabling the
annotate_overrides rule, so you can't depend on them using it to ensure that the
field is marked as overriding another. I think we really want this rule to catch
all violations and not depend on the extra information, but that does require
more computational time because then you have to check every field in every
class.

[Alexei Diaz, 2016/04/11 19:17:02]

Added the verification, my GUESS, is that the expensive computation was already
performed since I am only filtering, hopefully the extra cost is negligible.

+        .forEach(reportIfFinalFieldIsOverriden);
+  }
+
+  ExecutableElement _getOverriddenMember(Element member) {
+    if (member == null || _manager == null) {
+      return null;
+    }
+
+    ClassElement classElement =
+        member.getAncestor((element) => element is ClassElement);
+    if (classElement == null) {
+      return null;
+    }
+
+    return _manager.lookupInheritance(classElement, member.name);
+  }
+}