Changeset 18485 in josm

Timestamp:

2022-06-09T19:22:55+02:00 (23 months ago)

Author:

taylor.smock

Message:

SonarLint: Fix most recent code smells

This also fixes some tests so that the JOSMTestRules is more
accurate for them.

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/SessionSaveAction.java (modified) (9 diffs)
• src/org/openstreetmap/josm/actions/mapmode/SelectAction.java (modified) (3 diffs)
• src/org/openstreetmap/josm/data/imagery/vectortile/mapbox/Feature.java (modified) (4 diffs)
• test/unit/org/openstreetmap/josm/actions/CopyActionTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/actions/DeleteLayerActionTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/actions/PurgeActionTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/actions/UnJoinNodeWayActionTest.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/actions/UploadActionTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/actions/SessionSaveAction.java

@@ -81 +81 @@
     private static final String TOOLTIP_DEFAULT = tr("Save the current session.");
 
-    protected FileFilter joz = new ExtensionFileFilter("joz", "joz", tr("Session file (archive) (*.joz)"));
-    protected FileFilter jos = new ExtensionFileFilter("jos", "jos", tr("Session file (*.jos)"));
+    protected transient FileFilter joz = new ExtensionFileFilter("joz", "joz", tr("Session file (archive) (*.joz)"));
+    protected transient FileFilter jos = new ExtensionFileFilter("jos", "jos", tr("Session file (*.jos)"));
 
     private File removeFileOnSuccess;
 
     private static String tooltip = TOOLTIP_DEFAULT;
-    protected static File sessionFile;
-    protected static boolean isZipSessionFile;
-    protected static List<WeakReference<Layer>> layersInSessionFile;
+    static File sessionFile;
+    static boolean isZipSessionFile;
+    static List<WeakReference<Layer>> layersInSessionFile;
 
     private static final SessionSaveAction instance = new SessionSaveAction();
@@ -97 +97 @@
      * @return the instance
      */
-    public static final SessionSaveAction getInstance() {
+    public static SessionSaveAction getInstance() {
         return instance;
     }
@@ -132 +132 @@
         try {
             saveSession(false, false);
-        } catch (UserCancelException ignore) {
-            Logging.trace(ignore);
+        } catch (UserCancelException exception) {
+            Logging.trace(exception);
         }
     }
@@ -203 +203 @@
         setCurrentLayers(layersOut);
 
-
-        if (fn.indexOf('.') == -1) {
-            sessionFile = new File(sessionFile.getPath() + (isZipSessionFile ? ".joz" : ".jos"));
-            if (!SaveActionBase.confirmOverwrite(sessionFile)) {
-                throw new UserCancelException();
-            }
-        }
+        updateSessionFile(fn);
 
         Stream<Layer> layersToSaveStream = layersOut.stream()
@@ -219 +213 @@
 
         boolean success = true;
