Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

name clash with lambda as anon inner class compilation #1301

Closed
dpetroff opened this issue Mar 11, 2022 · 21 comments
Closed

name clash with lambda as anon inner class compilation #1301

dpetroff opened this issue Mar 11, 2022 · 21 comments
Assignees
Milestone

Comments

@dpetroff
Copy link

dpetroff commented Mar 11, 2022

Today I upgraded from Xtend 2.21 to 2.26 and my code base broke the IDE generator targeting Java 11.

Consider the method

def getMyAdapter(extension EObject it) {
	eAdapters.filter(AdapterImpl).head ?: (new AdapterImpl() => [eAdapters += it])
}

In the most recent Eclipse package (2022-03), the IDE generator generates distinctly different code from gradle for this example. In particular, the gradle generator uses Java lambda notation, while the IDE does not. Additionally, the IDE generated code also results in broken Java because it does not disambiguate between the EObject it and the Adapter it inside the lambda. Notice that the EObject it is declared as an extension, so the eAdapters call inside the lambda should apply to it. This works as expected when building with gradle and previous versions of the IDE, but the current IDE generator does not appear to disambiguate between the two its, which results in broken Java code.

myGradleProject.zip is a minimal gradle project reproducing the issue. Note that building with gradle is successful, including the test phase, and as long as the IDE generator doesn't rebuild the source after refreshing the gradle project in the IDE, there are no errors in the editor. Once the IDE has rebuilt the source, it is necessary to rebuild again with gradle and refresh the gradle project in the IDE again to get rid of the error.

@cdietrich
Copy link
Member

