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] UnusedImports: False positive if wildcard is used and only static methods #2016

Closed
albfernandez opened this issue Sep 14, 2019 · 7 comments · Fixed by #2138
Closed
Labels
a:false-positive PMD flags a piece of code that is not problematic in:type-resolution Affects the type resolution code
Milestone

Comments

@albfernandez
Copy link
Contributor

albfernandez commented Sep 14, 2019

Affects PMD Version:
confirmed from 6.9.0 to 6.17.0 and 6.18

Rule:
UnusedImports

Description:
False positives from UnusedImports in some specific conditions:

  • Import with wildcard (example: import java.util.*)
  • Using only calls to static methos who take parameters and return in java.lang (String, Object, etc)

Code Sample demonstrating the issue:

package demo; 

import java.util.*;

public class Demo {
    public void testFunction() {
        Objects.toString(null);
    }
}

Running PMD through: CLI and Ant

bash run.sh pmd -d src/ -R category/java/bestpractices.xml/UnusedImports -auxclasspath classes
sep 15, 2019 12:51:53 AM net.sourceforge.pmd.PMD processFiles
ADVERTENCIA: This analysis could be faster, please consider using Incremental Analysis: https://pmd.github.io/pmd-6.17.0/pmd_userdocs_incremental_analysis.html
src/demo/Demo.java:3:	Avoid unused imports such as 'java.util'

Curiously, placing the following test code in UnusedImports.xml works fine:

    <test-code>
    	<description>False positive in Unused imports with Wildcard</description>
    	<expected-problems>0</expected-problems>
    	<code><![CDATA[
package net.sourceforge.pmd.lang.java.rule.bestpractices.unusedimports;
	
import java.util.*;

public class Demo {
	
    public void testFunction() {
        Objects.toString(null);
    }
}    	
    	 ]]></code>    
    </test-code>
@jsotuyod
Copy link
Member

@albfernandez I can't reproduce this, not as a test case nor from CLI with 6.19.0-SNAPSHOT… Any further info you can provide?

@albfernandez

This comment has been minimized.

@albfernandez
Copy link
Contributor Author

albfernandez commented Sep 19, 2019

Hi

I've done some more researching:
The problem occurs when you set the auxclasspath. If you don't set the auxclasspath it works fine

This doesn't work (prints a violation)
bash pmd-bin-6.18.0/bin/run.sh pmd -d src/ -R category/java/bestpractices.xml/UnusedImports -auxclasspath classes

this works
bash pmd-bin-6.18.0/bin/run.sh pmd -d src/ -R category/java/bestpractices.xml/UnusedImports

The same applies to Ant.

I need to use auxclasspath for other rules to work

@jsotuyod jsotuyod added the a:false-positive PMD flags a piece of code that is not problematic label Oct 6, 2019
@adangel adangel added the in:type-resolution Affects the type resolution code label Nov 22, 2019
@adangel
Copy link
Member

adangel commented Nov 22, 2019

With type resolution enabled and a correct auxclasspath, we resolve the type of the ASTName node "Object.toString(null)" as "String" - which is the result type of the method call. I think, what we need to do instead is, assign the result type of the method call not to the ASTName, but to the expression:

PrimaryExpression  -> Type = java.lang.String
  |- PrimaryPrefix  -> Type = java.util.Objects
     |- Name "Objects.toString" -> Type = java.util.Objects
  |- PrimarySuffix
     |- Arguments
        |- ArgumentList

adangel added a commit to adangel/pmd that referenced this issue Nov 22, 2019
…tic methods

Added workaround for wrong typeresolution
(method result vs. class of method)

Refs pmd#2016
@albfernandez
Copy link
Contributor Author

I've tested it and seems to work fine for me.

Thanks

adangel added a commit to adangel/pmd that referenced this issue Nov 29, 2019
…cations

We already resolved the result type of a method invocation,
but in some cases, we also need the type of the target reference
for static method calls.

Reverts the previously introduced fix in UnusedImportsRule.

Fixes pmd#2016
@adangel adangel added this to the 6.20.0 milestone Nov 29, 2019
@adangel adangel added the has:pr label Nov 29, 2019
@jorn86
Copy link

jorn86 commented Nov 29, 2021

This is in the 6.20 milestone, does it mean it's fixed in that version? Because I'm using 6.26 and this issue still occurs.

@adangel
Copy link
Member

adangel commented Nov 29, 2021

@jorn86 Yes, this is what it means. This issue is supposed to be fixed with 6.20.0 already.
If you still encounter this, then you might have a different case that's not covered. Please open a new issue then. Thanks!

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 in:type-resolution Affects the type resolution code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants