Support @Nullable annotations?

[sdegutis]

It'd be cool if the VS Code Java extensions could recognize that a method is annotated with javax.annotation.Nullable and then warn if it notices potential null-exception usages. I have no idea if this is possible in this plugin or where the appropriate place to request it as a feature would be. But your plugins are generally awesome so I wanted to suggest it as a further improvement. Thanks for the great work :)

[rgrunber]

This is supported to some extent in JDT Core :

`org.eclipse.jdt.core.compiler.annotation.nullanalysis` *Annotation-based Null Analysis.* `{ "enabled", "disabled" }`

and

<dependency>
      <groupId>org.eclipse.jdt</groupId>
      <artifactId>org.eclipse.jdt.annotation</artifactId>
      <version>1.2.0</version>
</dependency>

Not sure how to get it working for javax.annotation.*. I see there are some settings like org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jdt.annotation.NonNull which I assume should have worked with javax.annotation.NonNull but it isn't working for me. Some more investigation may be required.

[rgrunber]

Ok, I got this working with javax.annotation.* or basically any set of annotations. It didn't work before because I had a typo. It's not javax.annotation.NonNull. That doesn't exist. It's javax.annotation.Nonnull.

So basically here was my setup :

.vscode/settings.json

{
	"java.settings.url": ".vscode/settings.prefs"
}

.vscode/settings.prefs

org.eclipse.jdt.core.compiler.annotation.nonnull=javax.annotation.Nonnull
org.eclipse.jdt.core.compiler.annotation.nonnull.secondary=
org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=javax.annotation.NonNullByDefault
org.eclipse.jdt.core.compiler.annotation.nonnullbydefault.secondary=
org.eclipse.jdt.core.compiler.annotation.nullable=javax.annotation.Nullable
org.eclipse.jdt.core.compiler.annotation.nullable.secondary=
org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled

That's it. Now that we have this working, I think it's just a matter of programmatically applying these settings (and potentially others) based on some client-side setting (eg. Enable null analysis). We could override the default values in JDT-LS so instead of using the org.eclipse.jdt.annotations classes, they use javax.annotations.* .

[CsCherrYY]

@rgrunber I've tested it locally and it works very well 👍. I'm also following the PR in JDT side to enable it by default but it seems not so easily in JDT. But never mind, we can have a patch in our side if it can't be a part of JDT :)

Some considerations:

• Is it better to make javax.annotation.* to primary ones or just keep current primary ones (org.eclipse.jdt.annotation.*) and make them secondary?

• How can we make it work? As we know, users can have a way to custom their compiler options (containing org.eclipse.jdt.core.compiler.annotation.nullanalysis) but it requires some manual steps. Most of users can't find a way to make it work unless they open or search for an issue here. So, a proper default value can help them well.

• Besides, we can consider making the compiler options configurable via a friendly wizard, a possible way is to use custom editor, like what we do for formatting settings in vscode-java-pack.

[rgrunber]

Some considerations:

* Is it better to make `javax.annotation.*` to primary ones or just keep current primary ones (`org.eclipse.jdt.annotation.*`) and make them secondary?

In my example above, I chose to use them as primary but maybe setting them as secondary (if it works) is probably the change that gets this working with minimal disruption for those who might choose to use the org.eclipse.jdt.annotations.*. So the only setting we would need to touch are :

org.eclipse.jdt.core.compiler.annotation.nonnull.secondary=javax.annotation.Nonnull
org.eclipse.jdt.core.compiler.annotation.nonnullbydefault.secondary=javax.annotation.NonNullByDefault
org.eclipse.jdt.core.compiler.annotation.nullable.secondary=javax.annotation.Nullable
org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled

If that works, we could just set that as the default on our end without the need for any "main" setting (eg. java.project.nullanalysis.enabled) that enables these.

* How can we make it work? As we know, users can have a way to custom their compiler options (containing `org.eclipse.jdt.core.compiler.annotation.nullanalysis`) but it requires some manual steps. Most of users can't find a way to make it work unless they open or search for an issue here. So, a proper default value can help them well.

Probably as above, where we just set those as the global defaults. I guess in https://github.com/eclipse/eclipse.jdt.ls/blob/3fb81cee130b526afb433c7b34dfc6593d66bffd/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/PreferenceManager.java#L140-L148

* Besides, we can consider making the compiler options configurable via a friendly wizard, a possible way is to use custom editor, like what we do for formatting settings in vscode-java-pack.

Do we want to further expose the options ? Like primary/secondary ? Right now it seems the request is mainly for javax.annotations.* . Not sure it makes sense to allow users to play with these unless it's requested.

[CsCherrYY]

As for the choice about making it primary/secondary, there is a potential issue about quick fix when we just change secondary settings, if I'm a fan of javax.annotations.*, the quick fix will introduce org.eclipse.jdt.annotation.*

https://user-images.githubusercontent.com/45906942/190077068-248263fb-f7a2-47bd-ae78-b47daa015f2e.mp4

So I'm afraid the better choice is to set it as primary one, if we can confirm the number of users of javax.annotations.* is much larger than those of org.eclipse.jdt.annotation.*.

The code hit in Github:

• The difference in Github result is about ~6.7 times (670, 000 hits from javax.annotations.* and 100, 000 hits from org.eclipse.jdt.annotation.*)

The maven artifact:

• The usage about JSR-305 (javax.annotation.*) is ~6249: https://mvnrepository.com/artifact/com.google.code.findbugs/jsr305

• The usage about JDT one (org.eclipse.jdt.annotation.*) is ~107: https://mvnrepository.com/artifact/org.eclipse.jdt/org.eclipse.jdt.annotation

And one possible solution is that we can make mentioned java.project.nullanalysis.enabled an enum configuration:

java.project.nullanalysis.annotation:

• disabled - current behavior
• org.eclipse.jdt.annotation - use org.eclipse.jdt.annotation as primary, in the jdtls side, just set org.eclipse.jdt.core.compiler.annotation.nullanalysis to true
• javax.annotations - use javax.annotations as primary (might be default value)

Do we want to further expose the options ? Like primary/secondary ? Right now it seems the request is mainly for javax.annotations.* . Not sure it makes sense to allow users to play with these unless it's requested.

I agree with that we should only resolve this issue first :), and I'm just wondering if there are further requests about primary/secondary or even other compiler options, we could have a better all-in-one solution in the long term. With this, we are able to not introduce new VS Code settings when supporting new compile configurations.

[rgrunber]

I see, so in this case, I think it should be fine to make javax.annotation.* the primary and I think we could keep the option enabled by default. Does this avoid us having to create a setting for it ?

[CsCherrYY]

Does this avoid us having to create a setting for it ?

IMO no. Let me draft a PR later.

[rgrunber]

Ok, I guess we can take the approach you have in the PR. I was hoping to avoid generating that "root" setting that gets sent from client to server because even if the null analysis is enabled by default, it isn't really affecting users who don't choose to use it. For those that do, it's one less setting to figure out. It just works.

I'm not sure if there's any disadvantage to such an approach but I guess we can decide to leave it up to clients to decide if they'd like to enable it by default or configure some other setting (eg. disabled, jsr305, jdt )