Changeset 15988 in josm for trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

Timestamp:

2020-03-01T23:36:07+01:00 (4 years ago)

Author:

simon04

Message:

see #18802 - MapCSSRule: support list of selectors

• allows to drop MapCSSTagChecker.GroupedMapCSSRule
• remove code duplication in MapCSSStyleSource/MapCSSTagCheckerIndex w.r.t. MapCSSRuleIndex

File:

• trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (modified) (15 diffs)

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

@@ -15 +15 @@
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -26 +25 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 import org.openstreetmap.josm.command.ChangePropertyCommand;
@@ -47 +47 @@
 import org.openstreetmap.josm.gui.mappaint.MultiCascade;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Condition;
-import org.openstreetmap.josm.gui.mappaint.mapcss.Declaration;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Expression;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction;
 import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule;
 import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource;
-import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource.MapCSSRuleIndex;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector;
@@ -76 +74 @@
 public class MapCSSTagChecker extends Test.TagTest {
     private MapCSSTagCheckerIndex indexData;
+    final Map<MapCSSRule, MapCSSTagCheckerAndRule> ruleToCheckMap = new HashMap<>();
     private final Set<OsmPrimitive> tested = new HashSet<>();
     private static final Map<IPrimitive, Area> mpAreaCache = new HashMap<>();
-
-    /**
-    * A grouped MapCSSRule with multiple selectors for a single declaration.
-    * @see MapCSSRule
-    */
-    public static class GroupedMapCSSRule {
-        /** MapCSS selectors **/
-        public final List<Selector> selectors;
-        /** MapCSS declaration **/
-        public final Declaration declaration;
-        /** MapCSS source **/
-        public final String source;
-
-        /**
-         * Constructs a new {@code GroupedMapCSSRule} with empty source
-         * @param selectors MapCSS selectors
-         * @param declaration MapCSS declaration
-         */
-        public GroupedMapCSSRule(List<Selector> selectors, Declaration declaration) {
-            this(selectors, declaration, "");
-        }
-
-        /**
-         * Constructs a new {@code GroupedMapCSSRule}.
-         * @param selectors MapCSS selectors
-         * @param declaration MapCSS declaration
-         * @param source the source of the rule
-         */
-        public GroupedMapCSSRule(List<Selector> selectors, Declaration declaration, String source) {
-            this.selectors = selectors;
-            this.declaration = declaration;
-            this.source = source;
-        }
-
-        @Override
-        public int hashCode() {
-            return Objects.hash(selectors, declaration);
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (this == obj) return true;
-            if (obj == null || getClass() != obj.getClass()) return false;
-            GroupedMapCSSRule that = (GroupedMapCSSRule) obj;
-            return Objects.equals(selectors, that.selectors) &&
-                    Objects.equals(declaration, that.declaration);
-        }
-
-        @Override
-        public String toString() {
-            return "GroupedMapCSSRule [selectors=" + selectors + ", declaration=" + declaration + ']';
-        }
-    }
 
     /**
@@ -281 +227 @@
     public static class TagCheck implements Predicate<OsmPrimitive> {
         /** The selector of this {@code TagCheck} */
-        protected final GroupedMapCSSRule rule;
+        protected final MapCSSRule rule;
         /** Commands to apply in order to fix a matching primitive */
         protected final List<FixCommand> fixCommands;
@@ -296 +242 @@
         protected String group;
 
-        TagCheck(GroupedMapCSSRule rule) {
+        TagCheck(MapCSSRule rule) {
             this.rule = rule;
             this.fixCommands = new ArrayList<>();
@@ -320 +266 @@
         private static final String POSSIBLE_THROWS = "throwError/throwWarning/throwOther";
 
-        static TagCheck ofMapCSSRule(final GroupedMapCSSRule rule, AssertionConsumer assertionConsumer) throws IllegalDataException {
+        static TagCheck ofMapCSSRule(final MapCSSRule rule, AssertionConsumer assertionConsumer) throws IllegalDataException {
             final TagCheck check = new TagCheck(rule);
             final Map<String, Boolean> assertions = new HashMap<>();
@@ -394 +340 @@
 
         static ParseResult readMapCSS(Reader css) throws ParseException {
-            return readMapCSS(css, "", null);
-        }
-
-        static ParseResult readMapCSS(Reader css, String url, AssertionConsumer assertionConsumer) throws ParseException {
+            return readMapCSS(css, null);
+        }
+
+        static ParseResult readMapCSS(Reader css, AssertionConsumer assertionConsumer) throws ParseException {
             CheckParameterUtil.ensureParameterNotNull(css, "css");
 
@@ -407 +353 @@
             // Ignore "meta" rule(s) from external rules of JOSM wiki
             source.removeMetaRules();
-            // group rules with common declaration block
-            Map<Declaration, List<Selector>> g = new LinkedHashMap<>();
+            List<TagCheck> parseChecks = new ArrayList<>();
             for (MapCSSRule rule : source.rules) {
-                if (!g.containsKey(rule.declaration)) {
-                    List<Selector> sels = new ArrayList<>();
-                    sels.add(rule.selector);
-                    g.put(rule.declaration, sels);
-                } else {
-                    g.get(rule.declaration).add(rule.selector);
-                }
-            }
-            List<TagCheck> parseChecks = new ArrayList<>();
-            for (Map.Entry<Declaration, List<Selector>> map : g.entrySet()) {
                 try {
-                    parseChecks.add(TagCheck.ofMapCSSRule(
-                            new GroupedMapCSSRule(map.getValue(), map.getKey(), url), assertionConsumer));
+                    parseChecks.add(TagCheck.ofMapCSSRule(rule, assertionConsumer));
                 } catch (IllegalDataException e) {
                     Logging.error("Cannot add MapCss rule: "+e.getMessage());
@@ -661 +595 @@
 
     static class MapCSSTagCheckerAndRule extends MapCSSTagChecker {
-        public final GroupedMapCSSRule rule;
-
-        MapCSSTagCheckerAndRule(GroupedMapCSSRule rule) {
+        public final MapCSSRule rule;
+        private final TagCheck tagCheck;
+        private final String source;
+
+        MapCSSTagCheckerAndRule(MapCSSRule rule) {
             this.rule = rule;
+            this.tagCheck = null;
+            this.source = "";
+        }
+
+        MapCSSTagCheckerAndRule(TagCheck tagCheck, String source) {
+            this.rule = tagCheck.rule;
+            this.tagCheck = tagCheck;
+            this.source = source;
         }
 
@@ -674 +618 @@
         @Override
         public String getSource() {
-            return tr("URL / File: {0}", rule.source);
-        }
+            return tr("URL / File: {0}", source);
+        }
+    }
+
+    static MapCSSTagCheckerIndex createMapCSSTagCheckerIndex(MultiMap<String, TagCheck> checks, boolean includeOtherSeverity, boolean allTests) {
+        final MapCSSTagCheckerIndex index = new MapCSSTagCheckerIndex();
+        final Stream<MapCSSRule> ruleStream = checks.values().stream()
+                .flatMap(Collection::stream)
+                // Ignore "information" level checks if not wanted, unless they also set a MapCSS class
+                .filter(c -> includeOtherSeverity || Severity.OTHER != c.getSeverity() || !c.setClassExpressions.isEmpty())
+                .filter(c -> allTests || c.rule.selectors.stream().anyMatch(Selector.ChildOrParentSelector.class::isInstance))
+                .map(c -> c.rule);
+        index.buildIndex(ruleStream);
+        return index;
     }
 
@@ -687 +643 @@
         final List<TestError> res = new ArrayList<>();
         if (indexData == null) {
-            indexData = new MapCSSTagCheckerIndex(checks, includeOtherSeverity, MapCSSTagCheckerIndex.ALL_TESTS);
-        }
-
-        MapCSSRuleIndex matchingRuleIndex = indexData.get(p);
+            indexData = MapCSSTagCheckerIndex.createMapCSSTagCheckerIndex(checks, includeOtherSeverity, MapCSSTagCheckerIndex.ALL_TESTS);
+        }
 
         Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
         env.mpAreaCache = mpAreaCache;
 
-        // the declaration indices are sorted, so it suffices to save the last used index
-        Declaration lastDeclUsed = null;
-
-        Iterator<MapCSSRule> candidates = matchingRuleIndex.getRuleCandidates(p);
+        Iterator<MapCSSRule> candidates = indexData.getRuleCandidates(p);
         while (candidates.hasNext()) {
             MapCSSRule r = candidates.next();
-            env.clearSelectorMatchingInformation();
-            if (r.matches(env)) { // as side effect env.parent will be set (if s is a child selector)
-                TagCheck check = indexData.getCheck(r);
+            for (Selector selector : r.selectors) {
+                env.clearSelectorMatchingInformation();
+                if (!selector.matches(env)) { // as side effect env.parent will be set (if s is a child selector)
+                    continue;
+                }
+                MapCSSTagCheckerAndRule test = ruleToCheckMap.computeIfAbsent(r, rule -> checks.entrySet().stream()
+                        .map(e -> e.getValue().stream()
+                                // rule.selectors might be different due to MapCSSStyleIndex, however, the declarations are the same object
+                                .filter(c -> c.rule.declaration == rule.declaration)
+                                .findFirst()
+                                .map(c -> new MapCSSTagCheckerAndRule(c, e.getKey()))
+                                .orElse(null))
+                        .filter(Objects::nonNull)
+                        .findFirst()
+                        .orElse(null));
+                TagCheck check = test == null ? null : test.tagCheck;
                 if (check != null) {
-                    if (r.declaration == lastDeclUsed)
-                        continue; // don't apply one declaration more than once
-                    lastDeclUsed = r.declaration;
-
                     r.declaration.execute(env);
                     if (!check.errors.isEmpty()) {
-                        for (TestError e: check.getErrorsForPrimitive(p, r.selector, env, new MapCSSTagCheckerAndRule(check.rule))) {
+                        for (TestError e: check.getErrorsForPrimitive(p, selector, env, test)) {
                             addIfNotSimilar(e, res);
                         }
@@ -831 +791 @@
             if (zip != null)
                 I18n.addTexts(cache.getFile());
-            result = TagCheck.readMapCSS(reader, url, assertionConsumer);
+            result = TagCheck.readMapCSS(reader, assertionConsumer);
             checks.remove(url);
             checks.putAll(url, result.parseChecks);
@@ -890 +850 @@
         super.setShowElements(true);
         if (indexData == null) {
-            indexData = new MapCSSTagCheckerIndex(checks, includeOtherSeverityChecks(), MapCSSTagCheckerIndex.ALL_TESTS);
+            indexData = MapCSSTagCheckerIndex.createMapCSSTagCheckerIndex(checks, includeOtherSeverityChecks(), MapCSSTagCheckerIndex.ALL_TESTS);
         }
         tested.clear();
@@ -904 +864 @@
             // rebuild index with a reduced set of rules (those that use ChildOrParentSelector) and thus may have left selectors
             // matching the previously tested elements
-            indexData = new MapCSSTagCheckerIndex(checks, includeOtherSeverityChecks(), MapCSSTagCheckerIndex.ONLY_SELECTED_TESTS);
+            indexData = MapCSSTagCheckerIndex.createMapCSSTagCheckerIndex(checks, includeOtherSeverityChecks(), MapCSSTagCheckerIndex.ONLY_SELECTED_TESTS);
 
             Set<OsmPrimitive> surrounding = new HashSet<>();