Skip to content

Commit

Permalink
Merge branch '__rultor'
Browse files Browse the repository at this point in the history
  • Loading branch information
rultor committed May 3, 2018
2 parents f8e8334 + 0ae8296 commit 9df81cd
Show file tree
Hide file tree
Showing 6 changed files with 295 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/**
* Copyright (c) 2011-2018, Qulice.com
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met: 1) Redistributions of source code must retain the above
* copyright notice, this list of conditions and the following
* disclaimer. 2) Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution. 3) Neither the name of the Qulice.com nor
* the names of its contributors may be used to endorse or promote
* products derived from this software without specific prior written
* permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
* NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
* FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
* THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
* INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
* OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package com.qulice.pmd.rules;

import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.rule.AbstractInefficientZeroCheck;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence;
import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
import net.sourceforge.pmd.util.StringUtil;

/**
* Rule to prohibit use of String.length() when checking for empty string.
* String.isEmpty() should be used instead.
* @author Mihai Andronache (amihaiemil@gmail.com)
* @version $Id$
* @since 0.18
* @todo #801:30min This rule currently doesn't complain if the string is
* prefixed with this when length is called (e.g. somestring.length() works
* but this.somestring.length() does not). The same happens in with a method
* call (this.method().length() does not work). Check if possible and if it
* is, extend this rule to also catch this case. More about how to write PMD
* rules here: http://pmd.sourceforge.net/pmd-4.3/howtowritearule.html.
* @checkstyle MultipleStringLiteralsCheck (300 lines)
*/
public final class UseStringIsEmptyRule extends AbstractInefficientZeroCheck {

@Override
public boolean appliesToClassName(final String name) {
return StringUtil.isSame(name, "String", true, true, true);
}

@Override
public Map<String, List<String>> getComparisonTargets() {
final Map<String, List<String>> rules = new HashMap<>();
rules.put("<", Arrays.asList("1"));
rules.put(">", Arrays.asList("0"));
rules.put("==", Arrays.asList("0"));
rules.put("!=", Arrays.asList("0"));
rules.put(">=", Arrays.asList("0", "1"));
rules.put("<=", Arrays.asList("0"));
return rules;
}

@Override
public boolean isTargetMethod(final JavaNameOccurrence occ) {
boolean target = false;
if (occ.getNameForWhichThisIsAQualifier() != null
&& occ.getLocation().getImage().endsWith(".length")
) {
target = true;
}
return target;
}

@Override
public Object visit(final ASTPrimarySuffix node, final Object data) {
if (node.getImage() != null && node.getImage().endsWith("length")) {
ASTClassOrInterfaceType type = getTypeOfPrimaryPrefix(node);
if (type == null) {
type = getTypeOfMethodCall(node);
}
if (type != null
&& this.appliesToClassName(type.getType().getSimpleName())
) {
checkNodeAndReport(
data, node, node.jjtGetParent().jjtGetParent()
);
}
}
return data;
}

/**
* Get the type returned by the method call.
* @param node Node suffix
* @return The type or null if it's not found
*/
private static ASTClassOrInterfaceType getTypeOfMethodCall(
final ASTPrimarySuffix node) {
ASTClassOrInterfaceType type = null;
final ASTName method = node.jjtGetParent()
.getFirstChildOfType(ASTPrimaryPrefix.class)
.getFirstChildOfType(ASTName.class);
if (method != null) {
final ClassScope scope = node.getScope()
.getEnclosingScope(ClassScope.class);
final Map<MethodNameDeclaration, List<NameOccurrence>> methods =
scope.getMethodDeclarations();
//@checkstyle LineLengthCheck (1 line)
for (final Map.Entry<MethodNameDeclaration, List<NameOccurrence>> entry
: methods.entrySet()) {
if (entry.getKey().getName().equals(method.getImage())) {
type = entry.getKey().getNode()
.getFirstParentOfType(ASTMethodDeclaration.class)
.getFirstChildOfType(ASTResultType.class)
.getFirstDescendantOfType(
ASTClassOrInterfaceType.class
);
break;
}
}
}
return type;
}

/**
* Get the type of the primary prefix.
* @param node Node suffix
* @return The type or null if it's not found.
*/
private static ASTClassOrInterfaceType getTypeOfPrimaryPrefix(
final ASTPrimarySuffix node) {
return node.jjtGetParent()
.getFirstChildOfType(ASTPrimaryPrefix.class)
.getFirstDescendantOfType(ASTClassOrInterfaceType.class);
}
}
9 changes: 9 additions & 0 deletions qulice-pmd/src/main/resources/com/qulice/pmd/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@
</description>
</rule>

<rule name="UseStringIsEmptyRule"
message="Use String.isEmpty() when checking for empty string"
class="com.qulice.pmd.rules.UseStringIsEmptyRule">
<description>
Method String.isEmpty() should be used when testing for empty string, rather than using
String.length() compared to 0
</description>
</rule>

<rule ref="rulesets/java/naming.xml">
<exclude name="ShortClassName"/>
<exclude name="ShortVariable"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright (c) 2011-2018, Qulice.com
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met: 1) Redistributions of source code must retain the above
* copyright notice, this list of conditions and the following
* disclaimer. 2) Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution. 3) Neither the name of the Qulice.com nor
* the names of its contributors may be used to endorse or promote
* products derived from this software without specific prior written
* permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
* NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
* FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
* THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
* INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
* OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package com.qulice.pmd;

import org.hamcrest.Matchers;
import org.junit.Test;

/**
* Test case for {@link UseStringIsEmptyRule}.
* @author Mihai Andronache (amihaiemil@gmail.com)
* @version $Id$
* @since 0.18
*/
public final class UseStringIsEmptyRuleTest {

/**
* Error message used to inform about using public static method.
*/
private static final String ERR_MESSAGE =
"Use String.isEmpty() when checking for empty string";

/**
* UseStringIsEmptyRule can detect when String.length() is compared to 0.
* @throws Exception If something goes wrong
*/
@Test
public void detectsLengthEqualsZero() throws Exception {
new PmdAssert(
"StringLengthEqualsZero.java", Matchers.is(false),
Matchers.containsString(
UseStringIsEmptyRuleTest.ERR_MESSAGE
)
).validate();
}

/**
* UseStringIsEmptyRule can detect when String.length() >= 1,
* when the String is returned by a method.
* @throws Exception If something goes wrong
*/
@Test
public void detectsLengthGreaterOrEqualOne() throws Exception {
new PmdAssert(
"StringFromMethodLength.java", Matchers.is(false),
Matchers.containsString(
UseStringIsEmptyRuleTest.ERR_MESSAGE
)
).validate();
}

/**
* UseStringIsEmptyRule can detect when String.length() < 1,
* when the String is a local variable.
* @throws Exception If something goes wrong
*/
@Test
public void detectsLengthLessThanOne() throws Exception {
new PmdAssert(
"LocalStringLength.java", Matchers.is(false),
Matchers.containsString(
UseStringIsEmptyRuleTest.ERR_MESSAGE
)
).validate();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package foo;

public final class LocalStringLength {

public boolean sizeLessThanOne() {
final String str = "somestring";
return str.length() < 1;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package foo;

public final class StringFromMethodLength {

public boolean sizeGreaterOrEqualOne() {
return getString().length() >= 1;
}

public String getString() {
return "somestring";
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package foo;

public final class StringLengthEqualsZero {

private final String somestring;

public StringLengthEqualsZero(final String str) {
this.somestring = str;
}

public boolean sizeIsZero() {
return somestring.length() == 0;
}

}

0 comments on commit 9df81cd

Please sign in to comment.