NonNullByDefault package annotations overwrite explicitly defined more specific EEAs

[blablubbabc]

• Eclipse ide 2024-06 (20240606-1231)
• Gradle project using Java 21
• Reproduction project: https://github.com/blablubbabc/jdt-testing

My situation / goal is as follows:

• I have an external library (in this example "org.bukkit:bukkit") that I want to essentially treat as un-annotated (they are using some jetbrains null annotations, but I have configured the project to not take those into account (see also #2734); only the default jdt annotations are taken into account). The Maven repository of this project sadly doesn't include the -source artifact. So to be able to later annotate it from inside Eclipse, I manually built and installed the source artifact in my local Maven repository. The artifact can be found in the linked repository, if needed for testing.
• In order to not have to manually annotate this whole external library, my idea was to manually annotate the packages that I use with the JDT NonNullByDefault annotation, i.e. locally in my project. And then for the few relevant nullable cases, I would explicitly define EEAs.

I currently run into the following issues:

• Issue 1: I am not able to annotate the library from within Eclipse. The source is linked, and the "Annotate" option shows up, but when clicking it I get this error in the workspace error log:

  
  eclipse.buildId=4.32.0.20240606-1231
  java.version=21.0.3
  java.vendor=Azul Systems, Inc.
  BootLoader constants: OS=win32, ARCH=x86_64, WS=win32, NL=de_DE
  Framework arguments:  -product org.eclipse.epp.package.java.product
  Command-line arguments:  -os win32 -ws win32 -arch x86_64 -product org.eclipse.epp.package.java.product

This is a continuation of log file C:\Users\lukas\workspace.metadata.bak_3.log Created Time: 2024-07-27 14:23:56.885

org.eclipse.jdt.ui Error Sat Jul 27 15:31:09 CEST 2024 Error during computation of Annotate proposals: Could not resolve type T

org.eclipse.jdt.internal.corext.fix.ExternalNullAnnotationChangeProposals$MissingBindingException: Could not resolve type T at org.eclipse.jdt.internal.corext.fix.ExternalNullAnnotationChangeProposals.resolveBinding(ExternalNullAnnotationChangeProposals.java:354) at org.eclipse.jdt.internal.corext.fix.ExternalNullAnnotationChangeProposals.collectExternalAnnotationProposals(ExternalNullAnnotationChangeProposals.java:481) at org.eclipse.jdt.internal.ui.text.correction.ExternalNullAnnotationQuickAssistProcessor.computeQuickAssistProposals(ExternalNullAnnotationQuickAssistProcessor.java:101) at org.eclipse.jface.text.quickassist.QuickAssistAssistant$ContentAssistProcessor.computeCompletionProposals(QuickAssistAssistant.java:71) at org.eclipse.jface.text.contentassist.ContentAssistant$2.lambda$0(ContentAssistant.java:2064) at java.base/java.util.Collections$SingletonSet.forEach(Collections.java:5125) at org.eclipse.jface.text.contentassist.ContentAssistant$2.run(ContentAssistant.java:2063) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:47) at org.eclipse.jface.text.contentassist.ContentAssistant.computeCompletionProposals(ContentAssistant.java:2060) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.computeProposals(CompletionProposalPopup.java:577) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.lambda$0(CompletionProposalPopup.java:507) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.showProposals(CompletionProposalPopup.java:502) at org.eclipse.jface.text.contentassist.ContentAssistant.showPossibleCompletions(ContentAssistant.java:1874) at org.eclipse.jface.text.quickassist.QuickAssistAssistant.showPossibleQuickAssists(QuickAssistAssistant.java:113) at org.eclipse.jdt.internal.ui.text.correction.JavaCorrectionAssistant.showPossibleQuickAssists(JavaCorrectionAssistant.java:194) at org.eclipse.jdt.internal.ui.javaeditor.ClassFileEditor$5.doOperation(ClassFileEditor.java:942) at org.eclipse.jdt.internal.ui.javaeditor.AnnotateClassFileAction.run(AnnotateClassFileAction.java:43) at org.eclipse.jface.action.Action.runWithEvent(Action.java:474) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:581) at org.eclipse.jface.action.ActionContributionItem.lambda$4(ActionContributionItem.java:415) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4285) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1160) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4083) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3673) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:152) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:639) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:546) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:668) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:605) at org.eclipse.equinox.launcher.Main.run(Main.java:1481)

* Additional notes:
  * I get this error regardless of what type I try to annotate within this (or any other external gradle) library. E.g. "Error during computation of Annotate proposals: Could not resolve type String"
  * If I try to annotate something inside the JDK, it works without issues.
  * An external annotations path is defined to point to the resources directory for the gradle libraries container.
  * I can manually create the EEA files, and they are also recognized correctly.
* Issue 2: My approach of adding a NonNullByDefault annotation to the package and then only annotating the nullable cases does not seem to work. The NonNullByDefault annotation always overrides my EEAs (maybe because it is "local").
  If I remove the NonNullByDefault package annotation, the EEA is correctly taken into account. Example:  

