Menu ▾ ▴

#1306 False positive on duplicate when using static imports

PMD-5.3.0
closed
Andreas Dangel
None
PMD
3-Major
Bug
5.1.1
2015-04-23
2015-01-09
Dimas
No

The DuplicateImportsRule handles static and normal imports absolutely the same way which is already wrong IMHO.
As the result, isDisambiguationImport method when it checks that a specific import can be a disambiguation checks if it can find a class of a given name. But in case of static imports, we are importing methods not classes.

As the result, this sequence of imports generates "duplicate import" warning:

import static org.hamcrest.Matchers.*;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.*;

although the last line is disambiguation because hamcrest.Matchers also has "any".

I would suggesthandling static imports separately from non-static ones. Or at the very least just ignore all static imports in DuplicateImportsRule.

Related

Issues: #914

Discussion

  • Andreas Dangel

    Andreas Dangel - 2015-01-22
    • Description has changed:

    Diff:

    --- old
+++ new
@@ -3,9 +3,9 @@

 As the result, this sequence of imports generates "duplicate import" warning:

- import static org.hamcrest.Matchers.*;
- import static org.mockito.Matchers.any;
- import static org.mockito.Matchers.*;
+    import static org.hamcrest.Matchers.*;
+    import static org.mockito.Matchers.any;
+    import static org.mockito.Matchers.*;

 although the last line is disambiguation because hamcrest.Matchers also has "any".
    • status: open --> closed
    • assigned_to: Andreas Dangel
    • Milestone: New Tickets --> PMD-Next
     

  • Andreas Dangel

    Andreas Dangel - 2015-01-22

    This will be fixed with the next version.

    It uses type resolution to verify the existence of the static method - if it exists in both types, the rule won't trigger anymore.

     

  • Dimas

    Dimas - 2015-04-22

    Are you sure it is fixed?
    I upgraded to version 3.4 maven-pmd-plugin and if I am not mistaken, it uses pmd-core 5.2.6 and the following block of imports still generates an error:

     import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.assertThat;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.*;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;

    [INFO] PMD Failure: Xxx.java:26 Rule:DuplicateImports Priority:3 Avoid duplicate imports such as 'org.mockito.Matchers.any'.

    Just by looking at the rule sources I am not sure I understand how it works

      if (!thisImportOnDemand.isStaticOnDemand()) {
    String fullyQualifiedClassName = thisImportOnDemand.getName() + "." + singleTypeName;
    if (node.getClassTypeResolver().classNameExists(fullyQualifiedClassName)) {
      return true;  //Class exists in another imported package
    }
  } else {

    Why does it check classNameExists? Static imports are not about class names, they are about method names...

     

    Last edit: Dimas 2015-04-22

  • Andreas Dangel

    Andreas Dangel - 2015-04-23

    Well, I hope it is fixed :)

    As maven-pmd-plugin 3.4 uses only pmd 5.2.6 - it is not fixed there. The bug has been fixed with pmd 5.3 (you can see this at the milestone field of this issue). So, it should be fixed when the next maven-pmd-plugin version is released and uses pmd >= 5.3.0.

     

Log in to post a comment.