Bugzilla – Full Text Bug Listing
| Summary: | Changing class name in source leads to missing inner class compilation artefact | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Simeon Andreev <simeon.danailov.andreev> | ||||
| Component: | Core | Assignee: | Simeon Andreev <simeon.danailov.andreev> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | loskutov, stephan.herrmann | ||||
| Version: | 3.8.2 | ||||||
| Target Milestone: | 4.8 M6 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| See Also: |
https://git.eclipse.org/r/116552 https://git.eclipse.org/c/jdt/eclipse.git/commit/?id=05b6ddd634f88cfd820ceaa84cbc7bf143602be6 |
||||||
| Whiteboard: | |||||||
| Bug Depends on: | 530634 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
Apparently the build thinks the renamed class is a secondary type, so it removes its .class files: Thread [Worker-14] (Suspended (breakpoint at line 724 in Resource)) File(Resource).delete(int, IProgressMonitor) line: 724 IncrementalImageBuilder.removeClassFile(IPath, IContainer) line: 800 IncrementalImageBuilder.removeSecondaryTypes() line: 813 IncrementalImageBuilder.incrementalBuildLoop() line: 184 IncrementalImageBuilder.build(SimpleLookupTable) line: 140 JavaBuilder.buildDeltas(SimpleLookupTable) line: 276 JavaBuilder.build(int, Map, IProgressMonitor) line: 197 BuildManager$2.run() line: 740 SafeRunner.run(ISafeRunnable) line: 42 BuildManager.basicBuild(int, IncrementalProjectBuilder, Map<String,String>, MultiStatus, IProgressMonitor) line: 210 BuildManager.basicBuild(IBuildConfiguration, int, IBuildContext, ICommand[], MultiStatus, IProgressMonitor) line: 253 BuildManager$1.run() line: 306 SafeRunner.run(ISafeRunnable) line: 42 BuildManager.basicBuild(IBuildConfiguration, int, IBuildContext, MultiStatus, IProgressMonitor) line: 309 BuildManager.basicBuildLoop(IBuildConfiguration[], IBuildConfiguration[], int, MultiStatus, IProgressMonitor) line: 361 BuildManager.build(IBuildConfiguration[], IBuildConfiguration[], int, IProgressMonitor) line: 382 AutoBuildJob.doBuild(IProgressMonitor) line: 142 AutoBuildJob.run(IProgressMonitor) line: 232 Worker.run() line: 56 Removed files in org.eclipse.jdt.internal.core.builder.IncrementalImageBuilder.removeSecondaryTypes() are: [a/B, a/B$Inner] > 5. In MyClass2.java, change class name back to MyClass2.
I.e. on this step, the builder will remove "secondary types", in this case MyClass1$InnerClass from MyClass2.java.
It doesn't rebuild MyClass1.java after this though.
The following code looks like it should filter out the generation of the inner class:
Thread [Worker-12] (Suspended)
State.isDuplicateLocator(String, String) line: 142
IncrementalImageBuilder(AbstractImageBuilder).acceptResult(CompilationResult) line: 167
Compiler.processCompiledUnits(int, boolean) line: 615
Compiler.compile(ICompilationUnit[], boolean) line: 472
Compiler.compile(ICompilationUnit[]) line: 423
IncrementalImageBuilder(AbstractImageBuilder).compile(SourceFile[], SourceFile[], boolean) line: 381
IncrementalImageBuilder.compile(SourceFile[], SourceFile[], boolean) line: 368
IncrementalImageBuilder(AbstractImageBuilder).compile(SourceFile[]) line: 313
IncrementalImageBuilder.incrementalBuildLoop() line: 183
IncrementalImageBuilder.build(SimpleLookupTable) line: 140
JavaBuilder.buildDeltas(SimpleLookupTable) line: 276
JavaBuilder.build(int, Map, IProgressMonitor) line: 197
BuildManager$2.run() line: 740
SafeRunner.run(ISafeRunnable) line: 42
BuildManager.basicBuild(int, IncrementalProjectBuilder, Map<String,String>, MultiStatus, IProgressMonitor) line: 210
BuildManager.basicBuild(IBuildConfiguration, int, IBuildContext, ICommand[], MultiStatus, IProgressMonitor) line: 253
BuildManager$1.run() line: 306
SafeRunner.run(ISafeRunnable) line: 42
BuildManager.basicBuild(IBuildConfiguration, int, IBuildContext, MultiStatus, IProgressMonitor) line: 309
BuildManager.basicBuildLoop(IBuildConfiguration[], IBuildConfiguration[], int, MultiStatus, IProgressMonitor) line: 361
BuildManager.build(IBuildConfiguration[], IBuildConfiguration[], int, IProgressMonitor) line: 382
AutoBuildJob.doBuild(IProgressMonitor) line: 142
AutoBuildJob.run(IProgressMonitor) line: 232
Worker.run() line: 56
However it does not, since no duplicate is detected:
public boolean isDuplicateLocator(String qualifiedTypeName, String typeLocator) {
String existing = (String) this.typeLocators.get(qualifiedTypeName);
return existing != null && !existing.equals(typeLocator);
}
When checking with the inner class SomeClass$InnerClass in SomeOtherclass.java, typeLocators contains:
somepackage/SomeClass -> src/somepackage/SomeClass.java
somepackage/SomeOtherClass -> src/somepackage/SomeOtherClass.java
The following piece of code doesn't make much sense as far as I can tell:
// Look for a possible collision, if one exists, report an error but do not write the class file
if (isNestedType) {
String qualifiedTypeName = new String(classFile.outerMostEnclosingClassFile().fileName());
if (this.newState.isDuplicateLocator(qualifiedTypeName, typeLocator))
continue;
} else {
...
this.newState.recordLocatorForType(qualifiedTypeName, typeLocator);
if (result.checkSecondaryTypes && !qualifiedTypeName.equals(compilationUnit.initialTypeName))
acceptSecondaryType(classFile);
}
There are no other entry points to State.typeLocators, aside from reading the build state from the disk.
So this code block will check for duplicates for nested types, but will not record nested types with which to check... This is as old as 2002, commit: https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1d631cdc6d8aa97d297a027f3883f9c7312c678d
So to sum up, there are two problems that I can see: 1. SomeClass$InnerClass.class from SomeOtherClass.java overwrites SomeClass$InnerClass.class from SomeClass.java, when the name of the type in SomeOtherClass.java is changed from SomeOtherClass to SomeClass. (comment #3) 2. SomeClass$InnerClass.class is removed, when renaming type SomeClass to SomeOtherClass in SomeOtherClass.java. (comments #1 and #2) Andrey please judge if its worth to try to fix those 2 points. The problem reported in our product, and the more general problem reported in this bug, can both be avoided by using refactor rename and copy-paste in Eclipse. New Gerrit change created: https://git.eclipse.org/r/116552 I believe I found an OK solution, submitted review https://git.eclipse.org/r/#/c/116552/. (In reply to Simeon Andreev from comment #6) > I believe I found an OK solution, submitted review > https://git.eclipse.org/r/#/c/116552/. FYI, your jenkins job seems to be affected / blocked by bug 530634. (In reply to Stephan Herrmann from comment #7) > (In reply to Simeon Andreev from comment #6) > > I believe I found an OK solution, submitted review > > https://git.eclipse.org/r/#/c/116552/. > > FYI, your jenkins job seems to be affected / blocked by bug 530634. Thanks for the info! Gerrit change https://git.eclipse.org/r/116552 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.git/commit/?id=05b6ddd634f88cfd820ceaa84cbc7bf143602be6 Thanks Simeon! Verified for Eclipse Photon (4.8) M6 with Build id: I20180306-0800 |
Created attachment 272414 [details] Example project to reproduce the problem with. Steps to reproduce: 1. Import project from attached archive. 2. Open MyClass2.java. 3. In MyClass2.java, change class name to MyClass1. 4. Observe expected error, that MyClass1 in MyClass2.java should be defined in its own class. 5. In MyClass2.java, change class name back to MyClass2. 6. Observe that bin/somepackage/MyClass1$InnerClass.class is gone. Or, from scratch: 1. Create Java project BugExample. 2. Create a package somepackage. 3. Create a class somepackage.MyClass1. 4. Define package private, non-static inner class MyClass1$InnerClass. 5. Copy MyClass1.java to MyClass2.java. 6. Open MyClass2.java. 7. In MyClass2.java, change class name to MyClass1. 8. Observe expected error, that MyClass1 in MyClass2.java should be defined in its own class. 9. In MyClass2.java, change class name back to MyClass2. 10. Observe that bin/somepackage/MyClass1$InnerClass.class is gone. A clean build or changing MyClass1.java results in restoring the missing artefact for MyClass1$InnerClass.class. Result here is that any program which needed MyClass1$InnerClass will run into a run-time error (due to the missing .class), although there is no actual compile error and the workspace is supposedly compiled. Reproduced with: Eclipse SDK Version: Photon (4.8) Build id: I20171217-2000 Originally found on Eclipse 3.8.2.