The type javax.annotation.Nonnull cannot be resolved #2264

snjeza · 2022-10-06T23:41:54Z

Fixes redhat-developer/vscode-java#2712
Fixes redhat-developer/vscode-java#2721
Fixes redhat-developer/vscode-java#2725

Signed-off-by: Snjezana Peco [email protected]

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java

rgrunber · 2022-10-07T13:43:31Z

...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/MavenProjectImporterTest.java

+			}
+			IJavaProject javaProject = JavaCore.create(project);
+			Map<String, String> options = javaProject.getOptions(true);
+			assertEquals(options.get(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS), JavaCore.DISABLED);


reverse the arguments here. It's expected, actual.

CsCherrYY · 2022-10-08T02:29:23Z

The changes LGTM, as for the test part, I suggest to add a test case not including jsr 305 in the pom (in any scope), and test this project would be imported successfully. e.g., sample project in redhat-developer/vscode-java#2712 (comment)

CsCherrYY · 2022-10-08T02:56:22Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java

@@ -2057,6 +2060,10 @@ private String getAnnotationType(IJavaProject javaProject, List<String> annotati
 				if (classpathStorage.keySet().contains(annotationType)) {
 					// for known types, check the classpath to achieve a better performance
 					for (String classpath : result.classpaths) {
+						IClasspathEntry classpathEntry = javaProject.getClasspathEntryFor(new Path(classpath));
+						if (isTest(classpathEntry, javaProject)) {


how about directly use ClasspathOptions.scope = "runtime" in https://github.com/eclipse/eclipse.jdt.ls/blob/f8452b422add64f9bbc80d0e22b48ed71797e563/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java#L2055

in this way, we can avoid checking classpath entry attribute here.

Agree. We could exclude test scope directly when getting the project classpaths.

it seems that using ClasspathOptions.scope = "runtime" doesn't work in this case. I just tested sample project in redhat-developer/vscode-java#2712 (comment) and see the introduced artifact jsr 305 is in runtime scope:

So the scope filter won't filter this dependency. However, in the classpath entry, it has test attribute:

So the new introduced method isTest() works. it will check the attributes of the classpath entry.

and I also tried ProjectCommand.isTestClasspathEntry() mentioned by @jdneo , it also checks the attribute and it works well. So it seems that we don't need to add a new method.

it seems that using ClasspathOptions.scope = "runtime" doesn't work in this case. I just tested sample project in redhat-developer/vscode-java#2712 (comment) and see the introduced artifact jsr 305 is in runtime scope:

~~This is a bug on the utility method ProjectCommand.getClasspathsFromJavaProject, which never uses the passed scope parameter.~~
https://github.com/eclipse/eclipse.jdt.ls/blob/f8452b422add64f9bbc80d0e22b48ed71797e563/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/JavaLaunchConfigurationInfo.java#L35-L50

In the launch configuration template JAVA_APPLICATION_LAUNCH, we could use the following attribute to exclude test code.
<booleanAttribute key="org.eclipse.jdt.launching.ATTR_EXCLUDE_TEST_CODE" value="true"/>

The launch configuration dynamically returns the scope value.
https://github.com/eclipse/eclipse.jdt.ls/blob/f8452b422add64f9bbc80d0e22b48ed71797e563/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/JavaApplicationLaunchConfiguration.java#L45

If the resolve logic doesn't filter the test scope directly, then it's a bug of JavaLaunchDelegate.getClasspathAndModulepath that doesn't handle the test scope well.

I'm OK to filter it on jdt.ls side.

It's interesting that the test attribute is set to true but the maven.scope is runtime.

Is that a bug of m2e?

which never uses the passed scope parameter.

It will,

https://github.com/eclipse/eclipse.jdt.ls/blob/f8452b422add64f9bbc80d0e22b48ed71797e563/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/JavaApplicationLaunchConfiguration.java#L43-L48

FYI the jsr305 dependency is declared as implementation scope, not test in spring-kafka: https://github.com/spring-projects/spring-kafka/blob/4bcb503a2fb33d20b45f4cc241c769c397bf0a7d/build.gradle#L173

CsCherrYY · 2022-10-08T09:10:42Z

it seems that jsr305 introduced by spring-kafka is in runtime scope, which make the null annotation types are not avaiable in project, but the jsr305 jar still appears in default classpath we get from getClasspathsFromJavaProject():

maybe we can check the classpath attribute maven.scope=runtime. Besides, I'm not so sure why test tag is introduced by m2e, maybe related to eclipse-m2e/m2e-core#543, but besides checking test tag, checking maven.scope is good IMO. (Do all the runtime classpath entries are tagged by test? maybe @fbricon and @rgrunber can provide some thoughts here)

As for Gradle projects, directly use implementation 'org.springframework.kafka:spring-kafka:2.9.1' will add the dependency in both compileClasspath and runtimeClasspath (which means the null annotation types are available in the project), there will be no such error.

snjeza · 2022-10-08T13:20:58Z

It's interesting that the test attribute is set to true but the maven.scope is runtime.
Is that a bug of m2e?

It is intentional. See https://github.com/eclipse-m2e/m2e-core/blob/master/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DefaultClasspathManagerDelegate.java#L164

snjeza · 2022-10-08T13:38:25Z

The changes LGTM, as for the test part, I suggest to add a test case not including jsr 305 in the pom (in any scope), and test this project would be imported successfully. e.g., sample project in redhat-developer/vscode-java#2712 (comment)

The https://github.com/yotov/vscode-nullable project includes jsr 305 indirectly. It is the same as the maven/null-analysis.

snjeza · 2022-10-08T13:40:32Z

@rgrunber @CsCherrYY @jdneo @testforstephen I have updated the PR.

Signed-off-by: Snjezana Peco <[email protected]>

snjeza · 2022-10-08T19:19:52Z

FYI the jsr305 dependency is declared as implementation scope, not test in spring-kafka: https://github.com/spring-projects/spring-kafka/blob/4bcb503a2fb33d20b45f4cc241c769c397bf0a7d/build.gradle#L173

maven recognizes it as runtime

$ cd vscode-nullable
$ mvn dependency:tree
[INFO] org.springframework.boot:example-app:jar:2.7.3
[INFO] +- org.springframework.boot:spring-boot-starter-jdbc:jar:2.7.3:compile
[INFO] |  +- org.springframework.boot:spring-boot-starter:jar:2.7.3:compile
[INFO] |  |  +- org.springframework.boot:spring-boot:jar:2.7.3:compile
[INFO] |  |  +- org.springframework.boot:spring-boot-autoconfigure:jar:2.7.3:compile
[INFO] |  |  +- org.springframework.boot:spring-boot-starter-logging:jar:2.7.3:compile
[INFO] |  |  |  +- ch.qos.logback:logback-classic:jar:1.2.11:compile
[INFO] |  |  |  |  \- ch.qos.logback:logback-core:jar:1.2.11:compile
[INFO] |  |  |  +- org.apache.logging.log4j:log4j-to-slf4j:jar:2.17.2:compile
[INFO] |  |  |  |  \- org.apache.logging.log4j:log4j-api:jar:2.17.2:compile
[INFO] |  |  |  \- org.slf4j:jul-to-slf4j:jar:1.7.36:compile
[INFO] |  |  +- jakarta.annotation:jakarta.annotation-api:jar:1.3.5:compile
[INFO] |  |  +- org.springframework:spring-core:jar:5.3.22:compile
[INFO] |  |  |  \- org.springframework:spring-jcl:jar:5.3.22:compile
[INFO] |  |  \- org.yaml:snakeyaml:jar:1.30:compile
[INFO] |  +- com.zaxxer:HikariCP:jar:4.0.3:compile
[INFO] |  |  \- org.slf4j:slf4j-api:jar:1.7.36:compile
[INFO] |  \- org.springframework:spring-jdbc:jar:5.3.22:compile
[INFO] |     \- org.springframework:spring-beans:jar:5.3.22:compile
[INFO] +- mysql:mysql-connector-java:jar:8.0.30:compile
[INFO] \- org.springframework.kafka:spring-kafka:jar:2.8.8:compile
[INFO]    +- org.springframework:spring-context:jar:5.3.22:compile
[INFO]    |  +- org.springframework:spring-aop:jar:5.3.22:compile
[INFO]    |  \- org.springframework:spring-expression:jar:5.3.22:compile
[INFO]    +- org.springframework:spring-messaging:jar:5.3.22:compile
[INFO]    +- org.springframework:spring-tx:jar:5.3.22:compile
[INFO]    +- org.springframework.retry:spring-retry:jar:1.3.3:compile
[INFO]    +- org.apache.kafka:kafka-clients:jar:3.1.1:compile
[INFO]    |  +- com.github.luben:zstd-jni:jar:1.5.0-4:runtime
[INFO]    |  +- org.lz4:lz4-java:jar:1.8.0:runtime
[INFO]    |  \- org.xerial.snappy:snappy-java:jar:1.1.8.4:runtime
[INFO]    \- com.google.code.findbugs:jsr305:jar:3.0.2:runtime

CsCherrYY · 2022-10-08T23:34:21Z

maven recognizes it as runtime

that's what I found and suggested in #2264 (comment), if all the runtime classpath entries have test attribute, then I'm OK with this change. It seems that they are related: https://github.com/eclipse-m2e/m2e-core/blob/0223a44258a6c92ac4a1943721a983743809ac37/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DefaultClasspathManagerDelegate.java#L125

The https://github.com/yotov/vscode-nullable project includes jsr 305 indirectly.

You're right. I just misunderstood that the project doesn't have jsr 305 dependency.

rgrunber · 2022-10-11T17:08:49Z

Yeah intuitively it made sense to me. runtime is basically saying "you don't need this dependency to compile, but it should be present when running the application". But running the tests also involves running the application, so the behaviour makes sense.

rgrunber

I think this is ready to be merged.

snjeza requested review from gorkem, fbricon, rgrunber, jdneo, testforstephen and CsCherrYY October 6, 2022 23:41

snjeza mentioned this pull request Oct 7, 2022

The type javax.annotation.Nonnull cannot be resolved redhat-developer/vscode-java#2712

Closed

rgrunber reviewed Oct 7, 2022

View reviewed changes

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java Show resolved Hide resolved

rgrunber reviewed Oct 7, 2022

View reviewed changes

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java Outdated Show resolved Hide resolved

rgrunber reviewed Oct 7, 2022

View reviewed changes

CsCherrYY reviewed Oct 8, 2022

View reviewed changes

snjeza force-pushed the issue-2712 branch from aaa8adc to 0d133c9 Compare October 8, 2022 13:29

The type javax.annotation.Nonnull cannot be resolved

2a171ba

Signed-off-by: Snjezana Peco <[email protected]>

snjeza force-pushed the issue-2712 branch from 0d133c9 to 2a171ba Compare October 8, 2022 14:57

CsCherrYY mentioned this pull request Oct 10, 2022

The type javax.annotation.Nonnull cannot be resolved redhat-developer/vscode-java#2722

Closed

rgrunber approved these changes Oct 11, 2022

View reviewed changes

CsCherrYY approved these changes Oct 12, 2022

View reviewed changes

rgrunber merged commit a00458c into eclipse-jdtls:master Oct 12, 2022

rgrunber added this to the End October milestone Oct 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The type javax.annotation.Nonnull cannot be resolved #2264

The type javax.annotation.Nonnull cannot be resolved #2264

snjeza commented Oct 6, 2022 •

edited

Loading

rgrunber Oct 7, 2022

snjeza Oct 8, 2022

CsCherrYY commented Oct 8, 2022

CsCherrYY Oct 8, 2022 •

edited

Loading

testforstephen Oct 8, 2022

CsCherrYY Oct 8, 2022

CsCherrYY Oct 8, 2022

testforstephen Oct 8, 2022 •

edited

Loading

jdneo Oct 8, 2022

jdneo Oct 8, 2022

CsCherrYY Oct 8, 2022

CsCherrYY commented Oct 8, 2022

snjeza commented Oct 8, 2022

snjeza commented Oct 8, 2022

snjeza commented Oct 8, 2022 •

edited

Loading

snjeza commented Oct 8, 2022

CsCherrYY commented Oct 8, 2022 •

edited

Loading

rgrunber commented Oct 11, 2022

rgrunber left a comment

The type javax.annotation.Nonnull cannot be resolved #2264

The type javax.annotation.Nonnull cannot be resolved #2264

Conversation

snjeza commented Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CsCherrYY commented Oct 8, 2022

CsCherrYY Oct 8, 2022 • edited Loading

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

testforstephen Oct 8, 2022 • edited Loading

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

CsCherrYY commented Oct 8, 2022

snjeza commented Oct 8, 2022

snjeza commented Oct 8, 2022

snjeza commented Oct 8, 2022 • edited Loading

snjeza commented Oct 8, 2022

CsCherrYY commented Oct 8, 2022 • edited Loading

rgrunber commented Oct 11, 2022

rgrunber left a comment

Choose a reason for hiding this comment

snjeza commented Oct 6, 2022 •

edited

Loading

CsCherrYY Oct 8, 2022 •

edited

Loading

testforstephen Oct 8, 2022 •

edited

Loading

snjeza commented Oct 8, 2022 •

edited

Loading

CsCherrYY commented Oct 8, 2022 •

edited

Loading