Skip to content
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

Xtend refactoring handling of anonymous class #2898

Merged
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
94cbea8
Added an Xtend validator test for cycles in internal types
LorenzoBettini Dec 18, 2023
2494947
also verify the error's position for hierarchy cycles
LorenzoBettini Dec 20, 2023
2d1328e
also verify error's position for INTERFACE_EXPECTED
LorenzoBettini Dec 20, 2023
839182e
also verify error's position for OVERRIDDEN_FINAL
LorenzoBettini Dec 20, 2023
b6d2aca
also verify error's position for CLASS_EXPECTED
LorenzoBettini Dec 20, 2023
b20135a
also verify error's position for field INVALID_USE_OF_TYPE
LorenzoBettini Dec 20, 2023
461db35
also verify error's position for parameter INVALID_USE_OF_TYPE
LorenzoBettini Dec 20, 2023
eb38b9d
fixed verify error's position for parameter INVALID_USE_OF_TYPE
LorenzoBettini Dec 20, 2023
5390514
also verify error's position for DUPLICATE_PARAMETER_NAME
LorenzoBettini Dec 20, 2023
061170c
also verify error's position for duplicates
LorenzoBettini Dec 20, 2023
ae07565
also verify error's position for duplicate methods
LorenzoBettini Dec 20, 2023
af65508
also verify error's message parts for duplicate methods
LorenzoBettini Dec 20, 2023
665d49c
added missing tests for doCheckOverriddenMethods
LorenzoBettini Dec 29, 2023
ab8c3fb
also verify error position for CONFLICTING_DEFAULT_METHODS
LorenzoBettini Dec 29, 2023
5d39e36
also verify error position for CLASS_MUST_BE_ABSTRACT
LorenzoBettini Dec 29, 2023
c2789f6
also verify error position for ANONYMOUS_CLASS_MISSING_MEMBERS
LorenzoBettini Dec 29, 2023
e424b1a
also verify error position for anonymous class override final class
LorenzoBettini Dec 29, 2023
944efb0
added test with anonymous class not implementing abstract methods
LorenzoBettini Dec 30, 2023
b2401fe
some experiments
LorenzoBettini Dec 31, 2023
85a0ea9
AnonymousClassAwareTreeAppendable
LorenzoBettini Dec 31, 2023
63d58e8
removed some customizations
LorenzoBettini Dec 31, 2023
d1d795b
simplified appendConstructedTypeName
LorenzoBettini Dec 31, 2023
e5b112f
simplified AnonymousClassAwareTreeAppendable
LorenzoBettini Jan 1, 2024
ad1b04c
added another createChild to make it easier for subclassing
LorenzoBettini Jan 1, 2024
1d822ef
don't store the converter
LorenzoBettini Jan 1, 2024
92242b1
simplified isVariableDeclarationRequired
LorenzoBettini Jan 1, 2024
c4491d4
simplified internalCanCompileToJavaExpression
LorenzoBettini Jan 1, 2024
c8b774d
simplified constructorCallToJavaExpression
LorenzoBettini Jan 1, 2024
ff120dc
simplified appendConstructedTypeName
LorenzoBettini Jan 1, 2024
34096f7
XtendCompilerUtil -> XtendCompilerHelper
LorenzoBettini Jan 1, 2024
d6a1a8b
use IResourceScopeCache in XtendCompilerHelper
LorenzoBettini Jan 1, 2024
f87ce5e
simplified appendConstructedTypeName
LorenzoBettini Jan 2, 2024
8f85eb7
canCompileToJavaAnonymousClass
LorenzoBettini Jan 2, 2024
f5251c1
check isAnonymousClassConstructorCall
LorenzoBettini Jan 2, 2024
e74011b
Xtend uses canCompileToJavaAnonymousClass
LorenzoBettini Jan 2, 2024
26d6c23
simplified isVariableDeclarationRequired
LorenzoBettini Jan 2, 2024
07a2f7e
simplified isVariableDeclarationRequired 2
LorenzoBettini Jan 2, 2024
723d944
isVariableDeclarationRequired for constructor in Xbase
LorenzoBettini Jan 2, 2024
74faea2
removed duplication by constructorCallToJavaExpression
LorenzoBettini Jan 2, 2024
15b244d
reduced visibility of some methods
LorenzoBettini Jan 2, 2024
4f44872
Merge branch 'lb_xbase_cleanup-1' into temp-xtend-anonymous-class-2
LorenzoBettini Jan 2, 2024
eb6665b
Added @since to JvmModelGenerator's new method
LorenzoBettini Jan 3, 2024
bc766b0
Merge branch 'main' into temp-xtend-anonymous-class-2
LorenzoBettini Jan 8, 2024
f8d46fd
XtendCompilerHelper -> AnonymousClassCompilerHelper
LorenzoBettini Feb 3, 2024
5d926c2
the added createAppendable is protected
LorenzoBettini Feb 3, 2024
6af742c
simplified check in generateVisibilityModifier
LorenzoBettini Feb 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class JvmModelTests extends AbstractXtendTestCase {
assertTrue(anonymous.final)
assertFalse(anonymous.static)
assertTrue(anonymous.local)
assertFalse(anonymous.anonymous) // additional member -> named local class
assertTrue(anonymous.anonymous)
assertEquals(JvmVisibility.DEFAULT, anonymous.visibility)
assertEquals(2, anonymous.superTypes.size)
assertEquals('java.lang.Runnable', anonymous.superTypes.last.qualifiedName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ public void testAnonymousClass_01() {
Assert.assertTrue(anonymous.isFinal());
Assert.assertFalse(anonymous.isStatic());
Assert.assertTrue(anonymous.isLocal());
Assert.assertFalse(anonymous.isAnonymous());
Assert.assertTrue(anonymous.isAnonymous());
Assert.assertEquals(JvmVisibility.DEFAULT, anonymous.getVisibility());
Assert.assertEquals(2, anonymous.getSuperTypes().size());
Assert.assertEquals("java.lang.Runnable", IterableExtensions.<JvmTypeReference>last(anonymous.getSuperTypes()).getQualifiedName());
Expand Down
1 change: 1 addition & 0 deletions org.eclipse.xtend.core/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Bundle-RequiredExecutionEnvironment: JavaSE-11
Export-Package: org.eclipse.xtend.core;version="2.34.0";x-friends:="org.eclipse.xtend.ide.common,org.eclipse.xtend.ide.tests,org.eclipse.xtend.core.tests",
org.eclipse.xtend.core.compiler;version="2.34.0";x-friends:="org.eclipse.xtend.m2e,org.eclipse.xtend.ide.tests,org.eclipse.xtend.core.tests",
org.eclipse.xtend.core.compiler.batch;version="2.34.0",
org.eclipse.xtend.core.compiler.output;version="2.34.0";x-internal:=true,
org.eclipse.xtend.core.conversion;version="2.34.0";x-friends:="org.eclipse.xtend.ide,org.eclipse.xtend.ide.common,org.eclipse.xtend.core.tests",
org.eclipse.xtend.core.documentation;version="2.34.0";x-internal:=true,
org.eclipse.xtend.core.findReferences;version="2.34.0";x-internal:=true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.eclipse.emf.common.util.TreeIterator;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.EStructuralFeature;
import org.eclipse.xtend.core.jvmmodel.IXtendJvmAssociations;
import org.eclipse.xtend.core.richstring.AbstractRichStringPartAcceptor;
import org.eclipse.xtend.core.richstring.DefaultIndentationHandler;
import org.eclipse.xtend.core.richstring.RichStringProcessor;
Expand All @@ -34,7 +33,6 @@
import org.eclipse.xtext.common.types.JvmFeature;
import org.eclipse.xtext.common.types.JvmField;
import org.eclipse.xtext.common.types.JvmFormalParameter;
import org.eclipse.xtext.common.types.JvmGenericType;
import org.eclipse.xtext.common.types.JvmIdentifiableElement;
import org.eclipse.xtext.common.types.JvmType;
import org.eclipse.xtext.common.types.JvmTypeReference;
Expand Down Expand Up @@ -85,7 +83,7 @@ public class XtendCompiler extends XbaseCompiler {
private IGeneratorConfigProvider generatorConfigProvider;

@Inject
private IXtendJvmAssociations associations;
private XtendCompilerHelper compilerHelper;

@Override
protected String getFavoriteVariableName(EObject ex) {
Expand Down Expand Up @@ -546,9 +544,7 @@ protected void _toJavaStatement(final AnonymousClass anonymousClass, ITreeAppend
@Override
protected boolean internalCanCompileToJavaExpression(XExpression expression, ITreeAppendable appendable) {
if(expression instanceof AnonymousClass) {
AnonymousClass anonymousClass = (AnonymousClass) expression;
JvmGenericType inferredLocalClass = associations.getInferredType(anonymousClass);
return inferredLocalClass.isAnonymous();
return compilerHelper.canCompileToJavaAnonymousClass((AnonymousClass) expression);
} else
return super.internalCanCompileToJavaExpression(expression, appendable);
}
Expand Down Expand Up @@ -611,5 +607,10 @@ protected boolean needSyntheticSelfVariable(XClosure closure, LightweightTypeRef
}
return false;
}


@Override
protected boolean canCompileToJavaAnonymousClass(XConstructorCall constructorCall) {
return super.canCompileToJavaAnonymousClass(constructorCall) &&
compilerHelper.canCompileToJavaAnonymousClass((AnonymousClass) constructorCall.eContainer());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*******************************************************************************
* Copyright (c) 2023 itemis AG (http://www.itemis.eu) and others.
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/
package org.eclipse.xtend.core.compiler;

import org.eclipse.emf.ecore.EObject;
import org.eclipse.xtend.core.xtend.AnonymousClass;
import org.eclipse.xtend.core.xtend.XtendField;
import org.eclipse.xtend.core.xtend.XtendFunction;
import org.eclipse.xtend.core.xtend.XtendMember;
import org.eclipse.xtext.common.types.JvmType;
import org.eclipse.xtext.util.IResourceScopeCache;
import org.eclipse.xtext.util.Tuples;
import org.eclipse.xtext.xbase.jvmmodel.IJvmModelAssociations;

import com.google.inject.Inject;

/**
* @author Lorenzo Bettini - Initial contribution and API
*/
public class XtendCompilerHelper {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


@Inject
private IResourceScopeCache cache;

@Inject
private IJvmModelAssociations associations;

/**
* Assumes that the passed type is anonymous.
*/
public boolean canCompileToJavaAnonymousClass(JvmType type) {
return cache.get(Tuples.pair("anonymousJava", type), type.eResource(), () -> {
EObject sourceElement = associations.getPrimarySourceElement(type);
return sourceElement instanceof AnonymousClass &&
canCompileToJavaAnonymousClass((AnonymousClass) sourceElement);
});
}

public boolean canCompileToJavaAnonymousClass(AnonymousClass anonymousClass) {
return cache.get(Tuples.pair("anonymousJava", anonymousClass), anonymousClass.eResource(), () -> {
for(XtendMember member: anonymousClass.getMembers()) {
if(member instanceof XtendField ||
(member instanceof XtendFunction && !((XtendFunction) member).isOverride()))
return false;
}
return true;
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ import org.eclipse.xtext.xbase.compiler.output.ITreeAppendable
import org.eclipse.xtext.xbase.compiler.output.SharedAppendableState
import org.eclipse.xtext.xbase.typesystem.IBatchTypeResolver
import org.eclipse.xtext.xbase.typesystem.references.ITypeReferenceOwner
import org.eclipse.xtext.xbase.compiler.ImportManager
import org.eclipse.xtext.generator.trace.ITraceURIConverter
import org.eclipse.xtext.resource.ILocationInFileProvider
import org.eclipse.xtext.xbase.jvmmodel.IJvmModelAssociations
import org.eclipse.emf.ecore.EObject
import org.eclipse.xtend.core.compiler.output.AnonymousClassAwareTreeAppendable

/**
* @author Sven Efftinge - Initial contribution and API
Expand All @@ -56,7 +62,8 @@ class XtendGenerator extends JvmModelGenerator implements IGenerator2 {
@Inject OperationCanceledManager operationCanceledManager

@Inject ElementIssueProvider.Factory issueProviderFactory

@Inject XtendCompilerHelper compilerHelper

override doGenerate(Resource input, IFileSystemAccess fsa) {
super.doGenerate(input, fsa)
callMacroProcessors(input)
Expand Down Expand Up @@ -164,9 +171,12 @@ class XtendGenerator extends JvmModelGenerator implements IGenerator2 {
}

def compileLocalTypeStubs(JvmFeature feature, ITreeAppendable appendable, GeneratorConfig config) {
feature.localClasses.filter[ !anonymous ].forEach[
appendable.newLine
feature.localClasses.forEach[
if (compilerHelper.canCompileToJavaAnonymousClass(it)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yet this code here doesn't do both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in the current implementation anonymous classes are local classes

return
}
val anonymousClass = sourceElements.head as AnonymousClass
appendable.newLine
val childAppendable = appendable.trace(anonymousClass)
childAppendable.append('abstract class ')
childAppendable.traceSignificant(anonymousClass).append(simpleName)
Expand Down Expand Up @@ -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 ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szarnekow I've just pushed 6af742c

that check for anonymous is redundant

!compilerHelper.canCompileToJavaAnonymousClass(declarator)
) {
return result
}
}
Expand Down Expand Up @@ -386,5 +398,9 @@ class XtendGenerator extends JvmModelGenerator implements IGenerator2 {
}

}


override createAppendable(ImportManager importManager, ITraceURIConverter converter, ILocationInFileProvider locationProvider, IJvmModelAssociations jvmModelAssociations, EObject source, String indentation, String lineSeparator) {
return new AnonymousClassAwareTreeAppendable(compilerHelper, importManager, converter, locationProvider, jvmModelAssociations, source, indentation, lineSeparator)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*******************************************************************************
* Copyright (c) 2023 itemis AG (http://www.itemis.eu) and others.
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/
package org.eclipse.xtend.core.compiler.output;

import java.util.Set;

import org.eclipse.emf.ecore.EObject;
import org.eclipse.xtend.core.compiler.XtendCompilerHelper;
import org.eclipse.xtext.generator.trace.ILocationData;
import org.eclipse.xtext.generator.trace.ITraceURIConverter;
import org.eclipse.xtext.resource.ILocationInFileProvider;
import org.eclipse.xtext.xbase.compiler.ImportManager;
import org.eclipse.xtext.xbase.compiler.output.SharedAppendableState;
import org.eclipse.xtext.xbase.compiler.output.TreeAppendable;
import org.eclipse.xtext.xbase.jvmmodel.IJvmModelAssociations;
import org.eclipse.xtext.xbase.typesystem.references.LightweightTypeReferenceSerializer;
import org.eclipse.xtext.xbase.typesystem.references.ParameterizedTypeReference;

/**
* A custom implementation that takes into consideration anonymous classes,
* which, in some cases, cannot be compiled into standard Java anonymous classes:
* they are compiled into nested local classes.
*
* It uses a custom {@link LightweightTypeReferenceSerializer}.
*
* @author Lorenzo Bettini - Initial contribution and API
*/
public class AnonymousClassAwareTreeAppendable extends TreeAppendable {

private XtendCompilerHelper compilerHelper;

public AnonymousClassAwareTreeAppendable(XtendCompilerHelper compilerHelper, ImportManager importManager, ITraceURIConverter converter,
ILocationInFileProvider locationProvider, IJvmModelAssociations jvmModelAssociations, EObject source, String indentation,
String lineSeparator) {
super(importManager, converter, locationProvider, jvmModelAssociations, source, indentation, lineSeparator);
this.compilerHelper = compilerHelper;
}

protected AnonymousClassAwareTreeAppendable(XtendCompilerHelper compilerHelper, SharedAppendableState state, ITraceURIConverter converter,
ILocationInFileProvider locationProvider, IJvmModelAssociations jvmModelAssociations, Set<ILocationData> sourceLocations,
boolean useForDebugging) {
super(state, converter, locationProvider, jvmModelAssociations, sourceLocations, useForDebugging);
this.compilerHelper = compilerHelper;
}

@Override
protected LightweightTypeReferenceSerializer createLightweightTypeReferenceSerializer() {
return new LightweightTypeReferenceSerializer(this) {
@Override
protected void doVisitParameterizedTypeReference(ParameterizedTypeReference reference) {
if (reference.isAnonymous() &&
!compilerHelper.canCompileToJavaAnonymousClass(reference.getType())) {
// serialize the type of the generated nested local class
AnonymousClassAwareTreeAppendable.this.append(reference.getType());
} else {
super.doVisitParameterizedTypeReference(reference);
}
}
};
}

@Override
protected TreeAppendable createChild(SharedAppendableState state, ITraceURIConverter converter, ILocationInFileProvider locationProvider,
IJvmModelAssociations jvmModelAssociations, Set<ILocationData> newData, boolean useForDebugging) {
return new AnonymousClassAwareTreeAppendable(compilerHelper, state, converter, locationProvider, jvmModelAssociations, newData, useForDebugging);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ public void inferLocalClass(
JvmFeature container) {
final JvmGenericType inferredType = typesFactory.createJvmGenericType();
inferredType.setSimpleName(localClassName);
inferredType.setAnonymous(!hasAdditionalMembers(anonymousClass));
inferredType.setAnonymous(true);
inferredType.setFinal(true);
inferredType.setVisibility(JvmVisibility.DEFAULT);
inferredType.getSuperTypes().add(jvmTypesBuilder.inferredType(anonymousClass));
Expand All @@ -873,14 +873,5 @@ public void inferLocalClass(
associator.associateLogicalContainer(actualParameter, container);
}
}

protected boolean hasAdditionalMembers(AnonymousClass anonymousClass) {
for(XtendMember member: anonymousClass.getMembers()) {
if(member instanceof XtendField ||
(member instanceof XtendFunction && !((XtendFunction) member).isOverride()))
return true;
}
return false;
}


}
Loading