Xtend refactoring handling of anonymous class #2898

LorenzoBettini · 2024-01-08T20:29:52Z

Another proposal for #2886

there's no need to store the converter

use getNamedType instead of manually retrieving the last element of supertypes

instead of redefining _toJavaStatement we redefine constructorCallToJavaExpression and we call it also in other parts

when redefining them it's useless to make them public; when used internally, make them private

LorenzoBettini · 2024-01-08T20:35:36Z

This is another working proposal for this issue that does not touch the inferred model.

the changes are rather minimal:

the inferrer always sets the inferred type for AnonymousClass as anonymous
a new XtendCompilerHelper performs the check the inferred used to do, i.e., the anonymous class does not contain fields or methods that are not overridden
the XtendGenerator uses this helper to know whether it has to create nested local classes instead of anonymous classes; previously, it only checked JvmGenericType.isAnonymous.
the XtendCompiler redefines the new XbaseCompiler.canCompileToJavaAnonymousClass(XConstructorCall constructorCall) introduced in Xbase/Xtend refactoring #2890 and delegates to XtendCompilerHelper
a custom TreeAppendable, I called that AnonymousClassAwareTreeAppendable, but it could be called XtendTreeAppendable, uses a custom LightweightTypeReferenceSerializer to decide what to do with JvmGenericType that are "anonymous", again, using the XtendCompiler. This is useful to intercept all the serialization of types related to anonymous classes correctly. While the previous customizations catch anonymous classes in "new" expressions, this one catches the occurrences of related types in other parts, e.g., in local variables in case anonymous classes are compiled into nested local classes.

This way, during the Xtend compilation process, anonymous classes are correctly generated as Java anonymous classes or nested local classes. All the tests are green. All the generated code is as before. No test has been touched apart from this one https://github.com/eclipse/xtext/pull/2898/files#diff-8d2c98464642a8554f494c6f4c1de6f865cbec0d6cb5b288412da41e6067cba1L222 of course.

szarnekow · 2024-02-01T16:59:38Z

org.eclipse.xtend.core/src/org/eclipse/xtend/core/compiler/XtendCompilerHelper.java

