Community
Participate
Working Groups
Dear Eclipse Team, Upon pressing Ctrl-1 over a previously undefined class, a new blank class is created. The new class always extends Object, which is rarely appropriate. I expected to be able to specify a superclass, a la "create class" wizard, which then overrides abstract methods etc. I'm bringing this feature request to your attention because it is the only case I'm aware of where auto-correction (Ctrl-1) generates more errors than it resolves -- the original error isn't fixed if the original class is an extended class. Maybe an example would help. I entered: Abstract a = new Derived(); The "Abstract" class exists, not the "Derived" one. "Derived" is therefore underlined. I press Ctrl-1 and select "Create class", it creates: public class Derived {} which doesn't really fix the error -- "public class Derived extends Abstract {}" would. The Derived class now exists, but my assignment statement is now wrong, when really it wasn't. The correction is. To summarize, I wish the "create class" auto-correction would respect the contract that the new class being created needs to implement. Or let me choose one. Surprisingly, the anonymous inner type completion already does something similar to this; it correctly identifies the class being completed, and offers to override methods etc. This bug is related, but not a duplicate of, 21782.
I believe I've located the code that relates to this bug. In org.eclipse.jdt.internal.ui.text.correction.\ UnresolvedElementsSubProcessor.getTypeProposals(), around lines number 195-200, it reads: if ((kind & SimilarElementsRequestor.CLASSES) != 0) { (*) String[] superTypes= (problemPos.getId() != IProblem.ExceptionTypeNotFound) ? null : new String[] { "java.lang.Exception" }; //$NON-NLS-1$ String label= CorrectionMessages.getFormattedString ("UnresolvedElementsSubProcessor.createclass.description", typeName); //$NON- NLS-1$ proposals.add(new NewCUCompletionProposal(label, addedCU, true, superTypes, 0)); } (*) above is what does it: the supertype of a class auto-generated when using quickfix is always empty (ie. Object) or Exception. The more I think about it, the more I'm inclined to think the proper way to solve this issue would be to display the "Create Class" wizard. I doubt the compiler has sufficient information to correctly generate a class that would really fix the error (ie. without introducing new errors, which is what I complain about in my original report). Basically I always have to retouch the auto-generated class somehow. Maybe this could be implemented as an extra choice in the corrections list? E.g. the list could be: Create class XXX Create class XXX using wizard Create interface XXX Create interface XXX using wizard
Bringing up the wizard is a good idea, I think we should add that. There are also possible improvements when guessing of the required type. As you can see in the code, I only touched this issue.
I've implemented what I suggested above, i.e. two additional proposals in the corrections list to "create class/interface using wizard". The wizard pops up and works okay. But I haven't figured out how to initialize the wizard with the correct type name, the name of the class being created. Without this, it is hardly a correction. Let's see how far I go with this, if I block I'll submit my patch in its current state, as a starting point.
I've found a way to do what I wanted, i.e. display the name of the type being corrected (the type that prompted a correction) in the wizard. I include a patch below. My patch is modeled after the existing correction which creates a new type without opening the wizard. My patch works as follows: * UnresolvedElementsSubProcessor.getTypeProposals() creates additional proposals of type NewCUCompletionUsingWizardProposal (this is a new class). * A NewCUCompletionUsingWizardProposal object creates a NewCUUsingWizardChange (new class). * A NewCUUsingWizardChange object creates a OpenClassWizardAction (resp. OpenInterfaceWizardAction) for a class (resp. interface) correction. These actions are initalized with the new compilation unit that was created in UnresolvedElementsSubProcessor.getTypeProposals(). I created three new constructors for this. * this new compilation unit is used by AbstractOpenWizardAction as the selection passed to the wizard itself. Two new lines in AbstractOpenWizardAction.getCurrentSelection(). * the wizard is then initialized from the selection, in NewTypeWizardPage.initTypePage(): the type name is retrieved from the compilation unit, and inserted into the appropriate text field. Although I've tried to mimic existing code style and conventions, this is my first patch to the Eclipse codebase; it is therefore candidate for a thorough review. I'll gladly answer any questions that may arise. Thanks!
Created attachment 1834 [details] open wizard to create type for correction
Cool, thanks a lot, nice work!! I will integrate it in the latest build. I plan to do some changes to the code: - Avoid changes to NewTypeWizardPage and the actions. Move this code to the NewCUCompletionUsingWizardProposal. NewTypeWizardPage.setTypeName is public and so can be set having the page. Having the wizards (NewClassCreationWizard & NewInterfaceCreationWizard we can access the page. - NewCUUsingWizardChange is not necessary (action can directly be done in NewCUCompletionUsingWizardProposal.apply) It might be even possible to take away the old create class/interface proposals. Let's see what users think. If you're interested in making the changes, let me know. Otherwise some improvements on the superclass guessing would be a good thing. For variable type guessing there is code in ASTResolving.getTypeBinding.
released code including the described changes > I20020806 NewCUCompletionUsingWizardProposal UnresolvedElementsSubProcessor CorrectionMessages.properties
Martin: I like your patch a lot better, it's simpler. Glad to see mine could be improved upon. Two minor bugs though: 1- the icon in the corrections list is now wrong; this comes from an incorrect boolean in UnresolvedElementsSubProcessor, line 219: the "isClass" boolean should be false. 2- because you initialize the wizard with a package fragment, the "enclosing type" isn't displayed in the wizard. How about passing the original compilation unit instead? I include below a patch which solves both problems. Regarding guessing a better type, I wouldn't mind taking a look at it, but don't wait for me. If you have time to do this feel free to blaze ahead. I'm just getting familiar with most of this stuff, and therefore very slow.
Created attachment 1838 [details] fixes icon bug and enclosing type
released the patch (attachment 1838 [details]) Thanks Renaud, using the CU is even better, you're right. For type guessing: I'm currently covered with other stuff; feel free to hack; I think it could be fun (I really like the AST API). Don't hesitate to send mail if you run into something.
I've made some progress on the initial bug, which was really about guessing the declared type. Patch (attached below) works as follows: * UnresolvedElementsSubProcessor.getTypeProposals() calls getSuperTypes (new method) to get a list of ITypeBindings for the super types of the type needing correction. * getSuperTypes() uses ASTResolving.getTypeBinding to find the supertype -- thanks for the tip. * the list returned by getSuperTypes() is converted to a String[] using getTypeNames() and passed to the NewCUCompletionProposal, left unchanged. * the list is passed to the NewCUCompletionUsingWizardProposal. * the NewCUCompletionUsingWizardProposal uses the ITypeBindings list to fill in the wizard page. Because they are ITypeBindings, the proposal is able to differentiate between a super class and a super interface, and correctly fill in the page. It works very nicely with the "wizard" proposal (I handled arrays too), less so with the "non-wizard" one. The CreateCompilationUnitChange, used by the NewCUCompletionProposal (the non- wizard proposal) doesn't handle all cases correctly. It can be fixed, but I'm starting to feel like you do, i.e. that the "wizard" proposal is more powerful and has more or less superceded the "non-wizard" one. In other words, is it really worth fixing the "non-wizard" proposal? The error happens with "Interface intf= new Concrete()" where Concrete needs correction. A class is created with "public class Concrete extends Interface" instead of "... implements Interface". Let me know what you think.
Created attachment 1850 [details] initialize wizard page with declared type
Very nice!! Ignore the old non-wizard quick fix, its fine for me if you just add the support for the new quick fix. (If we decide to keep the old ones, I can still adapt the feature). What do you think, shouldn't we move the super-class evaluating code to the proposal? The advantage is that then the evaluating code is only executed when really note that code like int i= new MyClass() throws an exception. To get the qualified name you can use Bindings.getFullyQualifiedName (thats another helper from us. I agree that we need some clean up here!) Let me know what you think. (I havent released the path yet, but I can if you prefer)
Martin, You're right. I wasn't comfortable either with parsing the entire CU just for the purpose of displaying a correction. I've reverted my changes to the UnresolvedElementsSubProcessor. I now do all the work of finding supertypes in the proposal. Makes more sense, and that way the original proposal is unchanged. I'm also more careful about what kind of nodes I get from the AST -- that should fix the exception you mention. Updated patch follows.
Created attachment 1858 [details] new proposal finds supertypes
Released patch > 20020820 I really like the feature! We should remove the old thing. From playing around I saw some more cases where we could improve: - When the new type is created in a different package, we must add an import statement to the edited type. Nice to have list...: - Type names in catch statements should implement Throwable } catch (NewClass e) { } - Same for types in the class declaration extends NewClass implements NewInterface - We could allow to change the name of the created class, and change quick fix selection to it. For the first two features, either we could improve ASTResolver.getTypeBinding (for now it handles mostly references) Fell free to continue, or let me know if you prefer me to go on. (I'm still busy with other things)
Thanks! > Type names in catch statements should implement Throwable > } catch (NewClass e) { Agreed. Although arguably this should be the compiler's job; since it is already able to tell when the correction happens inside a throws clause, it should be able to carry the same information when inside a catch (or throw new Xxx()). I'll look if I can get ASTResolving.getTypeBinding to detect this.
Well, it was dead easy. I include below a patch to ASTResolving that returns "java.lang.Exception" in a throw or catch context. I chose to return Exception (instead of Throwable as you initially suggested) because it seems more useful that way. Most application exception classes (should?) extend Exception, not Throwable. I still think the compiler should give us that info by setting the ProblemPosition.id to IProblem.ExceptionTypeNotFound. But I've looked into it a bit, and it looks damn hairy, so be it for now. The patch also removes the original correction.
Created attachment 1866 [details] better detection of exception contexts; removes NewCUCompletionProposal
Well, looks back my last fix has been fully integrated in the HEAD branch. Martin, wanna update the status of this bug?
I was cleaning up all the 'fix unresolved elements' quick fixes and intruduced test cases for it (org.eclipse.jdt.ui.tests / UnresolvedTypesQuickFixTest ). Made updates to the super class evaluation methods (now in ASTResolving.guessBindingForTypeReference). Have a look at it! > 20020924
