Ticket #19053: 19053.7.patch

src/org/openstreetmap/josm/data/preferences/sources/ValidatorPrefHelper.java

@@ -47 +47 @@
     /** The preferences key for the ignorelist */
     public static final String PREF_IGNORELIST = PREFIX + ".ignorelist";
 
+    /** The preferences key for the ignorelist format */
+    public static final String PREF_IGNORELIST_FORMAT = PREF_IGNORELIST + ".version";
+
     /**
      * The preferences key for enabling the permanent filtering
      * of the displayed errors in the tree regarding the current selection

src/org/openstreetmap/josm/data/validation/OsmValidator.java

@@ -18 +18 @@
 import java.util.EnumMap;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.TreeSet;
@@ -221 +223 @@
                         TreeSet<String> treeSet = new TreeSet<>();
                         treeSet.addAll(Files.readAllLines(path, StandardCharsets.UTF_8));
                         treeSet.forEach(ignore -> ignoredErrors.putIfAbsent(ignore, ""));
+                        removeLegacyEntries(true);
 
                         saveIgnoredErrors();
                         Files.deleteIfExists(path);
@@ -234 +237 @@
             } catch (SecurityException e) {
                 Logging.log(Logging.LEVEL_ERROR, "Unable to load ignored errors", e);
             }
-            // see #19053: remove invalid entry
-            ignoredErrors.remove("3000");
+            removeLegacyEntries(Config.getPref().get(ValidatorPrefHelper.PREF_IGNORELIST_FORMAT).isEmpty());
         }
     }
 
+    private static void removeLegacyEntries(boolean force) {
+        // see #19053:
+        boolean wasChanged = false;
+        if (force) {
+            Iterator<Entry<String, String>> iter = ignoredErrors.entrySet().iterator();
+            while (iter.hasNext()) {
+                Entry<String, String> entry = iter.next();
+                if (entry.getKey().startsWith("3000_")) {
+                    Logging.warn(tr("Cannot handle ignore list entry {0}", entry));
+                    iter.remove();
+                    wasChanged = true;
+                }
+            }
+        }
+        String legacyEntry = ignoredErrors.remove("3000");
+        if (legacyEntry != null) {
+            if (!legacyEntry.isEmpty()) {
+                addIgnoredError("3000_" + legacyEntry, legacyEntry);
+            }
+            wasChanged = true;
+        }
+        if (wasChanged) {
+            saveIgnoredErrors();
+        }
+    }
+
     /**
      * Adds an ignored error
      * @param s The ignore group / sub group name
@@ -265 +293 @@
      *  Make sure that we don't keep single entries for a "group ignore".
      */
     static void cleanupIgnoredErrors() {
+        cleanup3000();
         if (ignoredErrors.size() > 1) {
             List<String> toRemove = new ArrayList<>();
 
@@ -288 +317 @@
         }
     }
 
+    private static void cleanup3000() {
+        // see #19053
+        if (ignoredErrors.size() > 1) {
+            Set<String> toRemove = new HashSet<>();
+            for (Entry<String, String> entry : ignoredErrors.entrySet()) {
+                if (entry.getKey().equals("3000_" + entry.getValue()))
+                    toRemove.add(entry.getValue());
+            }
+            ignoredErrors.entrySet()
+                    .removeIf(e -> toRemove.contains(e.getValue()) && !e.getKey().equals("3000_" + e.getValue()));
+
+        }
+    }
+
     private static boolean sameCode(String key1, String key2) {
         return extractCodeFromIgnoreKey(key1).equals(extractCodeFromIgnoreKey(key2));
     }
@@ -482 +525 @@
         }
         if (list.isEmpty()) list = null;
         Config.getPref().putListOfMaps(ValidatorPrefHelper.PREF_IGNORELIST, list);
+        Config.getPref().put(ValidatorPrefHelper.PREF_IGNORELIST_FORMAT, "2");
     }
 
     /**

src/org/openstreetmap/josm/data/validation/TestError.java

@@ -338 +338 @@
      * @return The ignore sub group
      */
     public String getIgnoreSubGroup() {
+        if (code == 3000) {
+            // see #19053
+            return "3000_" + (description == null ? message : description);
+        }
         String ignorestring = getIgnoreGroup();
         if (descriptionEn != null) {
             ignorestring += '_' + descriptionEn;
@@ -351 +355 @@
      * @see TestError#getIgnoreSubGroup()
      */
     public String getIgnoreGroup() {
+        if (code == 3000) {
+            // see #19053
+            return "3000_" + getMessage();
+        }
         return Integer.toString(code);
     }
 
@@ -524 +532 @@
     public String toString() {
         return "TestError [tester=" + tester + ", code=" + code + ", message=" + message + ']';
     }
+
 }

src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java

@@ -410 +410 @@
 
         lastSelectedNode = node;
         if (node != null) {
-            final Set<String> codes = new HashSet<>();
             ValidatorTreePanel.visitTestErrors(node, error -> {
-                codes.add(error.getIgnoreSubGroup()); // see #19053
                 error.setSelected(true);
 
                 hasFixes.set(hasFixes.get() || error.isFixable());
@@ -424 +422 @@
             });
             selectAction.setEnabled(true);
             if (ignoreAction != null) {
-                ignoreAction.setEnabled(!(node.getUserObject() instanceof Severity) && codes.size() <= 1);
+                ignoreAction.setEnabled(!(node.getUserObject() instanceof Severity));
             }
         }
 

test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java

@@ -144 +144 @@
         final Collection<TestError> errors = test.getErrorsForPrimitive(p, false);
         assertEquals(1, errors.size());
         assertEquals("has alt_name but not name", errors.iterator().next().getMessage());
-        assertEquals("3000_*[.+_name][!name]", errors.iterator().next().getIgnoreSubGroup());
+        assertEquals("3000_has alt_name but not name", errors.iterator().next().getIgnoreSubGroup());
     }
 
     /**
@@ -159 +159 @@
         final Collection<TestError> errors = test.getErrorsForPrimitive(p, false);
         assertEquals(1, errors.size());
         assertEquals("footway used with foot=no", errors.iterator().next().getMessage());
-        assertEquals("3000_way[highway=footway][foot]", errors.iterator().next().getIgnoreSubGroup());
+        assertEquals("3000_footway used with foot=no", errors.iterator().next().getIgnoreSubGroup());
     }
 
     /**
@@ -413 +413 @@
         }
     }
 
+    /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/19053">Bug #19053</a>.
+     * Mapcss rule with group.
+     * @throws ParseException if a parsing error occurs
+     */
+    @Test
+    public void testTicket19053() throws ParseException {
+        final MapCSSTagChecker test = buildTagChecker(
+                "*[ele][ele =~ /^-?[0-9]+\\.[0-9][0-9][0-9]+$/] {"
+                        + "throwWarning: tr(\"{0}\",\"{0.tag}\");"
+                        + "group: tr(\"Unnecessary amount of decimal places\");" + "}");
+        final OsmPrimitive p = OsmUtils.createPrimitive("node ele=12.123456");
+        final Collection<TestError> errors = test.getErrorsForPrimitive(p, false);
+        assertEquals(1, errors.size());
+        assertEquals("Unnecessary amount of decimal places", errors.iterator().next().getMessage());
+        assertEquals("3000_ele=12.123456", errors.iterator().next().getIgnoreSubGroup());
+        assertEquals("3000_Unnecessary amount of decimal places", errors.iterator().next().getIgnoreGroup());
+    }
+
 }