Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] UnnecessaryImport: false positives with static imports #4602

Closed
BurovnikovEvgeniy opened this issue Jun 21, 2023 · 14 comments · Fixed by #4920
Closed

[java] UnnecessaryImport: false positives with static imports #4602

BurovnikovEvgeniy opened this issue Jun 21, 2023 · 14 comments · Fixed by #4920
Labels
a:false-positive PMD flags a piece of code that is not problematic
Milestone

Comments

@BurovnikovEvgeniy
Copy link

Affects PMD Version:
7.0.0-rc3
Rule:
UnnecessaryImport
Please provide the rule name and a link to the rule documentation:
https://docs.pmd-code.org/latest/pmd_rules_java_codestyle.html#unnecessaryimport

Description:
I'm trying to analyze a project with UI tests and check the imports used. But the result of the check is incorrect.

Suppressing these exceptions is too time-consuming for me, because these imports are in the entire project. There was no such problem in PMD 6.37, but when upgrading to 7.0.0 it showed up.

In CM 7.0.0-rc2, you wrote that this problem was detected and fixed (if I'm not confusing anything), but it didn't give a result (
Code Sample demonstrating the issue:

import static com.codeborne.selenide.Condition.visible;
import static com.codeborne.selenide.Selenide.$;

public class CallRateLayer extends ExtendsPage {
    @Override
    public void checkPage() {
        $(ROOT_ELEMENT).shouldBe(visible.because("msg")); // its import use
    }
}

Expected outcome:
Expected no violations

PMD reports a violation, but that's wrong. That's a false positive.
Example:
/myPath/test.java:8: UnnecessaryImport: Unused static import 'com.codeborne.selenide.Condition.visible'
/myPath/test.java:9: UnnecessaryImport: Unused static import 'com.codeborne.selenide.Selenide.$'

Running PMD through: Maven

@BurovnikovEvgeniy BurovnikovEvgeniy added the a:false-positive PMD flags a piece of code that is not problematic label Jun 21, 2023
@jsotuyod
Copy link
Member

We fixed a couple of false positives with this rule with #4551 and #4560, but you may have hit a different scenario.

Would you mind sharing which version of selenide you are using, just to ensure we are looking at the proper code?

@BurovnikovEvgeniy
Copy link
Author

I use com.codeborne:selenide:5.20.0

@BurovnikovEvgeniy
Copy link
Author

BurovnikovEvgeniy commented Jun 22, 2023

We fixed a couple of false positives with this rule with #4551 and #4560, but you may have hit a different scenario.

Would you mind sharing which version of selenide you are using, just to ensure we are looking at the proper code?

But to be honest, I don't think it's about Selenide. I haven't reached the code of the rule itself yet due to the large volume of work tasks, but the situation is similar with JUnit and Hamcrest

package ru.myorg.tests.calls;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import ru.myorg.Base;
import ru.myorg.CallLayer;
import ru.myorg.test.automation.junit.annotations.annotations.Tag;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class RateCallTest extends Base {

    @Before
    public void setUp() {
        createBaseLayer();
    }

    @Test
    @Tag(groups = {"testExample"})
    public void rateTest() {

        LOGGER.info("msg");
        CallLayer callLayer = new CallLayer();
        callLayer.rateCallExcellent();
        assertFalse("Where is layer?", callLayer.isCallLayerVisible());
        LOGGER.info("That all");
    }
}```

Report:
/ruletest/test.java:3:	UnnecessaryImport:	Unused import 'org.junit.After' // ok
/ruletest/test.java:10:	UnnecessaryImport:	Unused static import 'org.junit.Assert.assertFalse' // not ok
/ruletest/test.java:11:	UnnecessaryImport:	Unused static import 'org.junit.Assert.assertTrue' // ok

@BurovnikovEvgeniy
Copy link
Author

Another example. There are triggers not only for static imports:

package ru.myorg.test.session;

import com.forticom.api.base.ApiException;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import ru.myorg.test.Base;

import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertThat;
import static ru.myorg.CustomMatchers.throwsException;

public class TestTest extends Base {

    public final static String ERROR_NO_SESSION = "SESSION_ERROR";

    @Test
    @Tag("example")
    public void testWithoutSession() {
        assertThat(
                "Where is error?",
                () -> client.getService().register("id", null),
                throwsException(ApiException.class, containsString(ERROR_NO_SESSION))
        );
    }
}

/ruletest/test.java:3:	UnnecessaryImport:	Unused import 'com.forticom.api.base.ApiException' // not ok
/ruletest/test.java:8:	UnnecessaryImport:	Unused static import 'org.hamcrest.Matchers.containsString' // not ok
/ruletest/test.java:9:	UnnecessaryImport:	Unused static import 'org.junit.Assert.assertThat' // not ok
/ruletest/test.java:10:	UnnecessaryImport:	Unused static import 'ru.myorg.CustomMatchers.throwsException' // not ok

@adangel adangel changed the title Problem with UnnecessaryImport rule [java] UnnecessaryImport: false positives with static imports Jun 24, 2023
adangel added a commit to adangel/scratchpad that referenced this issue Jun 24, 2023
@adangel
Copy link
Member

adangel commented Jun 24, 2023

Hi @BurovnikovEvgeniy,
thanks for the report. However, I can't reproduce your problem.
See here for my reproducer project: https://github.com/adangel/scratchpad/tree/master/issue-4602-unnecessaryimport

In order to have this rule work correctly, you need to make sure, to use the correct classpath and have your project compiled. This is ensured by using maven and executing pmd in the "verify" phase.

Also note, that in order to use PMD 7.0.0-rc3, you'll need to use a snapshot version of maven-pmd-plugin (see also #4478 (comment) )

Looking at your additional examples, I suspect that you don't compile your test project and don't use maven to execute PMD, which makes it way harder to provide the correct auxclasspath. Why do I think you don't use maven/don't compile? Because your java source file is called test.java, while you top-level class inside is not called "test".

Could you please double check, that you execute PMD correctly? You can take my reproducer project as a starting point: https://github.com/adangel/scratchpad/tree/master/issue-4602-unnecessaryimport
Please also let us know, if you still see the false positives.

@BurovnikovEvgeniy
Copy link
Author

Hi @adangel.
I have PMD connected as a dependency to the maven project. The project itself contains our custom rules and configurations for working on our farm. In this way, the utility itself is connected to a separate project, which I compile and pass all the PMD parameters to the resulting jar file, including a list of files that need to be analyzed (they are in another project). My project is an add-on to PMD. In order for the port scanning to work correctly, I had to add all the dependencies to my PMD project, but if, for example, I make a static import of some enam constant in the analyzed file, then pmd does not understand where it came from, for him it is an unused import. Please tell me what to do with this, where am I wrong and how to solve the problem?

@adangel
Copy link
Member

adangel commented Jul 11, 2023

including a list of files that need to be analyzed (they are in another project).

This "another project" is the project you want to analyze, correct?
Then you need to compile this "another project" and call PMD with not only providing the list of files to analyze, but also with the full classpath of "another project" including the build output of "another project" (e.g. target/classes). This full classpath is called "aux-classpath" (CLI option --aux-classpath).

Which kind of project is "another project"? Does it use maven to build? If yes, then you can simply use the maven-pmd-plugin to execute PMD with the correct list of files and the maven-pmd-plugin will automatically calculate the correct aux-classpath.

Can you describe in more detail, how exactly you actually integrate PMD in order to analyze "another project"?

@jsotuyod jsotuyod added needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale and removed needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale labels Mar 17, 2024
@jjlharrison
Copy link
Contributor

This issue is still present in 7.0.0.

@oowekyala oowekyala added this to the 7.1.0 milestone Apr 3, 2024
jsotuyod added a commit that referenced this issue Apr 5, 2024
@mdagcilar
Copy link

mdagcilar commented Apr 25, 2024

I'm also still facing this issue, please re-open. I.

I'm using java 21, with maven-pmd-plugin 3.22.0 (should be using the default PMD 7.0.0)

I get over 500 PMD failures complaining about Rule:UnnecessaryImport Priority:4 Unused static import across imports which are used. I'm referring to this rule https://docs.pmd-code.org/latest/pmd_rules_java_codestyle.html#unnecessaryimport

@jsotuyod
Copy link
Member

As indicated on this issue, milestone is 7.1.0.

7.1.0 should be released before the end of the month.

@mdagcilar
Copy link

Thank you, i am also getting false positives for UnusedPrivateMethod when a private method is being used as part of @ParameterizedTest @MethodSource argument. Will this also be addressed alongside the false positives for UnnecessaryImport?

@jsotuyod
Copy link
Member

That's a separate issue, open new one.

@BurovnikovEvgeniy
Copy link
Author

As indicated on this issue, milestone is 7.1.0.

7.1.0 should be released before the end of the month.

Hey, guys! It hasn't been long, but I'm waiting for this feature. I really hope for you

@adangel
Copy link
Member

adangel commented Apr 26, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-positive PMD flags a piece of code that is not problematic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants