Null check annotations for Semantic Tokens API

[BoykoAlex]

The downstream plugins using the new ISemanticTokensProvider API and having JDT null analysis turned on having issues compiling classes deriving from the ISemanticTokensProvider. Whether I add @Nullable or @NonNull the class still fails to compile with

Error: 2,352 [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:4.0.10:compile (default-compile) on project org.eclipse.lsp4e.jdt: Compilation failure: Compilation failure: 
Error: 2,352 [ERROR] /Users/runner/work/lsp4e/lsp4e/org.eclipse.lsp4e.jdt/src/org/eclipse/lsp4e/jdt/LSJavaSemanticTokensProvider.java:[42] 
Error: 2,352 [ERROR] 	public Collection<ISemanticTokensProvider.SemanticToken> computeSemanticTokens(CompilationUnit ast) {
Error: 2,352 [ERROR] 	       ^^^^^^^^^^
Error: 2,352 [ERROR] The return type is incompatible with 'Collection<ISemanticTokensProvider.SemanticToken>' returned from ISemanticTokensProvider.computeSemanticTokens(CompilationUnit) (mismatching null constraints)
Error: 2,352 [ERROR] 1 problem (1 error)

Adding the @NonNull for the return type and the parameter sets the proper API expectations as well as hopefully would help sorting the downstream compiler errors out.

[BoykoAlex]

Is it okay to add jdt annotations plugin dependency to JDT UI? (cc: @jukzi @martinlippert)

[akurtakov]

@stephan-herrmann As this your area would you please jump in?

[stephan-herrmann]

@stephan-herrmann As this your area would you please jump in?

Thanks for the heads-up.

Some remarks:

• For interpreting unannotated library classes with intended null-contracts Eclipse External Annotations (eea) are the typical tool of choice.
  • If you need help setting this up for your project let me know
• ISemanticTokensProvider is API so any change in signature must be done in a compatible way.
  • For clients of this interface adding @NonNull on the return type strengthens the post condition, which is a compatible change, good.
  • Clients are also allowed to implement this interface. In that case the change could be considered as breaking if two conditions are met
    • The client has JDT annotation based null analysis enabled
    • The client has previously assumed that returning null from computeSemanticTokens() is allowed
  • Thus implementers have a non-null :) , albeit small, risk of breakage
• when adding null annotations to jdt.ui we should observe these:
  • require version >= 2.0.0 of the annotation bundle to ensure "modern" semantics (TYPE_USE)
  • enable annotation based analysis in the compiler preferences to ensure that jdt.ui doesn't violate its own contract
• In the past the existence of two versions of org.eclipse.jdt.annotation within the same build caused issues every now and then. This might have prevented adoption in some cases. Since the old version 1.x is no longer maintained and built, that risk is gone.

While I'm all for spreading null annotations into our own code, I'm not sure whether this example is the point where jdt.ui should flip the switch. @BoykoAlex as you seem to have experience with this analysis in the downstream plug-in, would you be interested in helping towards a null-checked implementation in jdt.ui, or some area of it?

[BoykoAlex]

@stephan-herrmann I don't think I'd get much time to dedicate to adopting null-check annotations in JDT UI. I looked into it only because LSP4E seems to have null-check analysis setting on and I had issues adopting new API of ISemanticTokensProvider in LSP4E because of this (see description).
However, if someone else starts this work I might as well jump in and help in a few areas... just don't know how much time I'd be able to give this work...

[stephan-herrmann]

@stephan-herrmann I don't think I'd get much time to dedicate to adopting null-check annotations in JDT UI. I looked into it only because LSP4E seems to have null-check analysis setting on and I had issues adopting new API of ISemanticTokensProvider in LSP4E because of this (see description). However, if someone else starts this work I might as well jump in and help in a few areas... just don't know how much time I'd be able to give this work...

In that case using external annotations in downstream projects looks like the path of least resistance ...