Bug 331996

Comment 1 Ankur Sharma 2010-12-07 05:29:51 EST

Created attachment 184702 [details]
Test Case patch to reproduce the NPE

Comment 2 Ankur Sharma 2010-12-07 05:32:00 EST

The problem occurs with the simple missing type problem as well.

Comment 3 Ankur Sharma 2010-12-07 06:37:23 EST

Created attachment 184705 [details]
Bug fix patch

Comment 4 Ankur Sharma 2010-12-07 06:38:13 EST

Created attachment 184706 [details]
test case patch to reproduce/catch the NPE

Comment 5 Curtis Windatt 2010-12-07 10:19:47 EST

+1 Fixed in HEAD.  Fixes the failing tests and adds another test case.

Comment 6 Curtis Windatt 2010-12-07 13:34:54 EST

Reviewing the code with Mike, we found that there was an unnecessary if statement.  If a type cannot be found, the util method to get the resource will return the manifest, so there is no need to check.

Comment 7 Ankur Sharma 2010-12-08 02:30:21 EST

That if statement was to put the marker for missing inner type on the parent type instead of the project.

Comment 8 Michael Rennie 2010-12-08 09:39:33 EST

(In reply to comment #7)
> That if statement was to put the marker for missing inner type on the parent
> type instead of the project.

The if statement assumed that the parent type was the name of the type proceeding the first '$' in the type name. This will not always give the parent type in the case of multiple inner types. Consider the example:

public class A {
  class B {
    class C {
      class D {
      }
    }
  }
}

If the reference to D is removed the marker will be placed on A, which is not the parent type of D.

We just need to tweak the if statement a bit to look for the last index of '$' in the type name and take the proceeding name.

Comment 9 Ankur Sharma 2010-12-08 10:55:58 EST

(In reply to comment #8)
> We just need to tweak the if statement a bit to look for the last index of '$'
> in the type name and take the proceeding name.

True and that would mean to recurse back until we find a type that still exists. For example, the reference is for D and type C is renamed. This would make D unresolvable and the problem marker then will have to be placed on B.

I feel we shall open a new bug for this that we will look in for M5

Comment 10 Michael Rennie 2010-12-08 11:21:58 EST

Created attachment 184794 [details]
fix

roll-up patch that includes the NPE check and walks the enclosing type names to find the smallest (existing) enclosing type to place the marker on.

Comment 11 Ankur Sharma 2010-12-08 12:11:18 EST

The 'inner.patch' fails the existing tests. This is because of the change

 			if(!Util.isManifest(res.getProjectRelativePath())) {
-				resource = res.getProjectRelativePath().toString();
+				rname = res.getProjectRelativePath().toString();
 			}
 			else {
-				resource = "."; //$NON-NLS-1$
+				rname = res.getProject().getName();
 			}


The reason it fails is because 'rname' is supposed to the the resource name which will be found using IJavaProject.findMember. That is why if a marker has to be placed on project, we have to set it to "." which will have findMember to return the project. The project name will make it return null and the marker will not get created.

Comment 12 Curtis Windatt 2010-12-08 12:40:56 EST

The fix that is going in to M4 will be the original bug fix patch attachment# 184705 [details].  We will improve the marker placement in M5 in bug 332157.