Bug 196104

Created attachment 106555 [details]
Rather then using the processor from IDE, always wait while its Bundle.class is generated and use that

The first approach is to give up trying to solve this in the IDE and rather give user full control. The processor output (when invoked from the IDE) is not used and user needs to do real build to generate Bundle.class first. The methods of Bundle.class are then visible up until user does another recompilation.

Cons of this solution is, that the code completion is not visible until the project can be recompiled. While common use of the code completion is to make the code compilable.

Created attachment 106556 [details]
Eliminate incremental effect

The other approach is to always collect whole sources in the package and read @Messages from it. Then the whole Bundle.java can be safely regenerated (which eliminates the problems of incremental compilation).

Cons: the patch is currently changing JavaC to know about a NetBeans specific class which is not aligned with the goal to make NetBeans JavaC patch as small as possible. The rumor however is that the annotation processor to do this kind of search for all @Message in a package by itself (although the code would be more complicated).

*** Bug 194958 has been marked as a duplicate of this bug. ***

core-main #679b6685185f

(In reply to comment #1)
> Rather then using the processor from IDE, always wait while its Bundle.class is
> generated and use that

Note that a patch along these lines would be potentially useful even for other processors - any which (1) generated Java sources, (2) is not desirable to run in the editor even on save (whether because of correctness or performance considerations). Currently an AnnotationProcessingQueryImplementation can only specify which processors to run, but if this SPI were extended so it could say "run anything except processors in the following list", then it would be easy to blacklist a few processors from the editor and let the usual incremental build process take care of source generation instead. I would suggest refining this patch for a subsequent release.

Integrated into 'main-golden', will be available in build *201103030057* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/679b6685185f
User: Jesse Glick <jglick@netbeans.org>
Log: #196104: NbBundleProcessor not handled well by java.source
Simply check all classes in the package (in addition to what was passed in root elements) to see if any have elements annotated with @Messages.
Seems to take care of all the serious issues with parsing only requested elements, such as IncorrectErrorBadges.
Still a few warnings can appear in log when keys are being edited, but these do not appear to cause any real problems:
Coupling error:
class file: file:.../var/cache/index/.../Bundle.sig
source file: file:.../var/cache/index/.../Bundle.java
METHOD: ...
INFO [org.netbeans.api.java.source.ElementHandle]: Cannot resolve: ElementHandle[kind=METHOD; sigs=....Bundle ... (Ljava/lang/Object;)Ljava/lang/String; ]

I occasionally get the following from Ant incremental builds, but I don't know what it means:

java.lang.NullPointerException
	at com.sun.tools.javac.jvm.ClassReader.findMethod(ClassReader.java:974)
	at com.sun.tools.javac.jvm.ClassReader.readEnclosingMethodAttr(ClassReader.java:926)
	at com.sun.tools.javac.jvm.ClassReader.readMemberAttr(ClassReader.java:909)
	at com.sun.tools.javac.jvm.ClassReader.readClassAttr(ClassReader.java:1053)
	at com.sun.tools.javac.jvm.ClassReader.readClassAttrs(ClassReader.java:1067)
	at com.sun.tools.javac.jvm.ClassReader.readClass(ClassReader.java:1560)
	at com.sun.tools.javac.jvm.ClassReader.readClassFile(ClassReader.java:1658)
	at com.sun.tools.javac.jvm.ClassReader.fillIn(ClassReader.java:1845)
	at com.sun.tools.javac.jvm.ClassReader.complete(ClassReader.java:1777)
	at com.sun.tools.javac.code.Symbol.complete(Symbol.java:386)
	at com.sun.tools.javac.code.Symbol$ClassSymbol.complete(Symbol.java:763)
	at com.sun.tools.javac.code.Symbol$ClassSymbol.flags(Symbol.java:695)
	at com.sun.tools.javac.code.Symbol$TypeSymbol.getEnclosedElements(Symbol.java:527)
	at org.netbeans.modules.openide.util.NbBundleProcessor.process(NbBundleProcessor.java:109)

javac bug? With some debugging of OpenJDK 7 langtools I can see that in ClassReader.readEnclosingMethodAttr, c.members_field == null where c is org.netbeans.modules.apisupport.hints.LayerHints$Prov$2, which is an anonymous inner class implementing Callable, whose call body method has a named local class. This is within a call to PackageElement.getEnclosedElements for org.netbeans.modules.apisupport.hints.

Note that all five classes in the package are in the explicit source list passed to javac. Only occurs during an incremental build, and seems to depend on what classes have already been compiled. What seems to work to reproduce is:

ant -f apisupport.refactoring/build.xml && ant -f openide.util.lookup/build.xml clean netbeans && ant -f apisupport.refactoring/build.xml

Something to do with LayerHints$Prov$2$1Handler.class. A simplistic patch which seems to solve the bug:

diff --git a/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java
--- a/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java
+++ b/src/share/classes/com/sun/tools/javac/jvm/ClassReader.java
@@ -1162,6 +1162,9 @@
         ClassSymbol c = readClassSymbol(nextChar());
         NameAndType nt = (NameAndType)readPool(nextChar());
 
+        if (c.members_field == null) {
+            return;
+        }
         MethodSymbol m = findMethod(nt, c.members_field, self.flags());
         if (nt != null && m == null)
             throw badClassFile("bad.enclosing.method", self);

My guess is that this is because an inner(-er) class is being completed before its outter class. The Symbols/Elements start as empty shells, and complete fills it with data (including the members_field) when needed, either from source or from class file. PackageElement is first filled with Symbols/Elements corresponding to all source+class files in the given directory, and getEnclosingElements is obviously completing them to ensure that the result will not contain classes that are not toplevel. But doing that is apparently incompatible with the implementation in ClassReader.

After code freeze I will see if I can make a small test case to report to the javac team. In the meantime the bug only seems to affect incremental builds on packages using @Messages as well as this particular nested class construct.

In reply to comment #5
Agree, I created an enhancement for it, issue #196391.

*** Bug 194664 has been marked as a duplicate of this bug. ***

I filed the NPE as bug #196556 for tracking.

Still sometimes have to make a few edits before the IDE's parser believes that a newly added bundle key exists. Command-line javac does not require this, so I think it is some kind of bug in the parser cache. Reproducible even in a single file, i.e. where round environment does contain all the relevant annotations, so not necessarily related to this bug. Occasionally triggers IncorrectErrorBadges. Seems that you ought to edit @Messages, then save, then use the new key. Otherwise if you add a key and its usage at once, I think what happens is that the parser signals an error for the usage in the modified document (since it has not yet rewritten Bundle.java internally), then when you save the document, the existence of an error prevents it from running the AP => catch-22. But just a guess, and seems to sometimes happen even if you save right after adding the key.

In replay to comment #13
1st) I've found a race that causes just partial re parse when full re parse is needed (but it affects only cases with 2 or more files).
2nd) The error cache does not affect AP behavior. The APs are executed even when there are errors in source. In the IDE mode the JavaCompiler.shouldStopPolicy is set to CompileState.GENERATE.

Are there any specific steps how to reproduce it?
Thanks

(In reply to comment #14)
> 1st) I've found a race that causes just partial re parse when full re parse is
> needed (but it affects only cases with 2 or more files).

Well, the problems I noticed happened with just one file.

> 2nd) The error cache does not affect AP behavior. The APs are executed even
> when there are errors in source. In the IDE mode the
> JavaCompiler.shouldStopPolicy is set to CompileState.GENERATE.

OK.

> Are there any specific steps how to reproduce it?

I just create a new standalone module, add a dep on openide.util, create a new class in a new package, edit to say

package p;
import org.openide.util.NbBundle.Messages;
public class C {
    @Messages({
        "k1=v",
    })
    public C() {
        Bundle.k1();
    }
}

and save => "cannot find symbol" on Bundle. After adding

        "k2=v",

and saving, the error clears. After adding

        Bundle.k2();

and saving, just this second line is marked as in error. Similarly when adding new keys and new usages, the old usages seem to be accepted but not the new ones, as if the model of Bundle.java were using old parser information. Seems to depend on timing of your edits, i.e. whether you save before or after the automatic reparse. If before => bogus error; if after => usually no error.