Bug 68305 - [navigation] mark occurrence of exception disregards inner exceptions
Summary: [navigation] mark occurrence of exception disregards inner exceptions
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.8 M2   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 222814 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-06-23 10:31 EDT by Timo Nentwig CLA
Modified: 2011-09-13 04:09 EDT (History)
5 users (show)

See Also:
raksha.vasisht: review+


Attachments
fix+tests (7.30 KB, patch)
2011-08-17 08:09 EDT, Deepak Azad CLA
no flags Details | Diff
final fix + tests (10.61 KB, patch)
2011-08-18 12:01 EDT, Deepak Azad CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timo Nentwig CLA 2004-06-23 10:31:19 EDT
Marking e1 will mark both URLs but the second one is never caught by e1 but by e2.

try
{
	URL u1 = new URL("mal://formed");
	try
	{
		URL u2 = new URL("mal://formed");
	}
	catch( Exception e2 )
	{
	}
}
catch( Exception e1 )
{
}
Comment 1 Dani Megert CLA 2004-06-23 17:28:40 EDT
Not critical for 3.0
Comment 2 Dani Megert CLA 2005-05-25 17:10:48 EDT
Deferred.
Comment 3 Dani Megert CLA 2008-03-15 05:31:56 EDT
*** Bug 222814 has been marked as a duplicate of this bug. ***
Comment 4 Deepak Azad CLA 2011-07-09 01:34:31 EDT
In the following snippet select 'Exception' in the throws clause
=> all three throw statements are marked even though 2 of the exceptions are caught. 
----------------------------------------------------------------------------
    void foo(String s) throws Exception {
        try {
            if (s == null)
                throw new NullPointerException();
            else if (s.length() > 10)
                throw new IllegalArgumentException();
            else
                throw new Exception();
        } catch (NullPointerException e) {
            e.printStackTrace();
        } catch (IllegalArgumentException e) {
            e.printStackTrace();
        }
    }
----------------------------------------------------------------------------
Comment 5 Deepak Azad CLA 2011-08-17 08:09:07 EDT
Created attachment 201636 [details]
fix+tests

The fix is on the lines of the functionality/code already there in MethodExitsFinder.

(Also I have added the tests to MarkOccurrenceTest17 and not to MarkOccurrenceTest because MarkOccurrenceTest is set up differently and it is just simpler to add new tests to MarkOccurrenceTest17)
Comment 6 Deepak Azad CLA 2011-08-17 08:11:08 EDT
Raksha, Markus can you please review the changes ? Note that I created the patch against HEAD.
Comment 7 Raksha Vasisht CLA 2011-08-18 10:05:34 EDT
(In reply to comment #6)
> Raksha, Markus can you please review the changes ? Note that I created the
> patch against HEAD.

Looks good. +1. You could may be replace '*Catched*' by '*Caught*' everywhere? , sounds better :)
Comment 8 Dani Megert CLA 2011-08-18 11:10:52 EDT
Strictly speaking this is not a 3.7.1 candidate. It made it on the 3.7.1 list while working on Java 7 support, see bug 351441 comment3.

>Note that I created the patch against HEAD.
I see that. Restructuring code should not happen for maintenance fixes. The changes should be as minimal as possible to fix the problem at hand. In that sense, the given patch is too big to fix a 3.4 problem.

If you attach a new patch with only the relevant changes and fix the typo that Raksha mentioned, then it might get the +1.
Comment 9 Deepak Azad CLA 2011-08-18 12:01:02 EDT
Created attachment 201727 [details]
final fix + tests

(In reply to comment #7)
> You could may be replace '*Catched*' by '*Caught*' everywhere?
> , sounds better :)
Also fixed in MethodExitsFinder.
Comment 10 Deepak Azad CLA 2011-08-18 12:01:41 EDT
(In reply to comment #8)
> Strictly speaking this is not a 3.7.1 candidate. 
Moving it to 3.8.

Fixed in HEAD.
Comment 11 Dani Megert CLA 2011-09-13 04:09:49 EDT
Verified in 3.8 (I20110912-2126) and 4.2 (I20110912-0200).