Revert "unreachable_from_main: Report unreachable constructors (#4162)" (#4253)
This reverts commit 644755218f410b4a68e801d0ba0be9788b42c89a.
diff --git a/lib/src/rules/unreachable_from_main.dart b/lib/src/rules/unreachable_from_main.dart
index 1de5135..8acb591 100644
--- a/lib/src/rules/unreachable_from_main.dart
+++ b/lib/src/rules/unreachable_from_main.dart
@@ -8,16 +8,15 @@
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
-import 'package:collection/collection.dart';
import '../analyzer.dart';
const _desc = 'Unreachable top-level members in executable libraries.';
const _details = r'''
-Top-level members and static members in an executable library should be used
-directly inside this library. An executable library is a library that contains
-a `main` top-level function or that contains a top-level function annotated with
+Top-level members in an executable library should be used directly inside this
+library. An executable library is a library that contains a `main` top-level
+function or that contains a top-level function annotated with
`@pragma('vm:entry-point')`). Executable libraries are not usually imported
and it's better to avoid defining unused members.
@@ -44,7 +43,7 @@
class UnreachableFromMain extends LintRule {
static const LintCode code = LintCode('unreachable_from_main',
- "Unreachable member '{0}' in an executable library.",
+ 'Unreachable top-level member in an executable library.',
correctionMessage: 'Try referencing the member or removing it.');
UnreachableFromMain()
@@ -100,12 +99,7 @@
}
void _addStaticMember(ClassMember member) {
- if (member is ConstructorDeclaration) {
- var e = member.declaredElement;
- if (e != null && e.isPublic && member.parent is! EnumDeclaration) {
- declarations.add(member);
- }
- } else if (member is FieldDeclaration && member.isStatic) {
+ if (member is FieldDeclaration && member.isStatic) {
for (var field in member.fields.variables) {
var e = field.declaredElement;
if (e != null && e.isPublic) {
@@ -121,26 +115,13 @@
}
}
-/// A visitor which gathers the declarations of the "references" it visits.
-///
-/// "References" are most often [SimpleIdentifier]s, but can also be other
-/// nodes which refer to a declaration.
-// TODO(srawlins): Add support for patterns.
-class _ReferenceVisitor extends RecursiveAstVisitor {
+/// A visitor which gathers the declarations of the identifiers it visits.
+class _IdentifierVisitor extends RecursiveAstVisitor {
Map<Element, Declaration> declarationMap;
Set<Declaration> declarations = {};
- _ReferenceVisitor(this.declarationMap);
-
- @override
- void visitAnnotation(Annotation node) {
- var e = node.element;
- if (e != null) {
- _addDeclaration(e);
- }
- super.visitAnnotation(node);
- }
+ _IdentifierVisitor(this.declarationMap);
@override
void visitAssignmentExpression(AssignmentExpression node) {
@@ -149,45 +130,6 @@
}
@override
- void visitClassDeclaration(ClassDeclaration node) {
- var element = node.declaredElement;
- if (element != null) {
- var hasConstructors =
- node.members.any((e) => e is ConstructorDeclaration);
- if (!hasConstructors) {
- // The default constructor will have an implicit super-initializer to
- // the super-type's unnamed constructor.
- _addDefaultSuperConstructorDeclaration(node);
- }
- }
- super.visitClassDeclaration(node);
- }
-
- @override
- void visitConstructorDeclaration(ConstructorDeclaration node) {
- // If a constructor does not have an explicit super-initializer (or redirection?)
- // then it has an implicit super-initializer to the super-type's unnamed constructor.
- var hasSuperInitializer =
- node.initializers.any((e) => e is SuperConstructorInvocation);
- if (!hasSuperInitializer) {
- var enclosingClass = node.parent;
- if (enclosingClass is ClassDeclaration) {
- _addDefaultSuperConstructorDeclaration(enclosingClass);
- }
- }
- super.visitConstructorDeclaration(node);
- }
-
- @override
- void visitConstructorName(ConstructorName node) {
- var e = node.staticElement;
- if (e != null) {
- _addDeclaration(e);
- }
- super.visitConstructorName(node);
- }
-
- @override
void visitPostfixExpression(PostfixExpression node) {
_visitCompoundAssignmentExpression(node);
super.visitPostfixExpression(node);
@@ -200,16 +142,6 @@
}
@override
- void visitRedirectingConstructorInvocation(
- RedirectingConstructorInvocation node) {
- var element = node.staticElement;
- if (element != null) {
- _addDeclaration(element);
- }
- super.visitRedirectingConstructorInvocation(node);
- }
-
- @override
void visitSimpleIdentifier(SimpleIdentifier node) {
if (!node.inDeclarationContext()) {
var e = node.staticElement;
@@ -220,15 +152,6 @@
super.visitSimpleIdentifier(node);
}
- @override
- void visitSuperConstructorInvocation(SuperConstructorInvocation node) {
- var e = node.staticElement;
- if (e != null) {
- _addDeclaration(e);
- }
- super.visitSuperConstructorInvocation(node);
- }
-
/// Adds the declaration of the top-level element which contains [element] to
/// [declarations], if it is found in [declarationMap].
///
@@ -244,8 +167,8 @@
declarations.add(enclosingTopLevelDeclaration);
}
- // Also add [element]'s declaration if it is a constructor, static accessor,
- // or static method.
+ // Also add [element]'s declaration if it is a static accessor or static
+ // method.
if (element.isPrivate) {
return;
}
@@ -255,7 +178,7 @@
}
if (enclosingElement is InterfaceElement ||
enclosingElement is ExtensionElement) {
- if (element is ConstructorElement) {
+ if (element is PropertyAccessorElement && element.isStatic) {
var declaration = declarationMap[element];
if (declaration != null) {
declarations.add(declaration);
@@ -265,23 +188,6 @@
if (declaration != null) {
declarations.add(declaration);
}
- } else if (element is PropertyAccessorElement && element.isStatic) {
- var declaration = declarationMap[element];
- if (declaration != null) {
- declarations.add(declaration);
- }
- }
- }
- }
-
- void _addDefaultSuperConstructorDeclaration(ClassDeclaration class_) {
- var classElement = class_.declaredElement;
- var supertype = classElement?.supertype;
- if (supertype != null) {
- var unnamedConstructor =
- supertype.constructors.firstWhereOrNull((e) => e.name.isEmpty);
- if (unnamedConstructor != null) {
- _addDeclaration(unnamedConstructor);
}
}
}
@@ -335,14 +241,13 @@
}
}
- // The set of the declarations which each top-level and static declaration
- // references.
+ // The set of the declarations which each top-level declaration references.
var dependencies = <Declaration, Set<Declaration>>{};
// Map each declaration to the collection of declarations which are
// referenced within its body.
for (var declaration in declarations) {
- var visitor = _ReferenceVisitor(declarationByElement);
+ var visitor = _IdentifierVisitor(declarationByElement);
declaration.accept(visitor);
dependencies[declaration] = visitor.declarations;
}
@@ -371,25 +276,15 @@
});
for (var member in unusedMembers) {
- if (member is ConstructorDeclaration) {
- if (member.name == null) {
- rule.reportLint(member.returnType, arguments: [member.nameForError]);
- } else {
- rule.reportLintForToken(member.name,
- arguments: [member.nameForError]);
- }
- } else if (member is NamedCompilationUnitMember) {
- rule.reportLintForToken(member.name, arguments: [member.nameForError]);
+ if (member is NamedCompilationUnitMember) {
+ rule.reportLintForToken(member.name);
} else if (member is VariableDeclaration) {
- rule.reportLintForToken(member.name, arguments: [member.nameForError]);
+ rule.reportLintForToken(member.name);
} else if (member is ExtensionDeclaration) {
- var memberName = member.name;
rule.reportLintForToken(
- memberName ?? member.firstTokenAfterCommentAndMetadata,
- arguments: [member.nameForError]);
+ member.name ?? member.firstTokenAfterCommentAndMetadata);
} else {
- rule.reportLintForToken(member.firstTokenAfterCommentAndMetadata,
- arguments: [member.nameForError]);
+ rule.reportLintForToken(member.firstTokenAfterCommentAndMetadata);
}
}
}
@@ -428,28 +323,3 @@
return type is InterfaceType && type.element.isPragma;
}
}
-
-extension on Declaration {
- String get nameForError {
- // TODO(srawlins): Move this to analyzer when other uses are found.
- // TODO(srawlins): Convert to switch-expression, hopefully.
- var self = this;
- if (self is ConstructorDeclaration) {
- return self.name?.lexeme ?? self.returnType.name;
- } else if (self is EnumConstantDeclaration) {
- return self.name.lexeme;
- } else if (self is ExtensionDeclaration) {
- var name = self.name;
- return name?.lexeme ?? 'the unnamed extension';
- } else if (self is MethodDeclaration) {
- return self.name.lexeme;
- } else if (self is NamedCompilationUnitMember) {
- return self.name.lexeme;
- } else if (self is VariableDeclaration) {
- return self.name.lexeme;
- }
-
- assert(false, 'Uncovered Declaration subtype: ${self.runtimeType}');
- return '';
- }
-}
diff --git a/test/rules/unreachable_from_main_test.dart b/test/rules/unreachable_from_main_test.dart
index 0ffcab8..9e47bd2 100644
--- a/test/rules/unreachable_from_main_test.dart
+++ b/test/rules/unreachable_from_main_test.dart
@@ -9,6 +9,7 @@
main() {
defineReflectiveSuite(() {
// TODO(srawlins): Add tests for unreachable public constructors.
+ // TODO(srawlins): Add tests for errors that should be reported in parts.
defineReflectiveTests(UnreachableFromMainTest);
});
}
@@ -116,10 +117,6 @@
}
''', [
lint(22, 1),
- // TODO(srawlins): See if we can skip reporting a declaration if it's
- //enclosing declaration is being reported.
- lint(28, 1),
- lint(37, 5),
]);
}
@@ -194,287 +191,6 @@
]);
}
- test_constructor_named_onEnum() async {
- await assertDiagnostics(r'''
-void main() {
- E.one;
- E.two;
-}
-
-enum E {
- one(), two();
-
- const E();
- const E.named();
-}
-''', [
- // No lint.
- error(HintCode.UNUSED_ELEMENT, 84, 5),
- ]);
- }
-
- test_constructor_named_reachableViaDirectCall() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.named();
-}
-
-class C {
- C.named();
-}
-''');
- }
-
- test_constructor_named_reachableViaExplicitSuperCall() async {
- await assertNoDiagnostics(r'''
-void main() {
- D();
-}
-
-class C {
- C.named();
-}
-
-class D extends C {
- D() : super.named();
-}
-''');
- }
-
- test_constructor_named_reachableViaRedirectedConstructor() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.two();
-}
-
-class C {
- C.named();
- factory C.two() = C.named;
-}
-''');
- }
-
- test_constructor_named_reachableViaRedirection() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.two();
-}
-
-class C {
- C.named();
-
- C.two() : this.named();
-}
-''');
- }
-
- test_constructor_named_reachableViaTearoff() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.named;
-}
-
-class C {
- C.named();
-}
-''');
- }
-
- test_constructor_named_unreachable() async {
- await assertDiagnostics(r'''
-void main() {
- C;
-}
-
-class C {
- C.named();
-}
-''', [
- lint(36, 5),
- ]);
- }
-
- test_constructor_named_unreachable_otherHasRedirectedConstructor() async {
- await assertDiagnostics(r'''
-void main() {
- C.two();
-}
-
-class C {
- C.named();
- C.one();
- factory C.two() = C.one;
-}
-''', [
- lint(42, 5),
- ]);
- }
-
- test_constructor_unnamed_reachableViaDefaultImplicitSuperCall() async {
- await assertNoDiagnostics(r'''
-void main() {
- D();
-}
-
-class C {
- C();
-}
-
-class D extends C {
- // Just a default constructor.
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaDirectCall() async {
- await assertNoDiagnostics(r'''
-void main() {
- C();
-}
-
-class C {
- C();
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaExplicitSuperCall() async {
- await assertNoDiagnostics(r'''
-void main() {
- D();
-}
-
-class C {
- C();
-}
-
-class D extends C {
- D() : super();
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaImplicitSuperCall() async {
- await assertNoDiagnostics(r'''
-void main() {
- D();
-}
-
-class C {
- C();
-}
-
-class D extends C {
- D();
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaImplicitSuperCall_indirectly() async {
- await assertNoDiagnostics(r'''
-void main() {
- E();
-}
-
-class C {
- C();
-}
-
-class D extends C {
- // Just a default constructor.
-}
-
-class E extends D {
- E();
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaImplicitSuperCall_superParameters() async {
- await assertNoDiagnostics(r'''
-void main() {
- D(1);
-}
-
-class C {
- C(int p);
-}
-
-class D extends C {
- D(super.p);
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaRedirectedConstructor() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.two();
-}
-
-class C {
- C();
- factory C.two() = C;
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaRedirection() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.two();
-}
-
-class C {
- C();
-
- C.two() : this();
-}
-''');
- }
-
- test_constructor_unnamed_reachableViaTearoff() async {
- await assertNoDiagnostics(r'''
-void main() {
- C.new;
-}
-
-class C {
- C();
-}
-''');
- }
-
- test_constructor_unnamed_unreachable() async {
- await assertDiagnostics(r'''
-void main() {
- C;
-}
-
-class C {
- C();
-}
-''', [
- lint(34, 1),
- ]);
- }
-
- test_constructor_unnamed_unreachable_otherHasRedirection() async {
- await assertDiagnostics(r'''
-void main() {
- C.two();
-}
-
-class C {
- C();
- C.one();
- C.two() : this.one();
-}
-''', [
- lint(40, 1),
- ]);
- }
-
test_enum_reachableViaValue() async {
await assertNoDiagnostics(r'''
void main() {