Skip to content

Commit

Permalink
split AstBuilder endClassConstructor out of endClassMethod event
Browse files Browse the repository at this point in the history
This CL moves the logic determining whether a declaration is a method
or constructor into the parser and splits the AstBuilder endClassMethod
event handler into simplified endClassConstructor and endClassMethod event handlers

Change-Id: I2d2124a2456f70d5511163cead25f6c586ea9c71
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116240
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
danrubel authored and commit-bot@chromium.org committed Sep 9, 2019
1 parent b6f6240 commit 355df2a
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 95 deletions.
187 changes: 107 additions & 80 deletions pkg/analyzer/lib/src/fasta/ast_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1574,11 +1574,11 @@ class AstBuilder extends StackListener {
assert(getOrSet == null ||
optional('get', getOrSet) ||
optional('set', getOrSet));
debugEvent("Method");
debugEvent("ClassMethod");

var bodyObject = pop();
List<ConstructorInitializer> initializers = pop() ?? const [];
Token separator = pop();
pop(); // initializers
pop(); // separator
FormalParameterList parameters = pop();
TypeParameterList typeParameters = pop();
var name = pop();
Expand All @@ -1589,96 +1589,45 @@ class AstBuilder extends StackListener {

assert(parameters != null || optional('get', getOrSet));

ConstructorName redirectedConstructor;
FunctionBody body;
if (bodyObject is FunctionBody) {
body = bodyObject;
} else if (bodyObject is _RedirectingFactoryBody) {
separator = bodyObject.equalToken;
redirectedConstructor = bodyObject.constructorName;
body = ast.emptyFunctionBody(endToken);
} else {
unhandled("${bodyObject.runtimeType}", "bodyObject",
beginToken.charOffset, uri);
}

void constructor(
SimpleIdentifier prefixOrName, Token period, SimpleIdentifier name) {
if (typeParameters != null) {
// Outline builder also reports this error message.
handleRecoverableError(messageConstructorWithTypeParameters,
typeParameters.beginToken, typeParameters.endToken);
}
if (modifiers?.constKeyword != null &&
body != null &&
(body.length > 1 || body.beginToken?.lexeme != ';')) {
// This error is also reported in BodyBuilder.finishFunction
Token bodyToken = body.beginToken ?? modifiers.constKeyword;
handleRecoverableError(
messageConstConstructorWithBody, bodyToken, bodyToken);
}
if (returnType != null) {
// This error is also reported in OutlineBuilder.endMethod
handleRecoverableError(messageConstructorWithReturnType,
returnType.beginToken, returnType.beginToken);
}
ConstructorDeclaration constructor = ast.constructorDeclaration(
comment,
metadata,
modifiers?.externalKeyword,
modifiers?.finalConstOrVarKeyword,
null,
// TODO(paulberry): factoryKeyword
ast.simpleIdentifier(prefixOrName.token),
period,
name,
parameters,
separator,
initializers,
redirectedConstructor,
body);
currentDeclarationMembers.add(constructor);
if (mixinDeclaration != null) {
// TODO (danrubel): Report an error if this is a mixin declaration.
}
}

void method(Token operatorKeyword, SimpleIdentifier name) {
if (modifiers?.constKeyword != null) {
// This error is also reported in OutlineBuilder.endMethod
handleRecoverableError(
messageConstMethod, modifiers.constKeyword, modifiers.constKeyword);
}
checkFieldFormalParameters(parameters);
currentDeclarationMembers.add(ast.methodDeclaration(
comment,
metadata,
modifiers?.externalKeyword,
modifiers?.abstractKeyword ?? modifiers?.staticKeyword,
returnType,
getOrSet,
operatorKeyword,
name,
typeParameters,
parameters,
body));
}

Token operatorKeyword;
SimpleIdentifier nameId;
if (name is SimpleIdentifier) {
if (name.name == currentDeclarationName?.name && getOrSet == null) {
constructor(name, null, null);
} else if (initializers.isNotEmpty && getOrSet == null) {
constructor(name, null, null);
} else {
method(null, name);
}
nameId = name;
} else if (name is _OperatorName) {
method(name.operatorKeyword, name.name);
} else if (name is PrefixedIdentifier) {
constructor(name.prefix, name.period, name.identifier);
operatorKeyword = name.operatorKeyword;
nameId = name.name;
} else {
throw new UnimplementedError();
}

if (modifiers?.constKeyword != null) {
// This error is also reported in OutlineBuilder.endMethod
handleRecoverableError(
messageConstMethod, modifiers.constKeyword, modifiers.constKeyword);
}
checkFieldFormalParameters(parameters);
currentDeclarationMembers.add(ast.methodDeclaration(
comment,
metadata,
modifiers?.externalKeyword,
modifiers?.abstractKeyword ?? modifiers?.staticKeyword,
returnType,
getOrSet,
operatorKeyword,
nameId,
typeParameters,
parameters,
body));
}

@override
Expand All @@ -1700,9 +1649,87 @@ class AstBuilder extends StackListener {
@override
void endClassConstructor(Token getOrSet, Token beginToken, Token beginParam,
Token beginInitializers, Token endToken) {
assert(getOrSet == null ||
optional('get', getOrSet) ||
optional('set', getOrSet));
debugEvent("ClassConstructor");
endClassMethod(
getOrSet, beginToken, beginParam, beginInitializers, endToken);

var bodyObject = pop();
List<ConstructorInitializer> initializers = pop() ?? const [];
Token separator = pop();
FormalParameterList parameters = pop();
TypeParameterList typeParameters = pop();
var name = pop();
TypeAnnotation returnType = pop();
_Modifiers modifiers = pop();
List<Annotation> metadata = pop();
Comment comment = _findComment(metadata, beginToken);

assert(parameters != null || optional('get', getOrSet));

ConstructorName redirectedConstructor;
FunctionBody body;
if (bodyObject is FunctionBody) {
body = bodyObject;
} else if (bodyObject is _RedirectingFactoryBody) {
separator = bodyObject.equalToken;
redirectedConstructor = bodyObject.constructorName;
body = ast.emptyFunctionBody(endToken);
} else {
unhandled("${bodyObject.runtimeType}", "bodyObject",
beginToken.charOffset, uri);
}

SimpleIdentifier prefixOrName;
Token period;
SimpleIdentifier nameOrNull;
if (name is SimpleIdentifier) {
prefixOrName = name;
} else if (name is PrefixedIdentifier) {
prefixOrName = name.prefix;
period = name.period;
nameOrNull = name.identifier;
} else {
throw new UnimplementedError();
}

if (typeParameters != null) {
// Outline builder also reports this error message.
handleRecoverableError(messageConstructorWithTypeParameters,
typeParameters.beginToken, typeParameters.endToken);
}
if (modifiers?.constKeyword != null &&
body != null &&
(body.length > 1 || body.beginToken?.lexeme != ';')) {
// This error is also reported in BodyBuilder.finishFunction
Token bodyToken = body.beginToken ?? modifiers.constKeyword;
handleRecoverableError(
messageConstConstructorWithBody, bodyToken, bodyToken);
}
if (returnType != null) {
// This error is also reported in OutlineBuilder.endMethod
handleRecoverableError(messageConstructorWithReturnType,
returnType.beginToken, returnType.beginToken);
}
ConstructorDeclaration constructor = ast.constructorDeclaration(
comment,
metadata,
modifiers?.externalKeyword,
modifiers?.finalConstOrVarKeyword,
null,
// TODO(paulberry): factoryKeyword
ast.simpleIdentifier(prefixOrName.token),
period,
nameOrNull,
parameters,
separator,
initializers,
redirectedConstructor,
body);
currentDeclarationMembers.add(constructor);
if (mixinDeclaration != null) {
// TODO (danrubel): Report an error if this is a mixin declaration.
}
}

@override
Expand Down
61 changes: 46 additions & 15 deletions pkg/front_end/lib/src/fasta/parser/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3352,27 +3352,57 @@ class Parser {
(staticToken == null || externalToken != null) && inPlainSync);
}
asyncState = savedAsyncModifier;
bool isConstructor = name.lexeme == enclosingDeclarationName;
if (!isConstructor &&
(optional('.', name.next) || beforeInitializers != null)) {
// Recovery: The name does not match,
// but the name is prefixed or the declaration contains initializers.

bool isConstructor = false;
if (optional('.', name.next) || beforeInitializers != null) {
isConstructor = true;
// TODO(danrubel): report invalid constructor name
// Currently multiple listeners report this error, but that logic should
// be removed and the error reported here instead.
if (name.lexeme != enclosingDeclarationName) {
// Recovery: The name does not match,
// but the name is prefixed or the declaration contains initializers.
// Report an error and continue with invalid name.
// TODO(danrubel): report invalid constructor name
// Currently multiple listeners report this error, but that logic should
// be removed and the error reported here instead.
}
if (getOrSet != null) {
// Recovery
if (optional('.', name.next)) {
// Unexpected get/set before constructor.
// Report an error and skip over the token.
// TODO(danrubel): report an error on get/set token
// This is currently reported by listeners other than AstBuilder.
// It should be reported here rather than in the listeners.
} else {
isConstructor = false;
if (beforeInitializers != null) {
// Unexpected initializers after get/set declaration.
// Report an error on the initializers
// and continue with the get/set declaration.
// TODO(danrubel): report invalid initializers error
// Currently multiple listeners report this error, but that logic
// should be removed and the error reported here instead.
}
}
}
} else if (name.lexeme == enclosingDeclarationName) {
if (getOrSet != null) {
// Recovery: The get/set member name is invalid.
// Report an error and continue with invalid name.
// TODO(danrubel): report invalid get/set member name
// Currently multiple listeners report this error, but that logic should
// be removed and the error reported here instead.
} else {
isConstructor = true;
}
}

if (isConstructor) {
//
// constructor
//
if (getOrSet != null) {
// TODO(danrubel): report an error on get/set token
// This is currently reported by listeners other than AstBuilder.
// It should be reported here rather than in the listeners.
}
switch (kind) {
case DeclarationKind.Class:
// TODO(danrubel): Remove getOrSet from constructor events
listener.endClassConstructor(getOrSet, beforeStart.next,
beforeParam.next, beforeInitializers?.next, token);
break;
Expand All @@ -3390,7 +3420,7 @@ class Parser {
beforeParam.next, beforeInitializers?.next, token);
break;
case DeclarationKind.TopLevel:
throw "Internal error: TopLevel method/constructor.";
throw "Internal error: TopLevel constructor.";
break;
}
} else {
Expand All @@ -3399,6 +3429,7 @@ class Parser {
//
switch (kind) {
case DeclarationKind.Class:
// TODO(danrubel): Remove beginInitializers token from method events
listener.endClassMethod(getOrSet, beforeStart.next, beforeParam.next,
beforeInitializers?.next, token);
break;
Expand All @@ -3415,7 +3446,7 @@ class Parser {
beforeParam.next, beforeInitializers?.next, token);
break;
case DeclarationKind.TopLevel:
throw "Internal error: TopLevel method/constructor.";
throw "Internal error: TopLevel method.";
break;
}
}
Expand Down

0 comments on commit 355df2a

Please sign in to comment.