private static void registry() { Registry registry = Registry.FROG_VARIANT; // Registry.get can return null. // External project can be assumed un-annotated (its jetbrains annotations are not // configured to be taken into account by Eclipse). // The package is annotated here (locally) as "NonNullByDefault", to apply a default to all // non-annotated external types (to not have to manually annotated the whole external // library). // Registry.get is explicitly annotated as nullable via EEA (see resources). // The EEA seems to be ignored in favor of the NonNullByDefault package declaration. Frog.@Nullable Variant variant = registry.get(NamespacedKey.minecraft("non_existent")); // Eclipse warning: "Null comparison always yields false: The variable variant cannot be // null at this location" if (variant == null) return; }

  Is this behavior intended? If yes, is there any other solution that doesn't require me to fully annotate the whole library?
  Edit: I just found this issue that might be related: #2512
* Question 3: The JDT tool has the option to alternatively use other null annotations instead of the default JDT ones. Is there a way to use an alternative `NonNullByDefault` annotation and give it the same semantic as the default JDT `NonNullByDefault` annotation for Java 8+ (i.e. to also apply to type parameters by default, i.e. to be able to write `List<String>` instead of `List<@NonNull String>` even when using a third-party annotation)? The documentation mentions that some aspects "are not supported when using 3rd party annotation types", such as configuring the `DefaultLocation`s. But even on Java 8+, the default for third-party NonNullByDefault annotations seems to be the Java 7 behavior.

[stephan-herrmann]

• In order to not have to manually annotate this whole external library, my idea was to manually annotate the packages that I use with the JDT NonNullByDefault annotation, i.e. locally in my project. And then for the few relevant nullable cases, I would explicitly define EEAs.

To check I understand: do you want to compose a Java package from these three sources?

• package-info.java in some source folder
• all the .class files from a jar
• annotations from .eea files in some source / resource folder

Is that it?

Technically, mixing package-info.java from source and .class from a jar sounds like a split package, and it's quite possible that ecj doesn't see the source fragment when reading .class files.

The natural follow-up question would be: do we need to support @NNBD in .eea? With all the configurability this annotation has in source code?

[blablubbabc]

To check I understand: do you want to compose a Java package from these three sources?

The final artifact of this project will contain the annotated (@NNBD) package-info.class files and the EEA files from the resource folder (the external library's original class files are not included in this package). In my specific case, this will be one module of a multi-module Gradle project, and the other modules depend on it to resolve their null annotations from it (via Search for external annotations in all build path locations). This seems to work fine so far, the only issue I am running into (both within this annotation module, as well as outside of it in the modules that depend on in) is that the @NNBD seems to always take precendece over the information from the EEA files.

Maybe JDT finds the un-annotated class + the NNBD annotation on the package and then decides to stop looking for any EEA because it assumes that the package is annotated, so there is apparently no need to check for EEAs?

it's quite possible that ecj doesn't see the source fragment when reading .class files.

You are referring to issue 1 I assume. This issue of not being able to annotate seems to apply to all Gradle dependencies. Is there anything I can do for Eclipse to let me conveniently annotate those external libraries?

The natural follow-up question would be: do we need to support @NNBD in .eea? With all the configurability this annotation has in source code?

Any working solution that doesn't require me to fully annotate the library would be fine with me ;) For the split package solution to work, Eclipse would probably need to always check for EEAs, even if some package is apparently already annotated. Maybe this would also be useful in other scenarios, e.g. if some external library is incorrectly annotated, one could manually fix those specific cases by annotating it locally via an EEA that then overrides the information derived from within the package. This might be a question of precendence among the different sources of null annotations. In my opinion, it would make sense to give EEAs more precedence, since this is what a developer defines locally when there is some issue with the annotations of the imported package.

A related question would be how to handle cases in which different packages include EEAs for the same class, possibly contradicting each other. I am not aware of there currently being a way to tell Eclipse in which order to check packages for EEAs. Example I have run into: For the JDK annotations, I am currently using a dependency on https://github.com/vegardit/no-npe . But there are some cases in which some Java classes are not correctly annotated. Currently, I am then adding annotations for those classes myself inside my module, which seems to currently take precedence over the EEAs of this external package. But I am not sure if this behavior is actually defined currently. Also, related to that (but now probably getting a bit off-topic ..), there doesn't seems to be a way to partially define the EEA for a class: If there is an issue with one method in a large class, I need to annotate the fully class currently, even if the other methods are annotated fine in this external shared EEA package.

[stephan-herrmann]

The final artifact of this project will contain the annotated (@NNBD) package-info.class files and the EEA files from the resource folder

Thanks for clarification. So the general structure (.class + .eea) is the expected one.

When I find the time I'll need to check if indeed finding a "real" @NNBD lets ecj take the shortcut and never look for additional annotations from .eea.

[blablubbabc]

Issue 1 (not being able to edit the annotations / resolve types): This seems to be somehow related to the presence of these NNBD package annotations. Once I remove them again, I can annotate the external library just fine and the error no longer occurs.

Regarding issue 2: I have thought about this a bit more, and maybe my approach of adding NNBD package declarations and then only annotating the few nullable cases is a bit flawed: The NNBD annotations effectively disable the null checking for all external return types by default, making it very risky to accidentally miss cases in which null checks are actually required (but that were not yet annotated as nullable). So for the project in question, I think I will go with the approach of fully annotating all uses of this external library explicitly.

Unless there is something for you to derive from this ticket as a potential improvement / usecase to support, feel free to close this ticket.