Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

Revert "unreachable_from_main: Report unreachable constructors (#4162)" #4253

Merged
pq merged 1 commit into from
Apr 3, 2023
Merged

Revert "unreachable_from_main: Report unreachable constructors (#4162)" #4253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 18 additions & 148 deletions lib/src/rules/unreachable_from_main.dart
View file
Open in desktop
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