Skip to content

Commit ed16c21

Browse files
authored
Revert "Revert "unreachable_from_main: Report unreachable constructors (dart-archive/linter#4162)" (dart-archive/linter#4253)" (dart-archive/linter#4337)
This reverts commit ed1dc5d. Also: Add support for test_reflective_loader
1 parent 5878cde commit ed16c21

File tree

2 files changed

+483
-20
lines changed

2 files changed

+483
-20
lines changed

lib/src/rules/unreachable_from_main.dart

Lines changed: 167 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,16 @@ import 'package:analyzer/dart/ast/ast.dart';
88
import 'package:analyzer/dart/ast/visitor.dart';
99
import 'package:analyzer/dart/element/element.dart';
1010
import 'package:analyzer/dart/element/type.dart';
11+
import 'package:collection/collection.dart';
1112

1213
import '../analyzer.dart';
1314

1415
const _desc = 'Unreachable top-level members in executable libraries.';
1516

1617
const _details = r'''
17-
Top-level members in an executable library should be used directly inside this
18-
library. An executable library is a library that contains a `main` top-level
19-
function or that contains a top-level function annotated with
18+
Top-level members and static members in an executable library should be used
19+
directly inside this library. An executable library is a library that contains
20+
a `main` top-level function or that contains a top-level function annotated with
2021
`@pragma('vm:entry-point')`). Executable libraries are not usually imported
2122
and it's better to avoid defining unused members.
2223
@@ -43,7 +44,7 @@ void f() {}
4344

4445
class UnreachableFromMain extends LintRule {
4546
static const LintCode code = LintCode('unreachable_from_main',
46-
'Unreachable top-level member in an executable library.',
47+
"Unreachable member '{0}' in an executable library.",
4748
correctionMessage: 'Try referencing the member or removing it.');
4849

4950
UnreachableFromMain()
@@ -99,7 +100,12 @@ class _DeclarationGatherer {
99100
}
100101

101102
void _addStaticMember(ClassMember member) {
102-
if (member is FieldDeclaration && member.isStatic) {
103+
if (member is ConstructorDeclaration) {
104+
var e = member.declaredElement;
105+
if (e != null && e.isPublic && member.parent is! EnumDeclaration) {
106+
declarations.add(member);
107+
}
108+
} else if (member is FieldDeclaration && member.isStatic) {
103109
for (var field in member.fields.variables) {
104110
var e = field.declaredElement;
105111
if (e != null && e.isPublic) {
@@ -115,20 +121,90 @@ class _DeclarationGatherer {
115121
}
116122
}
117123

118-
/// A visitor which gathers the declarations of the identifiers it visits.
119-
class _IdentifierVisitor extends RecursiveAstVisitor {
124+
/// A visitor which gathers the declarations of the "references" it visits.
125+
///
126+
/// "References" are most often [SimpleIdentifier]s, but can also be other
127+
/// nodes which refer to a declaration.
128+
// TODO(srawlins): Add support for patterns.
129+
class _ReferenceVisitor extends RecursiveAstVisitor {
120130
Map<Element, Declaration> declarationMap;
121131

122132
Set<Declaration> declarations = {};
123133

124-
_IdentifierVisitor(this.declarationMap);
134+
_ReferenceVisitor(this.declarationMap);
135+
136+
@override
137+
void visitAnnotation(Annotation node) {
138+
var e = node.element;
139+
if (e != null) {
140+
_addDeclaration(e);
141+
}
142+
super.visitAnnotation(node);
143+
}
125144

126145
@override
127146
void visitAssignmentExpression(AssignmentExpression node) {
128147
_visitCompoundAssignmentExpression(node);
129148
super.visitAssignmentExpression(node);
130149
}
131150

151+
@override
152+
void visitClassDeclaration(ClassDeclaration node) {
153+
var element = node.declaredElement;
154+
if (element != null) {
155+
var hasConstructors =
156+
node.members.any((e) => e is ConstructorDeclaration);
157+
if (!hasConstructors) {
158+
// The default constructor will have an implicit super-initializer to
159+
// the super-type's unnamed constructor.
160+
_addDefaultSuperConstructorDeclaration(node);
161+
}
162+
163+
var metadata = element.metadata;
164+
// This for-loop style is copied from analyzer's `hasX` getters on Element.
165+
for (var i = 0; i < metadata.length; i++) {
166+
var annotation = metadata[i].element;
167+
if (annotation is PropertyAccessorElement &&
168+
annotation.name == 'reflectiveTest' &&
169+
annotation.library.name == 'test_reflective_loader') {
170+
// The class is instantiated through the use of mirrors in
171+
// 'test_reflective_loader'.
172+
var unnamedConstructor = element.constructors
173+
.firstWhereOrNull((constructor) => constructor.name == '');
174+
if (unnamedConstructor != null) {
175+
_addDeclaration(unnamedConstructor);
176+
}
177+
}
178+
}
179+
}
180+
super.visitClassDeclaration(node);
181+
}
182+
183+
@override
184+
void visitConstructorDeclaration(ConstructorDeclaration node) {
185+
// If a constructor does not have an explicit super-initializer (or
186+
// redirection?) then it has an implicit super-initializer to the
187+
// super-type's unnamed constructor.
188+
var hasSuperInitializer =
189+
node.initializers.any((e) => e is SuperConstructorInvocation);
190+
if (!hasSuperInitializer) {
191+
var enclosingClass = node.parent;
192+
if (enclosingClass is ClassDeclaration) {
193+
_addDefaultSuperConstructorDeclaration(enclosingClass);
194+
}
195+
}
196+
super.visitConstructorDeclaration(node);
197+
}
198+
199+
@override
200+
void visitConstructorName(ConstructorName node) {
201+
var e = node.staticElement;
202+
if (e != null) {
203+
_addDeclaration(e);
204+
}
205+
super.visitConstructorName(node);
206+
}
207+
132208
@override
133209
void visitPostfixExpression(PostfixExpression node) {
134210
_visitCompoundAssignmentExpression(node);
@@ -141,6 +217,16 @@ class _IdentifierVisitor extends RecursiveAstVisitor {
141217
super.visitPrefixExpression(node);
142218
}
143219

220+
@override
221+
void visitRedirectingConstructorInvocation(
222+
RedirectingConstructorInvocation node) {
223+
var element = node.staticElement;
224+
if (element != null) {
225+
_addDeclaration(element);
226+
}
227+
super.visitRedirectingConstructorInvocation(node);
228+
}
229+
144230
@override
145231
void visitSimpleIdentifier(SimpleIdentifier node) {
146232
if (!node.inDeclarationContext()) {
@@ -152,6 +238,15 @@ class _IdentifierVisitor extends RecursiveAstVisitor {
152238
super.visitSimpleIdentifier(node);
153239
}
154240

241+
@override
242+
void visitSuperConstructorInvocation(SuperConstructorInvocation node) {
243+
var e = node.staticElement;
244+
if (e != null) {
245+
_addDeclaration(e);
246+
}
247+
super.visitSuperConstructorInvocation(node);
248+
}
249+
155250
/// Adds the declaration of the top-level element which contains [element] to
156251
/// [declarations], if it is found in [declarationMap].
157252
///
@@ -167,8 +262,8 @@ class _IdentifierVisitor extends RecursiveAstVisitor {
167262
declarations.add(enclosingTopLevelDeclaration);
168263
}
169264

170-
// Also add [element]'s declaration if it is a static accessor or static
171-
// method.
265+
// Also add [element]'s declaration if it is a constructor, static accessor,
266+
// or static method.
172267
if (element.isPrivate) {
173268
return;
174269
}
@@ -178,7 +273,7 @@ class _IdentifierVisitor extends RecursiveAstVisitor {
178273
}
179274
if (enclosingElement is InterfaceElement ||
180275
enclosingElement is ExtensionElement) {
181-
if (element is PropertyAccessorElement && element.isStatic) {
276+
if (element is ConstructorElement) {
182277
var declaration = declarationMap[element];
183278
if (declaration != null) {
184279
declarations.add(declaration);
@@ -188,6 +283,23 @@ class _IdentifierVisitor extends RecursiveAstVisitor {
188283
if (declaration != null) {
189284
declarations.add(declaration);
190285
}
286+
} else if (element is PropertyAccessorElement && element.isStatic) {
287+
var declaration = declarationMap[element];
288+
if (declaration != null) {
289+
declarations.add(declaration);
290+
}
291+
}
292+
}
293+
}
294+
295+
void _addDefaultSuperConstructorDeclaration(ClassDeclaration class_) {
296+
var classElement = class_.declaredElement;
297+
var supertype = classElement?.supertype;
298+
if (supertype != null) {
299+
var unnamedConstructor =
300+
supertype.constructors.firstWhereOrNull((e) => e.name.isEmpty);
301+
if (unnamedConstructor != null) {
302+
_addDeclaration(unnamedConstructor);
191303
}
192304
}
193305
}
@@ -241,13 +353,14 @@ class _Visitor extends SimpleAstVisitor<void> {
241353
}
242354
}
243355

244-
// The set of the declarations which each top-level declaration references.
356+
// The set of the declarations which each top-level and static declaration
357+
// references.
245358
var dependencies = <Declaration, Set<Declaration>>{};
246359

247360
// Map each declaration to the collection of declarations which are
248361
// referenced within its body.
249362
for (var declaration in declarations) {
250-
var visitor = _IdentifierVisitor(declarationByElement);
363+
var visitor = _ReferenceVisitor(declarationByElement);
251364
declaration.accept(visitor);
252365
dependencies[declaration] = visitor.declarations;
253366
}
@@ -276,15 +389,25 @@ class _Visitor extends SimpleAstVisitor<void> {
276389
});
277390

278391
for (var member in unusedMembers) {
279-
if (member is NamedCompilationUnitMember) {
280-
rule.reportLintForToken(member.name);
392+
if (member is ConstructorDeclaration) {
393+
if (member.name == null) {
394+
rule.reportLint(member.returnType, arguments: [member.nameForError]);
395+
} else {
396+
rule.reportLintForToken(member.name,
397+
arguments: [member.nameForError]);
398+
}
399+
} else if (member is NamedCompilationUnitMember) {
400+
rule.reportLintForToken(member.name, arguments: [member.nameForError]);
281401
} else if (member is VariableDeclaration) {
282-
rule.reportLintForToken(member.name);
402+
rule.reportLintForToken(member.name, arguments: [member.nameForError]);
283403
} else if (member is ExtensionDeclaration) {
404+
var memberName = member.name;
284405
rule.reportLintForToken(
285-
member.name ?? member.firstTokenAfterCommentAndMetadata);
406+
memberName ?? member.firstTokenAfterCommentAndMetadata,
407+
arguments: [member.nameForError]);
286408
} else {
287-
rule.reportLintForToken(member.firstTokenAfterCommentAndMetadata);
409+
rule.reportLintForToken(member.firstTokenAfterCommentAndMetadata,
410+
arguments: [member.nameForError]);
288411
}
289412
}
290413
}
@@ -323,3 +446,29 @@ extension on Annotation {
323446
return type is InterfaceType && type.element.isPragma;
324447
}
325448
}
449+
450+
extension on Declaration {
451+
String get nameForError {
452+
// TODO(srawlins): Move this to analyzer when other uses are found.
453+
// TODO(srawlins): Convert to switch-expression, hopefully.
454+
var self = this;
455+
if (self is ConstructorDeclaration) {
456+
var name = self.name?.lexeme ?? 'new';
457+
return '${self.returnType.name}.$name';
458+
} else if (self is EnumConstantDeclaration) {
459+
return self.name.lexeme;
460+
} else if (self is ExtensionDeclaration) {
461+
var name = self.name;
462+
return name?.lexeme ?? 'the unnamed extension';
463+
} else if (self is MethodDeclaration) {
464+
return self.name.lexeme;
465+
} else if (self is NamedCompilationUnitMember) {
466+
return self.name.lexeme;
467+
} else if (self is VariableDeclaration) {
468+
return self.name.lexeme;
469+
}
470+
471+
assert(false, 'Uncovered Declaration subtype: ${self.runtimeType}');
472+
return '';
473+
}
474+
}

0 commit comments

Comments
 (0)