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

NPE happens in Java Builder with enabled annotation-based analysis and groovy #931

Closed
sergebg opened this issue Aug 3, 2019 · 14 comments
Closed
Assignees
Labels
Milestone

Comments

@sergebg
Copy link

sergebg commented Aug 3, 2019

error.txt
NPE happens when Groovy plugin installed and annotation-based null analysis enabled.
Disabling of annotation-based null analysis everything fixes errors. (As well as Groovy plugin removal)

My project is java project with many modules. Main sources include only java. Groovy is used for testing. Tests are based on Spock framework. In these tests we have non-standard naming (from the java point of view), something like this:

def "run scenario and perform checks"() { ... }

Otherwise this project is standard maven-based project. Internal compiler error appearing in "Problems" never relates to groovy code. Usually I can see 2 or 3 errors relating java files. For the same set of modules I get the same errors.

Groovy plugin: Groovy Development Tools 3.5.0.SNAPSHOT (installed from marketplace)
The error was discovered long time ago, and can be reproduced with earlier versions of eclipse and groovy plugin.

@eric-milles
Copy link
Member

eric-milles commented Aug 3, 2019

Can you provide the stacktrace of the exception and possibly a small project that demonstrates the problem? I can enable the feature without issue.

@eric-milles
Copy link
Member

The one addition GDT makes to BinaryTypeBinding does show up in your stack trace:

java.lang.NullPointerException
               at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.getMethods(BinaryTypeBinding.java:1375)
               at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.findMethod(BinaryTypeBinding.java:1171)
               at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.cachePartsFrom(BinaryTypeBinding.java:493)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createBinaryTypeFrom(LookupEnvironment.java:1057)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createBinaryTypeFrom(LookupEnvironment.java:1038)
               at org.eclipse.jdt.internal.compiler.Compiler.accept(Compiler.java:318)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.askForType(LookupEnvironment.java:256)
               at org.eclipse.jdt.internal.compiler.lookup.UnresolvedReferenceBinding.resolve(UnresolvedReferenceBinding.java:114)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.annotateType(LookupEnvironment.java:1914)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.getTypeFromSignature(LookupEnvironment.java:1897)
               at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.createMethod(BinaryTypeBinding.java:810)
               at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.createMethods(BinaryTypeBinding.java:1025) <---
               at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.cachePartsFrom(BinaryTypeBinding.java:555)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createBinaryTypeFrom(LookupEnvironment.java:1057)
               at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createBinaryTypeFrom(LookupEnvironment.java:1038)
               at org.eclipse.jdt.internal.compiler.Compiler.accept(Compiler.java:318)
