diff --git a/docs/pages/pmd/rules/ecmascript/errorprone.md b/docs/pages/pmd/rules/ecmascript/errorprone.md index a572b3ce4c..123103175b 100644 --- a/docs/pages/pmd/rules/ecmascript/errorprone.md +++ b/docs/pages/pmd/rules/ecmascript/errorprone.md @@ -103,11 +103,7 @@ precision in a floating point number. This may result in numeric calculations b **This rule is defined by the following XPath expression:** ``` -//NumberLiteral[ - @Image != @Number - and translate(@Image, "e", "E") != @Number - and concat(@Image, ".0") != @Number - and @Image != substring-before(translate(@Number, ".", ""), "E")] +//NumberLiteral[@NormalizedImage != @Number] ``` **Example(s):** diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 2da4e9bbde..af83be2dd6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,11 @@ This is a minor release. ### New and noteworthy ### Fixed Issues + +* ecmascript + * [#861](https://github.com/pmd/pmd/issues/861): \[ecmascript] InnaccurateNumericLiteral false positive with hex literals +* java-codestyle + * [#1158](https://github.com/pmd/pmd/issues/1158): \[java] Fix IdenticalCatchBranches false positive * xml * [#715](https://github.com/pmd/pmd/issues/715): \[xml] ProjectVersionAsDependencyVersion false positive diff --git a/pmd-dist/src/main/scripts/run.sh b/pmd-dist/src/main/scripts/run.sh index 7ff7e02438..5069b6a930 100755 --- a/pmd-dist/src/main/scripts/run.sh +++ b/pmd-dist/src/main/scripts/run.sh @@ -75,8 +75,8 @@ check_lib_dir() { } jre_specific_vm_options() { - # java_ver is eg "18" for java 1.8, "90" for java 9.0 - java_ver=$(java -version 2>&1 | sed -n ';s/^.* version "\(.*\)\.\(.*\)\..*".*$/\1\2/p;') + # java_ver is eg "18" for java 1.8, "90" for java 9.0, "100" for java 10.0.x + java_ver=$(java -version 2>&1 | sed -n -e 's/-ea/.0.0/i' -e 's/^.* version "\(.*\)\.\(.*\)\..*".*$/\1\2/p') options="" if [ "$java_ver" -ge 90 ] && [ "${APPNAME}" = "designer" ] diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java index bd07b1291e..ba10b3f3c7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java @@ -12,6 +12,9 @@ import java.util.Set; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTCatchStatement; +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.ASTTryStatement; import net.sourceforge.pmd.lang.java.ast.ASTType; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; @@ -31,7 +34,7 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule { } - // groups catch statements by equivalence class, according to the string value of their tokens. + /** groups catch statements by equivalence class, according to the equivalence {@link #areEquivalent(ASTCatchStatement, ASTCatchStatement)}. */ private Set> equivalenceClasses(List catches) { Set> result = new HashSet<>(catches.size()); for (ASTCatchStatement stmt : catches) { @@ -103,19 +106,15 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule { return super.visit(node, data); } + /** - * Checks whether two nodes has same subtree. - * Note, it wouldn't be sensitive to changes in the exception variable name. + * Checks whether two nodes define the same subtree, + * up to the renaming of one local variable. * - * @param node1 - * the first node to check - * @param node2 - * the second node to check - * @param exceptionName1 - * the first exception variable name - * @param exceptionName2 - * the second exception variable name - * @return ture if two nodes has same subtree, otherwise false + * @param node1 the first node to check + * @param node2 the second node to check + * @param exceptionName1 the first exception variable name + * @param exceptionName2 the second exception variable name */ private boolean hasSameSubTree(Node node1, Node node2, String exceptionName1, String exceptionName2) { if (node1 == null && node2 == null) { @@ -131,18 +130,7 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule { for (int num = 0; num < node1.jjtGetNumChildren(); num++) { - //type of nodes are different - if (node1.jjtGetChild(num).getClass() != node2.jjtGetChild(num).getClass()) { - return false; - } - - String image1 = node1.jjtGetChild(num).getImage(); - String image2 = node2.jjtGetChild(num).getImage(); - - //image of nodes are different - if (!Objects.equals(image1, image2) - // wouldn't be sensitive to changes in the exception variable name - && !Objects.equals(image1, exceptionName1) && Objects.equals(image2, exceptionName2)) { + if (!basicEquivalence(node1.jjtGetChild(num), node2.jjtGetChild(num), exceptionName1, exceptionName2)) { return false; } @@ -154,4 +142,44 @@ public class IdenticalCatchBranchesRule extends AbstractJavaRule { } return true; } + + + // no subtree comparison + private boolean basicEquivalence(Node node1, Node node2, String varName1, String varName2) { + // Nodes must have the same type + if (node1.getClass() != node2.getClass()) { + return false; + } + + String image1 = node1.getImage(); + String image2 = node2.getImage(); + + // image of nodes must be the same + return Objects.equals(image1, image2) + // or must be references to the variable we allow to interchange + || Objects.equals(image1, varName1) && Objects.equals(image2, varName2) + // which means we must filter out method references. + && isNoMethodName(node1) && isNoMethodName(node2); + + } + + + private boolean isNoMethodName(Node name) { + + if (name instanceof ASTName + && (name.jjtGetParent() instanceof ASTPrimaryPrefix || name.jjtGetParent() instanceof ASTPrimarySuffix)) { + + Node prefixOrSuffix = name.jjtGetParent(); + + if (prefixOrSuffix.jjtGetParent().jjtGetNumChildren() > 1 + prefixOrSuffix.jjtGetChildIndex()) { + // there's one next sibling + + Node next = prefixOrSuffix.jjtGetParent().jjtGetChild(prefixOrSuffix.jjtGetChildIndex() + 1); + if (next instanceof ASTPrimarySuffix) { + return !((ASTPrimarySuffix) next).isArguments(); + } + } + } + return true; + } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/IdenticalCatchBranches.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/IdenticalCatchBranches.xml index 082448145b..30b250f928 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/IdenticalCatchBranches.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/IdenticalCatchBranches.xml @@ -71,5 +71,74 @@ + + #1158 false positive 1 + 0 + + + + + + + #1158 false positive 2 + 0 + + + + + + + False positive with method name mistaken for exception parameter + 0 + + + + + \ No newline at end of file diff --git a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/ASTNumberLiteral.java b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/ASTNumberLiteral.java index a0eda4c93e..9adcfd447d 100644 --- a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/ASTNumberLiteral.java +++ b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/ASTNumberLiteral.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.ecmascript.ast; import org.mozilla.javascript.ast.NumberLiteral; -// TODO The Rhino node does not tell us whether this was specified via decimal, octal or hexidecimal. public class ASTNumberLiteral extends AbstractEcmascriptNode { public ASTNumberLiteral(NumberLiteral numberLiteral) { super(numberLiteral); @@ -21,8 +20,20 @@ public class ASTNumberLiteral extends AbstractEcmascriptNode { } public String getNormalizedImage() { - // TODO Implement - return super.getImage(); + String image = getImage(); + image = normalizeHexIntegerLiteral(image); + image = image.replace('e', 'E'); + if (image.indexOf('.') == -1 && image.indexOf('E') == -1) { + image = image + ".0"; + } + return image; + } + + private String normalizeHexIntegerLiteral(String image) { + if (image.startsWith("0x") || image.startsWith("0X")) { + return String.valueOf(Integer.parseInt(image.substring(2), 16)); + } + return image; } public double getNumber() { diff --git a/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml b/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml index 6ece0ac336..1aea173480 100644 --- a/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml +++ b/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml @@ -107,11 +107,7 @@ precision in a floating point number. This may result in numeric calculations b diff --git a/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/errorprone/xml/InnaccurateNumericLiteral.xml b/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/errorprone/xml/InnaccurateNumericLiteral.xml index 6659953521..d5123e1887 100644 --- a/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/errorprone/xml/InnaccurateNumericLiteral.xml +++ b/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/errorprone/xml/InnaccurateNumericLiteral.xml @@ -3,64 +3,66 @@ xmlns="http://pmd.sourceforge.net/rule-tests" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> + - + Ok integer 0 ecmascript 3 + - + Bad integer 1 - + var x = 999999999999999999999999; ecmascript 3 + - + Ok float 0 ecmascript 3 + - + Bad float 1 ecmascript 3 + - + Ok float w/ exponent 0 ecmascript 3 + - + Bad float w/ exponent 1 ecmascript 3 + + + #861 [ecmascript] InnaccurateNumericLiteral false positive + 0 + + ecmascript 3 + diff --git a/pmd-ui/pom.xml b/pmd-ui/pom.xml index 57940bc958..047796f660 100644 --- a/pmd-ui/pom.xml +++ b/pmd-ui/pom.xml @@ -27,6 +27,15 @@ ${java.version} + + org.apache.maven.plugins + maven-pmd-plugin + + + /net/sourceforge/pmd/pmd-ui-dogfood-config.xml + + + diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/EditPropertyDialogController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/EditPropertyDialogController.java index 1c692c8740..05f37a9d1a 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/EditPropertyDialogController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/EditPropertyDialogController.java @@ -115,6 +115,7 @@ public class EditPropertyDialogController implements Initializable { backingDescriptor.ifPresent(PropertyDescriptorSpec::unbind); backingDescriptor.setValue(null); backingDescriptorList.setValue(null); + this.nameProperty().setValue(""); // necessary to get the validator to reevaluate each time } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java index a8560e87a0..f3fddaf9e8 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java @@ -47,11 +47,11 @@ public class NodeInfoPanelController implements Initializable { @FXML private TabPane nodeInfoTabPane; @FXML - private Tab xpathAttributesTitledPane; + private Tab xpathAttributesTab; @FXML private ListView xpathAttributesListView; @FXML - private Tab metricResultsTitledPane; + private Tab metricResultsTab; @FXML private ListView metricResultsListView; @FXML @@ -118,9 +118,9 @@ public class NodeInfoPanelController implements Initializable { private void notifyMetricsAvailable(long numMetrics) { - metricResultsTitledPane.setText("Metrics\t(" + (numMetrics == 0 ? "none" : numMetrics) + ")"); + metricResultsTab.setText("Metrics\t(" + (numMetrics == 0 ? "none" : numMetrics) + ")"); metricsTitleLabel.setText("Metrics\t(" + (numMetrics == 0 ? "none" : numMetrics) + " available)"); - metricResultsTitledPane.setDisable(numMetrics == 0); + metricResultsTab.setDisable(numMetrics == 0); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java index ae9a85a18c..f9959b4062 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java @@ -73,7 +73,7 @@ public class XPathPanelController implements Initializable, SettingsOwner { private final ObservableXPathRuleBuilder ruleBuilder = new ObservableXPathRuleBuilder(); @FXML - private PropertyTableView propertyView; + private PropertyTableView propertyTableView; @FXML private CustomCodeArea xpathExpressionArea; @FXML @@ -167,7 +167,7 @@ public class XPathPanelController implements Initializable, SettingsOwner { DesignerUtil.rewire(getRuleBuilder().xpathExpressionProperty(), xpathExpressionProperty()); DesignerUtil.rewire(getRuleBuilder().rulePropertiesProperty(), - propertyView.rulePropertiesProperty(), propertyView::setRuleProperties); + propertyTableView.rulePropertiesProperty(), propertyTableView::setRuleProperties); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java index 29df7f6021..ee234a62ae 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerUtil.java @@ -157,14 +157,13 @@ public final class DesignerUtil { */ public static void rewire(Property underlying, ObservableValue ui, Consumer setter) { setter.accept(underlying.getValue()); - underlying.unbind(); - underlying.bind(ui); // Bindings are garbage collected after the popup dies + rewire(underlying, ui); } /** Like rewire, with no initialisation. */ public static void rewire(Property underlying, ObservableValue source) { underlying.unbind(); - underlying.bind(source); + underlying.bind(source); // Bindings are garbage collected after the popup dies } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/beans/RestorePropertyVisitor.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/beans/RestorePropertyVisitor.java index a06b19d222..40dc63eb6f 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/beans/RestorePropertyVisitor.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/beans/RestorePropertyVisitor.java @@ -6,9 +6,11 @@ package net.sourceforge.pmd.util.fxdesigner.util.beans; import java.beans.PropertyDescriptor; import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.stream.Collectors; @@ -90,6 +92,9 @@ public class RestorePropertyVisitor extends BeanNodeVisitor { Iterator existingItems = container.iterator(); Class itemType = null; + // use a buffer to avoid concurrent modification + List itemsToAdd = new ArrayList<>(); + for (SimpleBeanModelNode child : model.getChildrenNodes()) { SettingsOwner item; if (existingItems.hasNext()) { @@ -108,9 +113,11 @@ public class RestorePropertyVisitor extends BeanNodeVisitor { } child.accept(this, item); - container.add(item); + itemsToAdd.add(item); } + container.addAll(itemsToAdd); + try { PropertyUtils.setProperty(target, model.getPropertyName(), container); } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/node-info.fxml b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/node-info.fxml index 4ed500e4b1..7a0559ecad 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/node-info.fxml +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/node-info.fxml @@ -17,7 +17,7 @@ - + @@ -41,7 +41,7 @@ - + diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/xpath.fxml b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/xpath.fxml index c5fcef3939..dcd3129843 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/xpath.fxml +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/xpath.fxml @@ -25,7 +25,7 @@ AnchorPane.topAnchor="0.0"> - -Xmx512m -Dfile.encoding=${project.build.sourceEncoding} - 1.1.0 + 1.1.1-SNAPSHOT @@ -448,12 +448,12 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code net.sourceforge.pmd pmd-core - 6.1.0 + 6.4.0 net.sourceforge.pmd pmd-java - 6.1.0 + 6.4.0