Changeset 19655 in josm

Timestamp:

2026-10-22T17:13:42+02:00 (3 hours ago)

Author:

stoecker

Message:

fix #24951 - Fix Sonar issues in recent Grid Layer and OAuth - patches by DanProgs & wangi

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/AlignGridRotationAction.java (modified) (5 diffs)
• src/org/openstreetmap/josm/gui/MapFrame.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/layer/MapGridPaintable.java (modified) (12 diffs)
• src/org/openstreetmap/josm/gui/layer/imagery/ShowTileBordersAction.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/preferences/display/GridPreference.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/actions/GridActionsTest.java (modified) (1 diff)

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

@@ -27 +27 @@
 public class AlignGridRotationAction extends JosmAction {
 
+    /** returned when the selection does not define a direction */
+    private static final EastNorth[] EMPTY_DIRECTION = new EastNorth[0];
+
     /**
      * Constructs a new {@code AlignGridRotationAction}.
@@ -40 +43 @@
      * Determines the direction given by the selection: a single way with at least two nodes, or exactly two nodes.
      * @param ds the data set, may be null
-     * @return start and end point, or {@code null} if the selection does not define a direction
+     * @return start and end point, or an empty array if the selection does not define a direction
      */
     static EastNorth[] getSelectedDirection(DataSet ds) {
         if (ds == null) {
-            return null;
+            return EMPTY_DIRECTION;
         }
         Collection<Way> ways = ds.getSelectedWays();
@@ -62 +65 @@
         }
         if (a == null || b == null || !a.isLatLonKnown() || !b.isLatLonKnown()) {
-            return null;
+            return EMPTY_DIRECTION;
         }
         EastNorth[] result = {a.getEastNorth(), b.getEastNorth()};
-        return result[0].equalsEpsilon(result[1], 1e-9) ? null : result;
+        return result[0].equalsEpsilon(result[1], 1e-9) ? EMPTY_DIRECTION : result;
     }
 
@@ -86 +89 @@
     @Override
     protected void updateEnabledState() {
-        setEnabled(getSelectedDirection(getLayerManager().getEditDataSet()) != null);
+        setEnabled(getSelectedDirection(getLayerManager().getEditDataSet()).length == 2);
     }
 
@@ -97 +100 @@
     public void actionPerformed(ActionEvent e) {
         EastNorth[] direction = getSelectedDirection(getLayerManager().getEditDataSet());
-        if (direction == null) {
+        if (direction.length != 2) {
             return;
         }

trunk/src/org/openstreetmap/josm/gui/MapFrame.java

@@ -125 +125 @@
     public final MapView mapView;
     /** The grid drawn over the map view, see {@link MapGridPaintable} */
-    private final MapGridPaintable gridOverlay = new MapGridPaintable();
+    private final transient MapGridPaintable gridOverlay = new MapGridPaintable();
 
     /**

trunk/src/org/openstreetmap/josm/gui/layer/MapGridPaintable.java

@@ -13 +13 @@
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.ProjectionBounds;
 import org.openstreetmap.josm.data.coor.EastNorth;
-import org.openstreetmap.josm.data.coor.ILatLon;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.preferences.BooleanProperty;
@@ -138 +138 @@
 
     /** written by {@link #paint} and read by {@link #preferenceChanged}, which may run on another thread */
-    private volatile MapView mapView;
+    private final AtomicReference<MapView> mapView = new AtomicReference<>();
 
     /**
@@ -149 +149 @@
     @Override
     public void paint(Graphics2D g, MapView mv, Bounds bbox) {
-        mapView = mv;
+        mapView.set(mv);
         if (!Boolean.TRUE.equals(ENABLED.get()) || mv.getWidth() <= 0 || mv.getHeight() <= 0) {
             return;
@@ -319 +319 @@
                 return 1;
             }
-            double metres = a.greatCircleDistance((ILatLon) b);
+            double metres = a.greatCircleDistance(b);
             return metres > 0 && Double.isFinite(metres) ? step / metres : 1;
         } catch (IllegalArgumentException e) {
@@ -358 +358 @@
             double rotationDegrees, double originX, double originY) {
         List<ProjectedGridLine> lines = new ArrayList<>();
-        if (!(spacingX > 0) || !(spacingY > 0)) {
+        if (!isPositive(spacingX) || !isPositive(spacingY)) {
             return lines;
         }
@@ -421 +421 @@
             double originLon, double originLat, int segments) {
         List<LatLonGridLine> lines = new ArrayList<>();
-        if (!(spacingLon > 0) || !(spacingLat > 0) || segments < 1) {
+        if (!isPositive(spacingLon) || !isPositive(spacingLat) || segments < 1) {
             return lines;
         }
@@ -446 +446 @@
         for (long k = kLonMin; k <= kLonMax; k++) {
             double lon = LatLon.toIntervalLon(originLon + k * spacingLon);
-            if (limitedLon && (lon < world.getMinLon() || lon > world.getMaxLon())) {
-                continue;
-            }
-            List<LatLon> line = new ArrayList<>(segments + 1);
-            for (int i = 0; i <= segments; i++) {
-                line.add(new LatLon(minLat + (maxLat - minLat) * i / segments, lon));
-            }
-            lines.add(new LatLonGridLine(line, true));
+            if (!limitedLon || (lon >= world.getMinLon() && lon <= world.getMaxLon())) {
+                addMeridian(lines, lon, minLat, maxLat, segments);
+            }
         }
         for (long k = kLatMin; k <= kLatMax; k++) {
@@ -463 +458 @@
         }
         return lines;
+    }
+
+    /**
+     * Adds a meridian running from one latitude to another.
+     * @param lines the list to add to
+     * @param lon the longitude of the meridian
+     * @param minLat the latitude to start at
+     * @param maxLat the latitude to end at
+     * @param segments number of segments of the polyline
+     */
+    private static void addMeridian(List<LatLonGridLine> lines, double lon, double minLat, double maxLat, int segments) {
+        List<LatLon> line = new ArrayList<>(segments + 1);
+        for (int i = 0; i <= segments; i++) {
+            line.add(new LatLon(minLat + (maxLat - minLat) * i / segments, lon));
+        }
+        lines.add(new LatLonGridLine(line, true));
     }
 
@@ -481 +492 @@
             return;
         }
-        if (!(maxLon > minLon)) {
+        if (maxLon <= minLon) {
             return;
         }
@@ -489 +500 @@
         }
         lines.add(new LatLonGridLine(line, false));
+    }
+
+    /**
+     * Determines if a value is strictly positive. Unlike {@code value <= 0}, this rejects NaN.
+     * @param value the value
+     * @return {@code true} if the value is greater than 0
+     */
+    private static boolean isPositive(double value) {
+        return value > 0;
     }
 
