Chromium Code Reviews| OLD | NEW |
|---|---|
| (Empty) | |
| 1 // Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file | |
| 2 // for details. All rights reserved. Use of this source code is governed by a | |
| 3 // BSD-style license that can be found in the LICENSE file. | |
| 4 | |
| 5 library linter.src.rules.final_override_non_final; | |
|
pquitslund
2016/04/11 16:20:32
As per the previous comment, I think a rule name l
Alexei Diaz
2016/04/11 17:30:49
Done
| |
| 6 | |
| 7 import 'package:linter/src/linter.dart'; | |
|
pquitslund
2016/04/11 16:20:32
We generally sort our members. (Alphabetically.)
Alexei Diaz
2016/04/11 17:30:49
I guess I overlooked it because I took for granted
| |
| 8 import 'package:analyzer/dart/ast/visitor.dart'; | |
| 9 import 'package:analyzer/dart/ast/ast.dart'; | |
| 10 import 'package:analyzer/dart/element/element.dart'; | |
| 11 import 'package:analyzer/src/generated/resolver.dart'; | |
| 12 | |
| 13 const desc = r'Field overriden'; | |
|
pquitslund
2016/04/11 16:20:32
To stay consistent with the other lint description
Alexei Diaz
2016/04/11 17:30:49
Done
| |
| 14 | |
| 15 const details = r''' | |
| 16 | |
| 17 **DO** Do not override fields. | |
| 18 | |
| 19 **BAD:** | |
| 20 ``` | |
| 21 class Base { | |
| 22 Object field = 'lorem'; | |
| 23 | |
| 24 Object something = 'change'; | |
| 25 } | |
| 26 | |
| 27 class Bad1 extends Base { | |
| 28 @override | |
| 29 final field = 'ipsum'; // LINT | |
| 30 } | |
| 31 | |
| 32 class Bad2 extends Base { | |
| 33 @override | |
| 34 Object something = 'done'; // LINT | |
| 35 } | |
| 36 ``` | |
| 37 | |
| 38 **GOOD:** | |
| 39 ``` | |
| 40 class Base { | |
| 41 Object field = 'lorem'; | |
| 42 | |
| 43 Object something = 'change'; | |
| 44 } | |
| 45 | |
| 46 class Ok extends Base { | |
| 47 Object newField; // OK | |
| 48 | |
| 49 final Object newFinal = 'ignore'; // OK | |
| 50 } | |
| 51 ``` | |
| 52 | |
| 53 '''; | |
| 54 | |
| 55 class FinalOverrideNonFinal extends LintRule { | |
| 56 FinalOverrideNonFinal() | |
| 57 : super( | |
| 58 name: 'final_override_non_final', | |
| 59 description: desc, | |
| 60 details: details, | |
| 61 group: Group.style); | |
| 62 | |
| 63 @override | |
| 64 AstVisitor getVisitor() => new _Visitor(this); | |
| 65 } | |
| 66 | |
| 67 class _Visitor extends SimpleAstVisitor { | |
| 68 InheritanceManager _manager; | |
| 69 | |
| 70 final LintRule rule; | |
| 71 _Visitor(this.rule); | |
| 72 | |
| 73 @override | |
| 74 visitCompilationUnit(CompilationUnit node) { | |
| 75 LibraryElement library = node?.element?.library; | |
| 76 _manager = library == null ? null : new InheritanceManager(library); | |
| 77 } | |
| 78 | |
| 79 @override | |
| 80 visitFieldDeclaration(FieldDeclaration node) { | |
| 81 void reportIfFinalFieldIsOverriden(VariableDeclaration variable) { | |
| 82 ExecutableElement member = _getOverriddenMember(variable.element); | |
| 83 final parentClass = member.enclosingElement as ClassElement; | |
| 84 parentClass.fields.forEach((FieldElement field) { | |
| 85 if (field.name == node.fields.variables.first.name.name && | |
| 86 field.getter != null && | |
| 87 field.setter != null) { | |
| 88 rule.reportLint(variable.name); | |
| 89 } | |
| 90 }); | |
| 91 } | |
| 92 ; | |
|
pquitslund
2016/04/11 16:20:32
Unneeded `;`?
Alexei Diaz
2016/04/11 17:30:50
Done
| |
| 93 | |
| 94 node.fields.variables | |
| 95 .where((v) => v?.element != null && v.element.isOverride) | |
| 96 .forEach(reportIfFinalFieldIsOverriden); | |
| 97 } | |
| 98 | |
| 99 ExecutableElement _getOverriddenMember(Element member) { | |
| 100 if (member == null || _manager == null) { | |
| 101 return null; | |
| 102 } | |
| 103 | |
| 104 ClassElement classElement = | |
| 105 member.getAncestor((element) => element is ClassElement); | |
| 106 if (classElement == null) { | |
| 107 return null; | |
| 108 } | |
| 109 | |
| 110 return _manager.lookupInheritance(classElement, member.name); | |
| 111 } | |
| 112 } | |
| OLD | NEW |