Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 403258

Summary: Use problem on anonymous type in incorrect location
Product: [Eclipse Project] PDE Reporter: Michael Rennie <Michael_Rennie>
Component: API ToolsAssignee: Michael Rennie <Michael_Rennie>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: curtis.windatt.public, markus.kell.r
Version: 4.3   
Target Milestone: 4.3 M7   
Hardware: PC   
OS: Mac OS X   
Whiteboard:

Description Michael Rennie CLA 2013-03-13 15:25:07 EDT
While working on the API for e4 there is a problem like the following:

org.eclipse.e4.ui.internal.workbench.Activator illegally implements LogService

in the above example the illegal implements is on an anonymous type but the error marker is on the root type, causing confusion when you otry to figure out what is going on.

We should try to place the error marker on the class instance creation, failing that at least have a different message like: Actvator illegally implements LogService in an anonymous type
Comment 1 Curtis Windatt CLA 2013-03-13 17:43:47 EDT
To correct the source range we would need to modify AbstractProblemDetector.createProblem() to find the anonymous type rather than its enclosing type (done in getTypeName(member)).  I'm not sure what options exist to find an anonymous type given the api member and the java project. The member has the name org.eclipse.foo.EnclosingType$1.

Another option may be to simply improve the problem message.
Comment 2 Michael Rennie CLA 2013-03-14 10:54:55 EDT
I was pretty sure we already did do this, and we do:

Description	Resource	Path	Location	Type
An anonymous type defined in org.eclipse.ui.internal.menus.MenuHelper illegally extends ActionContributionItem	MenuHelper.java	/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus	line 491	API Usage Problem


So it would be a bug that we failed to correctly place the marker in this case.
Comment 3 Michael Rennie CLA 2013-03-18 17:14:45 EDT
Pushed fix to:

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=83655c5d1780a74e2d7c6b667c44ad80f8afb577

Our extends problem detector already had all of the code to properly resolve local and anonymous types, so I pulled that support up to the root class for illegal type references. I also fixed a bug where only one problem would be reported when more than one local type indirectly illegally implemented the same interface.

I still need to add more tests, so oI'll leave the bug open until I get them done.
Comment 4 Michael Rennie CLA 2013-03-19 12:15:56 EDT
I pushed some tests, but for some reason the reference extractor is failing to pull out the implements references...

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=856bc4b097dac02c6c1703e17560d3ea22861b05
Comment 5 Michael Rennie CLA 2013-03-19 23:23:44 EDT
I fixed the tests here: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=3e42568cc8bc38b275ac9d2eed1aa5a40cb6102d

The oddity was that when compiled at 1.4 (or less) the local types in the inner type do not report illegal use. When compiled 1.5+ it works just fine. So the test fix removes checking for those refs in our 1.4 test project and checks for all of them in the 1.5+ test project (for now).

I think this might be a bug in our classfile reader - I noticed that the names of the local types are different in the classfile, and I bet we are doing something like dividing up a signature on '$', which would cause us to ignore the one in 1.4:

 - in 1.5+: class x.y.z.testC11$inner1$1local3 implements i.INoImpl3
 - in 1.4: class x.y.z.testC11$1$local3 implements i.INoImpl3
Comment 6 Michael Rennie CLA 2013-03-20 00:05:38 EDT
(In reply to comment #5)

> I think this might be a bug in our classfile reader - I noticed that the
> names of the local types are different in the classfile, and I bet we are
> doing something like dividing up a signature on '$', which would cause us to
> ignore the one in 1.4:
> 
>  - in 1.5+: class x.y.z.testC11$inner1$1local3 implements i.INoImpl3
>  - in 1.4: class x.y.z.testC11$1$local3 implements i.INoImpl3

Yup. In Referenceextractor#visitInnerClass we have this:

if(fType.getName().equals(pname) || !pname.startsWith(fType.getName())) {
  return;
}

In the 1.4 case pname equals x.y.z.testC12$1$local4 and fType is x.y.z.testC12$inner1 for the following code pattern (from one of the tests):

public class testC12  {
  class inner1 {
	void method2() {
		class local4 implements INoImpl5 { //indirect illegal implement
	        }
        }
  }
	
  public void method1() {
	class local2 implements INoImpl5 { //indirect illegal implement
	}
  }
}

So we end up skipping the local type in the inner type.

To makes matters a bit worse, we end up creating an IApiType for the local type that cannot resolve it proper enclosing type nor its correct enclosing method.
Comment 7 Michael Rennie CLA 2013-03-20 17:11:46 EDT
(In reply to comment #6)

I added some more tests for anonymous implements:

http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=ceaa6dab7f98363efa6d80e9ca1eb5604ccd3af7
Comment 8 Michael Rennie CLA 2013-03-20 18:09:55 EDT
Marking fixed. I opened bug 403953 to track improving support for anonymous and local types in nested types in the pre-1.5 world.