hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Merge branch 'pr-1954'

Browse Source
This commit is contained in:
Andreas Dangel 2019-08-04 11:22:23 +02:00
commit 1a5486dc30
6 changed files with 170 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
docs/pages/release_notes.md
View File

@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release.
### Fixed Issues
* java-codestyle
* [#1951](https://github.com/pmd/pmd/issues/1951): \[java] UnnecessaryFullyQualifiedName rule triggered when variable name clashes with package name
### API Changes
#### Deprecated APIs

3
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/TypeNode.java
View File

@ -5,13 +5,12 @@
package net.sourceforge.pmd.lang.java.ast;
import net.sourceforge.pmd.annotation.InternalApi;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition;
/**
* This interface allows a Java Class to be associated with a node.
*/
public interface TypeNode extends Node {
public interface TypeNode extends JavaNode {
/**
* Get the Java Class associated with this node.

67
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java
View File

@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.java.rule.codestyle;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
@ -20,10 +21,13 @@ import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.AbstractJavaTypeNode;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.SourceFileScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.lang.symboltable.Scope;
public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
@ -102,8 +106,16 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
return false;
}
private void checkImports(AbstractJavaTypeNode node, Object data) {
String name = node.getImage();
private void checkImports(TypeNode node, Object data) {
final String name = node.getImage();
// variable names shadow everything else
// If the first segment is a variable, then all
// the following are field accesses and it's not an FQCN
if (isVariable(node.getScope(), name)) {
return;
}
List<ASTImportDeclaration> matches = new ArrayList<>();
// Find all "matching" import declarations
@ -172,7 +184,7 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
if (matches.isEmpty()) {
if (isJavaLangImplicit(node)) {
addViolation(data, node, new Object[] { node.getImage(), "java.lang.*", "implicit "});
} else if (isSamePackage(node)) {
} else if (isSamePackage(node, name)) {
addViolation(data, node, new Object[] { node.getImage(), currentPackage + ".*", "same package "});
}
} else {
@ -211,12 +223,45 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
return result;
}
private boolean isSamePackage(AbstractJavaTypeNode node) {
String name = node.getImage();
return name.substring(0, name.lastIndexOf('.')).equals(currentPackage);
private boolean isVariable(Scope scope, String name) {
String firstSegment = name.substring(0, name.indexOf('.'));
while (scope != null) {
for (Entry<VariableNameDeclaration, List<NameOccurrence>> entry : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) {
if (entry.getKey().getName().equals(firstSegment)) {
return true;
}
}
scope = scope.getParent();
}
return false;
}
private boolean isJavaLangImplicit(AbstractJavaTypeNode node) {
private boolean isSamePackage(TypeNode node, String name) {
if (node.getType() != null) {
// with type resolution we can do an exact package match
Package packageOfType = node.getType().getPackage();
if (packageOfType != null) {
return node.getType().getPackage().getName().equals(currentPackage);
}
}
int i = name.lastIndexOf('.');
while (i > 0) {
name = name.substring(0, i);
if (name.equals(currentPackage)) {
return true;
}
i = name.lastIndexOf('.');
}
return false;
}
private boolean isJavaLangImplicit(TypeNode node) {
String name = node.getImage();
boolean isJavaLang = name != null && name.startsWith("java.lang.");
@ -232,8 +277,8 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRule {
return false;
}
private boolean isAvoidingConflict(final AbstractJavaTypeNode node, final String name,
final ASTImportDeclaration firstMatch) {
private boolean isAvoidingConflict(final TypeNode node, final String name,
final ASTImportDeclaration firstMatch) {
// is it a conflict between different imports?
if (firstMatch.isImportOnDemand() && firstMatch.isStatic()) {
final String methodCalled = name.substring(name.indexOf('.') + 1);

11
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryfullyqualifiedname/TestClass.java Normal file
View File

@ -0,0 +1,11 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryfullyqualifiedname;
public class TestClass {
}

10
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryfullyqualifiedname/subpackage/MyClass.java Normal file
View File

@ -0,0 +1,10 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryfullyqualifiedname.subpackage;
public class MyClass {
}

90
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml
View File

@ -488,7 +488,7 @@ public class TestArrayType {
}
]]></code>
</test-code>
<test-code>
<description>#1199 false negative for same package FQCN</description>
<expected-problems>1</expected-problems>
@ -499,6 +499,94 @@ package org.foo;
public class SamePackage {
public void convert(org.foo.Bar s) { // violation
}
}
]]></code>
</test-code>
<test-code>
<description>#1951 false positive when package name is obscured by variable</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
package threads;
public class FQNTest {
public static void main(String[] args) {
Thread[] threads = new Thread[5];
int i = threads.length;
}
}
]]></code>
</test-code>
<test-code>
<description>False positive when package name is obscured by variable (2)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
package threads;
public class FQNTest {
public static void main(String[] args) {
Thread[] threads = new Thread[5];
int i = threads.length.foo;
}
}
class length {
static int foo;
}
]]></code>
</test-code>
<test-code>
<description>False negative when name refers to static field</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
package threads;
public class FQNTest {
public static void main(String[] args) {
// threads.length is the class
int i = threads.length.foo;
}
}
class length {
static int foo;
}
]]></code>
</test-code>
<test-code>
<description>False positive when type name is obscured by variable</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class FQNTest {
public static void main(String[] args) {
length length = new length();
// the type name 'length' is obscured.
int i = length.foo;
}
}
class length {
static int foo;
}
]]></code>
</test-code>
<test-code>
<description>False positive when subpackage is referenced</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>6</expected-linenumbers>
<code><![CDATA[
package net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryfullyqualifiedname;
public class FQNTest {
public void foo() {
new net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryfullyqualifiedname.subpackage.MyClass(); // no violation
new net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryfullyqualifiedname.TestClass(); // violation
}
}
]]></code>
</test-code>