Skip to content

Commit

Permalink
Merge pull request #77 from adangel:check-with-pmd7
Browse files Browse the repository at this point in the history
Enable PMD checks with PMD 7 #77
  • Loading branch information
adangel committed Feb 3, 2024
2 parents c0f94ef + acec04f commit ed9d9f1
Show file tree
Hide file tree
Showing 24 changed files with 78 additions and 75 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
branches:
- main
- master
- compat-7.0
tags:
- '**'
pull_request:
Expand Down Expand Up @@ -37,7 +36,7 @@ jobs:
run: |
echo "LANG=en_US.UTF-8" >> $GITHUB_ENV
echo "MAVEN_OPTS=-Dmaven.wagon.httpconnectionManager.ttlSeconds=180 -Dmaven.wagon.http.retryHandler.count=3" >> $GITHUB_ENV
echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/20/scripts" >> $GITHUB_ENV
echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/master/scripts" >> $GITHUB_ENV
- name: Check Environment
shell: bash
run: |
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

* **Bump required pmd-core version to 7.0.0-SNAPSHOT.**

**Merged pull requests:**

* [#77](https://github.com/pmd/pmd-designer/pull/77) Enable PMD checks with PMD 7 by [@adangel](https://github.com/adangel)

## 7.0.0-rc4

* **Bump required pmd-core version to 7.0.0-rc4.**
Expand Down
42 changes: 19 additions & 23 deletions config/pmd-check.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@
</description>
<priority>1</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value><![CDATA[
//ImportDeclaration/Name[starts-with(@Image, "net.sourceforge.pmd.") and contains(@Image, ".internal")]
//ImportDeclaration[starts-with(@PackageName, "net.sourceforge.pmd.") and contains(@PackageName, ".internal")]
]]></value>
</property>
</properties>
Expand All @@ -47,7 +46,6 @@
<priority>1</priority>
<properties>
<property name="apiVersion" type="String" value="8" description="JavaFX API version to enforce"/>
<property name="version" value="2.0"/>
<property name="xpath">
<value><![CDATA[
//*[starts-with(@xmlns,"http://javafx.com/javafx/") and xmlns != string-join(("http://javafx.com/javafx/", $apiVersion), "")]
Expand All @@ -67,28 +65,26 @@
</description>
<priority>1</priority>
<properties>
<property name="version" value="2.0" />
<property name="xpath">
<value><![CDATA[
//ClassOrInterfaceBodyDeclaration[Annotation[pmd-java:typeIs('javafx.fxml.FXML')]]
/FieldDeclaration/VariableDeclarator/VariableDeclaratorId
[not(ends-with(@Name, ../../Type/@TypeImage))]
(: The following are the exceptions to the rule :)
[not(ends-with(@Name, 'Controller') and ends-with(../../Type/@TypeImage, 'Controller'))]
[not(../../Type/@TypeImage = 'TextField' and ends-with(@Name, 'Field'))]
[not(../../Type/@TypeImage = 'ToggleButton' and ends-with(@Name, 'Toggle'))]
[not(../../Type/@TypeImage = 'TextArea' or ends-with(../../Type/@TypeImage, 'CodeArea') and ends-with(@Name, 'Area'))]
[not(../../Type/@TypeImage = 'TableColumn' and ends-with(@Name, 'Column'))]
[not(../../Type/@TypeImage = 'MenuItem' and ends-with(@Name, 'Button'))]
[not(ends-with(../../Type/@TypeImage, 'Choicebox') and ends-with(@Name, 'Choicebox'))]
[not(ends-with(../../Type/@TypeImage, 'TitledPane') and ends-with(@Name, 'Pane'))]
(: This last clause allows variables to be named the same as their type, modulo Camel case :)
(: Ideally we would only allow this for our custom types, but there's currently no easy :)
(: way to get the type name of a node to check the package. :)
(: We should create a function for that, eg typeNameOf :)
[not(string-length(../../Type/@TypeImage) = string-length(@Name)
and substring(../../Type/@TypeImage, 2) = substring(@Name, 2))]
//FieldDeclaration[ModifierList/Annotation[pmd-java:typeIs('javafx.fxml.FXML')]]
/VariableDeclarator/VariableDeclaratorId
[not(ends-with(@Name, ../../ClassOrInterfaceType/@SimpleName))]
(: The following are the exceptions to the rule :)
[not(ends-with(@Name, 'Controller') and ends-with(../../ClassOrInterfaceType/@SimpleName, 'Controller'))]
[not(../../ClassOrInterfaceType/@SimpleName = 'TextField' and ends-with(@Name, 'Field'))]
[not(../../ClassOrInterfaceType/@SimpleName = 'ToggleButton' and ends-with(@Name, 'Toggle'))]
[not(../../ClassOrInterfaceType/@SimpleName = 'TextArea' or ends-with(../../ClassOrInterfaceType/@SimpleName, 'CodeArea') and ends-with(@Name, 'Area'))]
[not(../../ClassOrInterfaceType/@SimpleName = 'TableColumn' and ends-with(@Name, 'Column'))]
[not(../../ClassOrInterfaceType/@SimpleName = 'MenuItem' and ends-with(@Name, 'Button'))]
[not(ends-with(../../ClassOrInterfaceType/@SimpleName, 'Choicebox') and ends-with(@Name, 'Choicebox'))]
[not(ends-with(../../ClassOrInterfaceType/@SimpleName, 'TitledPane') and ends-with(@Name, 'Pane'))]
(: This last clause allows variables to be named the same as their type, modulo Camel case :)
(: Ideally we would only allow this for our custom types, but there's currently no easy :)
(: way to get the type name of a node to check the package. :)
(: We should create a function for that, eg typeNameOf :)
[not(string-length(../../ClassOrInterfaceType/@SimpleName) = string-length(@Name)
and substring(../../ClassOrInterfaceType/@SimpleName, 2) = substring(@Name, 2))]
]]></value>
</property>
</properties>
Expand Down
21 changes: 18 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>

<pmd.build-tools.version>21</pmd.build-tools.version>
<pmd.plugin.version>3.21.0</pmd.plugin.version>
<pmd.check.version>6.55.0</pmd.check.version>
<pmd.build-tools.version>23-SNAPSHOT</pmd.build-tools.version>
<pmd.plugin.version>3.21.2</pmd.plugin.version>
<pmd.check.version>7.0.0-rc4</pmd.check.version>

<jflex-output>${project.build.directory}/generated-sources/jflex</jflex-output>

Expand Down Expand Up @@ -280,6 +280,11 @@
<printFailingErrors>true</printFailingErrors>
</configuration>
<dependencies>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-compat6</artifactId>
<version>${pmd.check.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-core</artifactId>
Expand All @@ -295,6 +300,16 @@
<artifactId>pmd-java</artifactId>
<version>${pmd.check.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-javascript</artifactId>
<version>${pmd.check.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-jsp</artifactId>
<version>${pmd.check.version}</version>
</dependency>
<!-- This contains the main dogfood ruleset -->
<dependency>
<groupId>net.sourceforge.pmd</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import net.sourceforge.pmd.lang.rule.xpath.Attribute;
import net.sourceforge.pmd.util.fxdesigner.app.DesignerParams;
import net.sourceforge.pmd.util.fxdesigner.app.DesignerRoot;
import net.sourceforge.pmd.util.fxdesigner.app.DesignerRootImpl;
Expand Down Expand Up @@ -78,8 +75,6 @@ public void start(Stage stage, DesignerRoot owner) throws IOException {
stage.setTitle("PMD Rule Designer (v " + DesignerVersion.getCurrentVersion() + ')');
setIcons(stage);

Logger.getLogger(Attribute.class.getName()).setLevel(Level.OFF);

System.out.println(stage.getTitle() + " initializing... ");

FXMLLoader loader = new FXMLLoader(DesignerUtil.getFxml("designer"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Deque;
import java.util.List;
import java.util.Objects;
import java.util.Stack;
import java.util.stream.Collectors;

import org.checkerframework.checker.nullness.qual.NonNull;
Expand Down Expand Up @@ -123,7 +123,7 @@ public class MainDesignerController extends AbstractController {
private DynamicWidthChoicebox<Language> languageChoicebox;

// Other fields
private final Stack<File> recentFiles = new LimitedSizeStack<>(5);
private final Deque<File> recentFiles = new LimitedSizeStack<>(5);

public MainDesignerController(@NamedArg("designerRoot") DesignerRoot designerRoot) {
super(designerRoot);
Expand Down Expand Up @@ -293,7 +293,7 @@ private void updateRecentFilesMenu() {

@PersistentProperty
public List<File> getRecentFiles() {
return recentFiles;
return new ArrayList<>(recentFiles);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package net.sourceforge.pmd.util.fxdesigner;

import java.util.ArrayList;
import java.util.List;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.reactfx.EventStreams;
Expand Down Expand Up @@ -88,7 +89,7 @@ private ObservableList<MetricResult<?>> evaluateAllMetrics(Node n) {
if (provider == null) {
return FXCollections.emptyObservableList();
}
ArrayList<MetricResult<?>> results = new ArrayList<>();
List<MetricResult<?>> results = new ArrayList<>();
for (Metric<?, ?> metric : provider.getMetrics()) {
MetricResult<?> result = computeMetric(metric, n);
if (result != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ default Val<Node> initNodeSelectionHandling(DesignerRoot root,
}


class NodeSelectionEvent {
final class NodeSelectionEvent {

// RRR data class

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.time.Duration;
import java.util.EnumSet;
import java.util.Objects;
import java.util.Set;

import org.reactfx.EventSource;
import org.reactfx.EventStream;
Expand Down Expand Up @@ -95,8 +96,8 @@ public void logEvent(LogEntry event) {


private static EventStream<LogEntry> filterOnCategory(EventStream<LogEntry> input, boolean complement, Category first, Category... selection) {
EnumSet<Category> considered = EnumSet.of(first, selection);
EnumSet<Category> complemented = complement ? EnumSet.complementOf(considered) : considered;
EnumSet<Category> considered = EnumSet.of(first, selection); // NOPMD - need to use EnumSet for next line EnumSet.complementOf
Set<Category> complemented = complement ? EnumSet.complementOf(considered) : considered;

return input.filter(e -> complemented.contains(e.getCategory()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private static RebindSubscription<PropertyDescriptorSpec> makeDefault(PropertyDe
return RebindSubscription.make(
() -> mapping.remove(p),
addedP -> {
if (addedP == p) {
if (addedP.equals(p)) {
return makeDefault(p, mapping);
} else {
mapping.remove(p);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private void appendSingle(Element testCode, LiveTestCase descriptor, Document do
numViolations.setTextContent(expectedViolations.size() + "");
testCode.appendChild(numViolations);

if (expectedViolations.size() > 0 && expectedViolations.stream().allMatch(it -> it.getMessage() != null)) {
if (!expectedViolations.isEmpty() && expectedViolations.stream().allMatch(it -> it.getMessage() != null)) {
Element messages = doc.createElementNS(NS, "expected-messages");
for (LiveViolationRecord record : expectedViolations) {
Element r = doc.createElementNS(NS, "message");
Expand All @@ -96,7 +96,7 @@ private void appendSingle(Element testCode, LiveTestCase descriptor, Document do
testCode.appendChild(messages);
}

if (expectedViolations.size() > 0) {
if (!expectedViolations.isEmpty()) {
Element linenos = doc.createElementNS(NS, "expected-linenumbers");

// create a text doc just to ask for line numbers
Expand All @@ -119,7 +119,7 @@ private void appendSingle(Element testCode, LiveTestCase descriptor, Document do

boolean hasNonDefaultVersion = descriptor.languageVersionProperty()
.getOpt()
.filter(it -> it.getLanguage().getDefaultVersion() != it)
.filter(it -> !it.getLanguage().getDefaultVersion().equals(it))
.isPresent();
if (hasNonDefaultVersion) {
Element sourceType = doc.createElementNS(NS, "source-type");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ChangeListener;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.collections.transformation.SortedList;
import javafx.css.PseudoClass;
import javafx.fxml.FXML;
Expand Down Expand Up @@ -183,7 +184,7 @@ private Subscription bindPopupToThisController() {
// reset error nodes on closing
binding = binding.and(() -> selectedErrorNodes.setValue(Collections.emptyList()));

SortedList<LogEntry> logEntries = new SortedList<>(getLogger().getLog(), Comparator.reverseOrder());
ObservableList<LogEntry> logEntries = new SortedList<>(getLogger().getLog(), Comparator.reverseOrder());
eventLogTableView.itemsProperty().setValue(logEntries);
binding = binding.and(
() -> eventLogTableView.itemsProperty().setValue(FXCollections.emptyObservableList())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private DesignerUtil() {


public static <T> Set<T> setOf(T... ts) {
LinkedHashSet<T> set = new LinkedHashSet<>(ts.length);
Set<T> set = new LinkedHashSet<>(ts.length);
Collections.addAll(set, ts);
return set;
}
Expand Down Expand Up @@ -400,7 +400,7 @@ public static BuilderFactory customBuilderFactory(@NonNull DesignerRoot owner) {
// Controls that need the DesignerRoot can declare a constructor
// with a parameter w/ signature @NamedArg("designerRoot") DesignerRoot
// to be injected with the relevant instance of the app.
ProxyBuilder<Object> builder = new ProxyBuilder<>(type);
ProxyBuilder<Object> builder = new ProxyBuilder<>(type); // NOPMD - can't use Map<> here: it's a builder, not a map.
builder.put("designerRoot", owner);
return builder;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

package net.sourceforge.pmd.util.fxdesigner.util;

import java.util.Stack;
import java.util.ArrayDeque;

/**
* Stack with a limited size, without duplicates, without null value. Used to store recent files.
Expand All @@ -14,7 +14,7 @@
* @author Clément Fournier
* @since 6.0.0
*/
public class LimitedSizeStack<E> extends Stack<E> {
public class LimitedSizeStack<E> extends ArrayDeque<E> {

private final int maxSize;

Expand All @@ -25,19 +25,17 @@ public LimitedSizeStack(int maxSize) {


@Override
public E push(E item) {
public void push(E item) {
if (item == null) {
return null;
return;
}

this.remove(item);

super.push(item);

if (size() > maxSize) {
this.removeElementAt(size() - 1);
this.removeLast();
}

return item;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.logging.Logger;

import org.w3c.dom.Element;
import org.w3c.dom.Node;
Expand All @@ -30,8 +29,6 @@
*/
public class XmlInterfaceImpl extends XmlInterface {

private static final Logger LOGGER = Logger.getLogger(XmlInterface.class.getName());

// names used in the Xml schema
private static final String SCHEMA_NODE_ELEMENT = "node";
private static final String SCHEMA_NODESEQ_ELEMENT = "nodeseq";
Expand Down Expand Up @@ -83,7 +80,7 @@ protected SimpleBeanModelNode parseSettingsOwnerNode(Element nodeElement) {
node.addChild(parseSettingsOwnerNode(child));
}
} catch (ClassNotFoundException e) {
LOGGER.warning("Ignoring unknown settings node of type " + child.getAttribute(SCHEMA_NODE_CLASS_ATTRIBUTE));
System.out.println("Ignoring unknown settings node of type " + child.getAttribute(SCHEMA_NODE_CLASS_ATTRIBUTE));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private StyleSpans<Collection<String>> recomputePainting() {
final StyleSpans<Collection<String>> base = allSpans.get(0);

return allSpans.stream()
.filter(spans -> spans != base)
.filter(spans -> !base.equals(spans))
.filter(spans -> spans.length() <= getLength())
.reduce(base, (accumulator, elt) -> accumulator.overlay(elt, SyntaxHighlightingCodeArea::additiveOverlay));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,6 @@ private void setCurrentSyntaxHighlight(final @Nullable StyleSpans<Collection<Str
setStyleSpans(0, styleSyntaxHighlightChange(oldSyntaxHighlight, newSyntax));
}

@Override
public void setStyleSpans(int from, @NonNull StyleSpans<? extends Collection<String>> styleSpans) {
super.setStyleSpans(from, styleSpans);
}

/**
* Given the old value of the highlighting spans, and a newly computed value,
* computes the spans as they should be applied to the codearea. The default behaviour
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public static void subscribeOnSkin(Control node,

public static void copyToClipboardButton(Button button, Supplier<String> copiedText) {
button.setOnAction(e -> {
final ClipboardContent content = new ClipboardContent();
final ClipboardContent content = new ClipboardContent(); // NOPMD - can't use Map<> because of putString(...) method
content.putString(copiedText.get());
Clipboard.getSystemClipboard().setContent(content);
SimplePopups.showActionFeedback(button, AlertType.CONFIRMATION, "Copied to clipboard");
Expand Down
Loading

0 comments on commit ed9d9f1

Please sign in to comment.