Skip to content

Commit

Permalink
Revert "unreachable_from_main: Report unreachable constructors (#4162)…
Browse files Browse the repository at this point in the history 
…" (#4253)

This reverts commit 6447552.
  • Loading branch information
@pq
pq authored Apr 3, 2023
1 parent 1fde5fc commit b561905
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 433 deletions.
166 changes: 18 additions & 148 deletions lib/src/rules/unreachable_from_main.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@ import 'package:analyzer/dart/ast/ast.dart';
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.
Expand All @@ -44,7 +43,7 @@ void f() {}

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()
Expand Down Expand Up @@ -100,12 +99,7 @@ class _DeclarationGatherer {
}

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) {
Expand All @@ -121,72 +115,20 @@ class _DeclarationGatherer {
}
}

/// 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) {
_visitCompoundAssignmentExpression(node);
super.visitAssignmentExpression(node);
}

@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);
Expand All @@ -199,16 +141,6 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
super.visitPrefixExpression(node);
}

@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()) {
Expand All @@ -220,15 +152,6 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
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].
///
Expand All @@ -244,8 +167,8 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
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;
}
Expand All @@ -255,7 +178,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
}
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);
Expand All @@ -265,23 +188,6 @@ class _ReferenceVisitor extends RecursiveAstVisitor {
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);
}
}
}
Expand Down Expand Up @@ -335,14 +241,13 @@ class _Visitor extends SimpleAstVisitor<void> {
}
}

// 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;
}
Expand Down Expand Up @@ -371,25 +276,15 @@ class _Visitor extends SimpleAstVisitor<void> {
});

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);
}
}
}
Expand Down Expand Up @@ -428,28 +323,3 @@ extension on Annotation {
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 '';
}
}
Loading

0 comments on commit b561905

Please sign in to comment.