+/**
+ * @author Lorenzo Bettini - Initial contribution and API
+ */
+public class XtendCompilerHelper {


Since this can only answer whether a JvmType can be compiled to an anonymous class, maybe a more restrictive name should be chosen?
AnonymousClassCompilerHelper?

Indeed that was my second choice, so I'll rename it :)

By the way, we already have https://github.com/eclipse/xtext/blob/88acf1555988cd81e1756abc0d6d2c3f1f1efa11/org.eclipse.xtend.core/src/org/eclipse/xtend/core/jvmmodel/AnonymousClassUtil.java#L39
should I put the methods of XtendCompilerHelper directly there?

szarnekow · 2024-02-01T17:03:01Z

org.eclipse.xtend.core/src/org/eclipse/xtend/core/compiler/XtendGenerator.xtend

@@ -320,7 +330,9 @@ class XtendGenerator extends JvmModelGenerator implements IGenerator2 {
 			}
 			if (declaringType.local && it instanceof JvmOperation) {
 				val declarator = declaringType as JvmGenericType
-				if (!declarator.anonymous) {
+				if (!declarator.anonymous ||


I'm not sure whether I like the fact that the helper doesn't check the declarator itself but this code here has to do both.

Indeed I had put a Javadoc comment on the helper https://github.com/eclipse/xtext/pull/2898/files/bc766b0068465148679855a36c8c9c88e5c62065#diff-c6ea2a981e27e70dc2cd53ff982ee051ac65096607807462e10196d625518f88R35
to avoid a check there, but if you prefer we can check isAnonymous directly in the helper, if that's what you mean.

@szarnekow I've just pushed 6af742c

that check for anonymous is redundant

szarnekow · 2024-02-01T17:03:13Z

org.eclipse.xtend.core/src/org/eclipse/xtend/core/compiler/XtendGenerator.xtend

-		feature.localClasses.filter[ !anonymous ].forEach[
-			appendable.newLine
+		feature.localClasses.forEach[
+			if (compilerHelper.canCompileToJavaAnonymousClass(it)) {


Yet this code here doesn't do both.

At least in the current implementation anonymous classes are local classes

szarnekow · 2024-02-01T17:07:44Z

org.eclipse.xtext.xbase/src/org/eclipse/xtext/xbase/compiler/JvmModelGenerator.xtend

+	/**
+	 * @since 2.34
+	 */
+	def TreeAppendable createAppendable(ImportManager importManager, ITraceURIConverter converter,


Could this be protected?

Right! I think so.

szarnekow

Overall, I like this change. Just a few small comments / questions.

LorenzoBettini · 2024-02-05T21:17:47Z

Closing and re-opening to trigger CI rebuild

LorenzoBettini added 30 commits December 22, 2023 09:47

Added an Xtend validator test for cycles in internal types

94cbea8

also verify the error's position for hierarchy cycles

2494947

also verify error's position for INTERFACE_EXPECTED

2d1328e

also verify error's position for OVERRIDDEN_FINAL

839182e

also verify error's position for CLASS_EXPECTED

b6d2aca

also verify error's position for field INVALID_USE_OF_TYPE

b20135a

also verify error's position for parameter INVALID_USE_OF_TYPE

461db35

fixed verify error's position for parameter INVALID_USE_OF_TYPE

eb38b9d

also verify error's position for DUPLICATE_PARAMETER_NAME

5390514

also verify error's position for duplicates

061170c

also verify error's position for duplicate methods

ae07565

also verify error's message parts for duplicate methods

af65508

added missing tests for doCheckOverriddenMethods

665d49c

also verify error position for CONFLICTING_DEFAULT_METHODS

ab8c3fb

also verify error position for CLASS_MUST_BE_ABSTRACT

5d39e36

also verify error position for ANONYMOUS_CLASS_MISSING_MEMBERS

c2789f6

also verify error position for anonymous class override final class

e424b1a

added test with anonymous class not implementing abstract methods

944efb0

some experiments

b2401fe

AnonymousClassAwareTreeAppendable

85a0ea9

removed some customizations

63d58e8

simplified appendConstructedTypeName

d1d795b

simplified AnonymousClassAwareTreeAppendable

e5b112f

added another createChild to make it easier for subclassing

ad1b04c

there's no need to store the converter

don't store the converter

1d822ef

simplified isVariableDeclarationRequired

92242b1

simplified internalCanCompileToJavaExpression

c4491d4

simplified constructorCallToJavaExpression

c8b774d

simplified appendConstructedTypeName

ff120dc

XtendCompilerUtil -> XtendCompilerHelper

34096f7

LorenzoBettini added 12 commits January 2, 2024 11:47

simplified appendConstructedTypeName

f87ce5e

use getNamedType instead of manually retrieving the last element of supertypes

canCompileToJavaAnonymousClass

8f85eb7

check isAnonymousClassConstructorCall

f5251c1

Xtend uses canCompileToJavaAnonymousClass

e74011b

simplified isVariableDeclarationRequired

26d6c23

simplified isVariableDeclarationRequired 2

07a2f7e

isVariableDeclarationRequired for constructor in Xbase

723d944

removed duplication by constructorCallToJavaExpression

74faea2

instead of redefining _toJavaStatement we redefine constructorCallToJavaExpression and we call it also in other parts

reduced visibility of some methods

15b244d

when redefining them it's useless to make them public; when used internally, make them private

Merge branch 'lb_xbase_cleanup-1' into temp-xtend-anonymous-class-2

4f44872

Added @SInCE to JvmModelGenerator's new method

eb6665b

Merge branch 'main' into temp-xtend-anonymous-class-2

bc766b0

LorenzoBettini requested a review from szarnekow January 8, 2024 20:35

LorenzoBettini mentioned this pull request Jan 8, 2024

XtendJvmModelInferrer and treatment of anonymous classes #2886

Closed

szarnekow reviewed Feb 1, 2024

View reviewed changes

LorenzoBettini added 3 commits February 3, 2024 18:36

XtendCompilerHelper -> AnonymousClassCompilerHelper

f8d46fd

the added createAppendable is protected

5d926c2

simplified check in generateVisibilityModifier

6af742c

szarnekow approved these changes Feb 5, 2024

View reviewed changes

LorenzoBettini closed this Feb 5, 2024

LorenzoBettini reopened this Feb 5, 2024

LorenzoBettini merged commit 447dc2c into eclipse-xtext:main Feb 6, 2024
8 checks passed

LorenzoBettini deleted the temp-xtend-anonymous-class-2 branch February 6, 2024 08:34

LorenzoBettini added this to the Release_2.34 milestone Feb 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xtend refactoring handling of anonymous class #2898

Xtend refactoring handling of anonymous class #2898

LorenzoBettini commented Jan 8, 2024

LorenzoBettini commented Jan 8, 2024

szarnekow Feb 1, 2024

LorenzoBettini Feb 1, 2024

LorenzoBettini Feb 1, 2024

szarnekow Feb 1, 2024

LorenzoBettini Feb 1, 2024

LorenzoBettini Feb 3, 2024

szarnekow Feb 1, 2024

LorenzoBettini Feb 1, 2024

szarnekow Feb 1, 2024

LorenzoBettini Feb 1, 2024

szarnekow left a comment

LorenzoBettini commented Feb 5, 2024

Xtend refactoring handling of anonymous class #2898

Xtend refactoring handling of anonymous class #2898

Conversation

LorenzoBettini commented Jan 8, 2024

LorenzoBettini commented Jan 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szarnekow left a comment

Choose a reason for hiding this comment

LorenzoBettini commented Feb 5, 2024