@@ -495 +515 @@
         if (e.getKey().startsWith(PREFIX) || e.getKey().equals(COLOR_EAST.getKey()) || e.getKey().equals(COLOR_NORTH.getKey())) {
             invalidate();
-            if (mapView != null) {
-                mapView.repaint();
+            MapView mv = mapView.get();
+            if (mv != null) {
+                mv.repaint();
             }
         }
@@ -504 +525 @@
     public void destroy() {
         Config.getPref().removePreferenceChangeListener(this);
-        mapView = null;
+        mapView.set(null);
     }
 }

trunk/src/org/openstreetmap/josm/gui/layer/imagery/ShowTileBordersAction.java

@@ -21 +21 @@
 public class ShowTileBordersAction extends AbstractAction implements LayerAction {
 
-    private final AbstractTileSourceLayer<?> layer;
+    private final transient AbstractTileSourceLayer<?> layer;
 
     /**

trunk/src/org/openstreetmap/josm/gui/preferences/display/GridPreference.java

@@ -5 +5 @@
 
 import java.awt.Component;
+import java.awt.GridBagConstraints;
 import java.awt.GridBagLayout;
 import java.text.DecimalFormat;
@@ -101 +102 @@
         panel.add(enabled, GBC.eol().insets(0, 0, 0, 10));
         panel.add(new JLabel(tr("Grid type")), GBC.std().insets(5, 0, 5, 5));
-        panel.add(type, GBC.eol().fill(GBC.HORIZONTAL).insets(0, 0, 0, 5));
+        panel.add(type, GBC.eol().fill(GridBagConstraints.HORIZONTAL).insets(0, 0, 0, 5));
         panel.add(spacingXLabel, GBC.std().insets(5, 0, 5, 5));
         panel.add(spacingX, GBC.eol().insets(0, 0, 0, 5));
@@ -117 +118 @@
         panel.add(lineWidth, GBC.eol().insets(0, 0, 0, 5));
         panel.add(new JLabel(tr("Blend mode")), GBC.std().insets(5, 0, 5, 5));
-        panel.add(blendMode, GBC.eol().fill(GBC.HORIZONTAL).insets(0, 0, 0, 5));
-        panel.add(GBC.glue(0, 0), GBC.eol().fill(GBC.BOTH));
+        panel.add(blendMode, GBC.eol().fill(GridBagConstraints.HORIZONTAL).insets(0, 0, 0, 5));
+        panel.add(GBC.glue(0, 0), GBC.eol().fill(GridBagConstraints.BOTH));
 
         createPreferenceTabWithScrollPane(gui, panel);
@@ -143 +144 @@
     /** Renders enum values with a translated name */
     private static final class TranslatedRenderer<T> extends DefaultListCellRenderer {
-        private final Function<T, String> name;
+        private final transient Function<T, String> name;
 
         TranslatedRenderer(Function<T, String> name) {
@@ -179 +180 @@
         try {
             double value = FORMAT.parse(field.getText().trim()).doubleValue();
-            if (positive && !(value > 0)) {
+            if (positive && (Double.isNaN(value) || value <= 0)) {
                 Logging.warn("Ignoring invalid grid setting {0}: {1}", property.getKey(), field.getText());
                 return;

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

@@ -165 +165 @@
 
             ds.setSelected();
-            assertNull(AlignGridRotationAction.getSelectedDirection(ds));
+            assertEquals(0, AlignGridRotationAction.getSelectedDirection(ds).length);
             AlignGridRotationAction action = new AlignGridRotationAction();
             assertFalse(action.isEnabled());
             ds.setSelected(a);
-            assertNull(AlignGridRotationAction.getSelectedDirection(ds));
+            assertEquals(0, AlignGridRotationAction.getSelectedDirection(ds).length);
             assertFalse(action.isEnabled());
             ds.setSelected(a, b, c);
-            assertNull(AlignGridRotationAction.getSelectedDirection(ds));
+            assertEquals(0, AlignGridRotationAction.getSelectedDirection(ds).length);
             assertFalse(action.isEnabled());