Skip to content

[java] Fix unnecessaryFQCN false positive with import-on-demand #1220

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.

Sign up for GitHub

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

Merged
adangel merged 1 commit into from
Jul 8, 2018
+63 −3
Merged

[java] Fix unnecessaryFQCN false positive with import-on-demand #1220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTImportDeclaration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,21 @@
* Represents an import declaration in a Java file.
*
* <pre>
* ImportDeclaration ::= "import" [ "static" ] Name [ "." "*" ] ";"
*
* ImportDeclaration ::= "import" "static"? {@linkplain ASTName Name} ( "." "*" )? ";"
*
* </pre>
*
* @see <a href="https://docs.oracle.com/javase/specs/jls/se9/html/jls-7.html#jls-7.5">JLS 7.5</a>
*
*/
// TODO should this really be a type node?
// E.g. for on-demand imports, what's the type of this node? There's no type name, just a package name
// for on-demand static imports?
// for static imports of a field? the type of the field or the type of the enclosing type?
// for static imports of a method?
// I don't think we can work out a spec without surprising corner cases, and #1207 will abstract
// things away anyway, so I think we should make it a regular node
public class ASTImportDeclaration extends AbstractJavaTypeNode {

private boolean isImportOnDemand;
Expand All @@ -27,18 +38,47 @@ public ASTImportDeclaration(JavaParser p, int id) {
super(p, id);
}


/**
* @deprecated Will be made private with 7.0.0
*/
@Deprecated
public void setImportOnDemand() {
isImportOnDemand = true;
}


// @formatter:off
/**
* Returns true if this is an import-on-demand declaration,
* aka "wildcard import".
*
* <ul>
* <li>If this is a static import, then the imported names are those
* of the accessible static members of the named type;
* <li>Otherwise, the imported names are the names of the accessible types
* of the named type or named package.
* </ul>
*/
// @formatter:on
public boolean isImportOnDemand() {
return isImportOnDemand;
}


/**
* @deprecated Will be made private with 7.0.0
*/
@Deprecated
public void setStatic() {
isStatic = true;
}


/**
* Returns true if this is a static import. If this import is not on-demand,
* {@link #getImportedSimpleName()} returns the name of the imported member.
*/
public boolean isStatic() {
return isStatic;
}
Expand All @@ -52,7 +92,7 @@ public ASTName getImportedNameNode() {

/**
* Returns the full name of the import. For on-demand imports, this is the name without
* the final asterisk.
* the final dot and asterisk.
*/
public String getImportedName() {
return jjtGetChild(0).getImage();
Expand Down Expand Up @@ -106,6 +146,8 @@ public void setPackage(Package packge) {
* auxclasspath is not correctly set, as this method depends on correct
* type resolution.
*/
// TODO deprecate? This is only used in a test. I don't think it's really
// useful and it gives work to ClassTypeResolver.
public Package getPackage() {
return this.pkg;
}
Expand Down
2 changes: 1 addition & 1 deletion .../java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private void checkImports(AbstractJavaTypeNode node, Object data) {
matches.add(importDeclaration);
}
}
} else {
} else if (!importDeclaration.isImportOnDemand()) {
// last part matches?
if (nameParts[nameParts.length - 1].equals(importParts[importParts.length - 1])) {
matches.add(importDeclaration);
Expand Down
18 changes: 18 additions & 0 deletions ...ources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,22 @@ public class JavaLang {
}
]]></code>
</test-code>

<test-code>
<description>#1216 [java] UnnecessaryFullyQualifiedName false positive for the same name method</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.awt.Component;
import java.awt.Container;
import java.util.Arrays;
import java.util.asList.*; // we don't use stream since we're on java 7


public final class Test {
public static List<String> stream(Container parent) {
return Arrays.asList("", "");
}
}
]]></code>
</test-code>
</test-data>