apache / netbeans

Apache NetBeans
https://netbeans.apache.org/
Apache License 2.0
2.61k stars 836 forks source link

"Fix All Imports" importing invalid static methods #7073

Closed TFyre closed 2 months ago

TFyre commented 5 months ago

Apache NetBeans version

Apache NetBeans 20

What happened

Looks like "Fix All Imports" imports invalid static methods when the method name matches an existing import's root package.

image

Language / Project Type / NetBeans Component

Java Maven Application

How to reproduce

Create Maven Application and set source to 17 or 21 (works fine with 8):

pom.xml:

<maven.compiler.source>21</maven.compiler.source>

NewClass.java

package com.tfyre.test;

import java.util.List;
import javax.xml.parsers.DocumentBuilder;
import org.xml.sax.InputSource;

public class NewClass {

    private List<String> list;
    private DocumentBuilder foo;
    private InputSource is;

    public static class SomeClass2 {

        public static String java(String value) {
            return value;
        }

        public static String javax(String value) {
            return value;
        }

        public static String org(String value) {
            return value;
        }
    }

}
java -version
openjdk version "21.0.2" 2024-01-16 LTS
OpenJDK Runtime Environment Zulu21.32+17-CA (build 21.0.2+13-LTS)
OpenJDK 64-Bit Server VM Zulu21.32+17-CA (build 21.0.2+13-LTS, mixed mode, sharing)

Did this work correctly in an earlier version?

Apache NetBeans 19

Operating System

Windows

JDK

Zulu21.32+17-CA

Apache NetBeans packaging

Apache NetBeans binary zip

Anything else

Every time

Are you willing to submit a pull request?

Yes

TFyre commented 5 months ago

Possibly related to #6902?

TFyre commented 5 months ago

Duplicate of #5537?

mbien commented 2 months ago

interesting bug. While it is visiting the imports, e.g import java.util.List; it lands here: https://github.com/apache/netbeans/blob/668bca250cd66fd580b6bc3bc34412e80e324f66/java/java.editor/src/org/netbeans/modules/java/editor/imports/ComputeImports.java#L600-L610

where el is java, so it thinks it has to resolve this. Usually nothing would be found and no window would pop up. However if there is a static method somewhere carrying the name java, it will suggest it.

If I comment out L609 it would resolve this, however it will likely break in other situations like the case the comment mentions.

Reading the doc of getPackageElement, I believe it expects fully qualified non-empty packages, so java or javax would not be sufficient.

edit: is this even supposed to visit imports? Since it is adding the root of an import as unresolved element... which is then imported.

        @Override
        public Void visitImport(ImportTree node, Map<String, Object> p) {
            return null; // don't visit imports?
        }

edit2: this would only mitigate the issue, since any fully qualified class name later in code would still cause the same situation.

cc @jlahoda

mbien commented 2 months ago

not sure what account to ping @lahodaj

mbien commented 2 months ago

an attempt to fix this: https://github.com/apache/netbeans/pull/7363 (CI will produce a dev-build zip, linked from github action summary)