Bugzilla – Full Text Bug Listing
| Summary: | [rename] Renaming public class to name of compilation unit fatal error | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Andrew Eisenberg <andrew.eisenberg> |
| Component: | UI | Assignee: | Deepak Azad <deepakazad> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, deepakazad, eclipse, markus.kell.r |
| Version: | 3.7.1 | ||
| Target Milestone: | 3.7.2 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
I've been trying to create a proper patch for this, but it's been taking a while and I haven't gotten it yet. It's easier to simply describe the fix for this bug: In RenameTypeProcessor.java line 494 Change this: if (Checks.isTopLevel(fType) && (JdtFlags.isPublic(fType))) result.merge(Checks.checkCompilationUnitNewName(fType.getCompilationUnit(), getNewElementName())); To this: // Bug 365380: don't do this check if we are renaming the top-level type to the name of the enclosing compilation unit ICompilationUnit cu = fType.getCompilationUnit(); String newCUName= JavaModelUtil.getRenamedCUName(cu, getNewElementName()); if (!cu.getElementName().equals(newCUName) && Checks.isTopLevel(fType) && (JdtFlags.isPublic(fType))) result.merge(Checks.checkCompilationUnitNewName(fType.getCompilationUnit(), getNewElementName())); This works for me. I haven't written a unit test for this, but my description in the previous comment describes what such a test looks like. Thanks Andrew! I will take a look during M5. Thanks for having a look. Any chance that this could be backported to 3.7.2? Fixed in master - b7e4ce5095782863fc1b8d6abfaecc2e4f280f1a I check in Checks.checkCompilationUnitNewName(...) method if the new name for CU is the same as the old name. (In reply to comment #3) > Any chance that this could be backported to 3.7.2? The problem is not new in 3.7, and is also minor since you can use the quick fix to fix the compile error. Hence, this will not be backported to 3.7.2. Would be nice to have this in 3.7.2 since this is affecting Groovy-Eclipse, See: https://jira.codehaus.org/browse/GRECLIPSE-1049 This kind of situation is much more common in Groovy than it is in Java. (In reply to comment #5) > Would be nice to have this in 3.7.2 since this is affecting Groovy-Eclipse, > See: https://jira.codehaus.org/browse/GRECLIPSE-1049 > > This kind of situation is much more common in Groovy than it is in Java. Can you explain why it is more common in Groovy (how many bugs did you get about this?) and especially why it should now be considered important enough to be backported after being there for 10 years? *** Bug 185060 has been marked as a duplicate of this bug. *** (In reply to comment #6) > Can you explain why it is more common in Groovy (how many bugs did you get > about this?) and especially why it should now be considered important enough to > be backported after being there for 10 years? Sure. In Groovy, there may be more than one public class in a compilation unit. Also, the name of the compilation unit does not need to be related to any of the names of public classes. Although, we only have one bug report for this in our issue tracker, I have heard about this problem anecdotally in the mailing list and on twitter. I understand that this is a minor issue for Java users, but the Groovy community is growing and we are simply trying to provide the best experience for our users. > I understand that this is a minor issue for Java users, but the Groovy
> community is growing and we are simply trying to provide the best experience
> for our users.
OK, Markus will double-check the fix for potential unwanted side-effects and if he clears the fix I'm fine with backporting it.
(In reply to comment #4) > I check in Checks.checkCompilationUnitNewName(...) method if the new name for > CU is the same as the old name. That change was not good: - bad Javadoc of Checks#checkCompilationUnitNewName(ICompilationUnit, String) - allowed bad refactorings to proceed, e.g. Extract Superclass with same name Fixed in master along the lines of comment 1, but also removed the bogus check whether the type is public and fixed bad invocations of the helper method in RenameCompilationUnitProcessor. commit f56eb1cfb645cc3fe7b0cb9a70303c7f28979372 Deepak, can you please check the final solution and release to 3.7.2 if OK? Cherrypicked commits from comment 4 and comment 10 to R3_7_maintenance. Verified in M20120111-0800. |
In file MyClass.java: public class MyOtherClass { } (Note that this file has a compilation error since MyOtherClass is public, but not named after the compilation unit) Rename MyOtherClass to MyClass. Fatal error: Compilation unit 'MyClass.java' already exists. I expected that this would work even though there is a compilation error since executing the refactoring will fix the compile error. Regardless, the error message is confusing.