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] Update rule UseVarargs #3182

Merged
merged 8 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/files/all-java.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
<rule ref="category/java/bestpractices.xml/UseAssertTrueInsteadOfAssertEquals"/>
<rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>
<rule ref="category/java/bestpractices.xml/UseTryWithResources"/>
<!-- <rule ref="category/java/bestpractices.xml/UseVarargs"/> -->
<rule ref="category/java/bestpractices.xml/UseVarargs"/>
<rule ref="category/java/bestpractices.xml/WhileLoopWithLiteralBoolean"/>

<!-- codestyle.xml -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
import net.sourceforge.pmd.lang.java.symbols.JMethodSymbol;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.rule.xpath.DeprecatedAttribute;


Expand Down Expand Up @@ -145,4 +146,14 @@ public ASTArrayDimensions getExtraDimensions() {
return children(ASTArrayDimensions.class).first();
}

/**
* Returns whether this is a main method declaration.
*/
public boolean isMainMethod() {
return this.hasModifiers(JModifier.PUBLIC, JModifier.STATIC)
&& "main".equals(this.getName())
&& this.isVoid()
&& this.getArity() == 1
&& TypeTestUtil.isExactlyA(String[].class, this.getFormalParameters().get(0));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,7 @@ private static boolean isStringBufferOrBuilder(TypeNode node) {
*/
public static boolean isMainMethod(JavaNode node) {
if (node instanceof ASTMethodDeclaration) {
ASTMethodDeclaration decl = (ASTMethodDeclaration) node;


return decl.hasModifiers(JModifier.PUBLIC, JModifier.STATIC)
&& "main".equals(decl.getName())
&& decl.isVoid()
&& decl.getArity() == 1
&& TypeTestUtil.isExactlyA(String[].class, decl.getFormalParameters().get(0));
return ((ASTMethodDeclaration) node).isMainMethod();
}
return false;
}
Expand Down
25 changes: 5 additions & 20 deletions pmd-java/src/main/resources/category/java/bestpractices.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1806,29 +1806,14 @@ having to deal with the creation of an array.
</description>
<priority>4</priority>
<properties>
<property name="version" value="2.0" />
<property name="xpath">
<value>
<![CDATA[
//FormalParameters/FormalParameter
[position()=last()]
[VariableDeclaratorId/@ArrayType=true()]
[@Varargs=false()]
[not (./Type[@ArrayType=true()]/ReferenceType[PrimitiveType[@Image='byte']])]
[not (./Type/ReferenceType[ClassOrInterfaceType[@Image='Byte']])]
[not (./Type/PrimitiveType[@Image='byte'])]
[not (ancestor::MethodDeclaration/preceding-sibling::Annotation/*/Name[@Image='Override'])]
[not(
ancestor::MethodDeclaration
[ @Public=true()
and @Static=true()
and child::ResultType[@Void=true()]
and @Name = 'main'
and @Arity = 1
]
(: Type of the formal parameter here. :)
and pmd-java:typeIs('java.lang.String[]')
)]
//FormalParameters[not(parent::MethodDeclaration[@Overridden=true() or @MainMethod=true()])]
/FormalParameter[position()=last()]
[@Varargs=false()]
[ArrayType[not(PrimitiveType[@Kind = "byte"] or ClassOrInterfaceType[pmd-java:typeIs('java.lang.Byte')])]
or VariableDeclaratorId[ArrayDimensions] and (PrimitiveType[not(@Kind="byte")] or ClassOrInterfaceType[not(pmd-java:typeIs('java.lang.Byte'))])]
]]>
</value>
</property>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.test.RelevantAttributePrinter;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTModifierList;
import net.sourceforge.pmd.lang.java.ast.JModifier;
import net.sourceforge.pmd.lang.rule.xpath.Attribute;
Expand Down Expand Up @@ -40,7 +41,13 @@ protected boolean ignoreAttribute(@NonNull Node node, @NonNull Attribute attribu
// everytime. OTOH failing dump tests would warn us that we removed
// something that wasn't deprecated
|| attribute.isDeprecated()
|| attribute.getName().equals("Expression") && node instanceof ASTExpression;
|| "MainMethod".equals(attribute.getName()) && node instanceof ASTMethodDeclaration && !isBooleanTrue(attribute.getValue())
|| "Expression".equals(attribute.getName()) && node instanceof ASTExpression;
}

private boolean isBooleanTrue(Object o) {
// for some reason Boolean::new is called somewhere in the reflection layer
return o instanceof Boolean && (Boolean) o;
}

private AttributeInfo getModifierAttr(String name, Set<JModifier> mods) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,18 @@

import net.sourceforge.pmd.lang.ast.ParseException;
import net.sourceforge.pmd.lang.ast.test.BaseParsingHelper;
import net.sourceforge.pmd.lang.ast.test.BaseTreeDumpTest;
import net.sourceforge.pmd.lang.ast.test.RelevantAttributePrinter;
import net.sourceforge.pmd.lang.java.BaseJavaTreeDumpTest;
import net.sourceforge.pmd.lang.java.JavaParsingHelper;
import net.sourceforge.pmd.lang.java.symbols.JElementSymbol;
import net.sourceforge.pmd.lang.java.types.JPrimitiveType;

public class Java16TreeDumpTest extends BaseTreeDumpTest {
public class Java16TreeDumpTest extends BaseJavaTreeDumpTest {
private final JavaParsingHelper java16 =
JavaParsingHelper.WITH_PROCESSING.withDefaultVersion("16")
.withResourceContext(Java15TreeDumpTest.class, "jdkversiontests/java16/");
private final JavaParsingHelper java16p = java16.withDefaultVersion("16-preview");
private final JavaParsingHelper java15 = java16.withDefaultVersion("15");

public Java16TreeDumpTest() {
super(new RelevantAttributePrinter(), ".java");
oowekyala marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public BaseParsingHelper<?, ?> getParser() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import net.sourceforge.pmd.testframework.PmdRuleTst;

@org.junit.Ignore("Rule has not been updated yet")
public class UseVarargsTest extends PmdRuleTst {
// no additional unit tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
+- ClassOrInterfaceDeclaration[@Abstract = "false", @Annotation = "false", @Anonymous = "false", @BinaryName = "GitHubBug309", @CanonicalName = "GitHubBug309", @EffectiveVisibility = "public", @Enum = "false", @Final = "false", @Interface = "false", @Local = "false", @Nested = "false", @PackageName = "", @PackagePrivate = "false", @Record = "false", @RegularClass = "true", @RegularInterface = "false", @SimpleName = "GitHubBug309", @TopLevel = "true", @Visibility = "public"]
+- ModifierList[@EffectiveModifiers = "{public}", @ExplicitModifiers = "{public}"]
+- ClassOrInterfaceBody[@Size = "1"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @MainMethod = "true", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- ModifierList[@EffectiveModifiers = "{public, static}", @ExplicitModifiers = "{public, static}"]
+- VoidType[]
+- FormalParameters[@Size = "1"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
+- ClassOrInterfaceDeclaration[@Abstract = "false", @Annotation = "false", @Anonymous = "false", @BinaryName = "NonSealedIdentifier", @CanonicalName = "NonSealedIdentifier", @EffectiveVisibility = "public", @Enum = "false", @Final = "false", @Interface = "false", @Local = "false", @Nested = "false", @PackageName = "", @PackagePrivate = "false", @Record = "false", @RegularClass = "true", @RegularInterface = "false", @SimpleName = "NonSealedIdentifier", @TopLevel = "true", @Visibility = "public"]
+- ModifierList[@EffectiveModifiers = "{public}", @ExplicitModifiers = "{public}"]
+- ClassOrInterfaceBody[@Size = "1"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @MainMethod = "true", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- ModifierList[@EffectiveModifiers = "{public, static}", @ExplicitModifiers = "{public, static}"]
+- VoidType[]
+- FormalParameters[@Size = "1"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
+- ClassOrInterfaceDeclaration[@Abstract = "false", @Annotation = "false", @Anonymous = "false", @BinaryName = "TextBlocks", @CanonicalName = "TextBlocks", @EffectiveVisibility = "public", @Enum = "false", @Final = "false", @Interface = "false", @Local = "false", @Nested = "false", @PackageName = "", @PackagePrivate = "false", @Record = "false", @RegularClass = "true", @RegularInterface = "false", @SimpleName = "TextBlocks", @TopLevel = "true", @Visibility = "public"]
+- ModifierList[@EffectiveModifiers = "{public}", @ExplicitModifiers = "{public}"]
+- ClassOrInterfaceBody[@Size = "1"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @MainMethod = "true", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- ModifierList[@EffectiveModifiers = "{public, static}", @ExplicitModifiers = "{public, static}"]
+- VoidType[]
+- FormalParameters[@Size = "1"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
| +- InfixExpression[@CompileTimeConstant = "false", @Operator = "==", @ParenthesisDepth = "1", @Parenthesized = "true"]
| +- VariableAccess[@AccessType = "READ", @CompileTimeConstant = "false", @Image = "obj", @Name = "obj", @ParenthesisDepth = "0", @Parenthesized = "false"]
| +- VariableAccess[@AccessType = "READ", @CompileTimeConstant = "false", @Image = "s", @Name = "s", @ParenthesisDepth = "0", @Parenthesized = "false"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- MethodDeclaration[@Abstract = "false", @Arity = "1", @EffectiveVisibility = "public", @Image = "main", @MainMethod = "true", @Name = "main", @Overridden = "false", @Varargs = "false", @Visibility = "public", @Void = "true"]
+- ModifierList[@EffectiveModifiers = "{public, static}", @ExplicitModifiers = "{public, static}"]
+- VoidType[]
+- FormalParameters[@Size = "1"]
Expand Down