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 122658 - [68cat] Fix import does not remove not-needed imports of non-existant classes
Summary: [68cat] Fix import does not remove not-needed imports of non-existant classes
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Editor (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Max Sauer
URL:
Keywords:
: 150773 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-11-23 18:14 UTC by giorgio42
Modified: 2009-10-20 16:10 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description giorgio42 2007-11-23 18:14:18 UTC
NB6 RC2

Let's say you renamed a class Foo to Bar, and class Baz previously used Foo, but now has been changes, such that it does
not use it anymore. Then you have the an import statement with an error annotation, that is not used any more:

import my.mypackage.Foo;

Now if you press Ctrl-Shift-i and keep the Remove unused imports, the import statement should be removed, but isn't. I
have to remove it manually.

Georg
Comment 1 ats37 2008-10-13 16:35:23 UTC
This is still an issue in the 6.5 beta.

I created a javabean representing one of our database tables by switching the project settigns to JDK 1.5, using the
"New -> Entity Classes From Database" option, then reverting to the previous JDK 1.4 setting and deleting the
annotations from the source file.  The various remaining imports for javax.persistence.* are flagged with the error
"package javax.persistence does not exist".  However, when I select to Fix Imports, even though there are no references
to these types in the source file, the imports are not removed.

I'd say this is more than a P4, by the way.  There are other reasons for a previously imported class not to exist -
updating a library to a newer version that no longer has the same API, say, or a typo when adding an import by hand.  If
a type isn't used in a source file, Fix Imports should remove it whether it exists or not (in the case of the manual
typo, presumably it would add a second import with the correct spelling anyway).  As such I'd argue the "product feature
is affected", and should be a P3 or P2 depending whether you think the workaround of "go delete the affected lines
yourself" is "viable" or "impractical"...
Comment 2 giorgio42 2008-10-13 20:36:32 UTC
The bug report is one year old and nobody from the NB team has ever looked at it, it seems (which is no wonder, as the
QA requirements for NB 6.5 release state, that at least 50% of the P4s should be evaluated, which, in turn means, I have
a 50% chance that this bug report isn't cow time).

So let's make this a P3.
Comment 3 Jan Lahoda 2008-10-27 07:41:44 UTC
*** Issue 150773 has been marked as a duplicate of this issue. ***
Comment 4 Sergey Petrov 2009-04-13 15:37:28 UTC
yes, fix imports do not remove imports for not existent classes(in 6.7m3),
but these imports also cause compilation problem, this import are highlighted in editor, also how editor may consider
this error as need to be remove instead of need to correct. In most cases rename with nb cause also class anme
correction in import statement.
In my opinion it's not an issue.
Feel free to argure you position and reopen this issue.
Comment 5 giorgio42 2009-04-13 22:04:40 UTC
I sure don't understand your argument.

How could removing the import of a class that is not needed cause compilation problems?

Why are unnecessary imports of classes that exist, removed, but those of (obsolete and deliberately removed, therefore)
not existing classes are kept? Why, for God's sake, do I have to remove these imports manually?

Does this mean that if I intend to remove a class from my project, I have to remove all the unnecessary imports first,
and then remove the class from the project? Why should NB force me into this ordering of activities?

Comment 6 ats37 2009-04-14 11:33:28 UTC
sergeyp:
You say "these imports also cause compilation problem".  Surely that only makes it all the *more* important that "Fix"
(i.e. mend) imports should remove them, in order to allow the class to compile again...

Yes, if the appropriate refactoring tools are used for renaming/deleting classes the issue shouldn't arise.  But not
every code change is necessarily done using those tools (or even with Netbeans), but that shouldn't prevent this option
from working.  And I can think of other circumstances where non-existent classes might appear in the imports - e.g. if I
change the target Java version from 1.6 to 1.4, but have (unused) imports for classes that were introduced in JDK 1.5+.
 No refactoring or even any code changes involved, but a Fix Imports is still needed in order to get the project to
build again...

Besides, surely the real issue is consistency - if an import is unused in the code, Fix Imports should remove it. 
Whether the exists or not is irrelevant to this desired behaviour.
Comment 7 Max Sauer 2009-04-14 11:55:07 UTC
This should work. I'll have a look for 6.7.
Comment 8 Sergey Petrov 2009-04-14 15:11:50 UTC
ok, I miss some point, if import is corrupted "Fix import" can create correct correct import and remove wrong one and
all should be good then. it seems I miss use case also, will try again.
Comment 9 Sergey Petrov 2009-04-14 15:19:02 UTC
msauer 
just tried again with 6.7m3 and import for absent class wasn't removed with fix import but to existent one import was
removed, was it fixed later?

giorgio42 
as for "Does this mean that if I intend to remove a class from my project, I have to remove all the unnecessary imports
first,
and then remove the class from the project? Why should NB force me into this ordering of activities?"
if class is removed without refactoring, yes, it's necessary to remove class and imports manually (or semi-manually if
this issue will be fixed/implemented), but nb do not force you to proceed this way, you can always use safe deletion.
Comment 10 matthies 2009-04-15 21:16:51 UTC
It would be nice though if "Safely Delete" of a class would be smart and remove unused imports of the class to be 
deleted itself as part of the refactoring, rather than to complain about such imports and letting the user do the work. 
But that's a separate issue.
Comment 11 giorgio42 2009-09-29 21:54:30 UTC
This issue basically is about completeness and correctness, something the NB engineers are contiuously fighting with
(just remember the incorrect red error bagdes...). Maybe leaning towards the same culture and just don't care anymore
would actually be the best option...
Comment 12 Jan Lahoda 2009-10-05 10:20:07 UTC
I tried to implement removal of imports that are provably unused. E.g. if there is a single class import, and there is
no unresolvable element with the same name in the class, the import is considered to be unused. If there is an
unresolved element in the class with the same name as is the imported name, the import is not considered unused.

