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

[BUG] @With generates non-compiling code under Eclipse #2571

Open
Gaibhne opened this issue Aug 31, 2020 · 7 comments · May be fixed by #2574
Open

[BUG] @With generates non-compiling code under Eclipse #2571

Gaibhne opened this issue Aug 31, 2020 · 7 comments · May be fixed by #2574

Comments

@Gaibhne
Copy link

Gaibhne commented Aug 31, 2020

Describe the bug
Note that the entire issue is about compilation in Eclipse; I am not able to reproduce the issue via Maven.

@With cannot be used on an inner class where the outer class is generic (I think). An error message such as this is generated when compiled with a recent Java version (used to work on 8):

Cannot allocate the member type TestCase<T>.Inner using a parameterized compound name; use its simple name and an enclosing instance of type TestCase<T>

To Reproduce
The following code, compiled with Java 11, generates the above error:

import lombok.AllArgsConstructor;
import lombok.With;

public class TestCase<T> {
        @AllArgsConstructor
	public class Inner {
		@With
		Object object;
	}
}

Expected behavior
I expect this to compile under Eclipse, as it used to when running with JDK 8.

Version info (please complete the following information):

  • Lombok version: 1.18.12
  • Eclipse: Version: 2020-06 (4.16.0) / Build id: 20200615-1200 (I do not believe this is relevant, as it occurs with multiple Eclipse versions)
@Rawi01
Copy link
Collaborator

Rawi01 commented Aug 31, 2020

You are right, it does not work. Actually this one is quite interesting, even the delomboked result fails in eclipse while it works in javac. I will check out how we can generate code that eclipse likes too.

Delomboked code:

public class WithInnerGeneric<Z> {
	class Inner {
		final String x;

		@java.lang.SuppressWarnings("all")
		public Inner(final String x) {
			this.x = x;
		}

		@java.lang.SuppressWarnings("all")
		public WithInnerGeneric<Z>.Inner withX(final String x) {
			return this.x == x ? this : new WithInnerGeneric<Z>.Inner(x);
		}
	}
}
}

@Rawi01
Copy link
Collaborator

Rawi01 commented Sep 1, 2020

It should be possible to always call the constructor using the nested class name instead of also adding the parent class. This seems to work in all cases I tried. I will change the behaviour and add test cases to cover all the different cases.

Rawi01 added a commit to Rawi01/lombok that referenced this issue Sep 2, 2020
@Rawi01 Rawi01 linked a pull request Sep 2, 2020 that will close this issue
@rzwitserloot
Copy link
Collaborator

We can't generate just public Inner withX() {} - see issue #2268 for why that's problematic (and why this until recently did work; the fix for #2268 is recent and added the more partially qualified names to the output of @With and @Builder and such. We'll need to investigate if ecj is right or javac is right (usually ecj is right, but I do have my doubts here), and if there's anything we can generate that satisfies both ecj and javac and still addresses #2268. Tricky one.

@rzwitserloot
Copy link
Collaborator

Note that javac goes out of its way to allow you to have generics on each 'element' in a dotted type name, even though in common java you just never encounter generics anywhere except on the final element. This makes the code of javac considerably more convoluted, and ecj has this as well, which is a very light indication that in this particular instance, ecj's got it wrong, and this IS proper java syntax after all.

Not that the slam dunk case is then 'eh, whatever, ecj's problem, we shall ignore it' - but we're still just gathering info.

@Rawi01
Copy link
Collaborator

Rawi01 commented Sep 17, 2020

Uh, I totally missed that bug, sorry about that. I'm courious why all test cases still pass, I guess I have to understand #2268 completly but I'm to tired for that right now. One thing I noticed is that I only changed the constructor call to use the simple name and the returned type is still Outer.Inner.

@rspilker
Copy link
Collaborator

@Rawi01 I don't think we ever created a test for #2268.

@Rawi01
Copy link
Collaborator

Rawi01 commented Sep 24, 2020

Yes, I added one and think that I found a solution, I need some more time to properly test all the different cases.

Rawi01 added a commit to Rawi01/lombok that referenced this issue Nov 16, 2020
new Outer<String>.Inner<Integer>() -> new new Outer.Inner()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants