Community
Participate
Working Groups
Created attachment 79253 [details] proposed patch against HEAD (tested only in 3.3) Build ID: I20070625-1500 Simply compile the following class: public class Foo<T> { /** @see T.R */ void foo() {} } This causes the following NPE: java.lang.NullPointerException at org.eclipse.jdt.internal.compiler.ast.JavadocQualifiedTypeReference.internalResolveType(JavadocQualifiedTypeReference.java:66) at org.eclipse.jdt.internal.compiler.ast.JavadocQualifiedTypeReference.resolveType(JavadocQualifiedTypeReference.java:84) at org.eclipse.jdt.internal.compiler.ast.TypeReference.resolveType(TypeReference.java:125) at org.eclipse.jdt.internal.compiler.ast.Javadoc.resolveReference(Javadoc.java:353) at org.eclipse.jdt.internal.compiler.ast.Javadoc.resolve(Javadoc.java:257) at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolveJavadoc(AbstractMethodDeclaration.java:417) at org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolve(AbstractMethodDeclaration.java:398) at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1085) at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1164) at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.resolve(CompilationUnitDeclaration.java:366) at org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:623) at org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:392) at org.eclipse.jdt.internal.core.builder.AbstractImageBuilder.compile(AbstractImageBuilder.java:362) at org.eclipse.jdt.internal.core.builder.IncrementalImageBuilder.compile(IncrementalImageBuilder.java:302) ... The problem is: QualifiedTypeReference.getTypeBinding() may return null, which is not handled by JavadocQualifiedTypeReference.internalResolveType().
Created attachment 79254 [details] updated patch Oops, previous patch was not a good idea in HEAD, since reportInvalidType() would again cause an NPE. This patch should actually suffice.
Thanks for the patch but we do prefer fix the origin of the problem than just avoid the NPE. The problem here must be fixed by overriding getTypeBinding(Scope) method in JavadocQualifiedTypeReference. It was not necessary before 5.0 and generics, but now we need to do it to have a specific javadoc message "Javadoc: Illegal access from the type parameter T" instead of the basic compiler one...
(In reply to comment #2) > [...], but now we need to do it to have > a specific javadoc message "Javadoc: Illegal access from the type parameter T" > instead of the basic compiler one... Why can't you just add a little snippet to ProblemReporter.invalidType() et al, which reads something like (after id has been determined): if ((location.bits & ASTNode.InsideJavadoc) != 0) id += Javadoc; Wouldn't that be easier than duplicating all the AST-code? I seem to be missing something here ;-)
(In reply to comment #3) > (In reply to comment #2) > > [...], but now we need to do it to have > > a specific javadoc message "Javadoc: Illegal access from the type parameter T" > > instead of the basic compiler one... > > Why can't you just add a little snippet to ProblemReporter.invalidType() > et al, which reads something like (after id has been determined): > if ((location.bits & ASTNode.InsideJavadoc) != 0) > id += Javadoc; > Wouldn't that be easier than duplicating all the AST-code? > I seem to be missing something here ;-) > Thanks to wonder about it, but fortunately nothing is missing here... We cannot do that as Javadoc preferences are set separately from other compiler preferences. Typically, the javadoc issues are configured to be warnings although similar compiler issues are errors... That's why we need to create specific ProblemReporter methods and also why we needed to create specific compiler AST nodes. I know it looks sometimes redundant but it was absolutely necessary to implement it this way...
(In reply to comment #4) > We cannot do that as Javadoc preferences are set separately from other compiler > preferences. Typically, the javadoc issues are configured to be warnings > although similar compiler issues are errors... isn't that treated in ProblemReporter.computeSeverity based solely on the problem ID? That's why I thought tweeking the ID would suffice. That view might be too simplistic, though. > That's why we need to create specific ProblemReporter methods and also why we > needed to create specific compiler AST nodes. I know it looks sometimes > redundant but it was absolutely necessary to implement it this way... OK, I take your word. I don't mean to waste your time for explaining things that have been discussed before :)
Created attachment 85370 [details] [proposed patch + test cases] on top v829 - all jdt.core tests OK fix includes new test cases. No NPE anymore, and new Javadoc specific message added. Ex: "Javadoc: Illegal qualified access from the type parameter T"
Created attachment 85385 [details] [proposed patch + test case] on top v829 - all jdt.core tests OK as per Frederic remarks: more generic message
(In reply to comment #7) > Created an attachment (id=85385) [details] > [proposed patch + test case] on top v829 - all jdt.core tests OK > > as per Frederic remarks: more generic message > My remarks were more about fix for bug 209936 than for this one... But as I definitely like comments explaining code subtleties, I would expect for this fix, a comment explaining why getTypeBinding(scope) can return null. Something like: // End resolution when getTypeBinding(scope) returns null. This may happen in // certain circumstances, typically when an illegal access is done on a type // variable (see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=204749)
Created attachment 85440 [details] [proposed patch + test cases] on top v829 - all jdt.core tests OK (In reply to comment #8) > My remarks were more about fix for bug 209936 than for this one... Thought we had a discussion about this one, that conducted invalidate proposed patch and implement a new fix. That's what "as per Frederic remarks" was referring to. >But as I definitely like comments explaining code subtleties, I would expect for >this fix, a comment explaining why getTypeBinding(scope) can return null. Added your comment in the null test in JavadocQualifiedTypeReference#internalResolveType(...) Also added the same guard to JavadocSingleTypeReference#internalResolveType(...) as the same code pattern can be found. Thanks for the reviews.
Patch released for 3.4M5 in HEAD.
Verified for 3.4M5 using I20080204-0010
