languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 08:24:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

More improvements to DeclarationResolver.

Browse source 
This CL makes the following improvements:

- When visiting an executable element, local variables and labels are
  rebuilt rather than matching them up to the old element model.  This
  ensures that if the element model was resynthesized from an API
  summary (which doesn't contain local variables or labels), these
  elements are not lost.

- When matching up existing elements, if the elements did not
  previously contain offsets, offsets are recorded.  If they did
  previously contain offsets, the offsets are compared to verify that
  they match.  This ensures that if the element model was
  resynthesized from an API summary (which doesn't contain offsets),
  the offsets will be correct.

R=brianwilkerson@google.com, scheglov@google.com

Review URL: https://codereview.chromium.org/2435313002 .
This commit is contained in:
Paul Berry 2016-10-21 11:13:55 -07:00
parent f1aee7cf68
commit 396d7862b4
5 changed files with 215 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
pkg/analyzer/lib/src/dart/element/builder.dart
View file

@ -1078,8 +1078,11 @@ class LocalElementBuilder extends _BaseElementBuilder {
CompilationUnitElementImpl compilationUnitElement)
: super(initialHolder, compilationUnitElement);
@override
Object visitCatchClause(CatchClause node) {
/**
* Builds the variable elements associated with [node] and stores them in
* the element holder.
*/
void buildCatchVariableElements(CatchClause node) {
SimpleIdentifier exceptionParameter = node.exceptionParameter;
if (exceptionParameter != null) {
// exception
@ -1102,6 +1105,26 @@ class LocalElementBuilder extends _BaseElementBuilder {
stackTraceParameter.staticElement = stackTrace;
}
}
}
/**
* Builds the label elements associated with [labels] and stores them in the
* element holder.
*/
void buildLabelElements(
NodeList<Label> labels, bool onSwitchStatement, bool onSwitchMember) {
for (Label label in labels) {
SimpleIdentifier labelName = label.label;
LabelElementImpl element = new LabelElementImpl.forNode(
labelName, onSwitchStatement, onSwitchMember);
labelName.staticElement = element;
_currentHolder.addLabel(element);
}
}
@override
Object visitCatchClause(CatchClause node) {
buildCatchVariableElements(node);
return super.visitCatchClause(node);
}
@ -1226,37 +1249,19 @@ class LocalElementBuilder extends _BaseElementBuilder {
@override
Object visitLabeledStatement(LabeledStatement node) {
bool onSwitchStatement = node.statement is SwitchStatement;
for (Label label in node.labels) {
SimpleIdentifier labelName = label.label;
LabelElementImpl element =
new LabelElementImpl.forNode(labelName, onSwitchStatement, false);
_currentHolder.addLabel(element);
labelName.staticElement = element;
}
buildLabelElements(node.labels, onSwitchStatement, false);
return super.visitLabeledStatement(node);
}
@override
Object visitSwitchCase(SwitchCase node) {
for (Label label in node.labels) {
SimpleIdentifier labelName = label.label;
LabelElementImpl element =
new LabelElementImpl.forNode(labelName, false, true);
_currentHolder.addLabel(element);
labelName.staticElement = element;
}
buildLabelElements(node.labels, false, true);
return super.visitSwitchCase(node);
}
@override
Object visitSwitchDefault(SwitchDefault node) {
for (Label label in node.labels) {
SimpleIdentifier labelName = label.label;
LabelElementImpl element =
new LabelElementImpl.forNode(labelName, false, true);
_currentHolder.addLabel(element);
labelName.staticElement = element;
}
buildLabelElements(node.labels, false, true);
return super.visitSwitchDefault(node);
}

62
pkg/analyzer/lib/src/dart/element/element.dart
View file

@ -870,10 +870,11 @@ class ClassElementImpl extends AbstractClassElementImpl
@override
int get nameOffset {
if (_unlinkedClass != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedClass != null) {
return _unlinkedClass.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -1852,10 +1853,11 @@ class ConstFieldElementImpl_EnumValue extends ConstFieldElementImpl_ofEnum {
@override
int get nameOffset {
if (_unlinkedEnumValue != null) {
int offset = super.nameOffset;
if (offset == -1 && _unlinkedEnumValue != null) {
return _unlinkedEnumValue.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -3478,10 +3480,11 @@ class EnumElementImpl extends AbstractClassElementImpl {
@override
int get nameOffset {
if (_unlinkedEnum != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedEnum != null && _unlinkedEnum.nameOffset != 0) {
return _unlinkedEnum.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -3827,10 +3830,11 @@ abstract class ExecutableElementImpl extends ElementImpl
@override
int get nameOffset {
if (serializedExecutable != null) {
int offset = super.nameOffset;
if (offset == 0 && serializedExecutable != null) {
return serializedExecutable.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -4104,10 +4108,11 @@ class ExportElementImpl extends UriReferencedElementImpl
@override
int get nameOffset {
if (_unlinkedExportNonPublic != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedExportNonPublic != null) {
return _unlinkedExportNonPublic.offset;
}
return super.nameOffset;
return offset;
}
@override
@ -4677,10 +4682,11 @@ class FunctionTypeAliasElementImpl extends ElementImpl
@override
int get nameOffset {
if (_unlinkedTypedef != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedTypedef != null) {
return _unlinkedTypedef.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -5014,13 +5020,14 @@ class ImportElementImpl extends UriReferencedElementImpl
@override
int get nameOffset {
if (_unlinkedImport != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedImport != null) {
if (_unlinkedImport.isImplicit) {
return -1;
}
return _unlinkedImport.offset;
}
return super.nameOffset;
return offset;
}
PrefixElement get prefix {
@ -5223,10 +5230,13 @@ class LabelElementImpl extends ElementImpl implements LabelElement {
@override
int get nameOffset {
if (_unlinkedLabel != null) {
int offset = super.nameOffset;
if (offset == 0 &&
_unlinkedLabel != null &&
_unlinkedLabel.nameOffset != 0) {
return _unlinkedLabel.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -6764,10 +6774,11 @@ abstract class NonParameterVariableElementImpl extends VariableElementImpl {
@override
int get nameOffset {
if (_unlinkedVariable != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedVariable != null) {
return _unlinkedVariable.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -7068,13 +7079,14 @@ class ParameterElementImpl extends VariableElementImpl
@override
int get nameOffset {
if (_unlinkedParam != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedParam != null) {
if (isSynthetic) {
return -1;
}
return _unlinkedParam.nameOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -7390,10 +7402,11 @@ class PrefixElementImpl extends ElementImpl implements PrefixElement {
@override
int get nameOffset {
if (_unlinkedImport != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedImport != null) {
return _unlinkedImport.prefixOffset;
}
return super.nameOffset;
return offset;
}
@override
@ -8038,10 +8051,11 @@ class TypeParameterElementImpl extends ElementImpl
@override
int get nameOffset {
if (_unlinkedTypeParam != null) {
int offset = super.nameOffset;
if (offset == 0 && _unlinkedTypeParam != null) {
return _unlinkedTypeParam.nameOffset;
}
return super.nameOffset;
return offset;
}
TypeParameterType get type {

121
pkg/analyzer/lib/src/generated/declaration_resolver.dart
View file

@ -65,14 +65,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
@override
Object visitCatchClause(CatchClause node) {
SimpleIdentifier exceptionParameter = node.exceptionParameter;
if (exceptionParameter != null) {
_match(exceptionParameter, _walker.getVariable());
SimpleIdentifier stackTraceParameter = node.stackTraceParameter;
if (stackTraceParameter != null) {
_match(stackTraceParameter, _walker.getVariable());
}
}
_walker.elementBuilder.buildCatchVariableElements(node);
return super.visitCatchClause(node);
}
@ -98,8 +91,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
@override
Object visitConstructorDeclaration(ConstructorDeclaration node) {
ConstructorElement element = _match(node.name, _walker.getConstructor());
_walk(new ElementWalker.forExecutable(element), () {
ConstructorElement element = _match(node.name, _walker.getConstructor(),
offset: node.name?.offset ?? node.returnType.offset);
_walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
node.element = element;
super.visitConstructorDeclaration(node);
});
@ -109,9 +103,8 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
@override
Object visitDeclaredIdentifier(DeclaredIdentifier node) {
VariableElement element = _match(node.identifier, _walker.getVariable());
super.visitDeclaredIdentifier(node);
_resolveMetadata(node, node.metadata, element);
// Declared identifiers can only occur inside executable elements.
_walker.elementBuilder.visitDeclaredIdentifier(node);
return null;
}
@ -121,7 +114,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
_match(node.parameter.identifier, _walker.getParameter());
Expression defaultValue = node.defaultValue;
if (defaultValue != null) {
_walk(new ElementWalker.forExecutable(element.initializer), () {
_walk(
new ElementWalker.forExecutable(element.initializer, _enclosingUnit),
() {
defaultValue.accept(this);
});
}
@ -194,7 +189,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
}
}
node.functionExpression.element = element;
_walk(new ElementWalker.forExecutable(element), () {
_walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
super.visitFunctionDeclaration(node);
});
_resolveMetadata(node, node.metadata, element);
@ -205,8 +200,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
Object visitFunctionExpression(FunctionExpression node) {
if (node.parent is! FunctionDeclaration) {
FunctionElement element = _walker.getFunction();
_matchOffset(element, node.offset);
node.element = element;
_walk(new ElementWalker.forExecutable(element), () {
_walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
super.visitFunctionExpression(node);
});
return null;
@ -250,9 +246,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
@override
Object visitLabeledStatement(LabeledStatement node) {
for (Label label in node.labels) {
_match(label.label, _walker.getLabel());
}
bool onSwitchStatement = node.statement is SwitchStatement;
_walker.elementBuilder
.buildLabelElements(node.labels, onSwitchStatement, false);
return super.visitLabeledStatement(node);
}
@ -287,7 +283,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
elementName: nameOfMethod + '=');
}
}
_walk(new ElementWalker.forExecutable(element), () {
_walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
super.visitMethodDeclaration(node);
});
_resolveMetadata(node, node.metadata, element);
@ -325,17 +321,13 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
@override
Object visitSwitchCase(SwitchCase node) {
for (Label label in node.labels) {
_match(label.label, _walker.getLabel());
}
_walker.elementBuilder.buildLabelElements(node.labels, false, true);
return super.visitSwitchCase(node);
}
@override
Object visitSwitchDefault(SwitchDefault node) {
for (Label label in node.labels) {
_match(label.label, _walker.getLabel());
}
_walker.elementBuilder.buildLabelElements(node.labels, false, true);
return super.visitSwitchDefault(node);
}
@ -359,7 +351,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
VariableElement element = _match(node.name, _walker.getVariable());
Expression initializer = node.initializer;
if (initializer != null) {
_walk(new ElementWalker.forExecutable(element.initializer), () {
_walk(
new ElementWalker.forExecutable(element.initializer, _enclosingUnit),
() {
super.visitVariableDeclaration(node);
});
return null;
@ -370,12 +364,16 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
@override
Object visitVariableDeclarationList(VariableDeclarationList node) {
super.visitVariableDeclarationList(node);
if (node.parent is! FieldDeclaration &&
node.parent is! TopLevelVariableDeclaration) {
_resolveMetadata(node, node.metadata, node.variables[0].element);
if (_walker.elementBuilder != null) {
return _walker.elementBuilder.visitVariableDeclarationList(node);
} else {
super.visitVariableDeclarationList(node);
if (node.parent is! FieldDeclaration &&
node.parent is! TopLevelVariableDeclaration) {
_resolveMetadata(node, node.metadata, node.variables[0].element);
}
return null;
}
return null;
}
/**
@ -390,16 +388,26 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
*/
Element/*=E*/ _match/*<E extends Element>*/(
SimpleIdentifier identifier, Element/*=E*/ element,
{String elementName}) {
{String elementName, int offset}) {
elementName ??= identifier?.name ?? '';
offset ??= identifier.offset;
if (element.name != elementName) {
throw new StateError(
'Expected an element matching `$elementName`, got `${element.name}`');
}
identifier?.staticElement = element;
_matchOffset(element, offset);
return element;
}
void _matchOffset(Element element, int offset) {
if (element.nameOffset != 0 && element.nameOffset != offset) {
throw new StateError('Element offset mismatch');
} else {
(element as ElementImpl).nameOffset = offset;
}
}
/**
* Associate each of the annotation [nodes] with the corresponding
* [ElementAnnotation] in [annotations]. If there is a problem, report it
@ -461,6 +469,19 @@ class ElementWalker {
*/
final Element element;
/**
* If [element] is an executable element, an element builder which is
* accumulating the executable element's local variables and labels.
* Otherwise `null`.
*/
LocalElementBuilder elementBuilder;
/**
* If [element] is an executable element, the element holder associated with
* [elementBuilder]. Otherwise `null`.
*/
ElementHolder _elementHolder;
List<PropertyAccessorElement> _accessors;
int _accessorIndex = 0;
List<ClassElement> _classes;
@ -471,8 +492,6 @@ class ElementWalker {
int _enumIndex = 0;
List<ExecutableElement> _functions;
int _functionIndex = 0;
List<LabelElement> _labels;
int _labelIndex = 0;
List<ParameterElement> _parameters;
int _parameterIndex = 0;
List<FunctionTypeAliasElement> _typedefs;
@ -514,13 +533,9 @@ class ElementWalker {
* Creates an [ElementWalker] which walks the child elements of a compilation
* unit element.
*/
ElementWalker.forExecutable(ExecutableElement element)
: element = element,
_functions = element.functions,
_labels = element.labels,
_parameters = element.parameters,
_typeParameters = element.typeParameters,
_variables = element.localVariables;
ElementWalker.forExecutable(
ExecutableElement element, CompilationUnitElement compilationUnit)
: this._forExecutable(element, compilationUnit, new ElementHolder());
/**
* Creates an [ElementWalker] which walks the child elements of a parameter
@ -540,6 +555,16 @@ class ElementWalker {
_parameters = element.parameters,
_typeParameters = element.typeParameters;
ElementWalker._forExecutable(ExecutableElement element,
CompilationUnitElement compilationUnit, ElementHolder elementHolder)
: element = element,
elementBuilder =
new LocalElementBuilder(elementHolder, compilationUnit),
_elementHolder = elementHolder,
_functions = element.functions,
_parameters = element.parameters,
_typeParameters = element.typeParameters;
/**
* Returns the next non-synthetic child of [element] which is an accessor;
* throws an [IndexError] if there are no more.
@ -571,12 +596,6 @@ class ElementWalker {
*/
ExecutableElement getFunction() => _functions[_functionIndex++];
/**
* Returns the next non-synthetic child of [element] which is a label; throws
* an [IndexError] if there are no more.
*/
LabelElement getLabel() => _labels[_labelIndex++];
/**
* Returns the next non-synthetic child of [element] which is a parameter;
* throws an [IndexError] if there are no more.
@ -620,11 +639,15 @@ class ElementWalker {
check(_constructors, _constructorIndex);
check(_enums, _enumIndex);
check(_functions, _functionIndex);
check(_labels, _labelIndex);
check(_parameters, _parameterIndex);
check(_typedefs, _typedefIndex);
check(_typeParameters, _typeParameterIndex);
check(_variables, _variableIndex);
Element element = this.element;
if (element is ExecutableElementImpl) {
element.labels = _elementHolder.labels;
element.localVariables = _elementHolder.localVariables;
}
}
static bool _isNotSynthetic(Element e) => !e.isSynthetic;

77
pkg/analyzer/test/generated/declaration_resolver_test.dart
View file

@ -45,13 +45,16 @@ class DeclarationResolverMetadataTest extends ResolverTestCase {
CompilationUnit unit;
CompilationUnit unit2;
void checkMetadata(String search) {
void checkMetadata(String search, {bool expectDifferent: false}) {
NodeList<Annotation> metadata = _findMetadata(unit, search);
NodeList<Annotation> metadata2 = _findMetadata(unit2, search);
expect(metadata, isNotEmpty);
for (int i = 0; i < metadata.length; i++) {
expect(
metadata2[i].elementAnnotation, same(metadata[i].elementAnnotation));
Matcher expectation = same(metadata[i].elementAnnotation);
if (expectDifferent) {
expectation = isNot(expectation);
}
expect(metadata2[i].elementAnnotation, expectation);
}
}
@ -83,7 +86,7 @@ class DeclarationResolverMetadataTest extends ResolverTestCase {
void test_metadata_declaredIdentifier() {
setupCode('f(x, y) { for (@a var x in y) {} }');
checkMetadata('var');
checkMetadata('var', expectDifferent: true);
}
void test_metadata_enumDeclaration() {
@ -175,7 +178,7 @@ class DeclarationResolverMetadataTest extends ResolverTestCase {
void test_metadata_localVariableDeclaration() {
setupCode('f() { @a int x; }');
checkMetadata('x');
checkMetadata('x', expectDifferent: true);
}
void test_metadata_methodDeclaration_getter() {
@ -258,6 +261,70 @@ class DeclarationResolverTest extends ResolverTestCase {
super.setUp();
}
void test_closure_inside_catch_block() {
String code = '''
f() {
try {
} catch (e) {
return () => null;
}
}
''';
CompilationUnit unit = resolveSource(code);
// re-resolve
_cloneResolveUnit(unit);
// no other validations than built into DeclarationResolver
}
void test_closure_inside_labeled_statement() {
String code = '''
f(b) {
foo: while (true) {
if (b) {
break foo;
}
return () => null;
}
}
''';
CompilationUnit unit = resolveSource(code);
// re-resolve
_cloneResolveUnit(unit);
// no other validations than built into DeclarationResolver
}
void test_closure_inside_switch_case() {
String code = '''
void f(k, m) {
switch (k) {
case 0:
m.forEach((key, value) {});
break;
}
}
''';
CompilationUnit unit = resolveSource(code);
// re-resolve
_cloneResolveUnit(unit);
// no other validations than built into DeclarationResolver
}
void test_closure_inside_switch_default() {
String code = '''
void f(k, m) {
switch (k) {
default:
m.forEach((key, value) {});
break;
}
}
''';
CompilationUnit unit = resolveSource(code);
// re-resolve
_cloneResolveUnit(unit);
// no other validations than built into DeclarationResolver
}
void test_enumConstant_partiallyResolved() {
String code = r'''
enum Fruit {apple, pear}

6
pkg/analyzer/test/src/context/context_test.dart
View file

@ -5257,7 +5257,11 @@ class _ElementComparer extends GeneralizingElementVisitor {
@override
void visitElement(Element element) {
Element previousElement = previousElements[element];
if (!identical(previousElement, element)) {
bool expectIdentical = element is! LocalVariableElement;
bool ok = expectIdentical
? identical(previousElement, element)
: previousElement == element;
if (!ok) {
if (overwrittenCount == 0) {
buffer.writeln();
}