Bug 196104 - NbBundleProcessor not handled well by java.source
Summary: NbBundleProcessor not handled well by java.source
Alias: None
Product: platform
Classification: Unclassified
Component: -- Other -- (show other bugs)
Version: 7.0
Hardware: All All
: P2 normal (vote)
Assignee: Jesse Glick
Keywords: PLAN
: 194664 194958 (view as bug list)
Depends on: 196391 199086 196556
Blocks: 192750
  Show dependency tree
Reported: 2011-02-28 14:46 UTC by Jaroslav Tulach
Modified: 2011-06-01 14:12 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:

Rather then using the processor from IDE, always wait while its Bundle.class is generated and use that (19.38 KB, patch)
2011-02-28 14:51 UTC, Jaroslav Tulach
Details | Diff
Eliminate incremental effect (3.92 KB, patch)
2011-02-28 14:55 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Tulach 2011-02-28 14:46:18 UTC
The annotation processor for @NbBundle.Messages generates one Bundle.java file from many java files being present in the same package. These files moreover often reference methods of the generate Bundle.java files.

Currently the editor has huge troubles to provide reasonable code completion and error badges status. Many of the problems seem to come from a the fact that the processor is invoked incrementally, just on the edited file, and the generated Bundle.java code is then different than it would be if clean/build was invoked.
Comment 1 Jaroslav Tulach 2011-02-28 14:51:20 UTC
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.
Comment 2 Jaroslav Tulach 2011-02-28 14:55:04 UTC
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).
Comment 3 Jesse Glick 2011-03-01 17:30:23 UTC
*** Bug 194958 has been marked as a duplicate of this bug. ***
Comment 4 Jesse Glick 2011-03-01 20:37:20 UTC
core-main #679b6685185f
Comment 5 Jesse Glick 2011-03-01 20:43:32 UTC
(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.
Comment 6 Quality Engineering 2011-03-03 09:46:23 UTC
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
INFO [org.netbeans.api.java.source.ElementHandle]: Cannot resolve: ElementHandle[kind=METHOD; sigs=....Bundle ... (Ljava/lang/Object;)Ljava/lang/String; ]
Comment 7 Jesse Glick 2011-03-04 00:05:50 UTC
I occasionally get the following from Ant incremental builds, but I don't know what it means:

	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);
Comment 8 Jan Lahoda 2011-03-04 17:19:47 UTC
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.
Comment 9 Jesse Glick 2011-03-04 17:23:50 UTC
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.
Comment 10 Tomas Zezula 2011-03-07 10:07:25 UTC
In reply to comment #5
Agree, I created an enhancement for it, issue #196391.
Comment 11 Dusan Balek 2011-03-10 12:26:22 UTC
*** Bug 194664 has been marked as a duplicate of this bug. ***
Comment 12 Jesse Glick 2011-03-10 17:14:55 UTC
I filed the NPE as bug #196556 for tracking.
Comment 13 Jesse Glick 2011-03-30 20:21:31 UTC
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.
Comment 14 Tomas Zezula 2011-04-04 15:37:39 UTC
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?
Comment 15 Jesse Glick 2011-04-04 17:39:36 UTC
(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.


> 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 {
    public C() {

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


and saving, the error clears. After adding


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.