Skip to content

Commit

Permalink
Move more error detection out of Scope
Browse files Browse the repository at this point in the history 
R=scheglov@google.com

Review URL: https://codereview.chromium.org/2306223002 .
  • Loading branch information
@bwilkerson
bwilkerson committed Sep 3, 2016
1 parent 38482e5 commit 04d3701
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 63 deletions.
78 changes: 43 additions & 35 deletions pkg/analyzer/lib/src/dart/resolver/scope.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,49 @@ import 'package:analyzer/src/generated/java_core.dart';
import 'package:analyzer/src/generated/java_engine.dart';
import 'package:analyzer/src/generated/source.dart';

/**
* The scope defined by a block.
*/
class BlockScope extends EnclosedScope {
/**
* Initialize a newly created scope, enclosed within the [enclosingScope],
* based on the given [classElement].
*/
BlockScope(Scope enclosingScope, Block block) : super(enclosingScope) {
if (block == null) {
throw new IllegalArgumentException("block cannot be null");
}
_defineElements(block);
}

void _defineElements(Block block) {
for (Element element in elementsInBlock(block)) {
define(element);
}
}

/**
* Return the elements that are declared directly in the given [block]. This
* does not include elements declared in nested blocks.
*/
static Iterable<Element> elementsInBlock(Block block) sync* {
NodeList<Statement> statements = block.statements;
int statementCount = statements.length;
for (int i = 0; i < statementCount; i++) {
Statement statement = statements[i];
if (statement is VariableDeclarationStatement) {
NodeList<VariableDeclaration> variables = statement.variables.variables;
int variableCount = variables.length;
for (int j = 0; j < variableCount; j++) {
yield variables[j].element;
}
} else if (statement is FunctionDeclarationStatement) {
yield statement.functionDeclaration.element;
}
}
}
}