no no clue what change could have caused this. unfortunately not too many people seem to test milestones :(

@cdietrich
Copy link
Member

workaround:
have a org.eclipse.xtend.core.Xtend.prefs
with

useJavaCompilerCompliance=true

having

targetJavaVersion=Java11
useJavaCompilerCompliance=false

causes the problem

am not sure if this is a problem in the gradle plugin
downgrading the plugin to 2.1.0 helps
@szarnekow @oehme any idea?
Bildschirmfoto 2022-03-11 um 18 33 11

@cdietrich
Copy link
Member

cdietrich commented Mar 11, 2022

config.getJavaSourceVersion().isAtLeast(JAVA8) is false
this is cause there is no settings file
and the default is somewhere else.

the pref has
targetJavaVersion=Java11
and not
targetJavaVersion=JAVA11

i thought this was fixed in the plugin....

@dpetroff
Copy link
Author

dpetroff commented Mar 11, 2022

I noticed the workaround you suggested shortly before you posted it. However, it seems the Refresh Gradle Project action recreates the preference file and sets the option to false every time, which is not great as I frequently have to do it and have many subprojects that get reset. This is starting to look more like a gradle plugin issue.

@cdietrich
Copy link
Member

yes this is a bug in xtext-gradle-plugin

cdietrich added a commit to xtext/xtext-gradle-plugin that referenced this issue Mar 11, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
@cdietrich
Copy link
Member

can you build this branch locally and test it
https://github.com/xtext/xtext-gradle-plugin/compare/cd_xtendIssue1301
./gradlew pTML -PreleaseVersion=3.0.2-SNAPSHOT -x test
then add maven local and use classpath plugin application

(you may need to delete existing xtend and xtext.java prefs)

@dpetroff
Copy link
Author

dpetroff commented Mar 11, 2022

I still think there is also an Xtend generator component to it. Setting aside the failure to identify the correct Java compliance level, the correctly-generated code has a lambda with it_1 as the variable, while the incorrect code generates a procedure with it as the variable, which shadows the original it from the containing scope.

I'm also seeing a lot of weirdness with the useJavaCompilerCompliance setting being stuck to false in my projects, while at the same time being stuck to true in the gradle project I attached before, regardless of what the actual preference file says.

@cdietrich
Copy link
Member

yes it also should work conflict free with anonymous inner classes

@cdietrich
Copy link
Member

if you want an opt out for useJavaCompilerCompliance or settings generation,
maybe you can disable
xtextEclipseSettings task somehow.
if not file an enhancement request for the plugin

@cdietrich
Copy link
Member

i have no clue why we dont get the settings configured in e.g. xtext-core codebase

@cdietrich
Copy link
Member

@dpetroff any update?

@cdietrich
Copy link
Member

cdietrich commented Mar 14, 2022

@rubenporras the anonymous class has name clash issue would be something for you to look at if you want to dive into xbase

@rubenporras
Copy link

Hi @cdietrich ,

why do you think it would be something for me?

do you think a PR I merged in Xtext 2.25 could have broke something or do you think I could be interested in the topic?

Regards

@cdietrich
Copy link
Member

cdietrich commented Mar 14, 2022

do you think I could be interested in the topic?

i am actually asking if you are ...

@rubenporras
Copy link

Hi @cdietrich ,

thanks for asking. No, currently I am not interested. I am focused in migrating from xtext eclipse to xtext lsp, and that is enough of a challenge. In addition, we use Maven for our projects.

oehme added a commit to xtext/xtext-gradle-plugin that referenced this issue Mar 16, 2022
@cdietrich
Copy link
Member

gradle plugin is fixed.
open: anonymous class compilation

@cdietrich cdietrich changed the title IDE generated code does not match gradle generator results and generates broken Java name clash with lambda as anon inner class compilation Apr 1, 2022
@cdietrich
Copy link
Member

cdietrich commented Apr 1, 2022

Bildschirmfoto vom 2022-04-01 07-01-53
Bildschirmfoto vom 2022-04-01 07-02-23

	@Test
	def void testXtendIssue1301() {
		'''
		import org.eclipse.emf.ecore.EObject
		import org.eclipse.emf.common.notify.impl.AdapterImpl
		class Sample {
			def getMyAdapter(extension EObject it) {
				eAdapters.filter(AdapterImpl).head ?: (new AdapterImpl() => [eAdapters += it])
			}
		}
		'''.assertCompilesTo('''
		import com.google.common.collect.Iterables;
		import org.eclipse.emf.common.notify.Adapter;
		import org.eclipse.emf.common.notify.impl.AdapterImpl;
		import org.eclipse.emf.common.util.EList;
		import org.eclipse.emf.ecore.EObject;
		import org.eclipse.xtext.xbase.lib.Extension;
		import org.eclipse.xtext.xbase.lib.IterableExtensions;
		import org.eclipse.xtext.xbase.lib.ObjectExtensions;
		import org.eclipse.xtext.xbase.lib.Procedures.Procedure1;
		
		@SuppressWarnings("all")
		public class Sample {
		  public AdapterImpl getMyAdapter(@Extension final EObject it) {
		    AdapterImpl _elvis = null;
		    AdapterImpl _head = IterableExtensions.<AdapterImpl>head(Iterables.<AdapterImpl>filter(it.eAdapters(), AdapterImpl.class));
		    if (_head != null) {
		      _elvis = _head;
		    } else {
		      AdapterImpl _adapterImpl = new AdapterImpl();
		      final Procedure1<AdapterImpl> _function = new Procedure1<AdapterImpl>() {
		        public void apply(final AdapterImpl it_1) {
		          EList<Adapter> _eAdapters = it.eAdapters();
		          _eAdapters.add(it_1);
		        }
		      };
		      AdapterImpl _doubleArrow = ObjectExtensions.<AdapterImpl>operator_doubleArrow(_adapterImpl, _function);
		      _elvis = _doubleArrow;
		    }
		    return _elvis;
		  }
		}
		''')
	}

@szarnekow
the

scopeClosed = scopeClosed || !scope.pseudoScope;
			// If we left the current scope (incl. pseudo scopes) and the variable is not synthetic or unique,
			// we can stop collecting names. Overriding names from outside is ok in that case. 
			if (scopeClosed && !(synthetic || withUniqueName))
			```
leads to an early exit here.

@cdietrich
Copy link
Member

cdietrich commented Apr 1, 2022

should
org.eclipse.xtext.xbase.compiler.XbaseCompiler.appendClosureParameter(JvmFormalParameter, LightweightTypeReference, ITreeAppendable)
call declareUniqueVariableName?

or should
org.eclipse.xtext.xbase.compiler.XbaseCompiler.toAnonymousClass(XClosure, ITreeAppendable, LightweightTypeReference, JvmOperation)
call something else?

cdietrich added a commit to eclipse/xtext-extras that referenced this issue Apr 1, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
cdietrich added a commit that referenced this issue Apr 1, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
@cdietrich cdietrich added this to the Release_2.27 milestone Apr 1, 2022
@cdietrich cdietrich self-assigned this Apr 1, 2022
@cdietrich
Copy link
Member

alternatively: should the extension on out scope variable visible here?

cdietrich added a commit to eclipse/xtext-extras that referenced this issue Apr 1, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
cdietrich added a commit that referenced this issue Apr 1, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
@cdietrich
Copy link
Member

cdietrich added a commit that referenced this issue Apr 1, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
cdietrich added a commit to eclipse/xtext-extras that referenced this issue Apr 1, 2022
cdietrich added a commit that referenced this issue Apr 1, 2022
Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>
cdietrich added a commit that referenced this issue Apr 1, 2022
[#1301] fix name clashes with anony. inner classes
@cdietrich
Copy link
Member

fixed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants