Bug 315776 - 'illegally referenced method' warning not generated in method-local classes
Summary: 'illegally referenced method' warning not generated in method-local classes
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: API Tools (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.7 M4   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 324079 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-06-04 10:51 EDT by John Cortell CLA
Modified: 2010-11-15 12:06 EST (History)
4 users (show)

See Also:
Michael_Rennie: review+


Attachments
Screenshot showing the problem (26.61 KB, image/gif)
2010-06-04 10:51 EDT, John Cortell CLA
no flags Details
plugin projects showing problem (17.17 KB, application/octet-stream)
2010-06-04 11:09 EDT, John Cortell CLA
no flags Details
Proposed fix + regression tests (13.47 KB, patch)
2010-06-04 14:14 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix + regression tests (15.12 KB, patch)
2010-06-04 14:25 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix + regression tests (12.57 KB, patch)
2010-11-05 10:16 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix + regression tests (13.91 KB, patch)
2010-11-05 11:05 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix + regression tests (13.93 KB, patch)
2010-11-05 11:07 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix + regression tests (16.11 KB, patch)
2010-11-05 13:53 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Cortell CLA 2010-06-04 10:51:04 EDT
See attached screenshot. Executable.getSourceFiles() is in another plugin and is tagged with @noreference. This generates warnings in client code that invokes the method (expected), but not if the invocation is in a method-local class.
Comment 1 John Cortell CLA 2010-06-04 10:51:42 EDT
Created attachment 171106 [details]
Screenshot showing the problem
Comment 2 John Cortell CLA 2010-06-04 10:52:34 EDT
This is with 3.6 RC3
Comment 3 Olivier Thomann CLA 2010-06-04 10:55:30 EDT
Could you please attach a small set of bundles that reproduces the problem?
Thanks.
Comment 4 John Cortell CLA 2010-06-04 11:09:34 EDT
Created attachment 171110 [details]
plugin projects showing problem

Set of plugin projects showing problem. Search source for "SHOULD GENERATE WARNING" (in Activator of client plugin).
Comment 5 Olivier Thomann CLA 2010-06-04 14:14:37 EDT
Created attachment 171143 [details]
Proposed fix + regression tests

Michael, please review.
Comment 6 Olivier Thomann CLA 2010-06-04 14:17:12 EDT
Things to consider:
- when local and anonymous types should be scanned for api usage?
- can we do duplicate work for anonymous and local types ?

I think we should only fix the case of the type scope. There was a comment to say that local/anonymous/inner types should not be processed in the visitor.

Michael, I let you see what we want to do in these cases.
Comment 7 Olivier Thomann CLA 2010-06-04 14:25:21 EDT
Created attachment 171144 [details]
Proposed fix + regression tests

Same patch with updated copyrights.
Comment 8 Michael Rennie CLA 2010-06-18 12:07:19 EDT
(In reply to comment #6)
> Things to consider:
> - when local and anonymous types should be scanned for api usage?

For usage scans I think we always want to scan them

> - can we do duplicate work for anonymous and local types ?

I am not sure what you mean here - are you worried that we might do too much work?

> I think we should only fix the case of the type scope. There was a comment to
> say that local/anonymous/inner types should not be processed in the visitor.

The reason for the comment was that each class file in the location is processed by the TypeStructureBuilder, so we don't process any member types because we end up processing their class file separately anyway (this could be improved).
Comment 9 Michael Rennie CLA 2010-06-18 12:48:29 EDT
got the following NPE (and many "wrong number of errors reported") exceptions running the test suite with the patch applied:

java.lang.NullPointerException
	at org.eclipse.pde.api.tools.internal.builder.TypeScope.accept(TypeScope.java:103)
	at org.eclipse.pde.api.tools.internal.builder.ReferenceAnalyzer.extractReferences(ReferenceAnalyzer.java:213)
	at org.eclipse.pde.api.tools.internal.builder.ReferenceAnalyzer.analyze(ReferenceAnalyzer.java:244)
	at org.eclipse.pde.api.tools.internal.builder.BaseApiAnalyzer.checkApiUsage(BaseApiAnalyzer.java:912)
	at org.eclipse.pde.api.tools.internal.builder.BaseApiAnalyzer.analyzeComponent(BaseApiAnalyzer.java:262)
	at org.eclipse.pde.api.tools.internal.builder.IncrementalApiBuilder.build(IncrementalApiBuilder.java:272)
	at org.eclipse.pde.api.tools.internal.builder.IncrementalApiBuilder.build(IncrementalApiBuilder.java:230)
	at org.eclipse.pde.api.tools.internal.builder.ApiAnalysisBuilder.build(ApiAnalysisBuilder.java:314)
	at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:629)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:172)
	at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:203)
	at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:255)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:258)
	at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:311)
	at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:343)
	at org.eclipse.core.internal.resources.Workspace.build(Workspace.java:344)
	at org.eclipse.jdt.core.tests.builder.TestingEnvironment.incrementalBuild(TestingEnvironment.java:762)
	at org.eclipse.jdt.core.tests.builder.BuilderTests.incrementalBuild(BuilderTests.java:417)
	at org.eclipse.pde.api.tools.builder.tests.leak.LeakTest.deployLeakTest(LeakTest.java:150)
	at org.eclipse.pde.api.tools.builder.tests.leak.MethodParameterLeak.x20(MethodParameterLeak.java:538)
	at org.eclipse.pde.api.tools.builder.tests.leak.MethodParameterLeak.testMethodParameterLeak20I(MethodParameterLeak.java:527)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at junit.framework.TestCase.runTest(TestCase.java:168)
	at junit.framework.TestCase.runBare(TestCase.java:134)
	at junit.framework.TestResult$1.protect(TestResult.java:110)
	at junit.framework.TestResult.runProtected(TestResult.java:128)
	at junit.framework.TestResult.run(TestResult.java:113)
	at junit.framework.TestCase.run(TestCase.java:124)
	at junit.framework.TestSuite.runTest(TestSuite.java:232)
	at junit.framework.TestSuite.run(TestSuite.java:227)
	at junit.framework.TestSuite.runTest(TestSuite.java:232)
	at junit.framework.TestSuite.run(TestSuite.java:227)
	at junit.framework.TestSuite.runTest(TestSuite.java:232)
	at junit.framework.TestSuite.run(TestSuite.java:227)
	at junit.framework.TestSuite.runTest(TestSuite.java:232)
	at junit.framework.TestSuite.run(TestSuite.java:227)
	at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62)
	at org.eclipse.pde.internal.junit.runtime.UITestApplication$1.run(UITestApplication.java:116)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:134)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3527)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3174)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2629)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2593)
	at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2427)
	at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:670)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:663)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115)
	at org.eclipse.pde.internal.junit.runtime.UITestApplication.start(UITestApplication.java:47)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:369)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:619)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:574)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1407)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1383)

