Lint: Fix some PMD issues

Signed-off-by: Taylor Smock <[email protected]>

Author: tsmock

CHANGELOG.md

@@ -1,4 +1,7 @@
 ## What's Changed
+### [Unreleased](https://github.com/JOSM/Mapillary/compare/v2.0.0-beta.2...master)
+* Lint fixes (PMD)
+
 ### [v2.0.0-beta.2](https://github.com/JOSM/Mapillary/compare/v2.0.0-beta.1...v2.0.0-beta.2)
 * JOSM [#20274](https://josm.openstreetmap.de/ticket/20274): Cannot zoom in to detail.
   * Mapillary has added an API field for thumb_original_url, which has the original (processed) image.

build.gradle.kts

@@ -61,7 +61,7 @@ val versions = mapOf(
   "jdatepicker" to "1.3.4",
   "jmockit" to "1.49",
   "junit" to "5.8.2",
-  "pmd" to "6.41.0",
+  "pmd" to "6.42.0",
   "spotbugs" to "4.5.3",
   "wiremock" to "2.32.0"
 )

config/pmd/ruleset.xml

@@ -60,7 +60,8 @@
   <rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn" />
   <rule ref="category/java/codestyle.xml/UnnecessaryModifier" />
   <rule ref="category/java/codestyle.xml/UnnecessaryReturn" />
-  <rule ref="category/java/codestyle.xml/UselessParentheses" />
+  <!-- As of PMD 6.41.0, UselessParentheses conflicts with ErrorProne 2.11.0 OperatorPrecedence. -->
+  <!--<rule ref="category/java/codestyle.xml/UselessParentheses" />-->
   <rule ref="category/java/codestyle.xml/UselessQualifiedThis" />
 
   <rule ref="category/java/design.xml/AbstractClassWithoutAnyMethod" />

src/main/java/org/openstreetmap/josm/plugins/mapillary/MapillaryKeyListener.java

@@ -39,6 +39,7 @@
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.JosmRuntimeException;
 import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
  * Listen for Mapillary keys
@@ -80,7 +81,7 @@ public void popupMenuCanceled(PopupMenuEvent e) {
     }
 
     private void addAction(JPopupMenu popup, String key, String value) {
-        if (value == null || value.trim().isEmpty() || !MapillaryLayer.hasInstance()) {
+        if (Utils.isBlank(value) || !MapillaryLayer.hasInstance()) {
             return;
         }
         String lowerKey = key.toLowerCase(Locale.ENGLISH);

src/main/java/org/openstreetmap/josm/plugins/mapillary/MapillaryLayerListener.java

@@ -35,7 +35,7 @@ public void layerAdded(LayerManager.LayerAddEvent e) {
         if (e.getAddedLayer() instanceof MapillaryLayer) {
             VectorDataSet dataSet = ((MapillaryLayer) e.getAddedLayer()).getData();
             dataSet.addSelectionListener((VectorDataSelectionListener) ImageInfoPanel.getInstance());
-            dataSet.addSelectionListener(ImageDialogListener.instance);
+            dataSet.addSelectionListener(ImageDialogListener.INSTANCE);
             Stream.of(MapillaryPlugin.getMapillaryDataListeners()).forEach(dataSet::addSelectionListener);
         }
     }
@@ -45,7 +45,7 @@ public void layerRemoving(LayerManager.LayerRemoveEvent e) {
         if (e.getRemovedLayer() instanceof MapillaryLayer) {
             VectorDataSet dataSet = ((MapillaryLayer) e.getRemovedLayer()).getData();
             dataSet.removeSelectionListener((VectorDataSelectionListener) ImageInfoPanel.getInstance());
-            dataSet.removeSelectionListener(ImageDialogListener.instance);
+            dataSet.removeSelectionListener(ImageDialogListener.INSTANCE);
             Stream.of(MapillaryPlugin.getMapillaryDataListeners()).forEach(dataSet::removeSelectionListener);
         }
     }
@@ -64,7 +64,7 @@ public void destroy() {
      * A class that listens for dataset updates and updates the image viewer
      */
     private static class ImageDialogListener implements VectorDataSelectionListener {
-        private static final ImageDialogListener instance = new ImageDialogListener();
+        private static final ImageDialogListener INSTANCE = new ImageDialogListener();
 
         @Override
         public void selectionChanged(

src/main/java/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryDownloadAction.java

@@ -6,6 +6,7 @@
 import java.awt.event.ActionEvent;
 import java.awt.event.KeyEvent;
 import java.util.List;
+import java.util.Objects;
 
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.gui.MainApplication;
@@ -52,7 +53,7 @@ public void actionPerformed(ActionEvent ae) {
             // layer
             final OsmDataLayer editLayer = MainApplication.getLayerManager().getEditLayer();
             final Layer mapillaryLayer = MapillaryLayer.getInstance();
-            if (MainApplication.getLayerManager().getActiveLayer() != mapillaryLayer) {
+            if (Objects.equals(MainApplication.getLayerManager().getActiveLayer(), mapillaryLayer)) {
                 MainApplication.getLayerManager().setActiveLayer(mapillaryLayer);
             } else if (editLayer != null) {
                 MainApplication.getLayerManager().setActiveLayer(editLayer);

src/main/java/org/openstreetmap/josm/plugins/mapillary/actions/SmartEditAddAction.java

@@ -62,7 +62,7 @@ public void actionPerformed(ActionEvent e) {
     }
 
     @Override
-    public void updateEnabledState() {
+    public final void updateEnabledState() {
         this.setEnabled(!this.detection.getTaggingPresets().isEmpty());
     }
 
@@ -77,11 +77,11 @@ void addMapillaryPrimitiveToOsm(final IPrimitive mapillaryObject, final ObjectDe
         final Collection<TaggingPreset> presets = detection.getTaggingPresets();
         final DataSet dataSet = MainApplication.getLayerManager().getActiveDataSet();
         if (dataSet != null && !dataSet.isLocked() && presets.size() == 1) {
-            final TaggingPreset preset = presets.iterator().next();
             final List<OsmPrimitive> toAdd = generateToAdd();
             if (toAdd.isEmpty()) {
                 return;
             }
+            final TaggingPreset preset = presets.iterator().next();
             final OsmPrimitive basePrimitive = toAdd.get(0);
             basePrimitive.removeAll();
             detection.getOsmKeys().forEach(basePrimitive::put);
@@ -98,7 +98,6 @@ void addMapillaryPrimitiveToOsm(final IPrimitive mapillaryObject, final ObjectDe
                     preset.getChangedTags());
             });
             final int userSelection = tSelection[0];
-            final Command updateTagsCommand = tCommand[0];
             // Closing the window returns 0. Not in the TaggingPreset public answers at this time.
             if ((userSelection == 0 || userSelection == TaggingPreset.DIALOG_ANSWER_CANCEL)
                 && UndoRedoHandler.getInstance().hasUndoCommands()) {
@@ -113,7 +112,7 @@ void addMapillaryPrimitiveToOsm(final IPrimitive mapillaryObject, final ObjectDe
             }
             addMapillaryTags(addedPrimitive);
 
-            generateCommands(addedPrimitive, updateTagsCommand);
+            generateCommands(addedPrimitive, tCommand[0]);
             this.pointObjectLayer.getData().setSelected(this.pointObjectLayer.getData().getSelected().stream()
                 .filter(n -> !n.equals(this.mapillaryObject)).collect(Collectors.toList()));
         }

src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/ImageCheckBoxButton.java

@@ -44,7 +44,7 @@ public class ImageCheckBoxButton extends JPanel implements Destroyable, TableMod
     public ImageCheckBoxButton(ImageIcon icon, String detection, ObjectDetections... detections) {
         super(new GridBagLayout());
         this.detection = detection;
-        this.detections = detections;
+        this.detections = detections.clone();
         MapillaryExpertFilterDialog.getInstance().getFilterModel().addTableModelListener(this);
         splitName = detection.split("--", -1);
         image = new JButton();

src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/MapillaryExpertFilterDialog.java

@@ -1,6 +1,7 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.mapillary.gui.dialog;
 
+import static org.openstreetmap.josm.tools.I18n.marktr;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.Component;
@@ -60,6 +61,18 @@
  */
 public class MapillaryExpertFilterDialog extends ToggleDialog implements DataSetListener, MapModeChangeListener {
     private static final long serialVersionUID = -3067786777238478114L;
+    private static final String ENABLE_FILTER_STRING = marktr("Enable filter");
+    private static final Shortcut ENABLE_FILTER_SHORTCUT = Shortcut.registerShortcut("mapillary_multikey:enableFilter",
+        tr("Multikey: {0}", tr(ENABLE_FILTER_STRING)), KeyEvent.CHAR_UNDEFINED, Shortcut.NONE);
+
+    private static final Shortcut HIDING_FILTER_SHORTCUT = Shortcut.registerShortcut("mapillary_multikey:hidingFilter",
+        tr("Multikey: {0}", tr("Hide filter")), KeyEvent.CHAR_UNDEFINED, Shortcut.NONE);
+
+    private static final String[] COLUMN_TOOLTIPS = {
+        Shortcut.makeTooltip(tr(ENABLE_FILTER_STRING), ENABLE_FILTER_SHORTCUT.getKeyStroke()),
+        Shortcut.makeTooltip(tr("Hiding filter"), HIDING_FILTER_SHORTCUT.getKeyStroke()), null, tr("Inverse filter"),
+        tr("Filter mode") };
+
     private JTable userTable;
     private final MapillaryFilterTableModel filterModel = new MapillaryFilterTableModel(
         new DefaultListSelectionModel());
@@ -96,23 +109,17 @@ public MapillaryExpertFilterDialog() {
      * @return The instance for the toggle dialog
      */
     public static MapillaryExpertFilterDialog getInstance() {
-        if (instance == null) {
-            instance = new MapillaryExpertFilterDialog();
+        if (instance != null) {
+            return instance;
+        }
+        synchronized (MapillaryExpertFilterDialog.class) {
+            if (instance == null) {
+                instance = new MapillaryExpertFilterDialog();
+            }
         }
         return instance;
     }
 
-    private static final Shortcut ENABLE_FILTER_SHORTCUT = Shortcut.registerShortcut("mapillary_multikey:enableFilter",
-        tr("Multikey: {0}", tr("Enable filter")), KeyEvent.CHAR_UNDEFINED, Shortcut.NONE);
-
-    private static final Shortcut HIDING_FILTER_SHORTCUT = Shortcut.registerShortcut("mapillary_multikey:hidingFilter",
-        tr("Multikey: {0}", tr("Hide filter")), KeyEvent.CHAR_UNDEFINED, Shortcut.NONE);
-
-    private static final String[] COLUMN_TOOLTIPS = {
-        Shortcut.makeTooltip(tr("Enable filter"), ENABLE_FILTER_SHORTCUT.getKeyStroke()),
-        Shortcut.makeTooltip(tr("Hiding filter"), HIDING_FILTER_SHORTCUT.getKeyStroke()), null, tr("Inverse filter"),
-        tr("Filter mode") };
-
     private abstract class FilterAction extends AbstractAction implements IEnabledStateUpdating {
 
         FilterAction(String name, String description, String icon) {
@@ -253,7 +260,7 @@ public void updateEnabledState() {
     /**
      * Builds the GUI.
      */
-    protected void build() {
+    private void build() {
         userTable = new UserTable(filterModel);
 
         userTable.putClientProperty("terminateEditOnFocusLost", Boolean.TRUE);
@@ -306,11 +313,11 @@ public void actionPerformed(ActionEvent e) {
     @Override
     public void destroy() {
         if (!destroyed) {
+            destroyed = true;
             MultikeyActionsHandler.getInstance().removeAction(enableFilterAction);
             MultikeyActionsHandler.getInstance().removeAction(hidingFilterAction);
             instance = null;
             super.destroy();
-            destroyed = true;
         }
     }
 
@@ -464,7 +471,7 @@ public MultikeyInfo getLastMultikeyAction() {
     private class EnableFilterAction extends AbstractFilterAction {
 
         EnableFilterAction() {
-            putValue(SHORT_DESCRIPTION, tr("Enable filter"));
+            putValue(SHORT_DESCRIPTION, tr(ENABLE_FILTER_STRING));
             ENABLE_FILTER_SHORTCUT.setAccelerator(this);
         }
 

src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/MapillaryFilterDialog.java

@@ -519,7 +519,7 @@ public boolean test(INode img) {
         /**
          * Update the layer visibility
          */
-        void updateLayerVisible() {
+        final void updateLayerVisible() {
             this.layerVisible = MapillaryLayer.hasInstance() && MapillaryLayer.getInstance().isVisible();
         }
 
@@ -687,7 +687,7 @@ public void organizationAdded(OrganizationRecord organization) {
         }
         for (Component comp : Arrays.asList(organizationLabel, organizations)) {
             GuiHelper.runInEDT(() -> comp
-                .setEnabled(organizations.getItemCount() > 1 || organization != OrganizationRecord.NULL_RECORD));
+                .setEnabled(organizations.getItemCount() > 1 || !OrganizationRecord.NULL_RECORD.equals(organization)));
         }
     }