Bugzilla – Full Text Bug Listing
| Summary: | [1.7] Overlapping error ranges on multi-catch block in 1.5 mode | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Deepak Azad <deepakazad> |
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> |
| Status: | VERIFIED WONTFIX | QA Contact: | |
| Severity: | minor | ||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert, markus.kell.r, Olivier_Thomann, srikanth_sankaran |
| Version: | 3.7 | ||
| Target Milestone: | 3.7.1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
I'll follow up. (In reply to comment #0) > - Try the following snippet in a 1.5 (or even 1.4) project > => You get this error - "The exception FileNotFoundException is already caught > by the alternative IOException". This is wrong, in 1.5 mode the error should be > syntax error on |. > I get the two errors shown below: - The exception FileNotFoundException is already caught by the alternative IOException - Multi-catch parameters are not allowed for source level below 1.7 This is the intended behavior. Don't you see the second error ??? Ha! The 2 errors can be seen in Problems view. But when you hover over the red squiggly line in the editor you only see "The exception FileNotFoundException is already caught by the alternative IOException". When you hover over the 'X' in the vertical ruler you see "Multiple markers at this line - The exception FileNotFoundException is already caught by the alternative IOException - Multi-catch parameters are not allowed for source level below 1.7 " Can "Multi-catch parameters are not allowed for source level below 1.7" be shown as the first error ? Though I am not sure if showing 2 errors is correct - "The exception FileNotFoundException is already caught by the alternative IOException" does not make sense in the 1.5 mode. "Multi-catch parameters are not allowed for source level below 1.7" should be the only error shown. (In reply to comment #4) > Though I am not sure if showing 2 errors is correct - "The exception > FileNotFoundException is already caught by the alternative IOException" does > not make sense in the 1.5 mode. > > "Multi-catch parameters are not allowed for source level below 1.7" should be > the only error shown. Actually, having reported the error about 1.7 level, the compiler does try to analyze the code further to report additional errors. This is standard compiler strategy: Try the following in 1.4 mode for a comparison: public class X<T extends String> { X<Integer> y = null; } (In reply to comment #3) > Can "Multi-catch parameters are not allowed for source level below 1.7" be > shown as the first error ? Isn't this something that would fall under the UI's purview ?? (In reply to comment #5) > (In reply to comment #4) > > Though I am not sure if showing 2 errors is correct - "The exception > > FileNotFoundException is already caught by the alternative IOException" does > > not make sense in the 1.5 mode. > > > > "Multi-catch parameters are not allowed for source level below 1.7" should be > > the only error shown. > > Actually, having reported the error about 1.7 level, the compiler does > try to analyze the code further to report additional errors. This is > standard compiler strategy: hmm.. As a user I expect to see an error about the compiler compliance level or a syntax error, which is not the case currently 'in the Java editor'. Javac just reports about the compiler compliance level. (In reply to comment #6) > (In reply to comment #3) > > > Can "Multi-catch parameters are not allowed for source level below 1.7" be > > shown as the first error ? > > Isn't this something that would fall under the UI's purview ?? Maybe, I will have to take a look. In the following snippet with 1.5 the compiler does not say anything about 1.7 level, while javac does. I suppose this is also a bug, since the compiler error reporting behavior is not consistent? ------------------------------------------------------------- void foo(String s) { switch (s) { case "abc": System.out.println("abcd"); break; case "xyz": System.out.println("xyz"); break; } } ------------------------------------------------------------- (In reply to comment #6) > (In reply to comment #3) > > > Can "Multi-catch parameters are not allowed for source level below 1.7" be > > shown as the first error ? > > Isn't this something that would fall under the UI's purview ?? I also raised a similar doubt a few weeks back. I couldn't find any code in JDT/core which ranks the reported errors.(In reply to comment #7) > In the following snippet with 1.5 the compiler does not say anything about 1.7 > level, while javac does. I suppose this is also a bug, since the compiler error > reporting behavior is not consistent? > ------------------------------------------------------------- > void foo(String s) { > switch (s) { I don't think it is really necessary to either mention 1.7 or align the wording with javac's. So, I won't say its an inconsistency. Eg: if you use enum in 1.4 mode, we dont complain that enum is only available in 1.5. But I guess mentioning the 1.7 level in the error will make the diagnostic better for a user who's under the impression that his compliance is set to 1.7 but is actually not. I don't see a bug here - only intended behavior with analogous precedents. (see comment# 5) Hence resolving this as INVALID. (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #3) > > > > > Can "Multi-catch parameters are not allowed for source level below 1.7" be > > > shown as the first error ? > > > > Isn't this something that would fall under the UI's purview ?? > Maybe, I will have to take a look. Srikanth, at the very least the above needs to happen, so that the user sees the more appropriate error msg in the Java editor. I hope you agree to this much ? Whether the fix will go in Core or UI is a different issue.. (In reply to comment #10) > Srikanth, at the very least the above needs to happen, so that the user sees > the more appropriate error msg in the Java editor. I hope you agree to this > much ? Sure. Too sleepy - I overlooked this side issue and resolved this already. Can you take a look and see if the UI has the machinery to anything here ? As noted in comment# 5, this predates 1.7 work (i.e the bound check error is what is shown when you hover over the red underlined text) and even those cases will need similar handling. Not sure how much work it would take to recognize what should be shown ahead of others and whether it is worth it. I think the editor already orders overlapping problems and makes sure the longer error doesn't hide the shorter one. I just released the quick fix for the syntax error (change compliance to 1.7, see bug 348459), so at least Ctrl+1 now offers some help. But I agree with Deepak that the additional analysis inside an AST node with syntax errors is not that helpful. This is also bad in the example from comment 5. If you just look at X<Integer>, the bound mismatch error is just hiding the actual problem. Couldn't the compiler stop further analysis in declarations that already contain a syntax error? (In reply to comment #12) > Couldn't the compiler stop further analysis in declarations that already > contain a syntax error? What would be a "declaration" ? By extending this argument one level at a time, one can say if some error is found at all in a compilation unit, then don't analyze it further. The compiler will continue to analyze programs that have syntax errors and will do its best to report additional errors making reasonable efforts in the meantime to avoid a flurry of cascading errors where it can. That is what is happening here. I'll reassign it to inbox, so someone willing & able to pursue this can do so, if he/she deems fit to do so. > > Couldn't the compiler stop further analysis in declarations that already
> > contain a syntax error?
>
> What would be a "declaration" ? By extending this argument one level at a time,
> one can say if some error is found at all in a compilation unit, then don't
> analyze it further.
This would only apply to the first enclosing BodyDeclaration (recovered or real).
The second error basically comes from UnionTypeReference.resolveType(). While we could just suppress the error in source level below 1.7, I was thinking why do we need a resolved binding for the UnionTypeRef below 1.7 at all? Also, won't a client code that is not even aware of a "UnionTypeRef" break if we now start returning a UnionTypeRef binding even below 1.7? On these lines, I was wondering if we should just check the source level at the start of the resolution process and bail out with a null binding if we find it to be < 1.7. This would solve the problem of secondary errors automatically. (In reply to comment #16) > Also, > won't a client code that is not even aware of a "UnionTypeRef" break if we now > start returning a UnionTypeRef binding even below 1.7? There should be no clients as UnionTypeReference is JDT/Core internal. The question then shifts to whether the equivalent DOM type org.eclipse.jdt.core.dom.UnionType is exposed to clients which are unprepared for them. My understanding is that this is not the case and that we would flag the AST as being malformed (see org.eclipse.jdt.core.dom.ASTNode.MALFORMED), but this assumption needs to be verified. >On these lines, I was > wondering if we should just check the source level at the start of the > resolution process and bail out with a null binding if we find it to be < 1.7. > This would solve the problem of secondary errors automatically. This is a plausible approach, though this is not what we have traditionally done, per the earlier example with generics bounds check at 1.4 level. (In reply to comment #17) > (In reply to comment #16) > unprepared for them. My understanding is that this is not the case and > that we would flag the AST as being malformed (see > org.eclipse.jdt.core.dom.ASTNode.MALFORMED), but this assumption needs to > be verified. See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=339864 Is it possible to just suppress the error, like Bug 348406 ? The error msg "The exception FileNotFoundException is already caught by the alternative IOException" is not new in 1.5 mode, as opposed to Bug 348406 where the error msg referred to java.lang.AutoCloseable which does not even exist in 1.5. Hence, I think in this case it might also be ok to let things be as it is. (In reply to comment #17) > >On these lines, I was > > wondering if we should just check the source level at the start of the > > resolution process and bail out with a null binding if we find it to be < 1.7. > > This would solve the problem of secondary errors automatically. > > This is a plausible approach, though this is not what we have traditionally > done, per the earlier example with generics bounds check at 1.4 level. Ayush, please proceed on your suggestion and propose a tested patch and let us take a call after that. (In reply to comment #20) > (In reply to comment #17) Actually, my earlier concern about the unexpected UnionType node is not too valid since the UnionType is only obtained in AST level 4. And a user who choses to use the AST level 4 should, by all means, be prepared to handle a UnionType. In AST level < 4, we don't get the UnionType. Also, as per comment 19, the current warning seen on FileNotFoundException is relevant and not totally bogus. I'm closing as WONTFIX. OK, I filed bug 348860 for a quick assist for the redundant caught type. Verified. |
- Try the following snippet in a 1.5 (or even 1.4) project => You get this error - "The exception FileNotFoundException is already caught by the alternative IOException". This is wrong, in 1.5 mode the error should be syntax error on |. ------------------------------------------------------------------------ package org.eclipse; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InterruptedIOException; class MultiCatch { void foo() { try { throw new FileNotFoundException(); } catch (FileNotFoundException | IOException ex) { } } } ------------------------------------------------------------------------