Without the patch all tests pass.
Comment 10 Olivier Thomann CLA 2010-06-18 13:01:02 EDT
Ok, this means that something is not right. I'll continue to investigate.
Comment 11 Olivier Thomann CLA 2010-10-15 10:34:59 EDT
*** Bug 324079 has been marked as a duplicate of this bug. ***
Comment 12 Olivier Thomann CLA 2010-11-05 10:16:42 EDT
Created attachment 182474 [details]
Proposed fix + regression tests
Comment 13 Olivier Thomann CLA 2010-11-05 10:17:49 EDT
Michael, please verify.
The new patch is much simpler than the previous one.
Comment 14 Olivier Thomann CLA 2010-11-05 10:28:35 EDT
I'll revisit the patch. There is a location issue with the given test case.
New patch is coming soon.
Comment 15 Olivier Thomann CLA 2010-11-05 11:05:27 EDT
Created attachment 182480 [details]
Proposed fix + regression tests

New patch that fixed some issues with line numbers for the reported problems.
lineNumber-- should only be used when using document methods with are 0-based, but the actual line number for the problem should be 1-based.
Comment 16 Olivier Thomann CLA 2010-11-05 11:07:14 EDT
Created attachment 182481 [details]
Proposed fix + regression tests

Same patch with a non-nls tag added.
Comment 17 Olivier Thomann CLA 2010-11-05 13:53:37 EDT
Created attachment 182503 [details]
Proposed fix + regression tests

Fix issues with line numbers from the previous patch.
All tests passed.
Comment 18 Olivier Thomann CLA 2010-11-05 17:09:03 EDT
Released for 3.7M4.
Michael, please verify.
Comment 19 Michael Rennie CLA 2010-11-15 12:06:16 EST
+1 looks good. All Junit / smoke tests pass.