From 99e0e6f28482f8599b7808cf6076f444545ec5cd Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 10 Nov 2018 20:27:35 +0100 Subject: [PATCH 01/44] Rework pmd-dist so that it is usable for custom PMD distributions * the scripts are now in resources and included in pmd-dist jar artifact * the baseDirectory can be overridden using a property * the assembly descriptors are also in the pmd-dist jar artifact --- pmd-dist/pom.xml | 31 +++++++++++++++++-- pmd-dist/{ => src/main/resources}/LICENSE | 0 .../assemblies/pmd-bin.xml} | 10 +++--- .../assemblies/pmd-src.xml} | 2 +- .../{ => resources}/scripts/bgastviewer.bat | 0 .../src/main/{ => resources}/scripts/cpd.bat | 0 .../main/{ => resources}/scripts/cpdgui.bat | 0 .../main/{ => resources}/scripts/designer.bat | 0 .../{ => resources}/scripts/designerold.bat | 0 .../src/main/{ => resources}/scripts/pmd.bat | 0 .../src/main/{ => resources}/scripts/run.sh | 0 .../pmd/it/BinaryDistributionIT.java | 6 +++- 12 files changed, 40 insertions(+), 9 deletions(-) rename pmd-dist/{ => src/main/resources}/LICENSE (100%) rename pmd-dist/src/main/{assembly/bin.xml => resources/assemblies/pmd-bin.xml} (84%) rename pmd-dist/src/main/{assembly/src.xml => resources/assemblies/pmd-src.xml} (98%) rename pmd-dist/src/main/{ => resources}/scripts/bgastviewer.bat (100%) rename pmd-dist/src/main/{ => resources}/scripts/cpd.bat (100%) rename pmd-dist/src/main/{ => resources}/scripts/cpdgui.bat (100%) rename pmd-dist/src/main/{ => resources}/scripts/designer.bat (100%) rename pmd-dist/src/main/{ => resources}/scripts/designerold.bat (100%) rename pmd-dist/src/main/{ => resources}/scripts/pmd.bat (100%) rename pmd-dist/src/main/{ => resources}/scripts/run.sh (100%) diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index 7aa2c57082..046a2597d1 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -11,8 +11,33 @@ 6.10.0-SNAPSHOT + + pmd-bin-${project.version} + + + + maven-resources-plugin + + + copy-resources + prepare-package + + copy-resources + + + ${basedir}/target/extra-resources + + + src/main/resources + false + + + + + + maven-assembly-plugin @@ -30,9 +55,9 @@ single - pmd-bin-${project.version} + ${pmd.dist.bin.baseDirectory} - src/main/assembly/bin.xml + src/main/resources/assemblies/pmd-bin.xml @@ -45,7 +70,7 @@ pmd-src-${project.version} - src/main/assembly/src.xml + src/main/resources/assemblies/pmd-src.xml diff --git a/pmd-dist/LICENSE b/pmd-dist/src/main/resources/LICENSE similarity index 100% rename from pmd-dist/LICENSE rename to pmd-dist/src/main/resources/LICENSE diff --git a/pmd-dist/src/main/assembly/bin.xml b/pmd-dist/src/main/resources/assemblies/pmd-bin.xml similarity index 84% rename from pmd-dist/src/main/assembly/bin.xml rename to pmd-dist/src/main/resources/assemblies/pmd-bin.xml index 2b44241aa9..8b0c0134b2 100644 --- a/pmd-dist/src/main/assembly/bin.xml +++ b/pmd-dist/src/main/resources/assemblies/pmd-bin.xml @@ -1,13 +1,13 @@ - bin + pmd-bin zip true - pmd-bin-${project.version} + ${pmd.dist.bin.baseDirectory} @@ -18,7 +18,7 @@ designer.bat pmd.bat - src/main/scripts + target/extra-resources/scripts bin 0755 0755 @@ -29,7 +29,7 @@ run.sh - src/main/scripts + target/extra-resources/scripts bin 0755 0755 @@ -40,6 +40,8 @@ LICENSE + target/extra-resources + . 0755 0644 diff --git a/pmd-dist/src/main/assembly/src.xml b/pmd-dist/src/main/resources/assemblies/pmd-src.xml similarity index 98% rename from pmd-dist/src/main/assembly/src.xml rename to pmd-dist/src/main/resources/assemblies/pmd-src.xml index e69362fffc..8c8820d03c 100644 --- a/pmd-dist/src/main/assembly/src.xml +++ b/pmd-dist/src/main/resources/assemblies/pmd-src.xml @@ -1,7 +1,7 @@ - src + pmd-src zip diff --git a/pmd-dist/src/main/scripts/bgastviewer.bat b/pmd-dist/src/main/resources/scripts/bgastviewer.bat similarity index 100% rename from pmd-dist/src/main/scripts/bgastviewer.bat rename to pmd-dist/src/main/resources/scripts/bgastviewer.bat diff --git a/pmd-dist/src/main/scripts/cpd.bat b/pmd-dist/src/main/resources/scripts/cpd.bat similarity index 100% rename from pmd-dist/src/main/scripts/cpd.bat rename to pmd-dist/src/main/resources/scripts/cpd.bat diff --git a/pmd-dist/src/main/scripts/cpdgui.bat b/pmd-dist/src/main/resources/scripts/cpdgui.bat similarity index 100% rename from pmd-dist/src/main/scripts/cpdgui.bat rename to pmd-dist/src/main/resources/scripts/cpdgui.bat diff --git a/pmd-dist/src/main/scripts/designer.bat b/pmd-dist/src/main/resources/scripts/designer.bat similarity index 100% rename from pmd-dist/src/main/scripts/designer.bat rename to pmd-dist/src/main/resources/scripts/designer.bat diff --git a/pmd-dist/src/main/scripts/designerold.bat b/pmd-dist/src/main/resources/scripts/designerold.bat similarity index 100% rename from pmd-dist/src/main/scripts/designerold.bat rename to pmd-dist/src/main/resources/scripts/designerold.bat diff --git a/pmd-dist/src/main/scripts/pmd.bat b/pmd-dist/src/main/resources/scripts/pmd.bat similarity index 100% rename from pmd-dist/src/main/scripts/pmd.bat rename to pmd-dist/src/main/resources/scripts/pmd.bat diff --git a/pmd-dist/src/main/scripts/run.sh b/pmd-dist/src/main/resources/scripts/run.sh similarity index 100% rename from pmd-dist/src/main/scripts/run.sh rename to pmd-dist/src/main/resources/scripts/run.sh diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java index c9c52d291e..19d56ad9ab 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.it; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; @@ -59,6 +60,7 @@ public class BinaryDistributionIT { Set result = new HashSet<>(); String basedir = "pmd-bin-" + PMDVersion.VERSION + "/"; result.add(basedir); + result.add(basedir + "LICENSE"); result.add(basedir + "bin/run.sh"); result.add(basedir + "bin/pmd.bat"); result.add(basedir + "bin/cpd.bat"); @@ -81,7 +83,9 @@ public class BinaryDistributionIT { zip.close(); - assertTrue(expectedFileNames.isEmpty()); + if (!expectedFileNames.isEmpty()) { + fail("Missing files in archive: " + expectedFileNames); + } } @Test From b5d50068bdbb2bba5f149ff1991b74c52c4223f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Jan 2019 02:32:28 +0100 Subject: [PATCH 02/44] Add LanguageMetricsProvider --- .../lang/metrics/LanguageMetricsProvider.java | 88 ++++++++++++++++++ .../pmd/lang/metrics/MetricKey.java | 1 + .../pmd/lang/metrics/MetricResult.java | 64 +++++++++++++ .../AbstractLanguageMetricsProvider.java | 93 +++++++++++++++++++ .../metrics/internal/DummyMetricMemoizer.java | 44 +++++++++ .../internal/DummyProjectMemoizer.java | 45 +++++++++ .../fxdesigner/NodeInfoPanelController.java | 2 +- .../fxdesigner/model/MetricEvaluator.java | 2 + .../util/fxdesigner/model/MetricResult.java | 39 -------- .../util/controls/MetricResultListCell.java | 2 +- 10 files changed, 339 insertions(+), 41 deletions(-) create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java delete mode 100644 pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java new file mode 100644 index 0000000000..45fda79422 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java @@ -0,0 +1,88 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.metrics; + +import java.util.List; + +import net.sourceforge.pmd.annotation.Experimental; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.QualifiableNode; + + +/** + * Language-independent provider for metrics. Can be used to know applicable metrics + * on a node, e.g. to build GUI applications like the designer. + * + * Note: this is experimental, ie unstable until 7.0.0, after which it will probably + * be promoted to a real API. It also exposes some internal API. + * + * @param Type of type declaration nodes of the language + * @param Type of operation declaration nodes of the language + * + * @author Clément Fournier + * @since 7.0.0 + */ +@Experimental +public interface LanguageMetricsProvider { + + /** + * Returns a list of all supported type metric keys + * for the language. + */ + List> getAvailableTypeMetrics(); + + + /** + * Returns a list of all supported operation metric keys + * for the language. + */ + List> getAvailableOperationMetrics(); + + + /** + * Returns the given node casted to {@link T} if it's of the correct + * type, otherwise returns null. + */ + T asTypeNode(Node anyNode); + + + /** + * Returns the given node casted to {@link O} if it's of the correct + * type, otherwise returns null. + */ + O asOperationNode(Node anyNode); + + + /** + * Like {@link MetricsComputer#computeForType(MetricKey, QualifiableNode, boolean, MetricOptions, MetricMemoizer)}, + * but performs no memoisation. + */ + double computeForType(MetricKey key, T node, MetricOptions options); + + + /** + * Like {@link MetricsComputer#computeForOperation(MetricKey, QualifiableNode, boolean, MetricOptions, MetricMemoizer)} + * but performs no memoisation. + */ + double computeForOperation(MetricKey key, O node, MetricOptions options); + + + /** + * Like {@link MetricsComputer#computeWithResultOption(MetricKey, QualifiableNode, boolean, MetricOptions, ResultOption, ProjectMemoizer)} + * but performs no memoisation. + */ + double computeWithResultOption(MetricKey key, T node, MetricOptions options, ResultOption option); + + + /** + * Computes all metrics available on the given node. + * The returned results may contain Double.NaN as a value. + * + * @param node Node to inspect + * + * @return A list of metric results, possibly empty, but with no null element + */ + List computeAllMetricsFor(Node node); +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricKey.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricKey.java index 4f7521df0d..2259ed8956 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricKey.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricKey.java @@ -41,5 +41,6 @@ public interface MetricKey { */ boolean supports(N node); + // TODO the metric key should know about supported options } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java new file mode 100644 index 0000000000..da1f4cf660 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java @@ -0,0 +1,64 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.metrics; + +import java.util.AbstractMap.SimpleEntry; +import java.util.Map.Entry; +import java.util.Objects; + +import net.sourceforge.pmd.annotation.Experimental; +import net.sourceforge.pmd.lang.ast.Node; + +/** + * A data class pairing a computed metric with its key. + * This is used in {@link LanguageMetricsProvider#computeAllMetricsFor(Node)}. + * + * @author Clément Fournier + * @since 6.0.0 + */ +@Experimental +public final class MetricResult { + + private final SimpleEntry, Double> simpleEntry; + + + public MetricResult(MetricKey key, Double value) { + simpleEntry = new SimpleEntry, Double>(key, value); + } + + + MetricResult(Entry, Double> entry) { + simpleEntry = new SimpleEntry<>(entry); + } + + + public MetricKey getKey() { + return simpleEntry.getKey(); + } + + + public Double getValue() { + return simpleEntry.getValue(); + } + + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + MetricResult that = (MetricResult) o; + return Objects.equals(simpleEntry, that.simpleEntry); + } + + + @Override + public int hashCode() { + return Objects.hash(simpleEntry); + } +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java new file mode 100644 index 0000000000..745fbdcee0 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java @@ -0,0 +1,93 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.metrics.internal; + +import java.util.ArrayList; +import java.util.List; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.QualifiableNode; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; +import net.sourceforge.pmd.lang.metrics.MetricKey; +import net.sourceforge.pmd.lang.metrics.MetricOptions; +import net.sourceforge.pmd.lang.metrics.MetricResult; +import net.sourceforge.pmd.lang.metrics.MetricsComputer; +import net.sourceforge.pmd.lang.metrics.ResultOption; + + +/** + * Base implementation for {@link LanguageMetricsProvider}. + * + * @author Clément Fournier + * @since 7.0.0 + */ +public abstract class AbstractLanguageMetricsProvider implements LanguageMetricsProvider { + + private final Class tClass; + private final Class oClass; + private final MetricsComputer myComputer; + + + AbstractLanguageMetricsProvider(Class tClass, + Class oClass, + MetricsComputer computer) { + this.tClass = tClass; + this.oClass = oClass; + this.myComputer = computer; + } + + + @Override + public T asTypeNode(Node anyNode) { + return tClass.isInstance(anyNode) ? tClass.cast(anyNode) : null; + } + + + @Override + public O asOperationNode(Node anyNode) { + return oClass.isInstance(anyNode) ? oClass.cast(anyNode) : null; + } + + // une vraie arnaque + + + @Override + public double computeForType(MetricKey key, T node, MetricOptions options) { + return myComputer.computeForType(key, node, true, options, DummyMetricMemoizer.getInstance()); + } + + + @Override + public double computeForOperation(MetricKey key, O node, MetricOptions options) { + return myComputer.computeForOperation(key, node, true, options, DummyMetricMemoizer.getInstance()); + } + + + @Override + public double computeWithResultOption(MetricKey key, T node, MetricOptions options, ResultOption option) { + return myComputer.computeWithResultOption(key, node, true, options, option, DummyProjectMemoizer.getInstance()); + } + + + @Override + public List computeAllMetricsFor(Node node) { + List results = new ArrayList<>(); + T t = asTypeNode(node); + if (t != null) { + for (MetricKey tkey : getAvailableTypeMetrics()) { + results.add(new MetricResult(tkey, computeForType(tkey, t, MetricOptions.emptyOptions()))); + } + } + O o = asOperationNode(node); + if (o != null) { + for (MetricKey tkey : getAvailableOperationMetrics()) { + results.add(new MetricResult(tkey, computeForOperation(tkey, o, MetricOptions.emptyOptions()))); + } + } + + return results; + } + +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java new file mode 100644 index 0000000000..295d0c69d4 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java @@ -0,0 +1,44 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.metrics.internal; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.metrics.MetricMemoizer; +import net.sourceforge.pmd.lang.metrics.ParameterizedMetricKey; + + +/** + * Memoizes nothing. + * + * @author Clément Fournier + * @since 7.0.0 + */ +public class DummyMetricMemoizer implements MetricMemoizer { + + private static final DummyMetricMemoizer INSTANCE = new DummyMetricMemoizer<>(); + + + private DummyMetricMemoizer() { + + } + + + @Override + public Double getMemo(ParameterizedMetricKey key) { + return null; + } + + + @Override + public void memoize(ParameterizedMetricKey key, double value) { + + } + + + @SuppressWarnings("unchecked") + public static DummyMetricMemoizer getInstance() { + return (DummyMetricMemoizer) INSTANCE; + } +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java new file mode 100644 index 0000000000..5c0ddd6d02 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java @@ -0,0 +1,45 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.metrics.internal; + +import net.sourceforge.pmd.lang.ast.QualifiableNode; +import net.sourceforge.pmd.lang.ast.QualifiedName; +import net.sourceforge.pmd.lang.metrics.MetricMemoizer; +import net.sourceforge.pmd.lang.metrics.ProjectMemoizer; + + +/** + * Memoizes nothing. + * + * @author Clément Fournier + * @since 7.0.0 + */ +public class DummyProjectMemoizer implements ProjectMemoizer { + + private static final DummyProjectMemoizer INSTANCE = new DummyProjectMemoizer<>(); + + + private DummyProjectMemoizer() { + + } + + + @Override + public MetricMemoizer getOperationMemoizer(QualifiedName qname) { + return DummyMetricMemoizer.getInstance(); + } + + + @Override + public MetricMemoizer getClassMemoizer(QualifiedName qname) { + return DummyMetricMemoizer.getInstance(); + } + + + @SuppressWarnings("unchecked") + public static DummyProjectMemoizer getInstance() { + return (DummyProjectMemoizer) INSTANCE; + } +} 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 2acbff2c9a..7bd7d47fb1 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 @@ -17,7 +17,7 @@ import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; -import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; +import net.sourceforge.pmd.lang.metrics.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java index d590f97ddd..30e02d265c 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java @@ -18,6 +18,8 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.metrics.JavaMetrics; import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey; import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; +import net.sourceforge.pmd.lang.metrics.MetricResult; + /** * Evaluates metrics. diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java deleted file mode 100644 index 34b86828e1..0000000000 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java +++ /dev/null @@ -1,39 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.util.fxdesigner.model; - -import java.util.AbstractMap.SimpleEntry; -import java.util.Map.Entry; - -import net.sourceforge.pmd.lang.metrics.MetricKey; - -/** - * @author Clément Fournier - * @since 6.0.0 - */ -public class MetricResult { - - private final SimpleEntry, Double> simpleEntry; - - - public MetricResult(MetricKey key, Double value) { - simpleEntry = new SimpleEntry<>(key, value); - } - - - MetricResult(Entry, ? extends Double> entry) { - simpleEntry = new SimpleEntry<>(entry); - } - - - public MetricKey getKey() { - return simpleEntry.getKey(); - } - - - public Double getValue() { - return simpleEntry.getValue(); - } -} diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java index 77271ce948..622cb2adf0 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java @@ -6,7 +6,7 @@ package net.sourceforge.pmd.util.fxdesigner.util.controls; import java.util.Locale; -import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; +import net.sourceforge.pmd.lang.metrics.MetricResult; import javafx.scene.control.ListCell; import javafx.scene.control.ListView; From 5f2a5bb67813ac5acb38a8bac6d335181d79fd9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Jan 2019 02:47:19 +0100 Subject: [PATCH 03/44] Implement for Java --- .../lang/AbstractLanguageVersionHandler.java | 8 ++ .../pmd/lang/LanguageVersionHandler.java | 15 ++++ .../lang/metrics/LanguageMetricsProvider.java | 7 +- .../pmd/lang/metrics/MetricResult.java | 64 --------------- .../AbstractLanguageMetricsProvider.java | 17 ++-- .../pmd/lang/java/AbstractJavaHandler.java | 39 ++++++++++ .../java/metrics/JavaMetricsComputer.java | 10 ++- .../lang/java/metrics/JavaMetricsFacade.java | 2 +- .../java/metrics/api/JavaClassMetricKey.java | 1 - .../pmd/lang/java/ParserTstUtil.java | 8 +- .../java/metrics/JavaMetricsProviderTest.java | 77 +++++++++++++++++++ .../java/metrics/ProjectMemoizerTest.java | 4 +- .../fxdesigner/NodeInfoPanelController.java | 2 +- .../fxdesigner/model/MetricEvaluator.java | 2 - .../util/fxdesigner/model/MetricResult.java | 39 ++++++++++ .../util/controls/MetricResultListCell.java | 2 +- 16 files changed, 208 insertions(+), 89 deletions(-) delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java create mode 100644 pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java index 400be33c40..c00f38a05f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java @@ -7,6 +7,8 @@ package net.sourceforge.pmd.lang; import java.io.Writer; import net.sourceforge.pmd.lang.dfa.DFAGraphRule; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; + /** * This is a generic implementation of the LanguageVersionHandler interface. @@ -71,4 +73,10 @@ public abstract class AbstractLanguageVersionHandler implements LanguageVersionH public DFAGraphRule getDFAGraphRule() { return null; } + + + @Override + public LanguageMetricsProvider getLanguageMetricsProvider() { + return null; + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java index 5037c9f456..5474b1758a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java @@ -6,8 +6,10 @@ package net.sourceforge.pmd.lang; import java.io.Writer; +import net.sourceforge.pmd.annotation.Experimental; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.dfa.DFAGraphRule; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; import net.sourceforge.pmd.lang.rule.RuleViolationFactory; /** @@ -135,4 +137,17 @@ public interface LanguageVersionHandler { @Deprecated @InternalApi DFAGraphRule getDFAGraphRule(); + + + /** + * Returns the metrics provider for this language version, + * or null if it has none. + * + * Note: this is experimental, ie unstable until 7.0.0, after + * which it will probably be promoted to a stable API. For + * instance the return type will probably be changed to an Optional. + */ + @Experimental + LanguageMetricsProvider getLanguageMetricsProvider(); + } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java index 45fda79422..e2d34c47f4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.lang.metrics; import java.util.List; +import java.util.Map; import net.sourceforge.pmd.annotation.Experimental; import net.sourceforge.pmd.lang.ast.Node; @@ -16,7 +17,7 @@ import net.sourceforge.pmd.lang.ast.QualifiableNode; * on a node, e.g. to build GUI applications like the designer. * * Note: this is experimental, ie unstable until 7.0.0, after which it will probably - * be promoted to a real API. It also exposes some internal API. + * be promoted to a real API. * * @param Type of type declaration nodes of the language * @param Type of operation declaration nodes of the language @@ -82,7 +83,7 @@ public interface LanguageMetricsProvider computeAllMetricsFor(Node node); + Map, Double> computeAllMetricsFor(Node node); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java deleted file mode 100644 index da1f4cf660..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/MetricResult.java +++ /dev/null @@ -1,64 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.metrics; - -import java.util.AbstractMap.SimpleEntry; -import java.util.Map.Entry; -import java.util.Objects; - -import net.sourceforge.pmd.annotation.Experimental; -import net.sourceforge.pmd.lang.ast.Node; - -/** - * A data class pairing a computed metric with its key. - * This is used in {@link LanguageMetricsProvider#computeAllMetricsFor(Node)}. - * - * @author Clément Fournier - * @since 6.0.0 - */ -@Experimental -public final class MetricResult { - - private final SimpleEntry, Double> simpleEntry; - - - public MetricResult(MetricKey key, Double value) { - simpleEntry = new SimpleEntry, Double>(key, value); - } - - - MetricResult(Entry, Double> entry) { - simpleEntry = new SimpleEntry<>(entry); - } - - - public MetricKey getKey() { - return simpleEntry.getKey(); - } - - - public Double getValue() { - return simpleEntry.getValue(); - } - - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - MetricResult that = (MetricResult) o; - return Objects.equals(simpleEntry, that.simpleEntry); - } - - - @Override - public int hashCode() { - return Objects.hash(simpleEntry); - } -} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java index 745fbdcee0..9de2ca3b88 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java @@ -4,15 +4,14 @@ package net.sourceforge.pmd.lang.metrics.internal; -import java.util.ArrayList; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.QualifiableNode; import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; import net.sourceforge.pmd.lang.metrics.MetricKey; import net.sourceforge.pmd.lang.metrics.MetricOptions; -import net.sourceforge.pmd.lang.metrics.MetricResult; import net.sourceforge.pmd.lang.metrics.MetricsComputer; import net.sourceforge.pmd.lang.metrics.ResultOption; @@ -30,7 +29,7 @@ public abstract class AbstractLanguageMetricsProvider myComputer; - AbstractLanguageMetricsProvider(Class tClass, + protected AbstractLanguageMetricsProvider(Class tClass, Class oClass, MetricsComputer computer) { this.tClass = tClass; @@ -72,18 +71,18 @@ public abstract class AbstractLanguageMetricsProvider computeAllMetricsFor(Node node) { - List results = new ArrayList<>(); + public Map, Double> computeAllMetricsFor(Node node) { + Map, Double> results = new HashMap<>(); T t = asTypeNode(node); if (t != null) { for (MetricKey tkey : getAvailableTypeMetrics()) { - results.add(new MetricResult(tkey, computeForType(tkey, t, MetricOptions.emptyOptions()))); + results.put(tkey, computeForType(tkey, t, MetricOptions.emptyOptions())); } } O o = asOperationNode(node); if (o != null) { - for (MetricKey tkey : getAvailableOperationMetrics()) { - results.add(new MetricResult(tkey, computeForOperation(tkey, o, MetricOptions.emptyOptions()))); + for (MetricKey okey : getAvailableOperationMetrics()) { + results.put(okey, computeForOperation(okey, o, MetricOptions.emptyOptions())); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java index 725a4aae03..179ac6a6f1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java @@ -5,6 +5,8 @@ package net.sourceforge.pmd.lang.java; import java.io.Writer; +import java.util.Arrays; +import java.util.List; import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler; import net.sourceforge.pmd.lang.DataFlowHandler; @@ -14,11 +16,16 @@ import net.sourceforge.pmd.lang.XPathHandler; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler; import net.sourceforge.pmd.lang.dfa.DFAGraphRule; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.DumpFacade; import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.ast.MethodLikeNode; import net.sourceforge.pmd.lang.java.dfa.DataFlowFacade; import net.sourceforge.pmd.lang.java.dfa.JavaDFAGraphRule; +import net.sourceforge.pmd.lang.java.metrics.JavaMetricsComputer; +import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey; +import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; import net.sourceforge.pmd.lang.java.multifile.MultifileVisitorFacade; import net.sourceforge.pmd.lang.java.qname.QualifiedNameResolver; import net.sourceforge.pmd.lang.java.rule.JavaRuleViolationFactory; @@ -30,6 +37,9 @@ import net.sourceforge.pmd.lang.java.xpath.MetricFunction; import net.sourceforge.pmd.lang.java.xpath.TypeIsExactlyFunction; import net.sourceforge.pmd.lang.java.xpath.TypeIsFunction; import net.sourceforge.pmd.lang.java.xpath.TypeOfFunction; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; +import net.sourceforge.pmd.lang.metrics.MetricKey; +import net.sourceforge.pmd.lang.metrics.internal.AbstractLanguageMetricsProvider; import net.sourceforge.pmd.lang.rule.RuleViolationFactory; import net.sf.saxon.sxpath.IndependentContext; @@ -42,6 +52,8 @@ import net.sf.saxon.sxpath.IndependentContext; */ public abstract class AbstractJavaHandler extends AbstractLanguageVersionHandler { + private final LanguageMetricsProvider myMetricsProvider = new JavaMetricsProvider(); + @Override public DataFlowHandler getDataFlowHandler() { return new JavaDataFlowHandler(); @@ -147,4 +159,31 @@ public abstract class AbstractJavaHandler extends AbstractLanguageVersionHandler public DFAGraphRule getDFAGraphRule() { return new JavaDFAGraphRule(); } + + + @Override + public LanguageMetricsProvider getLanguageMetricsProvider() { + return myMetricsProvider; + } + + + private static class JavaMetricsProvider extends AbstractLanguageMetricsProvider { + + + JavaMetricsProvider() { + super(ASTAnyTypeDeclaration.class, MethodLikeNode.class, JavaMetricsComputer.getInstance()); + } + + + @Override + public List> getAvailableTypeMetrics() { + return Arrays.asList(JavaClassMetricKey.values()); + } + + + @Override + public List> getAvailableOperationMetrics() { + return Arrays.asList(JavaOperationMetricKey.values()); + } + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsComputer.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsComputer.java index b96d66b93a..9d95cb4eda 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsComputer.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsComputer.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.java.metrics; import java.util.ArrayList; import java.util.List; +import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; @@ -20,12 +21,19 @@ import net.sourceforge.pmd.lang.metrics.AbstractMetricsComputer; */ public final class JavaMetricsComputer extends AbstractMetricsComputer { - static final JavaMetricsComputer INSTANCE = new JavaMetricsComputer(); + private static final JavaMetricsComputer INSTANCE = new JavaMetricsComputer(); private JavaMetricsComputer() { } + + @InternalApi + public static JavaMetricsComputer getInstance() { + return INSTANCE; + } + + // TODO: doesn't consider lambdas @Override protected List findOperations(ASTAnyTypeDeclaration node) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsFacade.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsFacade.java index 6ddd2e1df9..f239407db7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsFacade.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsFacade.java @@ -33,7 +33,7 @@ class JavaMetricsFacade extends AbstractMetricsFacade getLanguageSpecificComputer() { - return JavaMetricsComputer.INSTANCE; + return JavaMetricsComputer.getInstance(); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java index 2d51621aa1..06ff9ed917 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/api/JavaClassMetricKey.java @@ -96,5 +96,4 @@ public enum JavaClassMetricKey implements MetricKey { return calculator.supports(node); } - } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java index cd3064565d..15352e90d1 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java @@ -205,12 +205,12 @@ public class ParserTstUtil { } - public static LanguageVersionHandler getLanguageVersionHandler(String version) { - return LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion(version).getLanguageVersionHandler(); + public static AbstractJavaHandler getLanguageVersionHandler(String version) { + return (AbstractJavaHandler) LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion(version).getLanguageVersionHandler(); } - public static LanguageVersionHandler getDefaultLanguageVersionHandler() { - return LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getDefaultVersion().getLanguageVersionHandler(); + public static AbstractJavaHandler getDefaultLanguageVersionHandler() { + return (AbstractJavaHandler) LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getDefaultVersion().getLanguageVersionHandler(); } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java new file mode 100644 index 0000000000..7be636b093 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java @@ -0,0 +1,77 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.metrics; + + +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import java.util.Map; + +import org.junit.Test; + +import net.sourceforge.pmd.lang.java.ParserTstUtil; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.MethodLikeNode; +import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey; +import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; +import net.sourceforge.pmd.lang.metrics.MetricKey; + + +/** + * @author Clément Fournier + */ +public class JavaMetricsProviderTest { + + @Test + public void testComputeAllMetrics() { + + LanguageMetricsProvider provider = ParserTstUtil.getLanguageVersionHandler("1.8").getLanguageMetricsProvider(); + + ASTCompilationUnit acu = ParserTstUtil.parseAndTypeResolveJava("1.8", + "class Foo { void bar() { System.out.println(1); } }"); + + ASTAnyTypeDeclaration type = acu.getFirstDescendantOfType(ASTAnyTypeDeclaration.class); + + Map, Double> results = provider.computeAllMetricsFor(type); + + for (JavaClassMetricKey key : JavaClassMetricKey.values()) { + assertTrue(results.containsKey(key)); + } + + MethodLikeNode op = acu.getFirstDescendantOfType(MethodLikeNode.class); + + Map, Double> opResults = provider.computeAllMetricsFor(op); + + for (JavaOperationMetricKey key : JavaOperationMetricKey.values()) { + assertTrue(opResults.containsKey(key)); + } + } + + + @Test + public void testTheresNoMemoisation() { + + LanguageMetricsProvider provider = ParserTstUtil.getLanguageVersionHandler("1.8").getLanguageMetricsProvider(); + + ASTAnyTypeDeclaration tdecl1 = ParserTstUtil.parseAndTypeResolveJava("1.8", + "class Foo { void bar() { System.out.println(1); } }").getFirstDescendantOfType(ASTAnyTypeDeclaration.class); + + Map, Double> reference = provider.computeAllMetricsFor(tdecl1); + + ASTAnyTypeDeclaration tdecl2 = ParserTstUtil.parseAndTypeResolveJava("1.8", + // same name, different characteristics + "class Foo { void bar(){} \npublic void hey() { System.out.println(1); } }").getFirstDescendantOfType(ASTAnyTypeDeclaration.class); + + Map, Double> secondTest = provider.computeAllMetricsFor(tdecl2); + + assertNotEquals(reference, secondTest); + + } + + +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/ProjectMemoizerTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/ProjectMemoizerTest.java index eee8a7152b..b40fa44b5f 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/ProjectMemoizerTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/ProjectMemoizerTest.java @@ -74,7 +74,7 @@ public class ProjectMemoizerTest { @Override public Object visit(ASTMethodOrConstructorDeclaration node, Object data) { MetricMemoizer op = toplevel.getOperationMemoizer(node.getQualifiedName()); - result.add((int) JavaMetricsComputer.INSTANCE.computeForOperation(opMetricKey, node, force, + result.add((int) JavaMetricsComputer.getInstance().computeForOperation(opMetricKey, node, force, MetricOptions.emptyOptions(), op)); return super.visit(node, data); } @@ -83,7 +83,7 @@ public class ProjectMemoizerTest { @Override public Object visit(ASTAnyTypeDeclaration node, Object data) { MetricMemoizer clazz = toplevel.getClassMemoizer(node.getQualifiedName()); - result.add((int) JavaMetricsComputer.INSTANCE.computeForType(classMetricKey, node, force, + result.add((int) JavaMetricsComputer.getInstance().computeForType(classMetricKey, node, force, MetricOptions.emptyOptions(), clazz)); return super.visit(node, data); } 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 7bd7d47fb1..2acbff2c9a 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 @@ -17,7 +17,7 @@ import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; -import net.sourceforge.pmd.lang.metrics.MetricResult; +import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java index 30e02d265c..d590f97ddd 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java @@ -18,8 +18,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; import net.sourceforge.pmd.lang.java.metrics.JavaMetrics; import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey; import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; -import net.sourceforge.pmd.lang.metrics.MetricResult; - /** * Evaluates metrics. diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java new file mode 100644 index 0000000000..34b86828e1 --- /dev/null +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricResult.java @@ -0,0 +1,39 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.util.fxdesigner.model; + +import java.util.AbstractMap.SimpleEntry; +import java.util.Map.Entry; + +import net.sourceforge.pmd.lang.metrics.MetricKey; + +/** + * @author Clément Fournier + * @since 6.0.0 + */ +public class MetricResult { + + private final SimpleEntry, Double> simpleEntry; + + + public MetricResult(MetricKey key, Double value) { + simpleEntry = new SimpleEntry<>(key, value); + } + + + MetricResult(Entry, ? extends Double> entry) { + simpleEntry = new SimpleEntry<>(entry); + } + + + public MetricKey getKey() { + return simpleEntry.getKey(); + } + + + public Double getValue() { + return simpleEntry.getValue(); + } +} diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java index 622cb2adf0..77271ce948 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/MetricResultListCell.java @@ -6,7 +6,7 @@ package net.sourceforge.pmd.util.fxdesigner.util.controls; import java.util.Locale; -import net.sourceforge.pmd.lang.metrics.MetricResult; +import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import javafx.scene.control.ListCell; import javafx.scene.control.ListView; From 0fec3640a30bd92e6a2d2a0f31888365edaa36e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Jan 2019 03:17:00 +0100 Subject: [PATCH 04/44] Implement for Apex --- .../pmd/lang/apex/ApexHandler.java | 40 +++++++++++++++++++ .../apex/metrics/ApexMetricsComputer.java | 9 ++++- .../lang/apex/metrics/ApexMetricsFacade.java | 2 +- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexHandler.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexHandler.java index 6984ff1a36..3a8663871d 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexHandler.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexHandler.java @@ -5,21 +5,34 @@ package net.sourceforge.pmd.lang.apex; import java.io.Writer; +import java.util.Arrays; +import java.util.List; import net.sourceforge.pmd.lang.AbstractLanguageVersionHandler; import net.sourceforge.pmd.lang.Parser; import net.sourceforge.pmd.lang.ParserOptions; import net.sourceforge.pmd.lang.VisitorStarter; import net.sourceforge.pmd.lang.XPathHandler; +import net.sourceforge.pmd.lang.apex.ast.ASTMethod; +import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface; import net.sourceforge.pmd.lang.apex.ast.ApexNode; import net.sourceforge.pmd.lang.apex.ast.DumpFacade; +import net.sourceforge.pmd.lang.apex.metrics.ApexMetricsComputer; +import net.sourceforge.pmd.lang.apex.metrics.api.ApexClassMetricKey; +import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey; import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileVisitorFacade; import net.sourceforge.pmd.lang.apex.rule.ApexRuleViolationFactory; import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; +import net.sourceforge.pmd.lang.metrics.internal.AbstractLanguageMetricsProvider; import net.sourceforge.pmd.lang.rule.RuleViolationFactory; + public class ApexHandler extends AbstractLanguageVersionHandler { + private final ApexMetricsProvider myMetricsProvider = new ApexMetricsProvider(); + + @Override public VisitorStarter getMultifileFacade() { return rootNode -> new ApexMultifileVisitorFacade().initializeWith((ApexNode) rootNode); @@ -51,4 +64,31 @@ public class ApexHandler extends AbstractLanguageVersionHandler { return rootNode -> new DumpFacade().initializeWith(writer, prefix, recurse, (ApexNode) rootNode); } + + @Override + public LanguageMetricsProvider, ASTMethod> getLanguageMetricsProvider() { + return myMetricsProvider; + } + + + private static class ApexMetricsProvider extends AbstractLanguageMetricsProvider, ASTMethod> { + + @SuppressWarnings("unchecked") + ApexMetricsProvider() { + // a wild double cast + super((Class>) (Object) ASTUserClassOrInterface.class, ASTMethod.class, ApexMetricsComputer.getInstance()); + } + + + @Override + public List getAvailableTypeMetrics() { + return Arrays.asList(ApexClassMetricKey.values()); + } + + + @Override + public List getAvailableOperationMetrics() { + return Arrays.asList(ApexOperationMetricKey.values()); + } + } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsComputer.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsComputer.java index d0d82c3d63..63ccc76f74 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsComputer.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsComputer.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.apex.metrics; import java.util.ArrayList; import java.util.List; +import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.apex.ast.ASTMethod; import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface; import net.sourceforge.pmd.lang.metrics.AbstractMetricsComputer; @@ -18,7 +19,13 @@ import net.sourceforge.pmd.lang.metrics.AbstractMetricsComputer; */ public class ApexMetricsComputer extends AbstractMetricsComputer, ASTMethod> { - static final ApexMetricsComputer INSTANCE = new ApexMetricsComputer(); + private static final ApexMetricsComputer INSTANCE = new ApexMetricsComputer(); + + + @InternalApi + public static ApexMetricsComputer getInstance() { + return INSTANCE; + } @Override diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsFacade.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsFacade.java index 0bcd271ab9..4ed4fa23d3 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsFacade.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetricsFacade.java @@ -27,7 +27,7 @@ public class ApexMetricsFacade extends AbstractMetricsFacade, ASTMethod> getLanguageSpecificComputer() { - return ApexMetricsComputer.INSTANCE; + return ApexMetricsComputer.getInstance(); } From 29272cc3c935c38392e571a5d53038fd442f7d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Jan 2019 03:20:37 +0100 Subject: [PATCH 05/44] Plug into the designer --- .../apex/metrics/ApexProjectMirrorTest.java | 4 +- pmd-ui/pom.xml | 8 ++ .../fxdesigner/NodeInfoPanelController.java | 22 ++--- .../fxdesigner/model/MetricEvaluator.java | 92 ------------------- .../util/controls/ScopeHierarchyTreeCell.java | 30 +----- 5 files changed, 22 insertions(+), 134 deletions(-) delete mode 100644 pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/metrics/ApexProjectMirrorTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/metrics/ApexProjectMirrorTest.java index 9c78229005..49d077318b 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/metrics/ApexProjectMirrorTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/metrics/ApexProjectMirrorTest.java @@ -91,7 +91,7 @@ public class ApexProjectMirrorTest { @Override public Object visit(ASTMethod node, Object data) { MetricMemoizer op = toplevel.getOperationMemoizer(node.getQualifiedName()); - result.add((int) ApexMetricsComputer.INSTANCE.computeForOperation(opMetricKey, node, force, + result.add((int) ApexMetricsComputer.getInstance().computeForOperation(opMetricKey, node, force, MetricOptions.emptyOptions(), op)); return super.visit(node, data); } @@ -100,7 +100,7 @@ public class ApexProjectMirrorTest { @Override public Object visit(ASTUserClass node, Object data) { MetricMemoizer> clazz = toplevel.getClassMemoizer(node.getQualifiedName()); - result.add((int) ApexMetricsComputer.INSTANCE.computeForType(classMetricKey, node, force, + result.add((int) ApexMetricsComputer.getInstance().computeForType(classMetricKey, node, force, MetricOptions.emptyOptions(), clazz)); return super.visit(node, data); } diff --git a/pmd-ui/pom.xml b/pmd-ui/pom.xml index 6c097dfd19..b2ed20fc3c 100644 --- a/pmd-ui/pom.xml +++ b/pmd-ui/pom.xml @@ -143,48 +143,56 @@ pmd-apex ${project.version} true + runtime net.sourceforge.pmd pmd-java ${project.version} true + runtime net.sourceforge.pmd pmd-javascript ${project.version} true + runtime net.sourceforge.pmd pmd-jsp ${project.version} true + runtime net.sourceforge.pmd pmd-plsql ${project.version} true + runtime net.sourceforge.pmd pmd-visualforce ${project.version} true + runtime net.sourceforge.pmd pmd-vm ${project.version} true + runtime net.sourceforge.pmd pmd-xml ${project.version} true + runtime junit 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 2acbff2c9a..4270821e5e 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 @@ -7,16 +7,18 @@ package net.sourceforge.pmd.util.fxdesigner; import java.net.URL; import java.util.Collections; import java.util.Iterator; +import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.ResourceBundle; +import java.util.stream.Collectors; import org.reactfx.EventStreams; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; -import net.sourceforge.pmd.lang.java.ast.TypeNode; +import net.sourceforge.pmd.lang.metrics.MetricKey; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; -import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; @@ -58,7 +60,6 @@ public class NodeInfoPanelController implements Initializable { private Label metricsTitleLabel; @FXML private TreeView scopeHierarchyTreeView; - private MetricEvaluator metricEvaluator = new MetricEvaluator(); private Node selectedNode; public NodeInfoPanelController(MainDesignerController mainController) { @@ -125,11 +126,9 @@ public class NodeInfoPanelController implements Initializable { private ObservableList evaluateAllMetrics(Node n) { - try { - return FXCollections.observableArrayList(metricEvaluator.evaluateAllMetrics(n)); - } catch (UnsupportedOperationException e) { - return FXCollections.emptyObservableList(); - } + Map, Double> results = parent.getLanguageVersion().getLanguageVersionHandler().getLanguageMetricsProvider().computeAllMetricsFor(n); + List resultList = results.entrySet().stream().map(e -> new MetricResult(e.getKey(), e.getValue())).collect(Collectors.toList()); + return FXCollections.observableArrayList(resultList); } @@ -146,9 +145,10 @@ public class NodeInfoPanelController implements Initializable { + ((attribute.getValue() != null) ? attribute.getStringValue() : "null")); } - if (node instanceof TypeNode) { - result.add("typeIs() = " + ((TypeNode) node).getType()); - } + // TODO maybe put some equivalent to TypeNode inside pmd-core + // if (node instanceof TypeNode) { + // result.add("typeIs() = " + ((TypeNode) node).getType()); + // } Collections.sort(result); return result; } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java deleted file mode 100644 index d590f97ddd..0000000000 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/model/MetricEvaluator.java +++ /dev/null @@ -1,92 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.util.fxdesigner.model; - -import java.util.ArrayList; -import java.util.List; - -import net.sourceforge.pmd.lang.apex.ast.ASTMethod; -import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; -import net.sourceforge.pmd.lang.apex.metrics.ApexMetrics; -import net.sourceforge.pmd.lang.apex.metrics.api.ApexClassMetricKey; -import net.sourceforge.pmd.lang.apex.metrics.api.ApexOperationMetricKey; -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; -import net.sourceforge.pmd.lang.java.metrics.JavaMetrics; -import net.sourceforge.pmd.lang.java.metrics.api.JavaClassMetricKey; -import net.sourceforge.pmd.lang.java.metrics.api.JavaOperationMetricKey; - -/** - * Evaluates metrics. - * - * @author Clément Fournier - * @since 6.0.0 - */ -public class MetricEvaluator { - - /** - * Evaluates all available metrics and returns a list of results. - * - * @param node Node - * - * @return List of all metric results (metric key + result), including NaN results - * - * @throws UnsupportedOperationException If no metrics are available for this node - */ - public List evaluateAllMetrics(Node node) throws UnsupportedOperationException { - if (ASTAnyTypeDeclaration.class.isInstance(node)) { - return evaluateAllMetrics((ASTAnyTypeDeclaration) node); - } else if (ASTMethodOrConstructorDeclaration.class.isInstance(node)) { - return evaluateAllMetrics((ASTMethodOrConstructorDeclaration) node); - } else if (ASTMethod.class.isInstance(node)) { - return evaluateAllMetrics((ASTMethod) node); - } else if (ASTUserClass.class.isInstance(node)) { - return evaluateAllMetrics((ASTUserClass) node); - } - throw new UnsupportedOperationException("That language does not support metrics"); - } - - - private List evaluateAllMetrics(ASTMethodOrConstructorDeclaration node) { - List metricResults = new ArrayList<>(); - for (JavaOperationMetricKey key : JavaOperationMetricKey.values()) { - metricResults.add(new MetricResult(key, JavaMetrics.get(key, node))); - } - - return metricResults; - } - - - private List evaluateAllMetrics(ASTAnyTypeDeclaration node) { - List metricResults = new ArrayList<>(); - for (JavaClassMetricKey key : JavaClassMetricKey.values()) { - metricResults.add(new MetricResult(key, JavaMetrics.get(key, node))); - } - - return metricResults; - } - - - private List evaluateAllMetrics(ASTMethod node) { - List metricResults = new ArrayList<>(); - for (ApexOperationMetricKey key : ApexOperationMetricKey.values()) { - metricResults.add(new MetricResult(key, ApexMetrics.get(key, node))); - } - - return metricResults; - } - - - private List evaluateAllMetrics(ASTUserClass node) { - List metricResults = new ArrayList<>(); - for (ApexClassMetricKey key : ApexClassMetricKey.values()) { - metricResults.add(new MetricResult(key, ApexMetrics.get(key, node))); - } - - return metricResults; - } - -} diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeCell.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeCell.java index 49958bd6e9..be51cc7c41 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeCell.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeCell.java @@ -4,10 +4,6 @@ package net.sourceforge.pmd.util.fxdesigner.util.controls; -import net.sourceforge.pmd.lang.java.ast.TypeNode; -import net.sourceforge.pmd.lang.java.symboltable.ClassNameDeclaration; -import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration; -import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.lang.symboltable.Scope; @@ -42,31 +38,7 @@ public class ScopeHierarchyTreeCell extends TreeCell { private String getTextForDeclaration(NameDeclaration declaration) { - - StringBuilder sb = new StringBuilder(); - if (declaration instanceof MethodNameDeclaration - || declaration instanceof net.sourceforge.pmd.lang.plsql.symboltable.MethodNameDeclaration) { - sb.append("Method "); - } else if (declaration instanceof VariableNameDeclaration - || declaration instanceof net.sourceforge.pmd.lang.plsql.symboltable.VariableNameDeclaration) { - sb.append("Variable "); - } else if (declaration instanceof ClassNameDeclaration - || declaration instanceof net.sourceforge.pmd.lang.plsql.symboltable.ClassNameDeclaration) { - sb.append("Class "); - } - - Class type = declaration.getNode() instanceof TypeNode ? ((TypeNode) declaration.getNode()).getType() - : null; - - sb.append(declaration.getName()); - - if (type != null) { - sb.append(" : ").append(type.getSimpleName()); - } - - sb.append(" (l. ").append(declaration.getNode().getBeginLine()).append(")"); - - return sb.toString(); + return declaration.toString(); // that's nice enough for now } } From 66f60e23502c658cd299a3dc1837ba7a00e4e013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Jan 2019 03:40:10 +0100 Subject: [PATCH 06/44] Update release notes, fixes 1496 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b6f74252f8..7e6849e2e7 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -37,6 +37,8 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues +* core + * [#1496](https://github.com/pmd/pmd/issues/1496) \[core] Refactor metrics to be dealt with generically from pmd-core * apex * [#1542](https://github.com/pmd/pmd/pull/1542): \[apex] Include the documentation category * java-bestpractices From fe961010557b44c36140fb070bdcd47cd2efb3b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Jan 2019 17:23:27 +0100 Subject: [PATCH 07/44] Handle null provider --- .../pmd/lang/metrics/LanguageMetricsProvider.java | 7 +++++-- .../internal/AbstractLanguageMetricsProvider.java | 2 -- .../java/metrics/JavaMetricsProviderTest.java | 2 +- .../util/fxdesigner/NodeInfoPanelController.java | 15 +++++++++++---- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java index e2d34c47f4..46edd263c2 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/LanguageMetricsProvider.java @@ -8,13 +8,16 @@ import java.util.List; import java.util.Map; import net.sourceforge.pmd.annotation.Experimental; +import net.sourceforge.pmd.lang.LanguageVersionHandler; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.QualifiableNode; /** - * Language-independent provider for metrics. Can be used to know applicable metrics - * on a node, e.g. to build GUI applications like the designer. + * Language-specific provider for metrics. Knows about all the metrics + * defined for a language. Can be used e.g. to build GUI applications + * like the designer, in a language independent way. Accessible through + * {@link LanguageVersionHandler#getLanguageMetricsProvider()}. * * Note: this is experimental, ie unstable until 7.0.0, after which it will probably * be promoted to a real API. diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java index 9de2ca3b88..8e3fa8dc28 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/AbstractLanguageMetricsProvider.java @@ -49,8 +49,6 @@ public abstract class AbstractLanguageMetricsProvider key, T node, MetricOptions options) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java index 7be636b093..919253837e 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/JavaMetricsProviderTest.java @@ -54,7 +54,7 @@ public class JavaMetricsProviderTest { @Test - public void testTheresNoMemoisation() { + public void testThereIsNoMemoisation() { LanguageMetricsProvider provider = ParserTstUtil.getLanguageVersionHandler("1.8").getLanguageMetricsProvider(); 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 4270821e5e..d6a8426c46 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 @@ -8,7 +8,6 @@ import java.net.URL; import java.util.Collections; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.ResourceBundle; import java.util.stream.Collectors; @@ -17,7 +16,7 @@ import org.reactfx.EventStreams; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; -import net.sourceforge.pmd.lang.metrics.MetricKey; +import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; @@ -126,8 +125,16 @@ public class NodeInfoPanelController implements Initializable { private ObservableList evaluateAllMetrics(Node n) { - Map, Double> results = parent.getLanguageVersion().getLanguageVersionHandler().getLanguageMetricsProvider().computeAllMetricsFor(n); - List resultList = results.entrySet().stream().map(e -> new MetricResult(e.getKey(), e.getValue())).collect(Collectors.toList()); + LanguageMetricsProvider provider = parent.getLanguageVersion().getLanguageVersionHandler().getLanguageMetricsProvider(); + if (provider == null) { + return FXCollections.emptyObservableList(); + } + List resultList = + provider.computeAllMetricsFor(n) + .entrySet() + .stream() + .map(e -> new MetricResult(e.getKey(), e.getValue())) + .collect(Collectors.toList()); return FXCollections.observableArrayList(resultList); } From efe5d44adf7fd776f7c513a941f84e3ff2b91b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 7 Jan 2019 02:50:49 +0100 Subject: [PATCH 08/44] Checkstyle --- .../pmd/lang/metrics/internal/DummyMetricMemoizer.java | 4 ++-- .../pmd/lang/metrics/internal/DummyProjectMemoizer.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java index 295d0c69d4..f9b9003148 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyMetricMemoizer.java @@ -15,7 +15,7 @@ import net.sourceforge.pmd.lang.metrics.ParameterizedMetricKey; * @author Clément Fournier * @since 7.0.0 */ -public class DummyMetricMemoizer implements MetricMemoizer { +public final class DummyMetricMemoizer implements MetricMemoizer { private static final DummyMetricMemoizer INSTANCE = new DummyMetricMemoizer<>(); @@ -33,7 +33,7 @@ public class DummyMetricMemoizer implements MetricMemoizer { @Override public void memoize(ParameterizedMetricKey key, double value) { - + // do nothing } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java index 5c0ddd6d02..d3775a7584 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/metrics/internal/DummyProjectMemoizer.java @@ -16,7 +16,7 @@ import net.sourceforge.pmd.lang.metrics.ProjectMemoizer; * @author Clément Fournier * @since 7.0.0 */ -public class DummyProjectMemoizer implements ProjectMemoizer { +public final class DummyProjectMemoizer implements ProjectMemoizer { private static final DummyProjectMemoizer INSTANCE = new DummyProjectMemoizer<>(); From 9673d51fbf2cc29cbeef3e36fd58573596cf76cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 9 Jan 2019 17:50:03 +0100 Subject: [PATCH 09/44] Add the attributes of the underlying node to Apex xpath nodes --- .../lang/apex/ast/ASTBooleanExpression.java | 1 + .../pmd/lang/apex/ast/AbstractApexNode.java | 23 ++++++++++ .../pmd/lang/ast/xpath/Attribute.java | 44 +++++++++++++++++-- .../lang/ast/xpath/AttributeAxisIterator.java | 24 +++++++--- .../lang/rule/xpath/SaxonXPathRuleQuery.java | 4 +- .../properties/AbstractPropertySource.java | 1 + .../pmd/properties/PropertySource.java | 3 ++ 7 files changed, 88 insertions(+), 12 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java index f2e44dbc4a..f73b5e8214 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java @@ -10,6 +10,7 @@ public class ASTBooleanExpression extends AbstractApexNode { public ASTBooleanExpression(BooleanExpression booleanExpression) { super(booleanExpression); + booleanExpression.getOp(); } @Override diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java index a41d152456..8a3b319d9a 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java @@ -4,7 +4,14 @@ package net.sourceforge.pmd.lang.apex.ast; +import java.util.Iterator; +import java.util.Spliterators; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + import net.sourceforge.pmd.lang.ast.SourceCodePositioner; +import net.sourceforge.pmd.lang.ast.xpath.Attribute; +import net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator; import apex.jorje.data.Location; import apex.jorje.data.Locations; @@ -57,4 +64,20 @@ public abstract class AbstractApexNode extends AbstractApexNo return "no location"; } } + + + @Override + public Iterator getXPathAttributesIterator() { + // Attributes of this node have precedence over same-name attributes of the underlying node + return Stream.concat(iteratorToStream(new AttributeAxisIterator(this)), + iteratorToStream(new AttributeAxisIterator(this, node))) + .distinct() + .iterator(); + + } + + + private static Stream iteratorToStream(Iterator it) { + return StreamSupport.stream(Spliterators.spliteratorUnknownSize(it, 0), false); + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java index 6f57f00dab..9657e6b033 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java @@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.ast.xpath; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; @@ -18,6 +19,9 @@ import net.sourceforge.pmd.lang.ast.Node; * Attributes know their name, the node they wrap, * and have access to their value. * + * Two attributes are equal iff their parent node is + * equal and they have the same name. + * * @author daniels */ public class Attribute { @@ -28,8 +32,9 @@ public class Attribute { private static final Object[] EMPTY_OBJ_ARRAY = new Object[0]; - private Node parent; - private String name; + private final Node parent; + private final Object myBean; + private final String name; private Method method; private Object value; private String stringValue; @@ -39,15 +44,25 @@ public class Attribute { this.parent = parent; this.name = name; this.method = m; + this.myBean = parent; } + /** Creates a new attribute belonging to the given node using its accessor. */ + public Attribute(Node parent, Object bean, String name, Method m) { + this.parent = parent; + this.name = name; + this.method = m; + this.myBean = bean; + } + /** Creates a new attribute belonging to the given node using its string value. */ public Attribute(Node parent, String name, String value) { this.parent = parent; this.name = name; this.value = value; this.stringValue = value; + this.myBean = parent; } @@ -73,7 +88,7 @@ public class Attribute { // this lazy loading reduces calls to Method.invoke() by about 90% try { - value = method.invoke(parent, EMPTY_OBJ_ARRAY); + value = method.invoke(myBean, EMPTY_OBJ_ARRAY); return value; } catch (IllegalAccessException | InvocationTargetException iae) { iae.printStackTrace(); @@ -93,12 +108,33 @@ public class Attribute { return stringValue; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Attribute attribute = (Attribute) o; + return Objects.equals(parent, attribute.parent) && + Objects.equals(name, attribute.name); + } + + + @Override + public int hashCode() { + return Objects.hash(parent, name); + } + + private String getLoggableAttributeName() { return parent.getXPathNodeName() + "/@" + name; } @Override public String toString() { - return name + ':' + getValue() + ':' + parent; + return name + ':' + getValue() + ':' + parent.getXPathNodeName(); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java index 75807ecd71..cdaccb305b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java @@ -10,6 +10,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -44,6 +45,7 @@ public class AttributeAxisIterator implements Iterator { private MethodWrapper[] methodWrappers; private int position; private Node node; + private final Object bean; /** @@ -52,24 +54,28 @@ public class AttributeAxisIterator implements Iterator { * use instead the overridable {@link Node#getXPathAttributesIterator()}. */ public AttributeAxisIterator(Node contextNode) { + this(contextNode, contextNode); + } + + public AttributeAxisIterator(Node contextNode, Object bean) { this.node = contextNode; - if (!METHOD_CACHE.containsKey(contextNode.getClass())) { - Method[] preFilter = contextNode.getClass().getMethods(); + this.bean = bean; + if (!METHOD_CACHE.containsKey(bean.getClass())) { + Method[] preFilter = bean.getClass().getMethods(); List postFilter = new ArrayList<>(); for (Method element : preFilter) { if (isAttributeAccessor(element)) { postFilter.add(new MethodWrapper(element)); } } - METHOD_CACHE.putIfAbsent(contextNode.getClass(), postFilter.toArray(new MethodWrapper[0])); + METHOD_CACHE.putIfAbsent(bean.getClass(), postFilter.toArray(new MethodWrapper[0])); } - this.methodWrappers = METHOD_CACHE.get(contextNode.getClass()); + this.methodWrappers = METHOD_CACHE.get(bean.getClass()); this.position = 0; this.currObj = getNextAttribute(); } - /** * Returns whether the given method is an attribute accessor, * in which case a corresponding Attribute will be added to @@ -80,12 +86,16 @@ public class AttributeAxisIterator implements Iterator { protected boolean isAttributeAccessor(Method method) { String methodName = method.getName(); - return CONSIDERED_RETURN_TYPES.contains(method.getReturnType()) + return isConsideredReturnType(method.getReturnType()) && method.getParameterTypes().length == 0 && !methodName.startsWith("jjt") && !FILTERED_OUT_NAMES.contains(methodName); } + private boolean isConsideredReturnType(Class klass) { + return CONSIDERED_RETURN_TYPES.contains(klass) || klass.isEnum(); + } + @Override public Attribute next() { @@ -115,7 +125,7 @@ public class AttributeAxisIterator implements Iterator { return null; } MethodWrapper m = methodWrappers[position++]; - return new Attribute(node, m.name, m.method); + return new Attribute(node, bean, m.name, m.method); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index ddbe17615b..b369007c68 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -236,7 +236,9 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { */ if (value == null) { return UntypedAtomicValue.ZERO_LENGTH_UNTYPED; - + } else if (value instanceof Enum) { + // enums use their toString + return new StringValue(value.toString()); } else if (value instanceof String) { return new StringValue((String) value); } else if (value instanceof Boolean) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/AbstractPropertySource.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/AbstractPropertySource.java index 386837f0a9..71463940cf 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/AbstractPropertySource.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/AbstractPropertySource.java @@ -164,6 +164,7 @@ public abstract class AbstractPropertySource implements PropertySource { @Override + @Deprecated public void setProperty(MultiValuePropertyDescriptor propertyDescriptor, V... values) { checkValidPropertyDescriptor(propertyDescriptor); propertyValuesByDescriptor.put(propertyDescriptor, Collections.unmodifiableList(Arrays.asList(values))); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertySource.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertySource.java index 7b24644164..7a516714bd 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertySource.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertySource.java @@ -112,7 +112,10 @@ public interface PropertySource { * @param propertyDescriptor The property descriptor for which to add a value * @param values Values * @param The type of the values + * + * @deprecated {@link MultiValuePropertyDescriptor} is deprecated */ + @Deprecated void setProperty(MultiValuePropertyDescriptor propertyDescriptor, V... values); From 2d0081a73af2667e6e543a673d71b4c3f7157e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 9 Jan 2019 18:07:33 +0100 Subject: [PATCH 10/44] Add tests --- .../pmd/lang/apex/ast/AbstractApexNode.java | 2 +- .../pmd/lang/apex/rule/ApexXpathRuleTest.java | 50 +++++++++++++++++++ .../pmd/internal/util/IteratorUtil.java | 49 ++++++++++++++++++ .../pmd/lang/ast/xpath/Attribute.java | 13 +++-- .../lang/ast/xpath/AttributeAxisIterator.java | 9 +++- .../ast/xpath/AttributeAxisIteratorTest.java | 29 +++++++++++ .../fxdesigner/SourceEditorController.java | 4 +- ...torUtil.java => DesignerIteratorUtil.java} | 23 ++------- .../fxdesigner/util/controls/ASTTreeItem.java | 4 +- 9 files changed, 152 insertions(+), 31 deletions(-) create mode 100644 pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/internal/util/IteratorUtil.java rename pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/{IteratorUtil.java => DesignerIteratorUtil.java} (81%) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java index 8a3b319d9a..ca71628b6e 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java @@ -76,7 +76,7 @@ public abstract class AbstractApexNode extends AbstractApexNo } - + // TODO move to IteratorUtil w/ java 8 private static Stream iteratorToStream(Iterator it) { return StreamSupport.stream(Spliterators.spliteratorUnknownSize(it, 0), false); } diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java new file mode 100644 index 0000000000..2b91d3158c --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java @@ -0,0 +1,50 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule; + +import static org.junit.Assert.assertTrue; + +import java.util.List; + +import org.junit.Test; + +import net.sourceforge.pmd.internal.util.IteratorUtil; +import net.sourceforge.pmd.lang.apex.ast.ASTBooleanExpression; +import net.sourceforge.pmd.lang.apex.ast.ApexNode; +import net.sourceforge.pmd.lang.apex.ast.ApexParserTestHelpers; +import net.sourceforge.pmd.lang.ast.xpath.Attribute; + +import apex.jorje.semantic.ast.compilation.Compilation; + + +/** + * @author Clément Fournier + * @since 7.0.0 + */ +public class ApexXpathRuleTest { + + @Test + public void testXPathAttributesOfUnderlyingNode() { + + String code = "class MyApexClass {\n" + + " void bar(){\n" + + "\tb f = a && b;\n" + + " if(!x.lit() && lis != null) {\n" + + " foo();\n" + + " }\n" + + " }\n" + + "}"; + + ApexNode compilation = ApexParserTestHelpers.parse(code); + + compilation.findDescendantsOfType(ASTBooleanExpression.class).forEach(expr -> { + List attributes = IteratorUtil.toList(expr.getXPathAttributesIterator()); + + assertTrue(attributes.stream().anyMatch(a -> a.getName().equals("Op"))); + }); + } + + +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/IteratorUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/IteratorUtil.java new file mode 100644 index 0000000000..e150ec0632 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/internal/util/IteratorUtil.java @@ -0,0 +1,49 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.internal.util; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + + +/** + * @author Clément Fournier + * @since 7.0.0 + */ +public final class IteratorUtil { + + private IteratorUtil() { + + } + + + public static Iterator reverse(Iterator it) { + List tmp = toList(it); + Collections.reverse(tmp); + return tmp.iterator(); + } + + + public static List toList(Iterator it) { + List list = new ArrayList<>(); + while (it.hasNext()) { + list.add(it.next()); + } + return list; + } + + + public static Iterable toIterable(final Iterator it) { + return new Iterable() { + @Override + public Iterator iterator() { + return it; + } + }; + } + +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java index 9657e6b033..a47c92a685 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java @@ -19,8 +19,8 @@ import net.sourceforge.pmd.lang.ast.Node; * Attributes know their name, the node they wrap, * and have access to their value. * - * Two attributes are equal iff their parent node is - * equal and they have the same name. + * Two attributes are equal iff they have the same name + * and their parent nodes are equal. * * @author daniels */ @@ -48,7 +48,10 @@ public class Attribute { } - /** Creates a new attribute belonging to the given node using its accessor. */ + /** + * Creates a new attribute belonging to the given node using an accessor + * for the given bean. + */ public Attribute(Node parent, Object bean, String name, Method m) { this.parent = parent; this.name = name; @@ -118,8 +121,8 @@ public class Attribute { return false; } Attribute attribute = (Attribute) o; - return Objects.equals(parent, attribute.parent) && - Objects.equals(name, attribute.name); + return Objects.equals(parent, attribute.parent) + && Objects.equals(name, attribute.name); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java index cdaccb305b..d1a347df94 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIterator.java @@ -10,7 +10,6 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -57,6 +56,14 @@ public class AttributeAxisIterator implements Iterator { this(contextNode, contextNode); } + + /** + * Creates a new iterator creating attributes belonging to the + * context node, but taken from the given bean. + * + * @param contextNode Owner of the attributes (from the point of view of XPath) + * @param bean Source of the attributes + */ public AttributeAxisIterator(Node contextNode, Object bean) { this.node = contextNode; this.bean = bean; diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java index e8326ca32d..a003c0be42 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java @@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.ast.xpath; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.util.HashMap; @@ -52,6 +53,18 @@ public class AttributeAxisIteratorTest { assertTrue(atts.containsKey("EndLine")); } + @Test + public void testAttributeAxisIteratorOnBean() { + DummyNode dummyNode = new DummyNode(1); + + AttributeAxisIterator it = new AttributeAxisIterator(dummyNode, new MyBean()); + Map atts = toMap(it); + Assert.assertEquals(2, atts.size()); + assertTrue(atts.containsKey("Enum")); + assertEquals(MyEnum.FOO, atts.get("Enum").getValue()); + assertTrue(atts.containsKey("XPath")); + } + private Map toMap(AttributeAxisIterator it) { Map atts = new HashMap<>(); @@ -61,4 +74,20 @@ public class AttributeAxisIteratorTest { } return atts; } + + private static class MyBean { + + + public String getXPath() { + return ""; + } + + public MyEnum getEnum() { + return MyEnum.FOO; + } + } + + private enum MyEnum { + FOO, BAR + } } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java index 0e7f6b60f2..92705a8206 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java @@ -6,8 +6,8 @@ package net.sourceforge.pmd.util.fxdesigner; import static java.util.Collections.emptyList; import static java.util.Collections.singleton; -import static net.sourceforge.pmd.util.fxdesigner.util.IteratorUtil.parentIterator; -import static net.sourceforge.pmd.util.fxdesigner.util.IteratorUtil.toIterable; +import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.parentIterator; +import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.toIterable; import java.io.File; import java.io.IOException; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java similarity index 81% rename from pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java rename to pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java index f8b86a8584..a671d049b3 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java @@ -20,29 +20,12 @@ import javafx.scene.control.TreeItem; * @author Clément Fournier * @since 6.11.0 */ -public final class IteratorUtil { +public final class DesignerIteratorUtil { - private IteratorUtil() { + // TODO move that into PMD core I can't stand it anymore - } + private DesignerIteratorUtil() { - public static Iterator reverse(Iterator it) { - List tmp = toList(it); - Collections.reverse(tmp); - return tmp.iterator(); - } - - public static List toList(Iterator it) { - List list = new ArrayList<>(); - while (it.hasNext()) { - list.add(it.next()); - } - return list; - } - - - public static Iterable toIterable(Iterator it) { - return () -> it; } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java index 6e6badd44e..5d174ae484 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java @@ -4,8 +4,8 @@ package net.sourceforge.pmd.util.fxdesigner.util.controls; -import static net.sourceforge.pmd.util.fxdesigner.util.IteratorUtil.parentIterator; -import static net.sourceforge.pmd.util.fxdesigner.util.IteratorUtil.reverse; +import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.parentIterator; +import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.reverse; import java.util.Arrays; import java.util.Collections; From d823a5db4ad1dcb980052e067aa4c66870df6860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 9 Jan 2019 18:26:11 +0100 Subject: [PATCH 11/44] Update ASTBooleanExpression.java --- .../net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java | 1 - 1 file changed, 1 deletion(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java index f73b5e8214..f2e44dbc4a 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTBooleanExpression.java @@ -10,7 +10,6 @@ public class ASTBooleanExpression extends AbstractApexNode { public ASTBooleanExpression(BooleanExpression booleanExpression) { super(booleanExpression); - booleanExpression.getOp(); } @Override From bfcc82918939d3d995c57df55e33325f3c436e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 9 Jan 2019 18:32:24 +0100 Subject: [PATCH 12/44] Update release notes, refs #1571 --- docs/pages/release_notes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3e111a7e7d..c1f9d02dc6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -39,6 +39,7 @@ This is a {{ site.pmd.release_type }} release. * apex * [#1542](https://github.com/pmd/pmd/pull/1542): \[apex] Include the documentation category + * [#1568](https://github.com/pmd/pmd/pull/1568): \[apex] AST node attribute @Image not usable / always null in XPath rule / Designer * java * [#1556](https://github.com/pmd/pmd/issues/1556): \[java] Default methods should not be considered abstract * java-bestpractices @@ -58,6 +59,10 @@ This is a {{ site.pmd.release_type }} release. ### API Changes + +* {% jdoc core::properties.PropertySource#setProperty(core::properties.MultiValuePropertyDescriptor, Object[]) %} is deprecated, +because {% jdoc core::properties.MultiValuePropertyDescriptor %} is deprecated as well + ### External Contributions * [#1503](https://github.com/pmd/pmd/pull/1503): \[java] Fix for ReturnFromFinallyBlock false-positives - [RishabhDeep Singh](https://github.com/rishabhdeepsingh) From ba8f318ef9405a2e90cef78cd6e349da68f06f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 9 Jan 2019 20:04:27 +0100 Subject: [PATCH 13/44] Fix designer imports --- .../pmd/util/fxdesigner/SourceEditorController.java | 2 +- .../pmd/util/fxdesigner/util/DesignerIteratorUtil.java | 3 --- .../pmd/util/fxdesigner/util/controls/ASTTreeItem.java | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java index 92705a8206..e83b3c08ba 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java @@ -6,8 +6,8 @@ package net.sourceforge.pmd.util.fxdesigner; import static java.util.Collections.emptyList; import static java.util.Collections.singleton; +import static net.sourceforge.pmd.internal.util.IteratorUtil.toIterable; import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.parentIterator; -import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.toIterable; import java.io.File; import java.io.IOException; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java index a671d049b3..b2beefe1cf 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/DesignerIteratorUtil.java @@ -4,10 +4,7 @@ package net.sourceforge.pmd.util.fxdesigner.util; -import java.util.ArrayList; -import java.util.Collections; import java.util.Iterator; -import java.util.List; import java.util.function.Function; import java.util.function.Predicate; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java index 5d174ae484..cfdbd81140 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ASTTreeItem.java @@ -4,8 +4,8 @@ package net.sourceforge.pmd.util.fxdesigner.util.controls; +import static net.sourceforge.pmd.internal.util.IteratorUtil.reverse; import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.parentIterator; -import static net.sourceforge.pmd.util.fxdesigner.util.DesignerIteratorUtil.reverse; import java.util.Arrays; import java.util.Collections; From dd061f2855c74e864fb429b27cd4ac60dea78d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 11 Jan 2019 02:05:08 +0100 Subject: [PATCH 14/44] Filter out some attributes --- .../fxdesigner/MainDesignerController.java | 4 +- .../fxdesigner/NodeInfoPanelController.java | 73 +++++++++++++++++-- .../pmd/util/fxdesigner/fxml/node-info.fxml | 44 ++++++----- 3 files changed, 95 insertions(+), 26 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index 823f93a3ba..f0d70a83b3 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -226,7 +226,7 @@ public class MainDesignerController extends AbstractController { * Executed when the user selects a node in a treeView or listView. */ public void onNodeItemSelected(Node selectedValue) { - nodeInfoPanelController.displayInfo(selectedValue); + nodeInfoPanelController.setFocusNode(selectedValue); sourceEditorController.setFocusNode(selectedValue); sourceEditorController.focusNodeInTreeView(selectedValue); } @@ -371,7 +371,7 @@ public class MainDesignerController extends AbstractController { public void invalidateAst() { - nodeInfoPanelController.invalidateInfo(); + nodeInfoPanelController.setFocusNode(null); xpathPanelController.invalidateResults(false); sourceEditorController.setFocusNode(null); } 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 677231b47e..7ddf5a4c9c 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 @@ -4,11 +4,14 @@ package net.sourceforge.pmd.util.fxdesigner; +import java.util.Arrays; import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Objects; import org.reactfx.EventStreams; +import org.reactfx.value.Var; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; @@ -17,12 +20,14 @@ import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; +import net.sourceforge.pmd.util.fxdesigner.util.beans.SettingsPersistenceUtil.PersistentProperty; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.fxml.FXML; +import javafx.scene.control.CheckMenuItem; import javafx.scene.control.Label; import javafx.scene.control.ListView; import javafx.scene.control.Tab; @@ -42,6 +47,12 @@ public class NodeInfoPanelController extends AbstractController { private final MainDesignerController parent; + /** List of attribute names that are ignored if {@link #isShowAllAttributes()} is false. */ + private static final List IGNORABLE_ATTRIBUTES = + Arrays.asList("BeginLine", "EndLine", "BeginColumn", "EndColumn", "FindBoundary", "SingleLine"); + + @FXML + private CheckMenuItem showAllAttributesMenuItem; @FXML private TabPane nodeInfoTabPane; @FXML @@ -65,6 +76,9 @@ public class NodeInfoPanelController extends AbstractController { @Override protected void beforeParentInit() { + + xpathAttributesListView.setPlaceholder(new Label("No available attributes")); + EventStreams.valuesOf(scopeHierarchyTreeView.getSelectionModel().selectedItemProperty()) .filter(Objects::nonNull) .map(TreeItem::getValue) @@ -72,38 +86,60 @@ public class NodeInfoPanelController extends AbstractController { .subscribe(parent::onNameDeclarationSelected); scopeHierarchyTreeView.setCellFactory(view -> new ScopeHierarchyTreeCell()); + + showAllAttributesProperty() + .values() + .distinct() + .subscribe(show -> displayAttributes(selectedNode)); + } /** - * Displays info about a node. + * Displays info about a node. If null, the panels are reset. * * @param node Node to inspect */ - public void displayInfo(Node node) { - Objects.requireNonNull(node, "Node cannot be null"); + public void setFocusNode(Node node) { + if (node == null) { + invalidateInfo(); + return; + } if (node.equals(selectedNode)) { return; } + selectedNode = node; + displayAttributes(node); + displayMetrics(node); + displayScopes(node); + } + + + private void displayAttributes(Node node) { ObservableList atts = getAttributes(node); xpathAttributesListView.setItems(atts); + } + + private void displayMetrics(Node node) { ObservableList metrics = evaluateAllMetrics(node); metricResultsListView.setItems(metrics); notifyMetricsAvailable(metrics.stream() .map(MetricResult::getValue) .filter(result -> !result.isNaN()) .count()); + } + + private void displayScopes(Node node) { // TODO maybe a better way would be to build all the scope TreeItem hierarchy once // and only expand the ascendants of the node. TreeItem rootScope = ScopeHierarchyTreeItem.buildAscendantHierarchy(node); scopeHierarchyTreeView.setRoot(rootScope); } - /** * Invalidates the info being displayed. */ @@ -130,17 +166,40 @@ public class NodeInfoPanelController extends AbstractController { } + @PersistentProperty + public boolean isShowAllAttributes() { + return showAllAttributesMenuItem.isSelected(); + } + + + public void setShowAllAttributes(boolean bool) { + showAllAttributesMenuItem.setSelected(bool); + } + + + public Var showAllAttributesProperty() { + return Var.fromVal(showAllAttributesMenuItem.selectedProperty(), showAllAttributesMenuItem::setSelected); + } + + /** * Gets the XPath attributes of the node for display within a listview. */ - private static ObservableList getAttributes(Node node) { + private ObservableList getAttributes(Node node) { + if (node == null) { + return FXCollections.emptyObservableList(); + } + ObservableList result = FXCollections.observableArrayList(); Iterator attributeAxisIterator = node.getXPathAttributesIterator(); while (attributeAxisIterator.hasNext()) { Attribute attribute = attributeAxisIterator.next(); - // TODO the display should be handled in a ListCell - result.add(attribute.getName() + " = " + + if (isShowAllAttributes() || !IGNORABLE_ATTRIBUTES.contains(attribute.getName())) { + // TODO the display should be handled in a ListCell + result.add(attribute.getName() + " = " + ((attribute.getValue() != null) ? attribute.getStringValue() : "null")); + } } if (node instanceof TypeNode) { 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 2561fb159b..e9075d601c 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 @@ -2,6 +2,8 @@ + + @@ -11,9 +13,11 @@ - - - + + + + + @@ -21,22 +25,28 @@ - - - + + + + + + - + - - -
- -
- + + + + + + From df88d16e93ae9750389472d3d7099608552626ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 11 Jan 2019 02:11:03 +0100 Subject: [PATCH 15/44] Replace old toolbars with shining titled panes --- .../fxdesigner/NodeInfoPanelController.java | 7 +- .../pmd/util/fxdesigner/fxml/node-info.fxml | 65 ++++++++----------- 2 files changed, 32 insertions(+), 40 deletions(-) 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 7ddf5a4c9c..b02e214c9c 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 @@ -23,6 +23,7 @@ import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; import net.sourceforge.pmd.util.fxdesigner.util.beans.SettingsPersistenceUtil.PersistentProperty; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; +import net.sourceforge.pmd.util.fxdesigner.util.controls.ToolbarTitledPane; import javafx.collections.FXCollections; import javafx.collections.ObservableList; @@ -51,6 +52,8 @@ public class NodeInfoPanelController extends AbstractController { private static final List IGNORABLE_ATTRIBUTES = Arrays.asList("BeginLine", "EndLine", "BeginColumn", "EndColumn", "FindBoundary", "SingleLine"); + @FXML + private ToolbarTitledPane metricsTitledPane; @FXML private CheckMenuItem showAllAttributesMenuItem; @FXML @@ -64,8 +67,6 @@ public class NodeInfoPanelController extends AbstractController { @FXML private ListView metricResultsListView; @FXML - private Label metricsTitleLabel; - @FXML private TreeView scopeHierarchyTreeView; private MetricEvaluator metricEvaluator = new MetricEvaluator(); private Node selectedNode; @@ -152,7 +153,7 @@ public class NodeInfoPanelController extends AbstractController { private void notifyMetricsAvailable(long numMetrics) { metricResultsTab.setText("Metrics\t(" + (numMetrics == 0 ? "none" : numMetrics) + ")"); - metricsTitleLabel.setText("Metrics\t(" + (numMetrics == 0 ? "none" : numMetrics) + " available)"); + metricsTitledPane.setTitle("Metrics\t(" + (numMetrics == 0 ? "none" : numMetrics) + " available)"); metricResultsTab.setDisable(numMetrics == 0); } 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 e9075d601c..f56bc53bdd 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 @@ -2,22 +2,17 @@ + - + + - - - - - - - @@ -55,22 +50,21 @@ - -
- - - - - -
- - - - - - - + + + + + + + @@ -79,19 +73,16 @@ - -
- -
- - - - - - - + + + From 144a9377a7a391312bbc0e1f4bb61bf5bdf90dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 11 Jan 2019 02:54:53 +0100 Subject: [PATCH 16/44] Fix Scope tab focus loss --- .../fxdesigner/MainDesignerController.java | 23 +++++++++---- .../fxdesigner/NodeInfoPanelController.java | 34 ++++++++++++++----- .../util/fxdesigner/util/IteratorUtil.java | 10 ++++++ .../util/controls/ScopeHierarchyTreeItem.java | 29 ++++++++++++++++ 4 files changed, 82 insertions(+), 14 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index f0d70a83b3..0ffb16c0b3 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -232,16 +232,27 @@ public class MainDesignerController extends AbstractController { } + /** + * Callback for {@link NodeInfoPanelController}. This method should + * not forward a focus request back to the {@link NodeInfoPanelController}, + * it takes care itself of calling itself. + */ public void onNameDeclarationSelected(NameDeclaration declaration) { - sourceEditorController.clearNameOccurences(); - List occurrences = declaration.getNode().getScope().getDeclarations().get(declaration); + Platform.runLater(() -> { + sourceEditorController.clearNameOccurences(); - if (occurrences != null) { - sourceEditorController.highlightNameOccurrences(occurrences); - } + List occurrences = declaration.getNode().getScope().getDeclarations().get(declaration); - Platform.runLater(() -> onNodeItemSelected(declaration.getNode())); + if (occurrences != null) { + sourceEditorController.highlightNameOccurrences(occurrences); + } + }); + + Platform.runLater(() -> { + sourceEditorController.setFocusNode(declaration.getNode()); + sourceEditorController.focusNodeInTreeView(declaration.getNode()); + }); } /** 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 b02e214c9c..a397696c5f 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 @@ -20,11 +20,13 @@ import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; +import net.sourceforge.pmd.util.fxdesigner.util.IteratorUtil; import net.sourceforge.pmd.util.fxdesigner.util.beans.SettingsPersistenceUtil.PersistentProperty; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; import net.sourceforge.pmd.util.fxdesigner.util.controls.ToolbarTitledPane; +import javafx.application.Platform; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.fxml.FXML; @@ -84,7 +86,10 @@ public class NodeInfoPanelController extends AbstractController { .filter(Objects::nonNull) .map(TreeItem::getValue) .filterMap(o -> o instanceof NameDeclaration, o -> (NameDeclaration) o) - .subscribe(parent::onNameDeclarationSelected); + .subscribe(declaration -> { + Platform.runLater(() -> setFocusNode(declaration.getNode(), true)); + parent.onNameDeclarationSelected(declaration); + }); scopeHierarchyTreeView.setCellFactory(view -> new ScopeHierarchyTreeCell()); @@ -102,6 +107,11 @@ public class NodeInfoPanelController extends AbstractController { * @param node Node to inspect */ public void setFocusNode(Node node) { + setFocusNode(node, false); + } + + + private void setFocusNode(Node node, boolean focusScopeView) { if (node == null) { invalidateInfo(); return; @@ -114,10 +124,9 @@ public class NodeInfoPanelController extends AbstractController { displayAttributes(node); displayMetrics(node); - displayScopes(node); + displayScopes(node, focusScopeView); } - private void displayAttributes(Node node) { ObservableList atts = getAttributes(node); xpathAttributesListView.setItems(atts); @@ -134,17 +143,26 @@ public class NodeInfoPanelController extends AbstractController { } - private void displayScopes(Node node) { - // TODO maybe a better way would be to build all the scope TreeItem hierarchy once - // and only expand the ascendants of the node. - TreeItem rootScope = ScopeHierarchyTreeItem.buildAscendantHierarchy(node); + private void displayScopes(Node node, boolean focusScopeView) { + + // current selection + TreeItem previousSelection = scopeHierarchyTreeView.getSelectionModel().getSelectedItem(); + + ScopeHierarchyTreeItem rootScope = ScopeHierarchyTreeItem.buildAscendantHierarchy(node); scopeHierarchyTreeView.setRoot(rootScope); + + if (focusScopeView && previousSelection != null) { + int maxDepth = IteratorUtil.count(IteratorUtil.parentIterator(previousSelection, true)); + + rootScope.tryFindNode(previousSelection.getValue(), maxDepth) + .ifPresent(scopeHierarchyTreeView.getSelectionModel()::select); + } } /** * Invalidates the info being displayed. */ - public void invalidateInfo() { + private void invalidateInfo() { metricResultsListView.setItems(FXCollections.emptyObservableList()); xpathAttributesListView.setItems(FXCollections.emptyObservableList()); scopeHierarchyTreeView.setRoot(null); diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java index f8b86a8584..5476683179 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java @@ -46,6 +46,16 @@ public final class IteratorUtil { } + /** Counts the items in this iterator, exhausting it. */ + public static int count(Iterator it) { + int count = 0; + for (Object o : toIterable(it)) { + count++; + } + return count; + } + + /** * Returns an iterator over the parents of the given node, in innermost to outermost order. */ diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeItem.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeItem.java index 36a8265981..680f4770d2 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeItem.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ScopeHierarchyTreeItem.java @@ -6,6 +6,8 @@ package net.sourceforge.pmd.util.fxdesigner.util.controls; import java.util.List; import java.util.Map.Entry; +import java.util.Objects; +import java.util.Optional; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; @@ -28,6 +30,33 @@ public final class ScopeHierarchyTreeItem extends TreeItem { } + /** + * Tries to find a node in the descendants of this node that has + * the same toString as the given value. + */ + public Optional tryFindNode(Object searched, int maxDepth) { + if (searched == null || maxDepth == 0) { + return Optional.empty(); + } + + if (Objects.equals(searched.toString(), this.getValue().toString())) { + return Optional.of(this); + } + + for (TreeItem child : getChildren()) { + + Optional childResult = + ((ScopeHierarchyTreeItem) child).tryFindNode(searched, maxDepth - 1); + + if (childResult.isPresent()) { + return childResult; + } + } + + return Optional.empty(); + } + + /** * Gets the scope hierarchy of a node. * From c676a4c22eb5411b075c927faa0f915b9d02eca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 14 Jan 2019 19:38:08 +0100 Subject: [PATCH 17/44] Replace with toggle button --- .../fxdesigner/NodeInfoPanelController.java | 22 +++++----- .../pmd/util/fxdesigner/fxml/node-info.fxml | 10 ++--- .../pmd/util/fxdesigner/less/designer.less | 44 +++++++++++++++++-- 3 files changed, 56 insertions(+), 20 deletions(-) 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 a397696c5f..4882790c9e 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 @@ -30,11 +30,11 @@ import javafx.application.Platform; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.fxml.FXML; -import javafx.scene.control.CheckMenuItem; import javafx.scene.control.Label; import javafx.scene.control.ListView; import javafx.scene.control.Tab; import javafx.scene.control.TabPane; +import javafx.scene.control.ToggleButton; import javafx.scene.control.TreeItem; import javafx.scene.control.TreeView; @@ -55,9 +55,9 @@ public class NodeInfoPanelController extends AbstractController { Arrays.asList("BeginLine", "EndLine", "BeginColumn", "EndColumn", "FindBoundary", "SingleLine"); @FXML - private ToolbarTitledPane metricsTitledPane; + private ToggleButton hideCommonAttributesToggle; @FXML - private CheckMenuItem showAllAttributesMenuItem; + private ToolbarTitledPane metricsTitledPane; @FXML private TabPane nodeInfoTabPane; @FXML @@ -93,7 +93,7 @@ public class NodeInfoPanelController extends AbstractController { scopeHierarchyTreeView.setCellFactory(view -> new ScopeHierarchyTreeCell()); - showAllAttributesProperty() + hideCommonAttributesProperty() .values() .distinct() .subscribe(show -> displayAttributes(selectedNode)); @@ -186,18 +186,18 @@ public class NodeInfoPanelController extends AbstractController { @PersistentProperty - public boolean isShowAllAttributes() { - return showAllAttributesMenuItem.isSelected(); + public boolean isHideCommonAttributes() { + return hideCommonAttributesToggle.isSelected(); } - public void setShowAllAttributes(boolean bool) { - showAllAttributesMenuItem.setSelected(bool); + public void setHideCommonAttributes(boolean bool) { + hideCommonAttributesToggle.setSelected(bool); } - public Var showAllAttributesProperty() { - return Var.fromVal(showAllAttributesMenuItem.selectedProperty(), showAllAttributesMenuItem::setSelected); + public Var hideCommonAttributesProperty() { + return Var.fromVal(hideCommonAttributesToggle.selectedProperty(), hideCommonAttributesToggle::setSelected); } @@ -214,7 +214,7 @@ public class NodeInfoPanelController extends AbstractController { while (attributeAxisIterator.hasNext()) { Attribute attribute = attributeAxisIterator.next(); - if (isShowAllAttributes() || !IGNORABLE_ATTRIBUTES.contains(attribute.getName())) { + if (!(isHideCommonAttributes() && IGNORABLE_ATTRIBUTES.contains(attribute.getName()))) { // TODO the display should be handled in a ListCell result.add(attribute.getName() + " = " + ((attribute.getValue() != null) ? attribute.getStringValue() : "null")); 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 f56bc53bdd..144fe6a347 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 @@ -13,6 +13,7 @@ + @@ -29,14 +30,11 @@ AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0"> - + - + - - - - + diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less index d7835c2ebf..72cb2ca57c 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less @@ -222,19 +222,57 @@ -fx-graphic-text-gap: 0; } - &.menu-button .ikonli-font-icon { + &.menu-button, &.toggle-button { + .ikonli-font-icon { // Fixes a bug where the cog icon is misplaced to the right // Probably needs to be synced with icon size -fx-translate-x: -4; + } } + .ikonli-font-icon, .svg-icon { -fx-fill: @fx-text-fill; } - &:showing, &:hover { - -fx-background-color: @selection-focus-color; + &.toggle-button:selected { + -fx-background-color: -fx-shadow-highlight-color, + linear-gradient(to bottom, derive(-fx-outer-border, -20%), -fx-outer-border), + linear-gradient(to bottom, + derive(-fx-color, -22%) 0%, + derive(-fx-color, -13%) 20%, + derive(-fx-color, -11%) 50%); + + -fx-background-insets: 0 0 -1 0, 0, 1; } + + .toggle-button-background(@base) { + -fx-background-color: -fx-focus-color, + linear-gradient(to bottom, + derive(@base, -22%) 0%, + derive(@base, -13%) 20%, + derive(@base, -11%) 50%), + -fx-faint-focus-color, + linear-gradient(to bottom, + derive(@base, -22%) 0%, + derive(@base, -13%) 20%, + derive(@base, -11%) 50%); + } + + .toggle-button:selected { + -fx-background-insets: -0.2, 1, -1.4, 2.6; + -fx-background-radius: 3, 2, 4, 0; + + &:focused { + .toggle-button-background(-fx-color); + } + + &:hover { + .toggle-button-background(darken(@selection-focus-color, 10%)); + } + } + + .label { -fx-background-color: transparent; //-fx-padding: 0; From fab97b7c37e1ff80dba3847041d60ece37fdb75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 15 Jan 2019 22:36:45 +0100 Subject: [PATCH 18/44] Fix css --- .../pmd/util/fxdesigner/fxml/node-info.fxml | 6 ++- .../pmd/util/fxdesigner/fxml/xpath.fxml | 4 +- .../pmd/util/fxdesigner/less/constants.less | 16 +++++- .../pmd/util/fxdesigner/less/designer.less | 54 ++++++++----------- 4 files changed, 44 insertions(+), 36 deletions(-) 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 144fe6a347..f4f35fc3a0 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 @@ -14,6 +14,7 @@ + @@ -30,7 +31,10 @@ AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0"> - + + + + 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 d7716eed92..54590b816d 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 @@ -65,7 +65,9 @@ AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0"> + @@ -93,7 +95,7 @@ - + diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/constants.less b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/constants.less index f0bae62c17..e27e43350f 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/constants.less +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/constants.less @@ -21,8 +21,11 @@ */ -// This is repeated to use it from Less -// Prefer using it over -fx-base +// This is repeated to use it from Less and fix the default values +// Important difference: +// * @foo is resolved at compile-time by Less +// * -foo is looked-up at runtime by the CSS engine. So it can't be used with Less +// compile-time functions, which will fail. @fx-base: #ececec; @fx-text-fill: ladder( -fx-color, @@ -31,6 +34,7 @@ -fx-dark-text-color 59%, -fx-mid-text-color 60%); +// TODO simplify that // Base colors for background slates @app-base-color: darken(@fx-base, 4%); @app-darker-slate-color: darken(@fx-base, 14%); @@ -46,6 +50,14 @@ @normal-font-size: 10pt; @smaller-font-size: 9pt; +.root { + + /* A light grey that is the base color for objects. Instead of using + * -fx-base directly, the sections in this file will typically use -fx-color. + */ + -fx-base: @app-base-color; +} + // mixin to fix the width of a component .fix-width(@width) { -fx-pref-width: @width; diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less index 72cb2ca57c..0ed95b9e2c 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/designer.less @@ -162,6 +162,12 @@ } } +.titled-pane > .title:hover { + // modena uses the rule -fx-color: -fx-hover-base; + // which causes an obnoxious flickering + -fx-color: -fx-base; +} + // Supports the ToolbarTitledPane .titled-pane.tool-bar-title > .title { @@ -214,7 +220,10 @@ .icon-button { -fx-background-color: transparent; + + -fx-content-display: CENTER; -fx-border-width: 0; + -fx-background-insets: 0; -fx-border-insets: 0; .force-square(24); @@ -222,12 +231,10 @@ -fx-graphic-text-gap: 0; } - &.menu-button, &.toggle-button { - .ikonli-font-icon { + &.menu-button .ikonli-font-icon { // Fixes a bug where the cog icon is misplaced to the right // Probably needs to be synced with icon size - -fx-translate-x: -4; - } + -fx-translate-x: -3; } .ikonli-font-icon, .svg-icon { @@ -235,47 +242,30 @@ } &.toggle-button:selected { - -fx-background-color: -fx-shadow-highlight-color, - linear-gradient(to bottom, derive(-fx-outer-border, -20%), -fx-outer-border), - linear-gradient(to bottom, - derive(-fx-color, -22%) 0%, - derive(-fx-color, -13%) 20%, - derive(-fx-color, -11%) 50%); - -fx-background-insets: 0 0 -1 0, 0, 1; - } + &, &:focused { + @grad: linear-gradient(to bottom, + derive(-fx-color, -22%) 0%, + derive(-fx-color, -13%) 20%, + derive(-fx-color, -11%) 50%); - .toggle-button-background(@base) { - -fx-background-color: -fx-focus-color, - linear-gradient(to bottom, - derive(@base, -22%) 0%, - derive(@base, -13%) 20%, - derive(@base, -11%) 50%), - -fx-faint-focus-color, - linear-gradient(to bottom, - derive(@base, -22%) 0%, - derive(@base, -13%) 20%, - derive(@base, -11%) 50%); - } + -fx-background-color: -fx-shadow-highlight-color, linear-gradient(to bottom, derive(-fx-outer-border, -20%), -fx-outer-border), @grad; - .toggle-button:selected { - -fx-background-insets: -0.2, 1, -1.4, 2.6; - -fx-background-radius: 3, 2, 4, 0; - - &:focused { - .toggle-button-background(-fx-color); + -fx-background-insets: 0 0 -1 0, 0, 1; } &:hover { - .toggle-button-background(darken(@selection-focus-color, 10%)); + -fx-hover-base: @selection-focus-color; } } + &.menu-button:showing, &:hover { + -fx-background-color: @selection-focus-color; + } .label { -fx-background-color: transparent; - //-fx-padding: 0; } } From 9e27bf2e6ba4ebb1bf4a2610f0918b84b94cf21d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 10:45:50 +0100 Subject: [PATCH 19/44] Convert XPath titledPanes to ToolBarTitledPanes --- .../util/fxdesigner/XPathPanelController.java | 6 +++--- .../pmd/util/fxdesigner/fxml/xpath.fxml | 18 +++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) 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 fdd40208da..76bc65f1ac 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 @@ -95,7 +95,7 @@ public class XPathPanelController extends AbstractController { @FXML private SyntaxHighlightingCodeArea xpathExpressionArea; @FXML - private TitledPane violationsTitledPane; + private ToolbarTitledPane violationsTitledPane; @FXML private ListView xpathResultListView; @@ -255,7 +255,7 @@ public class XPathPanelController extends AbstractController { ruleBuilder.getRuleProperties())); xpathResultListView.setItems(results.stream().map(parent::wrapNode).collect(Collectors.toCollection(LiveArrayList::new))); parent.highlightXPathResults(results); - violationsTitledPane.setText("Matched nodes\t(" + results.size() + ")"); + violationsTitledPane.setTitle("Matched nodes (" + results.size() + ")"); // Notify that everything went OK so we can avoid logging very recent exceptions designerRoot.getLogger().logEvent(new LogEntry(null, Category.XPATH_OK)); } catch (XPathEvaluationException e) { @@ -274,7 +274,7 @@ public class XPathPanelController extends AbstractController { public void invalidateResults(boolean error) { xpathResultListView.getItems().clear(); parent.resetXPathResults(); - violationsTitledPane.setText("Matched nodes" + (error ? "\t(error)" : "")); + violationsTitledPane.setTitle("Matched nodes" + (error ? "\t(error)" : "")); } 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 54590b816d..6c02fee03f 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 @@ -10,7 +10,6 @@ - @@ -24,6 +23,7 @@ fx:controller="net.sourceforge.pmd.util.fxdesigner.XPathPanelController"> - - + @@ -109,11 +109,11 @@ - - + - - - - From ac4c3d49fa412d1e2c80a73091df19ed9571a2f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 11:16:19 +0100 Subject: [PATCH 20/44] Simplify stylesheets --- .../fxdesigner/SourceEditorController.java | 10 +- .../util/controls/ToolbarTitledPane.java | 4 + .../pmd/util/fxdesigner/fxml/designer.fxml | 2 +- .../pmd/util/fxdesigner/fxml/editor.fxml | 28 +++--- .../pmd/util/fxdesigner/fxml/node-info.fxml | 6 +- .../pmd/util/fxdesigner/fxml/xpath.fxml | 4 - .../pmd/util/fxdesigner/less/designer.less | 99 ++++++++----------- 7 files changed, 65 insertions(+), 88 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java index dcd244e569..61717ea961 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java @@ -73,12 +73,10 @@ public class SourceEditorController extends AbstractController { @FXML private ToolbarTitledPane editorTitledPane; @FXML + private ToolbarTitledPane astViewTitledPane; + @FXML private MenuButton languageSelectionMenuButton; @FXML - private Label sourceCodeTitleLabel; - @FXML - private Label astTitleLabel; - @FXML private TreeView astTreeView; @FXML private HighlightLayerCodeArea codeEditorArea; @@ -214,7 +212,7 @@ public class SourceEditorController extends AbstractController { try { current = astManager.updateIfChanged(source, auxclasspathClassLoader.getValue()); } catch (ParseAbortedException e) { - astTitleLabel.setText("Abstract syntax tree (error)"); + astViewTitledPane.setTitle("Abstract syntax tree (error)"); return Optional.empty(); } @@ -231,7 +229,7 @@ public class SourceEditorController extends AbstractController { private void setUpToDateCompilationUnit(Node node) { parent.invalidateAst(); - astTitleLabel.setText("Abstract syntax tree"); + astViewTitledPane.setTitle("Abstract syntax tree"); ASTTreeItem root = ASTTreeItem.getRoot(node); astTreeView.setRoot(root); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ToolbarTitledPane.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ToolbarTitledPane.java index ca47da0bca..05be4d0d64 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ToolbarTitledPane.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/ToolbarTitledPane.java @@ -37,10 +37,14 @@ public final class ToolbarTitledPane extends TitledPane { getStyleClass().add("tool-bar-title"); + // change the default + setCollapsible(false); + toolBar.setPadding(Insets.EMPTY); Label titleLabel = new Label("Title"); titleLabel.textProperty().bind(title); + titleLabel.getStyleClass().add("title-label"); toolBar.getItems().add(titleLabel); diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml index 521e87ccfb..a3e4d01a24 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/designer.fxml @@ -84,7 +84,7 @@
- + diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/editor.fxml b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/editor.fxml index 9d6cf727a4..b9b5926026 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/editor.fxml +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/fxml/editor.fxml @@ -16,7 +16,6 @@ - -
+ + -
- - - - - - - + + 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 f4f35fc3a0..189652b7c8 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 @@ -24,7 +24,7 @@ .title { - .fix-height(30); - } - - &.tool-bar-title > .title > .tool-bar > .container > .label { - // TODO we can make that bigger when all titled panes rely on ToolbarTitledPane - -fx-font-size: @smaller-font-size; - } -} - .titled-pane > .title:hover { // modena uses the rule -fx-color: -fx-hover-base; // which causes an obnoxious flickering @@ -169,39 +139,52 @@ } // Supports the ToolbarTitledPane -.titled-pane.tool-bar-title > .title { +.titled-pane.tool-bar-title { - -fx-padding: 0 6 0 6; + &.full-size-title > .title { + .fix-height(30); + } - & > .tool-bar { - -fx-background-color: transparent; + & > .title { - & > .container { + -fx-padding: 0 6 0 6; + -fx-background-color: @app-darker-slate-color; + -fx-pref-height: 24; // TODO make the default at least 30 and prop up font size, maybe remove .full-size-title - -fx-background-insets: 0; - -fx-border-insets: 0; - -fx-border-image-insets: 0; + & > .tool-bar { + -fx-background-color: transparent; - & > .label { - -fx-background-color: transparent; - -fx-font-size: @smaller-font-size; - } + & > .container { - .separator { - //-fx-background-color: transparent; + -fx-background-insets: 0; + -fx-border-insets: 0; + -fx-border-image-insets: 0; - -fx-padding: 4 2 4 2; + & > .label { + -fx-background-color: transparent; + -fx-font-size: @smaller-font-size; - .line { - -fx-border-style: none solid none none; - -fx-border-width: 0.5px; - -fx-border-color: @fx-text-fill; + // This is the title label of the pane + &.title-label { + -fx-padding: 0 0 0 6; + } } - } + .separator { + //-fx-background-color: transparent; - .menu-bar { - -fx-background-color: transparent; + -fx-padding: 4 2 4 2; + + .line { + -fx-border-style: none solid none none; + -fx-border-width: 0.5px; + -fx-border-color: @fx-text-fill; + } + } + + .menu-bar { + -fx-background-color: transparent; + } } } } @@ -234,7 +217,7 @@ &.menu-button .ikonli-font-icon { // Fixes a bug where the cog icon is misplaced to the right // Probably needs to be synced with icon size - -fx-translate-x: -3; + -fx-translate-x: -4; } .ikonli-font-icon, .svg-icon { @@ -273,11 +256,7 @@ -fx-background-color: @darker-accent-focus; } -// TODO use ToolbarTitledPane everywhere and simplify stylesheets - -#main-toolbar, -.tool-bar.accent-header, -.split-pane.accent-header > .split-pane-divider { +#main-toolbar { .fix-height(30); -fx-background-color: @app-darker-slate-color; } @@ -302,7 +281,7 @@ .tab { -fx-background-insets: 0.0; -fx-background-radius: 0.0; - -fx-padding: 0 30 0 30; + -fx-padding: 0 20 0 20; -fx-border-color: transparent; -fx-background-color: transparent; From fe717c3af586142c5eb063d2cb6aef0d236cdc84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 11:54:16 +0100 Subject: [PATCH 21/44] Improve syntax highlight style on focused cells --- .../ApexSyntaxHighlighter.java | 2 ++ .../syntaxhighlighting/HighlightClasses.java | 26 +++++++++++-------- .../JavaSyntaxHighlighter.java | 2 ++ .../XPathSyntaxHighlighter.java | 2 ++ .../fxdesigner/less/syntax-highlighting.less | 21 +++++++++++++-- 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/ApexSyntaxHighlighter.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/ApexSyntaxHighlighter.java index 819a4df65e..cd41563968 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/ApexSyntaxHighlighter.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/ApexSyntaxHighlighter.java @@ -8,6 +8,7 @@ import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighti import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.BOOLEAN; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.BRACE; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.BRACKET; +import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.IDENTIFIER; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.KEYWORD; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.MULTIL_COMMENT; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.PAREN; @@ -65,6 +66,7 @@ public class ApexSyntaxHighlighter extends SimpleRegexSyntaxHighlighter { .or(STRING.css, "'[^'\\\\]*(\\\\.[^'\\\\]*)*'") .or(BOOLEAN.css, asWord("(?i)true|false")) .or(ANNOTATION.css, "@[\\w]+") + .or(IDENTIFIER.css, asWord("[\\w_$]+")) .create(Pattern.DOTALL | Pattern.CASE_INSENSITIVE); public ApexSyntaxHighlighter() { diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/HighlightClasses.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/HighlightClasses.java index d1148ab687..ce52de1c36 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/HighlightClasses.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/HighlightClasses.java @@ -8,7 +8,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; - /** * @author Clément Fournier * @since 6.0.0 @@ -33,23 +32,25 @@ public enum HighlightClasses { NULL("null", Constants.LITERAL), NUMBER("number", Constants.LITERAL), - KEYWORD("keyword"), + KEYWORD(Constants.KEYWORD), ANNOTATION("annotation"), - CLASS_IDENTIFIER("class-identifier"), - + + IDENTIFIER(Constants.IDENTIFIER), + CLASS_IDENTIFIER("class-identifier", Constants.IDENTIFIER), + // XPath specific - XPATH_ATTRIBUTE("attribute", Constants.XPATH), - XPATH_AXIS("axis", Constants.XPATH), - XPATH_FUNCTION("function", Constants.XPATH), + XPATH_ATTRIBUTE("attribute", Constants.XPATH, Constants.IDENTIFIER), + XPATH_AXIS("axis", Constants.XPATH, Constants.KEYWORD), + XPATH_FUNCTION(Constants.FUNCTION, Constants.XPATH, Constants.IDENTIFIER), XPATH_PATH("path", Constants.XPATH, Constants.PUNCTUATION), - XPATH_KIND_TEST("kind-test", "function", Constants.XPATH), + XPATH_KIND_TEST("kind-test", Constants.XPATH, Constants.FUNCTION), XML_CDATA_TAG("cdata-tag", Constants.XML), XML_CDATA_CONTENT("cdata-content", Constants.XML), XML_PROLOG("xml-prolog", Constants.XML), - XML_LT_GT("lt-gt", Constants.XML), - XML_TAG_NAME("tag-name", Constants.XML), - XML_ATTRIBUTE_NAME("attribute-name", Constants.XML); + XML_LT_GT("lt-gt", Constants.XML, Constants.PUNCTUATION), + XML_TAG_NAME("tag-name", Constants.XML, Constants.IDENTIFIER), + XML_ATTRIBUTE_NAME("attribute-name", Constants.XML, Constants.IDENTIFIER); /** Name of the css class. */ @@ -62,6 +63,9 @@ public enum HighlightClasses { private static final class Constants { + static final String IDENTIFIER = "identifier"; + static final String FUNCTION = "function"; + static final String KEYWORD = "keyword"; static final String LITERAL = "literal"; static final String COMMENT = "comment"; static final String PUNCTUATION = "punctuation"; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/JavaSyntaxHighlighter.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/JavaSyntaxHighlighter.java index 99ce7dda02..2c7f46ce97 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/JavaSyntaxHighlighter.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/JavaSyntaxHighlighter.java @@ -10,6 +10,7 @@ import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighti import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.BRACKET; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.CHAR; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.CLASS_IDENTIFIER; +import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.IDENTIFIER; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.KEYWORD; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.MULTIL_COMMENT; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.NULL; @@ -62,6 +63,7 @@ public final class JavaSyntaxHighlighter extends SimpleRegexSyntaxHighlighter { .or(BOOLEAN.css, asWord("true|false")) .or(ANNOTATION.css, "@[\\w]+") .or(CLASS_IDENTIFIER.css, asWord("[A-Z][\\w_$]*")) + .or(IDENTIFIER.css, asWord("[\\w_$]+")) .create(Pattern.DOTALL); diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/XPathSyntaxHighlighter.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/XPathSyntaxHighlighter.java index 538905b9a4..25da70f5d7 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/XPathSyntaxHighlighter.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/syntaxhighlighting/XPathSyntaxHighlighter.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.BRACKET; +import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.IDENTIFIER; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.KEYWORD; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.MULTIL_COMMENT; import static net.sourceforge.pmd.util.fxdesigner.util.codearea.syntaxhighlighting.HighlightClasses.NUMBER; @@ -61,6 +62,7 @@ public class XPathSyntaxHighlighter extends SimpleRegexSyntaxHighlighter { .or(NUMBER.css, "(\\.\\d++\\b|\\b\\d++\\.|(\\b\\d++(\\.\\d*+)?([eE][+-]?\\d+)?))") .or(STRING.css, "('([^']|'')*')|(\"([^\"]|\"\")*\")") .or(URI.css, "Q\\{[^{}]*}") + .or(IDENTIFIER.css, asWord("[\\w_$]+")) .create(); diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less index 5c8897f2da..2f45ffa0c0 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less @@ -4,11 +4,28 @@ /* Rules for the syntax highlighters. - This file doesn't include highlight rules, so that highlight is not displayed - on controls that display node rich text if only this file is set as stylesheet. + This file doesn't include layer highlight rules (eg .xpath-highlight .depth-0), so that + layer highlight is not displayed on controls that display node rich text if only this + file is set as stylesheet. */ +.list-cell:focused .code { + // more readable styles on focused cells w/ blue background + + &.identifier { + -fx-fill: white !important; + } + + &.keyword, &.literal { + -fx-fill: #dddddd; + } + + &.punctuation { + -fx-fill: black; + } +} + .code { /* Common classes */ From d0fbf4a31381c8c046ab23fff4152f93730d1cdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 12:02:29 +0100 Subject: [PATCH 22/44] Explanations --- .../pmd/util/fxdesigner/NodeInfoPanelController.java | 7 +++++-- .../pmd/util/fxdesigner/SourceEditorController.java | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) 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 4882790c9e..bd0f321b81 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 @@ -50,7 +50,7 @@ public class NodeInfoPanelController extends AbstractController { private final MainDesignerController parent; - /** List of attribute names that are ignored if {@link #isShowAllAttributes()} is false. */ + /** List of attribute names that are ignored if {@link #isHideCommonAttributes()} is true. */ private static final List IGNORABLE_ATTRIBUTES = Arrays.asList("BeginLine", "EndLine", "BeginColumn", "EndColumn", "FindBoundary", "SingleLine"); @@ -152,8 +152,11 @@ public class NodeInfoPanelController extends AbstractController { scopeHierarchyTreeView.setRoot(rootScope); if (focusScopeView && previousSelection != null) { + // Try to find the node that was previously selected and focus it in the new ascendant hierarchy + // Otherwise, when you select a node in the scope tree, since focus of the app is shifted to that + // node, the scope hierarchy is reset and you lose the selection - even though obviously the node + // you selected is in its own scope hierarchy so it looks buggy. int maxDepth = IteratorUtil.count(IteratorUtil.parentIterator(previousSelection, true)); - rootScope.tryFindNode(previousSelection.getValue(), maxDepth) .ifPresent(scopeHierarchyTreeView.getSelectionModel()::select); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java index 61717ea961..4b76e41638 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java @@ -70,13 +70,13 @@ public class SourceEditorController extends AbstractController { private static final Duration AST_REFRESH_DELAY = Duration.ofMillis(100); + @FXML + private MenuButton languageSelectionMenuButton; @FXML private ToolbarTitledPane editorTitledPane; @FXML private ToolbarTitledPane astViewTitledPane; @FXML - private MenuButton languageSelectionMenuButton; - @FXML private TreeView astTreeView; @FXML private HighlightLayerCodeArea codeEditorArea; From 845cb14a1f33841d2444ff9567b661633b03bab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 12:23:57 +0100 Subject: [PATCH 23/44] Highlight name occurences of methods --- .../fxdesigner/MainDesignerController.java | 24 +++++++++++++------ .../fxdesigner/NodeInfoPanelController.java | 4 +++- .../fxdesigner/SourceEditorController.java | 7 +++--- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index 662f9a1d94..5941a3d853 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -217,9 +217,9 @@ public class MainDesignerController extends AbstractController { * Executed when the user selects a node in a treeView or listView. */ public void onNodeItemSelected(Node selectedValue) { - nodeInfoPanelController.setFocusNode(selectedValue); - sourceEditorController.setFocusNode(selectedValue); - sourceEditorController.focusNodeInTreeView(selectedValue); + // doing that in parallel speeds it up + Platform.runLater(()-> nodeInfoPanelController.setFocusNode(selectedValue)); + Platform.runLater(()-> sourceEditorController.setFocusNode(selectedValue)); } @@ -231,19 +231,29 @@ public class MainDesignerController extends AbstractController { public void onNameDeclarationSelected(NameDeclaration declaration) { Platform.runLater(() -> { + // TODO highlight usages of regular node selection and move that logic to nodeInfoPanelController.setFocusNode + // In fact I think the current symbol table is too low level for that. You + // can map a NameDeclaration to its node but not the reverse... sourceEditorController.clearNameOccurences(); List occurrences = declaration.getNode().getScope().getDeclarations().get(declaration); + if (occurrences == null) { + // For MethodNameDeclaration the scope is the method scope, which is not the scope it is declared + // in but the scope it declares! That means that getDeclarations().get(declaration) returns null + // and no name occurrences are found. We thus look in the parent, but ultimately the name occurrence + // finder is broken since it can't find e.g. the use of a method in another scope. Plus in case of + // overloads both overloads are reported to have a usage. + // Plus this is some serious law of Demeter breaking there... + occurrences = declaration.getNode().getScope().getParent().getDeclarations().get(declaration); + } + if (occurrences != null) { sourceEditorController.highlightNameOccurrences(occurrences); } }); - Platform.runLater(() -> { - sourceEditorController.setFocusNode(declaration.getNode()); - sourceEditorController.focusNodeInTreeView(declaration.getNode()); - }); + Platform.runLater(() -> sourceEditorController.setFocusNode(declaration.getNode())); } /** 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 bd0f321b81..11c91d0123 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 @@ -17,6 +17,8 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; +import net.sourceforge.pmd.lang.symboltable.NameOccurrence; +import net.sourceforge.pmd.lang.symboltable.ScopedNode; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; @@ -152,7 +154,7 @@ public class NodeInfoPanelController extends AbstractController { scopeHierarchyTreeView.setRoot(rootScope); if (focusScopeView && previousSelection != null) { - // Try to find the node that was previously selected and focus it in the new ascendant hierarchy + // Try to find the node that was previously selected and focus it in the new ascendant hierarchy. // Otherwise, when you select a node in the scope tree, since focus of the app is shifted to that // node, the scope hierarchy is reset and you lose the selection - even though obviously the node // you selected is in its own scope hierarchy so it looks buggy. diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java index 4b76e41638..d7a0170339 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/SourceEditorController.java @@ -51,7 +51,6 @@ import net.sourceforge.pmd.util.fxdesigner.util.controls.TreeViewWrapper; import javafx.application.Platform; import javafx.css.PseudoClass; import javafx.fxml.FXML; -import javafx.scene.control.Label; import javafx.scene.control.MenuButton; import javafx.scene.control.RadioMenuItem; import javafx.scene.control.SelectionModel; @@ -249,7 +248,7 @@ public class SourceEditorController extends AbstractController { /** Clears the name occurences. */ public void clearNameOccurences() { - codeEditorArea.clearStyleLayer(StyleLayerIds.ERROR); + codeEditorArea.clearStyleLayer(StyleLayerIds.NAME_OCCURENCE); } @@ -268,6 +267,8 @@ public class SourceEditorController extends AbstractController { return; } + Platform.runLater(() -> focusNodeInTreeView(node)); + codeEditorArea.styleNodes(node == null ? emptyList() : singleton(node), StyleLayerIds.FOCUS, true); if (node != null) { @@ -324,7 +325,7 @@ public class SourceEditorController extends AbstractController { } - public void focusNodeInTreeView(Node node) { + private void focusNodeInTreeView(Node node) { SelectionModel> selectionModel = astTreeView.getSelectionModel(); // node is different from the old one From 4e3832b5cb9642e87ce6f833450982a382dc3b90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 12:42:19 +0100 Subject: [PATCH 24/44] Fix pmd warnings --- .../pmd/util/fxdesigner/MainDesignerController.java | 4 ++-- .../pmd/util/fxdesigner/NodeInfoPanelController.java | 2 -- .../sourceforge/pmd/util/fxdesigner/XPathPanelController.java | 1 - .../sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java | 3 ++- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index 5941a3d853..d016aa2a58 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -218,8 +218,8 @@ public class MainDesignerController extends AbstractController { */ public void onNodeItemSelected(Node selectedValue) { // doing that in parallel speeds it up - Platform.runLater(()-> nodeInfoPanelController.setFocusNode(selectedValue)); - Platform.runLater(()-> sourceEditorController.setFocusNode(selectedValue)); + Platform.runLater(() -> nodeInfoPanelController.setFocusNode(selectedValue)); + Platform.runLater(() -> sourceEditorController.setFocusNode(selectedValue)); } 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 11c91d0123..54643ccc2a 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 @@ -17,8 +17,6 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; -import net.sourceforge.pmd.lang.symboltable.NameOccurrence; -import net.sourceforge.pmd.lang.symboltable.ScopedNode; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; 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 76bc65f1ac..4ce4bdf28e 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 @@ -58,7 +58,6 @@ import javafx.scene.control.MenuButton; import javafx.scene.control.MenuItem; import javafx.scene.control.RadioMenuItem; import javafx.scene.control.TextArea; -import javafx.scene.control.TitledPane; import javafx.scene.control.ToggleGroup; import javafx.scene.input.MouseButton; import javafx.scene.input.MouseEvent; diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java index 5476683179..50406e2252 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/IteratorUtil.java @@ -49,7 +49,8 @@ public final class IteratorUtil { /** Counts the items in this iterator, exhausting it. */ public static int count(Iterator it) { int count = 0; - for (Object o : toIterable(it)) { + while (it.hasNext()) { + it.next(); count++; } return count; From 5a2202319f90080e9980735a37e4eb8e948d2f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 16 Jan 2019 14:27:44 +0100 Subject: [PATCH 25/44] Highlight name declarations when selecting from treeview --- .../fxdesigner/MainDesignerController.java | 47 ++++-------- .../fxdesigner/NodeInfoPanelController.java | 75 ++++++++++++++++--- .../fxdesigner/less/syntax-highlighting.less | 2 +- 3 files changed, 81 insertions(+), 43 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index d016aa2a58..c32e15c029 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -21,7 +21,6 @@ import org.reactfx.value.Val; import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.util.fxdesigner.model.XPathEvaluationException; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; @@ -217,43 +216,29 @@ public class MainDesignerController extends AbstractController { * Executed when the user selects a node in a treeView or listView. */ public void onNodeItemSelected(Node selectedValue) { + onNodeItemSelected(selectedValue, false); + } + + + /** + * Executed when the user selects a node in a treeView or listView. + * + * @param isFromNameDecl Whether the node was selected in the scope hierarchy treeview + */ + public void onNodeItemSelected(Node selectedValue, boolean isFromNameDecl) { // doing that in parallel speeds it up - Platform.runLater(() -> nodeInfoPanelController.setFocusNode(selectedValue)); + Platform.runLater(() -> nodeInfoPanelController.setFocusNode(selectedValue, isFromNameDecl)); Platform.runLater(() -> sourceEditorController.setFocusNode(selectedValue)); } /** - * Callback for {@link NodeInfoPanelController}. This method should - * not forward a focus request back to the {@link NodeInfoPanelController}, - * it takes care itself of calling itself. + * Highlight a list of name occurrences. + * + * @param occurrences May be empty but never null. */ - public void onNameDeclarationSelected(NameDeclaration declaration) { - - Platform.runLater(() -> { - // TODO highlight usages of regular node selection and move that logic to nodeInfoPanelController.setFocusNode - // In fact I think the current symbol table is too low level for that. You - // can map a NameDeclaration to its node but not the reverse... - sourceEditorController.clearNameOccurences(); - - List occurrences = declaration.getNode().getScope().getDeclarations().get(declaration); - - if (occurrences == null) { - // For MethodNameDeclaration the scope is the method scope, which is not the scope it is declared - // in but the scope it declares! That means that getDeclarations().get(declaration) returns null - // and no name occurrences are found. We thus look in the parent, but ultimately the name occurrence - // finder is broken since it can't find e.g. the use of a method in another scope. Plus in case of - // overloads both overloads are reported to have a usage. - // Plus this is some serious law of Demeter breaking there... - occurrences = declaration.getNode().getScope().getParent().getDeclarations().get(declaration); - } - - if (occurrences != null) { - sourceEditorController.highlightNameOccurrences(occurrences); - } - }); - - Platform.runLater(() -> sourceEditorController.setFocusNode(declaration.getNode())); + public void highlightAsNameOccurences(List occurrences) { + sourceEditorController.highlightNameOccurrences(occurrences); } /** 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 54643ccc2a..7087e96437 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 @@ -6,9 +6,13 @@ package net.sourceforge.pmd.util.fxdesigner; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.Set; import org.reactfx.EventStreams; import org.reactfx.value.Var; @@ -17,6 +21,9 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; +import net.sourceforge.pmd.lang.symboltable.NameOccurrence; +import net.sourceforge.pmd.lang.symboltable.Scope; +import net.sourceforge.pmd.lang.symboltable.ScopedNode; import net.sourceforge.pmd.util.fxdesigner.model.MetricEvaluator; import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.AbstractController; @@ -86,10 +93,8 @@ public class NodeInfoPanelController extends AbstractController { .filter(Objects::nonNull) .map(TreeItem::getValue) .filterMap(o -> o instanceof NameDeclaration, o -> (NameDeclaration) o) - .subscribe(declaration -> { - Platform.runLater(() -> setFocusNode(declaration.getNode(), true)); - parent.onNameDeclarationSelected(declaration); - }); + .filter(nd -> !Objects.equals(nd.getNode(), selectedNode)) + .subscribe(declaration -> Platform.runLater(() -> parent.onNodeItemSelected(declaration.getNode(), true))); scopeHierarchyTreeView.setCellFactory(view -> new ScopeHierarchyTreeCell()); @@ -101,17 +106,20 @@ public class NodeInfoPanelController extends AbstractController { } - /** - * Displays info about a node. If null, the panels are reset. - * - * @param node Node to inspect - */ + /** 
{@linkplain #setFocusNode(Node, boolean) setFocusNode}(node, false)
*/ public void setFocusNode(Node node) { setFocusNode(node, false); } - private void setFocusNode(Node node, boolean focusScopeView) { + /** + * Displays info about a node. If null, the panels are reset. + * + * @param node Node to inspect + * @param isFromNameDecl Whether the node was selected in the scope hierarchy treeview. + * If so we'll attempt to preserve that selection. + */ + public void setFocusNode(Node node, boolean isFromNameDecl) { if (node == null) { invalidateInfo(); return; @@ -124,9 +132,54 @@ public class NodeInfoPanelController extends AbstractController { displayAttributes(node); displayMetrics(node); - displayScopes(node, focusScopeView); + displayScopes(node, isFromNameDecl); + + if (node instanceof ScopedNode) { + // not null as well + highlightNameOccurences((ScopedNode) node); + } + } + + private void highlightNameOccurences(ScopedNode node) { + + // For MethodNameDeclaration the scope is the method scope, which is not the scope it is declared + // in but the scope it declares! That means that getDeclarations().get(declaration) returns null + // and no name occurrences are found. We thus look in the parent, but ultimately the name occurrence + // finder is broken since it can't find e.g. the use of a method in another scope. Plus in case of + // overloads both overloads are reported to have a usage. + + // Plus this is some serious law of Demeter breaking there... + + Set candidates = new HashSet<>(node.getScope().getDeclarations().keySet()); + + Optional.ofNullable(node.getScope().getParent()) + .map(Scope::getDeclarations) + .map(Map::keySet) + .ifPresent(candidates::addAll); + + List occurrences = + candidates.stream() + .filter(nd -> node.equals(nd.getNode())) + .findFirst() + .map(nd -> { + // nd.getScope() != nd.getNode().getScope()?? wtf? + + List usages = nd.getNode().getScope().getDeclarations().get(nd); + + if (usages == null) { + usages = nd.getNode().getScope().getParent().getDeclarations().get(nd); + } + + return usages; + }) + .orElse(Collections.emptyList()); + + parent.highlightAsNameOccurences(occurrences); + } + + private void displayAttributes(Node node) { ObservableList atts = getAttributes(node); xpathAttributesListView.setItems(atts); diff --git a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less index 2f45ffa0c0..b6c880de88 100644 --- a/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less +++ b/pmd-ui/src/main/resources/net/sourceforge/pmd/util/fxdesigner/less/syntax-highlighting.less @@ -10,7 +10,7 @@ */ -.list-cell:focused .code { +.list-cell:selected:focused .code { // more readable styles on focused cells w/ blue background &.identifier { From 7c1c5d54e7e89bc8368b783d6d064ae46fb2b5b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 23 Jan 2019 18:34:14 +0100 Subject: [PATCH 26/44] Expose jorje attributes manually --- .../apex/ast/ASTAssignmentExpression.java | 5 ++ .../lang/apex/ast/ASTBooleanExpression.java | 9 ++++ .../lang/apex/ast/ASTLiteralExpression.java | 8 +++ .../lang/apex/ast/ASTPostfixExpression.java | 8 +++ .../lang/apex/ast/ASTPrefixExpression.java | 7 +++ .../lang/apex/ast/ASTReferenceExpression.java | 14 ++++++ .../pmd/lang/apex/ast/AbstractApexNode.java | 8 +-- .../pmd/lang/apex/rule/ApexXpathRuleTest.java | 50 ------------------- 8 files changed, 53 insertions(+), 56 deletions(-) delete mode 100644 pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTAssignmentExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTAssignmentExpression.java index 29f466692b..de3531be71 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTAssignmentExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTAssignmentExpression.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.apex.ast; +import apex.jorje.data.ast.AssignmentOp; import apex.jorje.semantic.ast.expression.AssignmentExpression; public class ASTAssignmentExpression extends AbstractApexNode { @@ -16,4 +17,8 @@ public class ASTAssignmentExpression extends AbstractApexNode { public ASTBooleanExpression(BooleanExpression booleanExpression) { super(booleanExpression); } + @Override public Object jjtAccept(ApexParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + + public BooleanOp getOperator() { + return this.node.getOp(); + } + } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTLiteralExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTLiteralExpression.java index 19e7440fac..35b04e7d37 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTLiteralExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTLiteralExpression.java @@ -4,16 +4,24 @@ package net.sourceforge.pmd.lang.apex.ast; +import apex.jorje.data.ast.LiteralType; import apex.jorje.semantic.ast.expression.LiteralExpression; + public class ASTLiteralExpression extends AbstractApexNode { public ASTLiteralExpression(LiteralExpression literalExpression) { super(literalExpression); } + @Override public Object jjtAccept(ApexParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + + public LiteralType getLiteralType() { + return node.getLiteralType(); + } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPostfixExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPostfixExpression.java index 4d2bd2b017..5a760da8b2 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPostfixExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPostfixExpression.java @@ -4,16 +4,24 @@ package net.sourceforge.pmd.lang.apex.ast; +import apex.jorje.data.ast.PostfixOp; import apex.jorje.semantic.ast.expression.PostfixExpression; + public class ASTPostfixExpression extends AbstractApexNode { public ASTPostfixExpression(PostfixExpression postfixExpression) { super(postfixExpression); } + @Override public Object jjtAccept(ApexParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + + public PostfixOp getOperator() { + return node.getOp(); + } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPrefixExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPrefixExpression.java index 11db2fb4a3..4dd4a20cde 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPrefixExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTPrefixExpression.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.apex.ast; +import apex.jorje.data.ast.PrefixOp; import apex.jorje.semantic.ast.expression.PrefixExpression; public class ASTPrefixExpression extends AbstractApexNode { @@ -16,4 +17,10 @@ public class ASTPrefixExpression extends AbstractApexNode { public Object jjtAccept(ApexParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + + public PrefixOp getOperator() { + return node.getOp(); + } + } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java index 32e91d26bb..ef3db237a3 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java @@ -4,7 +4,10 @@ package net.sourceforge.pmd.lang.apex.ast; +import apex.jorje.semantic.ast.expression.IdentifierContext; import apex.jorje.semantic.ast.expression.ReferenceExpression; +import apex.jorje.semantic.ast.expression.ReferenceType; + public class ASTReferenceExpression extends AbstractApexNode { @@ -12,8 +15,19 @@ public class ASTReferenceExpression extends AbstractApexNode extends AbstractApexNo protected final T node; - public AbstractApexNode(T node) { + protected AbstractApexNode(T node) { super(node.getClass()); this.node = node; } @@ -69,11 +69,7 @@ public abstract class AbstractApexNode extends AbstractApexNo @Override public Iterator getXPathAttributesIterator() { // Attributes of this node have precedence over same-name attributes of the underlying node - return Stream.concat(iteratorToStream(new AttributeAxisIterator(this)), - iteratorToStream(new AttributeAxisIterator(this, node))) - .distinct() - .iterator(); - + return new AttributeAxisIterator(this); } // TODO move to IteratorUtil w/ java 8 diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java deleted file mode 100644 index 2b91d3158c..0000000000 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXpathRuleTest.java +++ /dev/null @@ -1,50 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.apex.rule; - -import static org.junit.Assert.assertTrue; - -import java.util.List; - -import org.junit.Test; - -import net.sourceforge.pmd.internal.util.IteratorUtil; -import net.sourceforge.pmd.lang.apex.ast.ASTBooleanExpression; -import net.sourceforge.pmd.lang.apex.ast.ApexNode; -import net.sourceforge.pmd.lang.apex.ast.ApexParserTestHelpers; -import net.sourceforge.pmd.lang.ast.xpath.Attribute; - -import apex.jorje.semantic.ast.compilation.Compilation; - - -/** - * @author Clément Fournier - * @since 7.0.0 - */ -public class ApexXpathRuleTest { - - @Test - public void testXPathAttributesOfUnderlyingNode() { - - String code = "class MyApexClass {\n" - + " void bar(){\n" - + "\tb f = a && b;\n" - + " if(!x.lit() && lis != null) {\n" - + " foo();\n" - + " }\n" - + " }\n" - + "}"; - - ApexNode compilation = ApexParserTestHelpers.parse(code); - - compilation.findDescendantsOfType(ASTBooleanExpression.class).forEach(expr -> { - List attributes = IteratorUtil.toList(expr.getXPathAttributesIterator()); - - assertTrue(attributes.stream().anyMatch(a -> a.getName().equals("Op"))); - }); - } - - -} From 032fae401e86d9e992de8b38e65963978c811df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 23 Jan 2019 18:41:11 +0100 Subject: [PATCH 27/44] Remove unnecessary override --- .../pmd/lang/apex/ast/AbstractApexNode.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java index e8d699fc00..d68166fa9f 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java @@ -64,16 +64,4 @@ public abstract class AbstractApexNode extends AbstractApexNo return "no location"; } } - - - @Override - public Iterator getXPathAttributesIterator() { - // Attributes of this node have precedence over same-name attributes of the underlying node - return new AttributeAxisIterator(this); - } - - // TODO move to IteratorUtil w/ java 8 - private static Stream iteratorToStream(Iterator it) { - return StreamSupport.stream(Spliterators.spliteratorUnknownSize(it, 0), false); - } } From ead4a78c3b888e47095df47bd36c0e776fa2859d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 23 Jan 2019 19:13:09 +0100 Subject: [PATCH 28/44] unused imports --- .../sourceforge/pmd/lang/apex/ast/AbstractApexNode.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java index d68166fa9f..df99747719 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java @@ -4,14 +4,7 @@ package net.sourceforge.pmd.lang.apex.ast; -import java.util.Iterator; -import java.util.Spliterators; -import java.util.stream.Stream; -import java.util.stream.StreamSupport; - import net.sourceforge.pmd.lang.ast.SourceCodePositioner; -import net.sourceforge.pmd.lang.ast.xpath.Attribute; -import net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator; import apex.jorje.data.Location; import apex.jorje.data.Locations; From b173d11508816c2f41aa531faa921996fe4998c3 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Wed, 23 Jan 2019 20:53:40 +0100 Subject: [PATCH 29/44] [plsql] Parse Exception with function calls in WHERE clause Fixes #1588 --- docs/pages/release_notes.md | 1 + pmd-plsql/etc/grammar/PldocAST.jjt | 30 ++++++++++++++++--- .../pmd/lang/plsql/ast/WhereClauseTest.java | 7 +++++ .../plsql/ast/WhereClauseFunctionCall.pls | 5 ++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 43a401f025..08d2129776 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -79,6 +79,7 @@ This is a {{ site.pmd.release_type }} release. * [#1511](https://github.com/pmd/pmd/issues/1511): \[plsql] Parse Exception with IS NOT NULL * [#1583](https://github.com/pmd/pmd/issues/1583): \[plsql] Update Set Clause should allow multiple columns * [#1586](https://github.com/pmd/pmd/issues/1586): \[plsql] Parse Exception when functions are used with LIKE + * [#1588](https://github.com/pmd/pmd/issues/1588): \[plsql] Parse Exception with function calls in WHERE clause ### API Changes diff --git a/pmd-plsql/etc/grammar/PldocAST.jjt b/pmd-plsql/etc/grammar/PldocAST.jjt index 066a9eab8a..551393e182 100644 --- a/pmd-plsql/etc/grammar/PldocAST.jjt +++ b/pmd-plsql/etc/grammar/PldocAST.jjt @@ -1414,16 +1414,38 @@ ASTSqlExpression SqlExpression() : | LOOKAHEAD(2) | - LOOKAHEAD(2) AdditiveExpression() // this can be a literal or a simple expression, but no conditional + AdditiveExpression() // this can be a literal or a simple expression, but no conditional ) { return jjtThis; } } +/** + * See also https://docs.oracle.com/en/database/oracle/oracle-database/18/sqlrf/About-User-Defined-Functions.html#GUID-4EB3E236-8216-471C-BA44-23D87BDFEA67 + * + * A function reference might be: + * function_name + * package.function_name + * package.schema.function_name + * optional: @ dblink . + */ ASTFunctionCall FunctionCall() : -{ ASTID id; } { - id = ID() { jjtThis.setImage(id.getImage()); } Arguments() - { return jjtThis; } + ASTID id; + StringBuilder name = new StringBuilder(); +} +{ + id = ID() { name.append(id.getImage()); } + [ "." id = ID() { name.append('.').append(id.getImage()); } + [ "." id = ID() { name.append('.').append(id.getImage()); } ] + ] + [ "@" id = ID() { name.append('@').append(id.getImage()); } ] + + Arguments() + + { + jjtThis.setImage(name.toString()); + return jjtThis; + } } ASTColumn Column() : diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/WhereClauseTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/WhereClauseTest.java index 3ccfd30b0c..18d549472a 100644 --- a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/WhereClauseTest.java +++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/WhereClauseTest.java @@ -5,8 +5,10 @@ package net.sourceforge.pmd.lang.plsql.ast; import java.nio.charset.StandardCharsets; +import java.util.List; import org.apache.commons.io.IOUtils; +import org.junit.Assert; import org.junit.Test; import net.sourceforge.pmd.lang.plsql.AbstractPLSQLParserTst; @@ -18,6 +20,11 @@ public class WhereClauseTest extends AbstractPLSQLParserTst { String code = IOUtils.toString(this.getClass().getResourceAsStream("WhereClauseFunctionCall.pls"), StandardCharsets.UTF_8); ASTInput input = parsePLSQL(code); + List selectStatements = input.findDescendantsOfType(ASTSelectIntoStatement.class); + Assert.assertEquals(3, selectStatements.size()); + + ASTFunctionCall functionCall = selectStatements.get(2).getFirstDescendantOfType(ASTFunctionCall.class); + Assert.assertEquals("utils.get_colname", functionCall.getImage()); } @Test diff --git a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/WhereClauseFunctionCall.pls b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/WhereClauseFunctionCall.pls index e2513cfe27..c777b1747c 100644 --- a/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/WhereClauseFunctionCall.pls +++ b/pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/WhereClauseFunctionCall.pls @@ -16,5 +16,10 @@ BEGIN OR rgn.cny_code = street_cny_code_in) AND UPPER(rgn.name) = UPPER(street_rgn_in); + SELECT value + INTO v_value + FROM table + WHERE colname = utils.get_colname('COLUMN_ID'); + END; / From 46ddd7d1f6cb4e296cd1e518128cc5618af5cb69 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 17 Jan 2019 21:12:34 +0100 Subject: [PATCH 30/44] [plsql] ParseException when using TableCollectionExpression Fixes #1526 Also adding InsertStatement --- docs/pages/release_notes.md | 1 + pmd-plsql/etc/grammar/PldocAST.jjt | 133 +++++++++++++++--- .../plsql/ast/PLSQLParserVisitorAdapter.java | 45 ++++++ .../lang/plsql/rule/AbstractPLSQLRule.java | 45 ++++++ .../plsql/ast/TableCollectionExpression.java | 32 +++++ .../ast/TableCollectionExpressionExamples.pls | 19 +++ .../TableCollectionExpressionIssue1526.pls | 9 ++ 7 files changed, 261 insertions(+), 23 deletions(-) create mode 100644 pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/ast/TableCollectionExpression.java create mode 100644 pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/TableCollectionExpressionExamples.pls create mode 100644 pmd-plsql/src/test/resources/net/sourceforge/pmd/lang/plsql/ast/TableCollectionExpressionIssue1526.pls diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 43a401f025..6f26bac871 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -77,6 +77,7 @@ This is a {{ site.pmd.release_type }} release. * [#1508](https://github.com/pmd/pmd/issues/1508): \[plsql] Parse Exception when using SELECT COUNT(\*) * [#1509](https://github.com/pmd/pmd/issues/1509): \[plsql] Parse Exception with OUTER/INNER Joins * [#1511](https://github.com/pmd/pmd/issues/1511): \[plsql] Parse Exception with IS NOT NULL + * [#1526](https://github.com/pmd/pmd/issues/1526): \[plsql] ParseException when using TableCollectionExpression * [#1583](https://github.com/pmd/pmd/issues/1583): \[plsql] Update Set Clause should allow multiple columns * [#1586](https://github.com/pmd/pmd/issues/1586): \[plsql] Parse Exception when functions are used with LIKE diff --git a/pmd-plsql/etc/grammar/PldocAST.jjt b/pmd-plsql/etc/grammar/PldocAST.jjt index 066a9eab8a..7b9f4337cc 100644 --- a/pmd-plsql/etc/grammar/PldocAST.jjt +++ b/pmd-plsql/etc/grammar/PldocAST.jjt @@ -220,9 +220,10 @@ ASTInput Input() : {} | LOOKAHEAD(6) Global() | LOOKAHEAD(6) DDLCommand() //Ignore any other DDL Event | LOOKAHEAD(2) SqlPlusCommand() - | LOOKAHEAD(2) UpdateStatement() - | LOOKAHEAD(2) DeleteStatement() - |(||||||) SkipPastNextTokenOccurrence(SQLPLUS_TERMINATOR) //Ignore SQL statements in scripts ) ("/")* )* @@ -1542,18 +1543,43 @@ ASTHostArrayName HostArrayName() : ASTTableReference TableReference() : {} { - // QueryTableExpression - ( - [ LOOKAHEAD(2) SchemaName() "." ] TableName() - | - "(" Subquery() ")" - ) + QueryTableExpression() [ LOOKAHEAD(2) TableAlias() ] { return jjtThis; } } +void QueryTableExpression() #void : +{} +{ + ( + LOOKAHEAD(2) [ LOOKAHEAD(2) SchemaName() "." ] TableName() + | + LOOKAHEAD(2) TableName() + | + TableCollectionExpression() + | + [ ] "(" Subquery() [ SubqueryRestrictionClause() ] ")" + ) +} + +ASTSubqueryRestrictionClause SubqueryRestrictionClause() : +{} +{ + ( |
"(" (LOOKAHEAD(3) Subquery() | Expression()) ")" [ "(" "+" ")" ] + + { return jjtThis; } +} + /** * Special production, used in joins. The table reference might have * a table alias, but this should not match any following NATURAL, CROSS, etc. @@ -1563,12 +1589,7 @@ ASTTableReference TableReference() : ASTTableReference TableReferenceInJoin() #TableReference : {} { - // QueryTableExpression - ( - [ LOOKAHEAD(2) SchemaName() "." ] TableName() - | - "(" Subquery() ")" - ) + QueryTableExpression() [ LOOKAHEAD(1, ID(), {getToken(1).kind != NATURAL && getToken(1).kind != CROSS @@ -1762,6 +1783,7 @@ ASTUnlabelledStatement UnlabelledStatement() : SelectIntoStatement() ";" | UpdateStatement() ";" | DeleteStatement() ";" | + InsertStatement() ";" | LOOKAHEAD(["("]
||) SqlStatement(null,";") [";"] | LOOKAHEAD(3) ContinueStatement() ";" // CONTINUE keyword was added in 11G, so Oracle compilation supports CONTINUE as a variable name | CaseStatement() ";" @@ -1871,6 +1893,52 @@ ASTCursorForLoopStatement CursorForLoopStatement() : { return jjtThis ; } } +ASTInsertStatement InsertStatement() : +{} +{ + ( SingleTableInsert() | MultiTableInsert() ) + { return jjtThis; } +} + +ASTSingleTableInsert SingleTableInsert() : +{} +{ + InsertIntoClause() ( ValuesClause() | Subquery() ) + { return jjtThis; } +} + +ASTInsertIntoClause InsertIntoClause() : +{} +{ + DMLTableExpressionClause() [ LOOKAHEAD(2) TableAlias() ] [ LOOKAHEAD(2) "(" Column() ( "," Column() )* ")" ] + { return jjtThis; } +} + +ASTValuesClause ValuesClause() : +{} +{ + "(" ( Expression() | <_DEFAULT> ) ( "," ( Expression() | <_DEFAULT> ) )* ")" + { return jjtThis; } +} +ASTMultiTableInsert MultiTableInsert() : +{} +{ + ( + LOOKAHEAD(2) ( InsertIntoClause() [ ValuesClause() ] )+ Subquery() + | + ConditionalInsertClause() + ) + { return jjtThis; } +} + +ASTConditionalInsertClause ConditionalInsertClause() : +{} +{ + [ | ] ( Condition() ( InsertIntoClause() [ ValuesClause() ] )+ )+ + [ ( InsertIntoClause() [ ValuesClause() ] )+ ] + { return jjtThis; } +} + ASTSelectStatement SelectStatement() : {} { @@ -1892,21 +1960,37 @@ ASTSelectStatement SelectStatement() : ASTUpdateStatement UpdateStatement() : {} { - SqlExpression() + DMLTableExpressionClause() [ LOOKAHEAD(2) TableAlias() ] UpdateSetClause() [ WhereClause() ] { return jjtThis; } } +ASTDMLTableExpressionClause DMLTableExpressionClause() : +{} +{ + ( + LOOKAHEAD(2) TableName() + | + TableCollectionExpression() + | + LOOKAHEAD(2) [ LOOKAHEAD(2) SchemaName() "." ] TableName() + | + [ ] "(" Subquery() [ SubqueryRestrictionClause() ] ")" + ) + + { return jjtThis; } +} + ASTUpdateSetClause UpdateSetClause() : {} { ( - "=" ID() + LOOKAHEAD(3) "=" ID() | - Column() "=" ( Expression() | <_DEFAULT> ) - ( "," Column() "=" ( Expression() | <_DEFAULT> )*)* + [ LOOKAHEAD(2) TableName() "." ] Column() "=" ( Expression() | <_DEFAULT> ) + ( "," [ LOOKAHEAD(2) TableName() "." ] Column() "=" ( Expression() | <_DEFAULT> ) )* ) { return jjtThis; } } @@ -4405,6 +4489,9 @@ TOKEN [IGNORE_CASE]: | | | + +| +| } /** @@ -4991,7 +5078,7 @@ ASTKEYWORD_UNRESERVED KEYWORD_UNRESERVED (): {} //| | | -//| +| //| //| //| @@ -5342,7 +5429,7 @@ ASTKEYWORD_UNRESERVED KEYWORD_UNRESERVED (): {} //| //| //| -//| +| //| | //| @@ -5447,7 +5534,7 @@ ASTKEYWORD_UNRESERVED KEYWORD_UNRESERVED (): {} //| | //| -| //SET is defined as a reserved word but is used in "SYS"."DBMS_RESULT_CACHE_API" as a function name and as a Pragma parameter +//| //SET is defined as a reserved word but is used in "SYS"."DBMS_RESULT_CACHE_API" as a function name and as a Pragma parameter //| | //| @@ -5833,7 +5920,7 @@ ASTID ID(): {} | | //RESERVED WORD | - |
//RESERVED WORD + //|
//RESERVED WORD | //SYNTAX //RESERVED WORD // |