private IBinaryMethod[] createMethods(IBinaryMethod[] iMethods, IBinaryType binaryType, long sourceLevel, char[][][] missingTypeNames) {
	...
		boolean hasRestrictedAccess = hasRestrictedAccess();
		MethodBinding[] methods1 = new MethodBinding[total];
		if (total == initialTotal) {
			...
		} else {
			IBinaryMethod[] mappedBinaryMethods = new IBinaryMethod[total];
			for (int i = 0, index = 0; i < initialTotal; i++) {
				if (iClinit != i && (toSkip == null || toSkip[i] != -1)) {
					MethodBinding method = createMethod(iMethods[i], binaryType, sourceLevel, missingTypeNames);
					if (hasRestrictedAccess)
						method.modifiers |= ExtraCompilerModifiers.AccRestrictedAccess;
					mappedBinaryMethods[index] = iMethods[i];
					methods1[index++] = method;
				}
			}
			this.methods = methods1;
			// GROOVY add -- save the bridge methods; Groovy needs to see them
			if (this.environment.globalOptions.buildGroovyFiles == 2) {
				int skipped = (initialTotal - this.methods.length - (iClinit == -1 ? 0 : 1));
				if (skipped == 0) {
					this.infraMethods = Binding.NO_METHODS;
				} else {
					this.infraMethods = new MethodBinding[skipped];
					for (int i = 0, index = 0; i < initialTotal; i += 1) {
						if (iClinit != i && (toSkip != null && toSkip[i] == -1)) {
							MethodBinding method = createMethod(iMethods[i], binaryType, sourceLevel, missingTypeNames); // this is line 1025
							if (hasRestrictedAccess)
								method.modifiers |= ExtraCompilerModifiers.AccRestrictedAccess;
							this.infraMethods[index++] = method;
						}
					}
				}
			}
			// GROOVY end

What's going on here is that Groovy needs to see the bridge methods for covariant checks. It uses the same call sequence that JDT does to create the method binding. It's possible this needs to shift up a line si that this.methods is not initialized until after the bridge methods are created.

@sergebg
Copy link
Author

sergebg commented Aug 5, 2019

I will try to provide an example. But the error happens on a big project with multiple modules. So that will take some time.

@eric-milles
Copy link
Member

Does this still happen with the latest snapshot?

If you're looking for something specific, I think the error is happening for an inner class (possibly anonymous).

@sergebg
Copy link
Author

sergebg commented Aug 7, 2019

Updated to 3.5.0.v201908052119-e1906
Still get NPE when enable null analysis

@sergebg
Copy link
Author

sergebg commented Aug 8, 2019

Reproduced NPE using public project spotify/apollo

  1. Forked it as https://github.com/sergebg/apollo
  2. Imported the project into eclipse (branch 1.x)
  3. Then I added groovy support using gmaveplus plugin.
    (commit 1fd1b43cc85c8bea520ae53342a93b8267cd44df)
  4. As result groovy nature was added to projects and...
  5. As result I got NPE.

@eric-milles
Copy link
Member

Does the NPE occur when you open a specific file or just build the project?

The projects built to completion for me:
image

@sergebg
Copy link
Author

sergebg commented Aug 9, 2019

I just repeated the all steps and reproduced errors.

  1. I have disabled analysis and rebuild the project. Today I have found 1 compilation error specific to eclipse compiler.

Screenshot 2019-08-09 at 19 33 02

2. Then I open settings and enable analysis.

Screenshot 2019-08-09 at 19 34 18

3. Eclipse suggested full rebuild and instantly after clicking "OK" I got errors

Screenshot 2019-08-09 at 19 34 33

@eric-milles
Copy link
Member

The error can be fixed by replacing Response::forPayload with Response::<R>forPayload.

I'm not getting the NPE. Are you running eclipse or building your project with Java 9+? I'm just trying to find something different about our setups.

@sergebg
Copy link
Author

sergebg commented Aug 10, 2019

I have tried Oracle JDK 1.8.0_171 and 1.8.0_201. The error happens after any full rebuild.
By the way, if you use the same settings in null analysis ("Error" for null access and potential null access), then you should get 5 compilation errors. On your screenshot I can't see such errors. So, I believe you have different config.

Description	Resource	Path	Location	Type
Null type mismatch: required '@Nonnull ResponseWithDelay' but the provided value is specified as @Nullable	StubClient.java	/apollo-test/src/main/java/com/spotify/apollo/test	line 277	Java Problem
Null type mismatch: required '@Nonnull T' but the provided value is null	PerformanceFixture.java	/apollo-test/src/main/java/com/spotify/apollo/test/experimental	line 88	Java Problem
Null type mismatch: required '@Nonnull Void' but the provided value is null	AsyncRequester.java	/apollo-test/src/main/java/com/spotify/apollo/test/experimental	line 118	Java Problem
Potential null pointer access: The field config is specified as @Nullable	DefaultMetaGatherer.java	/apollo-api-impl/src/main/java/com/spotify/apollo/meta/model	line 101	Java Problem
Potential null pointer access: The field responseWithDelay is specified as @Nullable	StubClient.java	/apollo-test/src/main/java/com/spotify/apollo/test	line 251	Java Problem

@eric-milles
Copy link
Member

eric-milles commented Aug 10, 2019

I need to know your annotation settings. I must be missing some of the annotations here:
image

Update: When I add ParametersAreNonnullByDefault and rebuild I get:

	at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.getMethods(BinaryTypeBinding.java:1374)
	at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.findMethod(BinaryTypeBinding.java:1170)
	at org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.cachePartsFrom(BinaryTypeBinding.java:493)
	at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createBinaryTypeFrom(LookupEnvironment.java:1057)
	at org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createBinaryTypeFrom(LookupEnvironment.java:1038)
	at org.eclipse.jdt.internal.compiler.Compiler.accept(Compiler.java:318)
	...

@sergebg
Copy link
Author

sergebg commented Aug 10, 2019

java-compiler-preferences.epf.txt

I am using javax.annotation.*:
javax.annotation.Nullable
javax.annotation.Nonnull
javax.annotation.ParametersAreNonnullByDefault

@eric-milles
Copy link
Member

Can you try again with the latest snapshot?

@sergebg
Copy link
Author

sergebg commented Aug 10, 2019

Now it works fine. Thanks

@sergebg sergebg closed this as completed Aug 10, 2019
@eric-milles eric-milles self-assigned this Aug 10, 2019
@eric-milles eric-milles added this to the v3.5.0 milestone Aug 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants