This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 88415 - [retouche]Implement all abstract methods generates methods that already exist if imports not resolved
Summary: [retouche]Implement all abstract methods generates methods that already exist...
Status: RESOLVED WONTFIX
Alias: None
Product: java
Classification: Unclassified
Component: Source (show other bugs)
Version: 6.x
Hardware: All All
: P4 blocker (vote)
Assignee: Jan Lahoda
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-11-01 14:29 UTC by _ tboudreau
Modified: 2009-11-02 11:02 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description _ tboudreau 2006-11-01 14:29:33 UTC 
Say you have an interface

public interface Foo {
   public Shape toShape();
}

and a concrete class

public class Bar {
   public Shape toShape() {
     //return something
   }
}

but the concrete class has no import for Shape.

Change bar to implement Foo.  Accept the implement all abstract methods hint.  

You get another implementation of toShape() (I guess because the import for
Shape is missing, so the hint does not think the existing implementation is an
implementation of Foo because its return type is unresolved - I think you need
to do a string match test as well as a type match test here).
Comment 1 Jan Lahoda 2007-01-09 10:54:41 UTC 
Sorry, but I think this is currently as designed.
Comment 2 _ tboudreau 2007-01-09 17:40:12 UTC 
Reopening - sorry.  The IDE should never generate uncompilable code - it makes
users not trust it.  Surely we can detect if we are generating a method with an
identical signature to one which already exists - just do a string compare of
the name of the unresolved class with the code that would be generated, and if
identical, don't generate the method stub.

Really - any case where we generate code that is obviously uncompilable is going
to make users not trust NetBeans and say bad things about it.  In this case, we
can do something about it - so we should.
Comment 3 Jan Lahoda 2007-01-09 23:17:46 UTC 
Sorry, but I do not think that string matching would be really error prone. What
I can do is to disable the Implement All Abstract Methods hint if there are any
unresolved identifiers in the given class, which should prevent this situation.
Comment 4 Jan Lahoda 2007-01-09 23:28:36 UTC 
The first sentence of my previous comment should obviously read "Sorry, but I
think that string matching would be really error prone." :-).
Comment 5 Pavel Flaska 2007-01-10 09:44:51 UTC 
Not sure disabling the hint is correct in this case. -- And definitely disagree
refactoring/suggestion should never create uncompilable code. Sometimes it is
reasonable and even desirable to create uncompilable code.

I remember such a discussion when I implemented 'Change method parameters'
refactoring. When updating method call with value, I intentionally used unique
non-existing value, then I compile and use errors to update all the calls with
correct value. Or, when removing parameter, I prefered just warn user about used
parameter when he tried to remove it. Then, he could simply remove all parameter
usages in source where errors occured. Just my opinion.
Comment 6 _ tboudreau 2007-01-10 18:55:31 UTC 
Given 

public Foo doSomething() {
}

and a hint that wants to generate "public Foo doSomething()", what is so error
prone about comparing "Foo" with "Foo"?  Even if the return type is Foo.Bar and
the existing method has an unresolved "Bar", it should be relatively simple to
handle.

Or couldn't invoking the hint first add the import for Foo, then recompute the
list of methods that need to be added, in which case "public Foo doSomething()"
will not be in the list?  In that case, it would do this correctly.
Comment 7 _ tboudreau 2007-01-10 19:01:54 UTC 
> Sometimes it is reasonable and even desirable to create uncompilable code.

Agree that there are such times;  disagree that this is one of them.
Comment 8 Jan Lahoda 2007-01-10 19:58:48 UTC 
I agree with Pavel that the idea with disabling the hint is not a good one.

Regarding the imports, the hint actually does not know how the type will be
imported, or even if the type will be imported at all (the settings may order
use of FQNs). This is done in the import management code to ensure consistency.

Imagine (public) classes a.B.C.D and a.E.D (B and E are top-level classes).
Imagine an interface X1 with a method like x(a.B.C.D d), an interface X2 with a
method like x(a.E.D d) and a class like:
package a;
class Y implements X1, X2 {
   x(D d) {}
}

What should the hint do?
Comment 9 _ tboudreau 2007-01-12 10:27:10 UTC 
In a super rare case like that?  Ask the user.
Comment 10 Jan Lahoda 2008-11-19 12:49:01 UTC 
later.
Comment 11 Quality Engineering 2009-11-02 11:02:09 UTC 
NetBeans.org Migration: changing resolution from LATER to WONTFIX