What needs to be done now is to rigorously test that this feature does not remove any import that should not be removed
and removes all imports that can be removed. This is to ensure the aforementioned completeness and correctness. I think
it would be best if the community members that commented in this bug would participate in this testing. Could you please
test a build with the below change and report results? Thanks.

http://hg.netbeans.org/jet-main/rev/257d9e94169d

(I have found one more problem with the above change, it removes imports like:
import something.broken.;
which is incorrect, IMO. I will fix this soon and comment here.)
Comment 13 Quality Engineering 2009-10-07 00:19:07 UTC
Integrated into 'main-golden', will be available in build *200910061401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/257d9e94169d
User: Jan Lahoda <jlahoda@netbeans.org>
Log: #122658: if an unresolvable import is (would be) provably unused, mark it as unused.
Comment 14 Quality Engineering 2009-10-07 12:50:38 UTC
Integrated into 'main-golden', will be available in build *200910070250* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/8ecc595ac50e
User: Jan Lahoda <jlahoda@netbeans.org>
Log: #122658(cont'd): imports with parse error (e.g. "some.thing.") should not be considered unused.
Comment 15 denbo 2009-10-12 16:58:49 UTC
'Log: #122658(cont'd): imports with parse error (e.g. "some.thing.") should not be considered unused.'

I am sure this is not what most users would expect.
If I invoke the action "Fix Imports" I want the IDE to _entirely fix_ the imports. I deliberately decide to have
Netbeans manage the imports section. Import statements containing parse errors are something I have to manually edit in
order to get my imports fixed.

On the other hand side, if I feel the need to keep my erroneous statements, I shouldn't invoke "Fix Imports", right?

Besides, what's the advantage of protecting faulty imports?
Comment 16 ulfzibis 2009-10-12 17:25:01 UTC
> Besides, what's the advantage of protecting faulty imports?
Sometimes NetBeans does not have access to all relevant resources/source root. E.g. note OpenJDK 7 freeform projects.
Comment 17 matthies 2009-10-12 21:35:48 UTC
Denbo: NetBeans may not always be able to "fix" all imports. (One particular example are static imports.) I think it's 
very reasonable that if there still are unresolved elements in the source file (after Fix Imports, that is), that 
invalid imports for these names are not removed. In those cases manual intervention is necessary anyway, and the faulty 
imports are likely to help in figuring out the cause of the error, and may turn out to be correct after all if NetBeans 
not finding them is just a configuration error, a stale cache or whatever.
Comment 18 Jiri Kovalsky 2009-10-20 16:10:36 UTC
Reported by NetCAT 6.8 participant.