Community
Participate
Working Groups
Created attachment 110722 [details] proposed patch Consider the following class: public class ClassWithAnon { int container() { new Missing() { void bar(int i) {} }.bar(false); return new Missing2() {}; } } Currently, the compiler correctly reports: ---------- 1. ERROR in ClassWithAnon.java (at line 3) new Missing() { ^^^^^^^ Missing cannot be resolved to a type ---------- 2. ERROR in ClassWithAnon.java (at line 7) return new Missing2() {}; ^^^^^^^^ Missing2 cannot be resolved to a type ---------- 2 problems (2 errors) However, by only deleting(!) two source lines in QualifiedAllocationExpression it can be made to report two more errors, which, too, are interesting: ---------- 1. ERROR in ClassWithAnon.java (at line 3) new Missing() { ^^^^^^^ Missing cannot be resolved to a type ---------- 2. ERROR in ClassWithAnon.java (at line 5) }.bar(false); ^^^ The method bar(int) in the type new Missing(){} is not applicable for the arguments (boolean) ---------- 3. ERROR in ClassWithAnon.java (at line 7) return new Missing2() {}; ^^^^^^^^^^^^^^^^^ Type mismatch: cannot convert from new Missing2(){} to int ---------- 4. ERROR in ClassWithAnon.java (at line 7) return new Missing2() {}; ^^^^^^^^ Missing2 cannot be resolved to a type ---------- 4 problems (4 errors) I'll attach a proposed micro-patch. With all the recently gained resilience I see no danger in proceeding one step further. Am I missing any thing?
I just now realize that the patch will also bring full analysis (resolve) to the *body* of those anonymous classes, which currently are skipped completely. Hm, so much more functionality by just *deleting* two lines ;-) (I *must* be missing something).
OK, a few tests change with my patch in place: ASTConverterTest.test0312, ASTConverterAST3Test.test0312: (declaring node is actually found) linking to http://dev.eclipse.org/bugs/show_bug.cgi?id=11638 ASTConverterTestAST3_2.test0652: (4 more errors reported) ASTConverterBugsTest.testBug218824a, ASTConverterBugsTestJLS3.testBug218824a: (one more error reported) ProblemTypeAndMethodTest.test059 (one more error reported) ExtractMethodTest.test016 (assumes statement cannot be selected, assumption no longer holds) All additional diagnostics look right to me. The negative extract method test might become just obsolete. To me, the test changes seem to indicate that the patch will make a difference to some people: more valid errors reported, more AST resolved.
I have the proposed patch in our production code. While updating JDT/Core and tests to 3.4.1 I had to adapt one more test: org.eclipse.jdt.ui.tests.quickfix.UnresolvedTypesQuickFixTest#testParameterizedType2 now detects two problems instead of one, new one is about incompatible assignment from missing type to known type. Just let me know if you'd like to see a patch with the changes I had to make to tests.
Need to be careful with reporting too many secondary errors in presence of missing types. In order to validate the idea, I would like to hear about a real world testcase. Like take an existing codebase, and discard some types (or remove binaries from the build path) and run a build; check the new errors to see how much value they provide. Like in a case like: new Listener() { void foo() {... } }.notify(...) How valuable is it going to claim #notify(...) cannot be bound, until you can resolve Listener ?
(In reply to comment #4) Not a real world example but a simple comparison: public class Test1 extends Missing { void buggy(Absent a) { notThere.dontEventSearch(unavailable); } } ---------- 1. ERROR in Test1.java (at line 1) public class Test1 extends Missing { ^^^^^^^ Missing cannot be resolved to a type ---------- 2. ERROR in Test1.java (at line 2) void buggy(Absent a) { ^^^^^^ Absent cannot be resolved to a type ---------- 3. ERROR in Test1.java (at line 3) notThere.dontEventSearch(unavailable); ^^^^^^^^ notThere cannot be resolved ---------- 4. ERROR in Test1.java (at line 3) notThere.dontEventSearch(unavailable); ^^^^^^^^^^^ unavailable cannot be resolved ---------- 4 problems (4 errors) GOOD. public class Test2 { Object anonymous() { return new Missing() { void buggy(Absent a) { notThere.dontEventSearch(unavailable); } }; } } ---------- 1. ERROR in Test2.java (at line 3) return new Missing() { ^^^^^^^ Missing cannot be resolved to a type ---------- 1 problem (1 error) Hmmm, why this difference? Reporting too many secondary errors is already avoided by checking HierarchyHasProblems further down, which I'd consider the most consistent solution.
Adjusting title to better reflect the discussion.
I thought this issue would be a quick-and-easy one: Normally, missing supertype does not prevent analysis of the type declaration. For anonymous types it does. Effects: - No errors inside are detected. - Code inside is not selectable (cf. ExtractMethodTest.test016) Change is tiny, changes in test results looked OK to me. Any comments?
Srikanth, please see what we can do in this case. I think with comment 5 Stephan has a case.
Created attachment 155967 [details] Plausible source + test patch Cumulative source + test patch (only JDT/Core tests)
I don't feel strongly one way or other about this issue. Olivier, what is your opinion on this ?
I would say that errors detected inside an anonymous type should not be much different from the error reported against a top level type when the super type is missing. So I would be inclined to go towards reporting more secondary errors. If we could report errors inside the anonymous type without reporting errors as mentioned in comment 4, this would be ideal.
Created attachment 156371 [details] Revised source + test patch This patch avoids reporting secondary problems when the super type of anonymous classes is missing.
Released in HEAD for 3.6M5. Markus, a heads up that some JDT/UI tests may need adjusting/remastering. FYI, Thanks.
(In reply to comment #12) > Created an attachment (id=156371) [details] > Revised source + test patch > > This patch avoids reporting secondary problems when > the super type of anonymous classes is missing. I basically agree. Just two comments: + why restrict the new conditional to anonymous types? Are secondary errors any more valuable when the type is not anonymous? + wouldn't TagBits.HierarchyHasProblems be a more systematic indicator that secondary errors are useless? (haven't tried it, just guessing) no big deal, though.
(In reply to comment #13) > Markus, a heads up that some JDT/UI tests may need adjusting/remastering. I've adjusted ExtractMethodTest.test016 and UnresolvedTypesQuickFixTest#testParameterizedType2 in HEAD.
Verified for 3.6M5 using I20100125-0800
