Changeset 16784 in josm

Timestamp:

2020-07-17T17:12:54+02:00 (4 years ago)

Author:

GerdP

Message:

see #19180: false positives from tagchecker with single letter differences

• avoid to produce "Value 'y' for key 'x' is unknown, maybe 'z' is meant?" when x=z produces a deprecated message. It uses all *.mapcss rules which are stored in files ending with deprecated.mapcss, so this will fail if user changed the name to something else, but it should work with e.g. my_deprecated.mapcss

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/validation/tests/MapCSSTagChecker.java (modified) (1 diff)
• data/validation/tests/TagChecker.java (modified) (8 diffs)
• gui/mappaint/mapcss/MapCSSRuleIndex.java (modified) (1 diff)
• gui/mappaint/mapcss/MapCSSStyleIndex.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

@@ -939 +939 @@
 
         if (partialSelection && !tested.isEmpty()) {
-            // #14287: see https://josm.openstreetmap.de/ticket/14287#comment:15
-            // execute tests for objects which might contain or cross previously tested elements
-
-            // rebuild index with a reduced set of rules (those that use ChildOrParentSelector) and thus may have left selectors
-            // matching the previously tested elements
-            indexData = createMapCSSTagCheckerIndex(currentCheck, includeOtherSeverityChecks(), ONLY_SELECTED_TESTS);
-
-            if (surrounding.isEmpty()) {
-                for (OsmPrimitive p : tested) {
-                    if (p.getDataSet() != null) {
-                        surrounding.addAll(p.getDataSet().searchWays(p.getBBox()));
-                        surrounding.addAll(p.getDataSet().searchRelations(p.getBBox()));
-                    }
-                }
-            }
-
-            final boolean includeOtherSeverity = includeOtherSeverityChecks();
-            for (OsmPrimitive p : surrounding) {
-                if (tested.contains(p))
-                    continue;
-                Collection<TestError> additionalErrors = getErrorsForPrimitive(p, includeOtherSeverity);
-                for (TestError e : additionalErrors) {
-                    if (e.getPrimitives().stream().anyMatch(tested::contains))
-                        addIfNotSimilar(e, errors);
-                }
-            }
-        }
+            testPartial(currentCheck, tested, surrounding);
+        }
+    }
+
+    private void testPartial(MultiMap<String, TagCheck> currentCheck, Set<OsmPrimitive> tested,
+            Set<OsmPrimitive> surrounding) {
+
+        // #14287: see https://josm.openstreetmap.de/ticket/14287#comment:15
+        // execute tests for objects which might contain or cross previously tested elements
+
+        final boolean includeOtherSeverity = includeOtherSeverityChecks();
+        // rebuild index with a reduced set of rules (those that use ChildOrParentSelector) and thus may have left selectors
+        // matching the previously tested elements
+        indexData = createMapCSSTagCheckerIndex(currentCheck, includeOtherSeverity, ONLY_SELECTED_TESTS);
+        if (indexData.isEmpty())
+            return; // performance: some *.mapcss rule files don't use ChildOrParentSelector
+
+        if (surrounding.isEmpty()) {
+            for (OsmPrimitive p : tested) {
+                if (p.getDataSet() != null) {
+                    surrounding.addAll(p.getDataSet().searchWays(p.getBBox()));
+                    surrounding.addAll(p.getDataSet().searchRelations(p.getBBox()));
+                }
+            }
+        }
+
+        for (OsmPrimitive p : surrounding) {
+            if (tested.contains(p))
+                continue;
+            Collection<TestError> additionalErrors = getErrorsForPrimitive(p, includeOtherSeverity);
+            for (TestError e : additionalErrors) {
+                if (e.getPrimitives().stream().anyMatch(tested::contains))
+                    addIfNotSimilar(e, errors);
+            }
+        }
+
+    }
+
+    /**
+     * Execute only the rules for the rules matching the given file name. See #19180
+     * @param ruleFile the name of the mapcss file, e.g. deprecated.mapcss
+     * @param selection collection of primitives
+     * @since 16784
+     */
+    public void runOnly(String ruleFile, Collection<OsmPrimitive> selection) {
+        mpAreaCache.clear();
+
+        Set<OsmPrimitive> surrounding = new HashSet<>();
+        for (Entry<String, Set<TagCheck>> entry : checks.entrySet()) {
+            if (isCanceled()) {
+                break;
+            }
+            if (entry.getKey().endsWith(ruleFile)) {
+                visit(entry.getKey(), entry.getValue(), selection, surrounding);
+            }
+        }
+
     }
 }

trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java

@@ -18 +18 @@
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -45 +46 @@
 import org.openstreetmap.josm.data.osm.Tagged;
 import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
+import org.openstreetmap.josm.data.validation.OsmValidator;
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test.TagTest;
@@ -100 +102 @@
     /** The preferences prefix */
     protected static final String PREFIX = ValidatorPrefHelper.PREFIX + "." + TagChecker.class.getSimpleName();
+
+    MapCSSTagChecker deprecatedChecker;
 
     /**
@@ -894 +898 @@
             // use Levenshtein distance to find typical typos
             int minDist = MAX_LEVENSHTEIN_DISTANCE + 1;
-            String closest = null;
             for (Set<String> possibleValues: sets) {
                 for (String possibleVal : possibleValues) {
@@ -911 +914 @@
                     }
                     if (dist < minDist) {
-                        closest = possibleVal;
                         minDist = dist;
                         fixVals.clear();
@@ -920 +922 @@
                 }
             }
-
+            filterDeprecatedTags(p, key, fixVals);
             if (minDist <= MAX_LEVENSHTEIN_DISTANCE && maxPresetValueLen > MAX_LEVENSHTEIN_DISTANCE
+                    && !fixVals.isEmpty()
                     && (harmonizedValue.length() > 3 || minDist < MAX_LEVENSHTEIN_DISTANCE)) {
                 if (fixVals.size() < 2) {
-                    fixedValue = closest;
+                    fixedValue = fixVals.get(0);
                 } else {
                     Collections.sort(fixVals);
@@ -958 +961 @@
     }
 
+    // see #19180
+    private void filterDeprecatedTags(OsmPrimitive p, String key, List<String> fixVals) {
+        if (fixVals.isEmpty() || deprecatedChecker == null)
+            return;
+
+        String origVal = p.get(key);
+        try {
+            int unchangedDeprecated = countDeprecated(p);
+            Iterator<String> iter = fixVals.iterator();
+            while (iter.hasNext()) {
+                p.put(key, iter.next());
+                if (countDeprecated(p) > unchangedDeprecated)
+                    iter.remove();
+            }
+        } finally {
+            // restore original value
+            p.put(key, origVal);
+        }
+    }
+
+    private int countDeprecated(OsmPrimitive p) {
+        if (deprecatedChecker == null)
+            return 0;
+        deprecatedChecker.getErrors().clear();
+        deprecatedChecker.runOnly("deprecated.mapcss", Collections.singleton(p));
+        return deprecatedChecker.getErrors().size();
+    }
+
     private static boolean isNum(String harmonizedValue) {
         try {
@@ -1008 +1039 @@
             checkPresetsTypes = checkPresetsTypes && Config.getPref().getBoolean(PREF_CHECK_PRESETS_TYPES_BEFORE_UPLOAD, true);
         }
+        deprecatedChecker = OsmValidator.getTest(MapCSSTagChecker.class);
+    }
+
+    @Override
+    public void endTest() {
+        deprecatedChecker = null;
+        super.endTest();
     }
 

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/MapCSSRuleIndex.java

@@ -247 +247 @@
         remaining.clear();
     }
+
+    /**
+     * Check if this index is empty.
+     * @return true if this index is empty.
+     * @since 16784
+     */
+    public boolean isEmpty() {
+        return rules.isEmpty();
+    }
 }

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/MapCSSStyleIndex.java

@@ -173 +173 @@
         return get(osm).getRuleCandidates(osm);
     }
+
+    /**
+     * Check if this index is empty.
+     * @return true if this index is empty.
+     * @since 16784
+     */
+    public boolean isEmpty() {
+        return nodeRules.isEmpty() && wayRules.isEmpty() && wayNoAreaRules.isEmpty() && relationRules.isEmpty()
+                && multipolygonRules.isEmpty() && canvasRules.isEmpty();
+    }
 }