-
Notifications
You must be signed in to change notification settings - Fork 320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some Xbase cleanup #2888
Some Xbase cleanup #2888
Conversation
The handling of anonymous class is already taken care by org.eclipse.xtext.xbase.typesystem.references.LightweightTypeReferenceSerializer.doVisitParameterizedTypeReference(ParameterizedTypeReference) so there's no need to consider anonymous class here
Since we already assume that the import manager is part of the appendable, which is a TreeAppendable, I think it's better to do the case and expose getImportManager in TreeAppendable than to call that method with two reflective calls. This would also allow clients to provide a custom TreeAppendable; with the reflective call this latter use-case would not be supported
typeAppendable.append(actualType); | ||
} | ||
IResolvedTypes resolvedTypes = batchTypeResolver.resolveTypes(constructorCall); | ||
LightweightTypeReference actualType = resolvedTypes.getActualType(constructorCall).getRawTypeReference(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use getNamedType to obtain the correct type if it’s an anonymous class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szarnekow This is done by org.eclipse.xtext.xbase.typesystem.references.LightweightTypeReferenceSerializer.doVisitParameterizedTypeReference(ParameterizedTypeReference)
@Override
protected void doVisitParameterizedTypeReference(ParameterizedTypeReference reference) {
if (reference.isAnonymous()) {
reference.getNamedType().accept(this);
} else {
appender.append(reference.getType());
if (!reference.getTypeArguments().isEmpty()) {
appender.append("<");
appendCommaSeparated(reference.getTypeArguments());
appender.append(">");
}
}
}
But I've just noted Xtend compilation tests fail: also the org.eclipse.xtext.xbase.compiler.XbaseCompiler.constructorCallToJavaExpression(XConstructorCall, ITreeAppendable) needs adjusting/simplification or it generates type arguments twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szarnekow the new commit fdad984 updates/simplified org.eclipse.xtext.xbase.compiler.XbaseCompiler.constructorCallToJavaExpression(XConstructorCall, ITreeAppendable) accordingly (at least locally Xtend tests are now green)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szarnekow it can be simplified even further 28a2e11
last message for this year: Happy new year :)
type arguments are already serialized by appendConstructedTypeName now
@szarnekow maybe now I understand better the meaning of the serialization of type arguments with traces, so my last commits would remove such a functionality for anonymous classes' constructor calls. Moreover, concerning traces, the intention really was to serialize the constructor type name separately from the type arguments. I'm closing this PR and creating a new one. |
The two commits' messages describe the proposed changes (it's a small cleanup)