Ticket #19180: 19180-check-deprecated.2.patch

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

@@ -938 +938 @@
         }
 
         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
+            testPartial(currentCheck, tested, surrounding);
+        }
+    }
 
-            // 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);
+    private void testPartial(MultiMap<String, TagCheck> currentCheck, Set<OsmPrimitive> tested,
+            Set<OsmPrimitive> surrounding) {
 
-            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()));
-                    }
+        // #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()));
                 }
             }
+        }
 
-            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);
-                }
+        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 xxx
+     */
+    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);
+            }
+        }
+
+    }
 }

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

@@ -17 +17 @@
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -44 +45 @@
 import org.openstreetmap.josm.data.osm.TagMap;
 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;
 import org.openstreetmap.josm.data.validation.TestError;
@@ -100 +102 @@
     /** The preferences prefix */
     protected static final String PREFIX = ValidatorPrefHelper.PREFIX + "." + TagChecker.class.getSimpleName();
 
+    MapCSSTagChecker deprecatedChecker;
+
     /**
      * The preference key to check values
      */
@@ -852 +856 @@
             fixedKey = prettifiedKey;
         }
 
+        // see #19180: Don't suggest a key which is deprecated
+        if (deprecatedChecker != null && !p.hasTag(fixedKey) && fixedKey != null && !"".equals(fixedKey)) {
+            String val = p.get(key);
+            boolean isDeprecated = false;
+            try {
+                int origDeprecated = countDeprecated(p);
+                p.remove(key);
+                p.put(fixedKey, val);
+                isDeprecated = countDeprecated(p) > origDeprecated;
+            } finally {
+                p.remove(fixedKey);
+                p.put(key, val);
+            }
+            if (isDeprecated) {
+                fixedKey = null;
+            }
+        }
+
         if (fixedKey != null && !"".equals(fixedKey) && !fixedKey.equals(key)) {
             final String proposedKey = fixedKey;
             // misspelled preset key
@@ -893 +915 @@
             List<String> fixVals = new ArrayList<>();
             // 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) {
                     if (possibleVal.isEmpty())
@@ -910 +931 @@
                         continue;
                     }
                     if (dist < minDist) {
-                        closest = possibleVal;
                         minDist = dist;
                         fixVals.clear();
                         fixVals.add(possibleVal);
@@ -919 +939 @@
                     }
                 }
             }
-
+            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);
                     // misspelled preset value with multiple good alternatives
@@ -957 +978 @@
         }
     }
 
+    // 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 {
             Double.parseDouble(harmonizedValue);
@@ -1007 +1056 @@
         if (isBeforeUpload) {
             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();
+    }
+
+    @Override
     public void visit(Collection<OsmPrimitive> selection) {
         if (checkKeys || checkValues || checkComplex || checkFixmes || checkPresetsTypes) {
             super.visit(selection);

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

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

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

@@ -172 +172 @@
     public Iterator<MapCSSRule> getRuleCandidates(IPrimitive osm) {
         return get(osm).getRuleCandidates(osm);
     }
+
+    /**
+     * Check if this index is empty.
+     * @return true if this index is empty.
+     * @since xxx
+     */
+    public boolean isEmpty() {
+        return nodeRules.isEmpty() && wayRules.isEmpty() && wayNoAreaRules.isEmpty() && relationRules.isEmpty()
+                && multipolygonRules.isEmpty() && canvasRules.isEmpty();
+    }
 }