-        if (forceSaveAll || SAVE_LOCAL_FILES_PROPERTY.get()) {
+        if (forceSaveAll || Boolean.TRUE.equals(SAVE_LOCAL_FILES_PROPERTY.get())) {
             // individual files must be saved before the session file as the location may change
             if (layersToSaveStream
@@ -394 +388 @@
 
             int numNoExporter = 0;
-            WHILE: while (numNoExporter != noExporter.size()) {
+            while (numNoExporter != noExporter.size()) {
                 numNoExporter = noExporter.size();
-                for (Layer layer : layers) {
-                    if (noExporter.contains(layer)) continue;
-                    for (Layer depLayer : dependencies.get(layer)) {
-                        if (noExporter.contains(depLayer)) {
-                            noExporter.add(layer);
-                            exporters.put(layer, null);
-                            break WHILE;
-                        }
+                updateExporters(noExporter);
+            }
+        }
+
+        private void updateExporters(Collection<Layer> noExporter) {
+            for (Layer layer : layers) {
+                if (noExporter.contains(layer)) continue;
+                for (Layer depLayer : dependencies.get(layer)) {
+                    if (noExporter.contains(depLayer)) {
+                        noExporter.add(layer);
+                        exporters.put(layer, null);
+                        return;
                     }
                 }
@@ -436 +434 @@
             op.add(tabs, GBC.eol().fill());
             JCheckBox chkSaveLocal = new JCheckBox(tr("Save all local files to disk"), SAVE_LOCAL_FILES_PROPERTY.get());
-            chkSaveLocal.addChangeListener(l -> {
-                SAVE_LOCAL_FILES_PROPERTY.put(chkSaveLocal.isSelected());
-            });
+            chkSaveLocal.addChangeListener(l -> SAVE_LOCAL_FILES_PROPERTY.put(chkSaveLocal.isSelected()));
             op.add(chkSaveLocal);
             return op;
@@ -496 +492 @@
 
     /**
+     * Update the session file
+     * @param fileName The filename to use. If there are no periods in the file, we update the extension.
+     * @throws UserCancelException If the user does not want to overwrite a previously existing file.
+     */
+    private static void updateSessionFile(String fileName) throws UserCancelException {
+        if (fileName.indexOf('.') == -1) {
+            sessionFile = new File(sessionFile.getPath() + (isZipSessionFile ? ".joz" : ".jos"));
+            if (!SaveActionBase.confirmOverwrite(sessionFile)) {
+                throw new UserCancelException();
+            }
+        }
+    }
+
+    /**
      * Sets the current session file and the layers included in that file
      * @param file file
@@ -528 +538 @@
     public static void setCurrentLayers(List<Layer> layers) {
         layersInSessionFile = layers.stream()
-                .filter(l -> l instanceof AbstractModifiableLayer)
+                .filter(AbstractModifiableLayer.class::isInstance)
                 .map(WeakReference::new)
                 .collect(Collectors.toList());

trunk/src/org/openstreetmap/josm/actions/mapmode/SelectAction.java

@@ -402 +402 @@
         mouseDownButton = e.getButton();
         // return early
-        if (!mv.isActiveLayerVisible() || !(Boolean) this.getValue("active") || mouseDownButton != MouseEvent.BUTTON1)
+        if (!mv.isActiveLayerVisible() || Boolean.FALSE.equals(this.getValue("active")) || mouseDownButton != MouseEvent.BUTTON1)
             return;
 
@@ -744 +744 @@
                                 dh += Math.PI/2;
                                 break;
+                            default:
+                                // Just in case we cannot clamp
                             }
                             clampedEastNorth = currentEN.rotate(startEN, -dh);
@@ -1149 +1151 @@
                         // special case:  for cycle groups of 2, we can toggle to the
                         // true nearest primitive on mousePressed right away
-                        if (cycleList.size() == 2 && !waitForMouseUpParameter) {
-                            if (!(osm.equals(old) || osm.isNew() || platformMenuShortcutKeyMask)) {
+                        if (cycleList.size() == 2 && !waitForMouseUpParameter
+                                // This was a nested if statement (see commented code below)
+                                && (!(osm.equals(old) || osm.isNew() || platformMenuShortcutKeyMask))) {
                                 cyclePrims = false;
                                 osm = old;
-                            } // else defer toggling to mouseRelease time in those cases:
+                            // else defer toggling to mouseRelease time in those cases:
                             /*
                              * osm == old -- the true nearest node is the

trunk/src/org/openstreetmap/josm/data/imagery/vectortile/mapbox/Feature.java

@@ -57 +57 @@
      *
      * @param layer  The layer the feature is part of (required for tags)
-     * @param record The record to create the feature from
+     * @param protobufRecord The record to create the feature from
      * @throws IOException - if an IO error occurs
      */
-    public Feature(Layer layer, ProtobufRecord record) throws IOException {
+    public Feature(Layer layer, ProtobufRecord protobufRecord) throws IOException {
         long tId = 0;
         GeometryTypes geometryTypeTemp = GeometryTypes.UNKNOWN;
@@ -69 +69 @@
         // By avoiding array copies in TagMap, Feature#init goes from 339 MB to 188 MB.
         ArrayList<String> tagList = null;
-        try (ProtobufParser parser = new ProtobufParser(record.getBytes())) {
+        try (ProtobufParser parser = new ProtobufParser(protobufRecord.getBytes())) {
             ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(4);
             while (parser.hasNext()) {
@@ -111 +111 @@
         this.id = tId;
         this.geometryType = geometryTypeTemp;
-        record.close();
+        protobufRecord.close();
         if (tagList != null && !tagList.isEmpty()) {
             this.tags = new TagMap(tagList.toArray(EMPTY_STRING_ARRAY));
@@ -128 +128 @@
      * @return The new key (if {@code null}, then a value was parsed and added to tags)
      */
-    private String parseTagValue(String key, Layer layer, Number number, List<String> tagList) {
+    private static String parseTagValue(String key, Layer layer, Number number, List<String> tagList) {
         if (key == null) {
             key = layer.getKey(number.intValue());

trunk/test/unit/org/openstreetmap/josm/actions/CopyActionTest.java

@@ -47 +47 @@
     @RegisterExtension
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().preferences().fakeAPI();
+    static public JOSMTestRules test = new JOSMTestRules().preferences().fakeAPI().main().projection();
 
     /**

trunk/test/unit/org/openstreetmap/josm/actions/DeleteLayerActionTest.java

@@ -24 +24 @@
     @RegisterExtension
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules();
+    static public JOSMTestRules test = new JOSMTestRules().main().projection();
 
     /**

trunk/test/unit/org/openstreetmap/josm/actions/PurgeActionTest.java

@@ -32 +32 @@
     @RegisterExtension
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().main();
+    static public JOSMTestRules test = new JOSMTestRules().main().projection();
 
     /**

trunk/test/unit/org/openstreetmap/josm/actions/UnJoinNodeWayActionTest.java

@@ -42 +42 @@
     @RegisterExtension
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules();
+    static public JOSMTestRules test = new JOSMTestRules().projection().main();
 
     /**

trunk/test/unit/org/openstreetmap/josm/actions/UploadActionTest.java

@@ -42 +42 @@
     // Note that the setUp method can be replaced by the @Territories extension, when that is merged.
     @RegisterExtension
-    JOSMTestRules josmTestRules = new JOSMTestRules().main().projection().fakeAPI();
+    static JOSMTestRules josmTestRules = new JOSMTestRules().main().projection().fakeAPI();
 
     @BeforeAll