/**
* The scope defined by a class.
*/
Expand Down Expand Up @@ -80,13 +123,6 @@ class EnclosedScope extends Scope {
@override
final Scope enclosingScope;

/**
* A table mapping names that will be defined in this scope, but right now are
* not initialized. According to the scoping rules these names are hidden,
* even if they were defined in an outer scope.
*/
HashMap<String, Element> _hiddenElements = null;

/**
* Initialize a newly created scope, enclosed within the [enclosingScope].
*/
Expand All @@ -95,41 +131,13 @@ class EnclosedScope extends Scope {
@override
AnalysisErrorListener get errorListener => enclosingScope.errorListener;

/**
* Record that given [element] is declared in this scope, but hasn't been
* initialized yet, so it is error to use. If there is already an element with
* the given name defined in an outer scope, then it will become unavailable.
*/
void hide(Element element) {
if (element != null) {
String name = element.name;
if (name != null && !name.isEmpty) {
_hiddenElements ??= new HashMap<String, Element>();
_hiddenElements[name] = element;
}
}
}

@override
Element internalLookup(
Identifier identifier, String name, LibraryElement referencingLibrary) {
Element element = localLookup(name, referencingLibrary);
if (element != null) {
return element;
}
// May be there is a hidden Element.
if (_hiddenElements != null) {
Element hiddenElement = _hiddenElements[name];
if (hiddenElement != null) {
errorListener.onError(new AnalysisError(
getSource(identifier),
identifier.offset,
identifier.length,
CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION,
[name]));
return hiddenElement;
}
}
// Check enclosing scope.
return enclosingScope.internalLookup(identifier, name, referencingLibrary);
}
Expand Down
98 changes: 95 additions & 3 deletions pkg/analyzer/lib/src/generated/error_verifier.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
HashSet<String> _namesForReferenceToDeclaredVariableInInitializer =
new HashSet<String>();

/**
* The elements that will be defined later in the current scope, but right
* now are not declared.
*/
HiddenElements _hiddenElements = null;

/**
* A list of types used by the [CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS]
* and [CompileTimeErrorCode.IMPLEMENTS_DISALLOWED_CLASS] error codes.
Expand Down Expand Up @@ -388,8 +394,13 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {

@override
Object visitBlock(Block node) {
_checkDuplicateDeclarationInStatements(node.statements);
return super.visitBlock(node);
_hiddenElements = new HiddenElements(_hiddenElements, node);
try {
_checkDuplicateDeclarationInStatements(node.statements);
return super.visitBlock(node);
} finally {
_hiddenElements = _hiddenElements.outerElements;
}
}

@override
Expand Down Expand Up @@ -747,14 +758,19 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {

@override
Object visitFunctionDeclaration(FunctionDeclaration node) {
ExecutableElement functionElement = node.element;
if (functionElement != null &&
functionElement.enclosingElement is! CompilationUnitElement) {
_hiddenElements.declare(functionElement);
}
ExecutableElement outerFunction = _enclosingFunction;
try {
SimpleIdentifier identifier = node.name;
String methodName = "";
if (identifier != null) {
methodName = identifier.name;
}
_enclosingFunction = node.element;
_enclosingFunction = functionElement;
TypeName returnType = node.returnType;
if (node.isSetter || node.isGetter) {
_checkForMismatchedAccessorTypes(node, methodName);
Expand Down Expand Up @@ -1111,6 +1127,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {

@override
Object visitSimpleIdentifier(SimpleIdentifier node) {
_checkForReferenceBeforeDeclaration(node);
_checkForImplicitThisReferenceInInitializer(node);
if (!_isUnqualifiedReferenceToNonLocalStaticMemberAllowed(node)) {
_checkForUnqualifiedReferenceToNonLocalStaticMember(node);
Expand Down Expand Up @@ -1220,6 +1237,15 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
_isInInstanceVariableInitializer = wasInInstanceVariableInitializer;
_namesForReferenceToDeclaredVariableInInitializer.remove(name);
}
// declare the variable
AstNode grandparent = node.parent.parent;
if (grandparent is! TopLevelVariableDeclaration &&
grandparent is! FieldDeclaration) {
VariableElement element = node.element;
if (element != null) {
_hiddenElements.declare(element);
}
}
// done
return null;
}
Expand Down Expand Up @@ -5420,6 +5446,17 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
redirectedConstructorNode);
}

void _checkForReferenceBeforeDeclaration(SimpleIdentifier node) {
if (!node.inDeclarationContext() &&
_hiddenElements != null &&
_hiddenElements.contains(node.staticElement)) {
_errorReporter.reportErrorForNode(
CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION,
node,
[node.name]);
}
}

/**
* Check that the given rethrow [expression] is inside of a catch clause.
*
Expand Down Expand Up @@ -6610,6 +6647,61 @@ class GeneralizingElementVisitor_ErrorVerifier_hasTypedefSelfReference
}
}

/**
* A record of the elements that will be declared in some scope (block), but are
* not yet declared.
*/
class HiddenElements {
/**
* The elements hidden in outer scopes, or `null` if this is the outermost
* scope.
*/
final HiddenElements outerElements;

/**
* A set containing the elements that will be declared in this scope, but are
* not yet declared.
*/
Set<Element> _elements = new HashSet<Element>();

/**
* Initialize a newly created set of hidden elements to include all of the
* elements defined in the set of [outerElements] and all of the elements
* declared in the given [block].
*/
HiddenElements(this.outerElements, Block block) {
_initializeElements(block);
}

/**
* Return `true` if this set of elements contains the given [element].
*/
bool contains(Element element) {
if (_elements.contains(element)) {
return true;
} else if (outerElements != null) {
return outerElements.contains(element);
}
return false;
}

/**
* Record that the given [element] has been declared, so it is no longer
* hidden.
*/
void declare(Element element) {
_elements.remove(element);
}

/**
* Initialize the list of elements that are not yet declared to be all of the
* elements declared somewhere in the given [block].
*/
void _initializeElements(Block block) {
_elements.addAll(BlockScope.elementsInBlock(block));
}
}

/**
* A class used to compute a list of the constants whose value needs to be
* computed before errors can be computed by the [VerifyUnitTask].
Expand Down
32 changes: 7 additions & 25 deletions pkg/analyzer/lib/src/generated/resolver.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6608,7 +6608,7 @@ class ResolverVisitor extends ScopedVisitor {
var typeParamS =
futureThenType.returnType.flattenFutures(typeSystem);
returnType =
FutureUnionType.from(typeParamS, typeProvider, typeSystem);
FutureUnionType.from(typeParamS, typeProvider, typeSystem);
} else {
returnType = _computeReturnOrYieldType(functionType.returnType);
}
Expand Down Expand Up @@ -7782,8 +7782,7 @@ abstract class ScopedVisitor extends UnifyingAstVisitor<Object> {
Object visitBlock(Block node) {
Scope outerScope = nameScope;
try {
EnclosedScope enclosedScope = new EnclosedScope(nameScope);
_hideNamesDefinedInBlock(enclosedScope, node);
EnclosedScope enclosedScope = new BlockScope(nameScope, node);
nameScope = enclosedScope;
super.visitBlock(node);
} finally {
Expand Down Expand Up @@ -8307,28 +8306,6 @@ abstract class ScopedVisitor extends UnifyingAstVisitor<Object> {
}
return outerScope;
}

/**
* Marks the local declarations of the given [Block] hidden in the enclosing scope.
* According to the scoping rules name is hidden if block defines it, but name is defined after
* its declaration statement.
*/
void _hideNamesDefinedInBlock(EnclosedScope scope, Block block) {
NodeList<Statement> statements = block.statements;
int statementCount = statements.length;
for (int i = 0; i < statementCount; i++) {
Statement statement = statements[i];
if (statement is VariableDeclarationStatement) {
NodeList<VariableDeclaration> variables = statement.variables.variables;
int variableCount = variables.length;
for (int j = 0; j < variableCount; j++) {
scope.hide(variables[j].element);
}
} else if (statement is FunctionDeclarationStatement) {
scope.hide(statement.functionDeclaration.element);
}
}
}
}

/**
Expand Down Expand Up @@ -8829,6 +8806,11 @@ class TypeNameResolver {
parent is WithClause ||
parent is ClassTypeAlias) {
// Ignored. The error will be reported elsewhere.
} else if (element is LocalVariableElement ||
(element is FunctionElement &&
element.enclosingElement is ExecutableElement)) {
reportErrorForNode(CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION,
typeName, [typeName.name]);
} else {
reportErrorForNode(
StaticWarningCode.NOT_A_TYPE, typeName, [typeName.name]);
Expand Down
22 changes: 22 additions & 0 deletions pkg/analyzer/test/generated/compile_time_error_code_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5675,6 +5675,28 @@ main() {
assertErrors(source, [CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION]);
}

void test_referencedBeforeDeclaration_type_localFunction() {
Source source = addSource(r'''
void testTypeRef() {
String s = '';
int String(int x) => x + 1;
}
''');
computeLibrarySourceErrors(source);
assertErrors(source, [CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION]);
}

void test_referencedBeforeDeclaration_type_localVariable() {
Source source = addSource(r'''
void testTypeRef() {
String s = '';
var String = '';
}
''');
computeLibrarySourceErrors(source);
assertErrors(source, [CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION]);
}

void test_rethrowOutsideCatch() {
Source source = addSource(r'''
f() {
Expand Down

0 comments on commit 04d3701

Please sign in to comment.