Changeset 6601 in josm for trunk

Timestamp:

2014-01-02T23:58:58+01:00 (11 years ago)

Author:

simon04

Message:

see #9414 fix #9485 - MapCSSTagChecker: add support for set class_name instruction and .class_name condition

Location:

trunk

Files:

• data/validator/highway.mapcss (modified) (3 diffs)
• src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (modified) (12 diffs)
• src/org/openstreetmap/josm/gui/mappaint/Environment.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java (modified) (1 diff)
• test/unit/org/openstreetmap/TestUtils.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (modified) (3 diffs)

trunk/data/validator/highway.mapcss

@@ +1 @@
+way[highway=~/^(motorway|trunk|primary|secondary|tertiary)(_link)?$/] {
+  set major_road;
+}
+way[highway=~/^(unclassified|residential|living_street|service)$/] {
+  set minor_road;
+}
+
 way[highway][name =~ /(?i).* (Ave|Blvd|Br|Brg|Cct|Cir|Cl|Cr|Crct|Cres|Crt|Ct|Dr|Drv|Esp|Espl|Hwy|Ln|Mw|Mwy|Pl|Rd|Qy|Qys|Sq|St|Str|Ter|Tce|Tr|Wy)[.]?$/] {
   throwWarning: tr("abbreviated street name");
@@ -40 +47 @@
 }
 
-way[highway =~ /motorway|trunk|primary|secondary|tertiary/][!ref] {
+way.major_road[!ref] {
   throwOther: tr("highway without a reference");
   assertMatch: "way highway=primary";
@@ -46 +53 @@
 }
 
-way[foot][highway =~ /motorway|trunk|primary|secondary|tertiary/] {
-  throwWarning: tr("{0} used with {1}", "{0.key}", "{1.tag}");
+way.major_road[foot] {
+  throwWarning: tr("{0} used with {1}", tr("major road"), "{0.tag}");
   suggestAlternative: "sidewalk";
-  suggestAlternative: "separate footway";
+  suggestAlternative: tr("separate footway");
   assertMatch: "way highway=primary foot=yes";
   assertNoMatch: "way highway=primary";

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

@@ -23 +23 @@
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.SequenceCommand;
-import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Tag;
-import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.preferences.CollectionProperty;
 import org.openstreetmap.josm.data.validation.FixableTestError;
@@ -34 +31 @@
 import org.openstreetmap.josm.data.validation.TestError;
 import org.openstreetmap.josm.gui.mappaint.Environment;
+import org.openstreetmap.josm.gui.mappaint.MultiCascade;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Condition;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Expression;
@@ -68 +66 @@
 
     static class TagCheck implements Predicate<OsmPrimitive> {
-        protected final List<Selector> selector;
+        protected final MapCSSRule rule;
         protected final List<PrimitiveToTag> change = new ArrayList<PrimitiveToTag>();
         protected final Map<String, String> keyChange = new LinkedHashMap<String, String>();
@@ -75 +73 @@
         protected final Map<String, Boolean> assertions = new HashMap<String, Boolean>();
 
-        TagCheck(List<Selector> selector) {
-            this.selector = selector;
+        TagCheck(MapCSSRule rule) {
+            this.rule = rule;
         }
 
@@ -113 +111 @@
 
         static TagCheck ofMapCSSRule(final MapCSSRule rule) {
-            final TagCheck check = new TagCheck(rule.selectors);
+            final TagCheck check = new TagCheck(rule);
+            boolean containsSetClassExpression = false;
             for (Instruction i : rule.declaration) {
                 if (i instanceof Instruction.AssignmentInstruction) {
@@ -142 +141 @@
                     } else if ("assertNoMatch".equals(ai.key) && val != null) {
                         check.assertions.put(val, false);
+                    } else if (ai.val instanceof Boolean && ((Boolean) ai.val)) {
+                        containsSetClassExpression = true;
                     } else {
                         throw new RuntimeException("Cannot add instruction " + ai.key + ": " + ai.val + "!");
@@ -147 +148 @@
                 }
             }
-            if (check.errors.isEmpty()) {
+            if (check.errors.isEmpty() && !containsSetClassExpression) {
                 throw new RuntimeException("No throwError/throwWarning/throwOther given! You should specify a validation error message for " + rule.selectors);
             } else if (check.errors.size() > 1) {
@@ -183 +184 @@
          * @param primitive the primitive to test
          * @return true when the primitive contains a deprecated tag
-         */
+         * @deprecated since it does not handle MapCSS-classes
+         */
+        @Deprecated
         boolean matchesPrimitive(OsmPrimitive primitive) {
             return whichSelectorMatchesPrimitive(primitive) != null;
@@ -189 +192 @@
 
         Selector whichSelectorMatchesPrimitive(OsmPrimitive primitive) {
-            final Environment env = new Environment().withPrimitive(primitive);
-            for (Selector i : selector) {
+            return whichSelectorMatchesEnvironment(new Environment().withPrimitive(primitive));
+        }
+
+        Selector whichSelectorMatchesEnvironment(Environment env) {
+            for (Selector i : rule.selectors) {
+                env.clearSelectorMatchingInformation();
                 if (i.matches(env)) {
                     return i;
@@ -318 +325 @@
          */
         TestError getErrorForPrimitive(OsmPrimitive p) {
-            final Selector matchingSelector = whichSelectorMatchesPrimitive(p);
-            if (matchingSelector != null) {
+            return getErrorForPrimitive(p, whichSelectorMatchesPrimitive(p));
+        }
+
+        TestError getErrorForPrimitive(OsmPrimitive p, Selector matchingSelector) {
+            if (matchingSelector != null && !errors.isEmpty()) {
                 final Command fix = fixPrimitive(p);
                 final String description = getDescriptionForMatchingSelector(matchingSelector);
@@ -331 +341 @@
             }
         }
+    }
+
+    static class MapCSSTagCheckerAndRule extends MapCSSTagChecker {
+        public final MapCSSRule rule;
+
+        MapCSSTagCheckerAndRule(MapCSSRule rule) {
+            this.rule = rule;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            return super.equals(obj)
+                    || (obj instanceof TagCheck && rule.equals(((TagCheck) obj).rule))
+                    || (obj instanceof MapCSSRule && rule.equals(obj));
+        }
+    }
+
+    /**
+     * Obtains all {@link TestError}s for the {@link OsmPrimitive} {@code p}.
+     */
+    public Collection<TestError> getErrorsForPrimitive(OsmPrimitive p) {
+        final ArrayList<TestError> r = new ArrayList<TestError>();
+        final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
+        for (TagCheck check : checks) {
+            final Selector selector = check.whichSelectorMatchesEnvironment(env);
+            if (selector != null) {
+                check.rule.execute(env);
+                final TestError error = check.getErrorForPrimitive(p, selector);
+                if (error != null) {
+                    error.setTester(new MapCSSTagCheckerAndRule(check.rule));
+                    r.add(error);
+                }
+            }
+        }
+        return r;
     }
 
@@ -340 +385 @@
     @Override
     public void check(OsmPrimitive p) {
-        for (TagCheck check : checks) {
-            final TestError error = check.getErrorForPrimitive(p);
-            if (error != null) {
-                error.setTester(this);
-                errors.add(error);
-            }
-        }
+        errors.addAll(getErrorsForPrimitive(p));
     }
 

trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java

@@ -21 +21 @@
     public StyleSource source;
     private Context context = Context.PRIMITIVE;
+    public static final String DEFAULT_LAYER = "default";
 
     /**

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

@@ -291 +291 @@
         @Override
         public boolean applies(Environment env) {
-            return not ^ env.mc.getCascade(env.layer).containsKey(id);
+            return env != null && env.mc != null && env.mc.getCascade(env.layer) != null && not ^ env.mc.getCascade(env.layer).containsKey(id);
         }
 

trunk/test/unit/org/openstreetmap/TestUtils.java

@@ -60 +60 @@
         assertThat(p.get("railway"), is("rail"));
     }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreatePrimitiveFail() throws Exception {
+        TestUtils.createPrimitive("noway name=Foo");
+    }
+
 }

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

@@ -9 +9 @@
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Tag;
-import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.validation.Severity;
-import org.openstreetmap.josm.tools.TextTagParser;
+import org.openstreetmap.josm.data.validation.TestError;
+import org.openstreetmap.josm.tools.Predicate;
+import org.openstreetmap.josm.tools.Predicates;
+import org.openstreetmap.josm.tools.Utils;
 
 import java.io.StringReader;
@@ -62 +63 @@
         n2.put("natural", "wood");
         assertFalse(check.matchesPrimitive(n2));
-        assertThat(MapCSSTagChecker.TagCheck.insertArguments(check.selector.get(0), "The key is {0.key} and the value is {0.value}"),
+        assertThat(MapCSSTagChecker.TagCheck.insertArguments(check.rule.selectors.get(0), "The key is {0.key} and the value is {0.value}"),
                 is("The key is natural and the value is marsh"));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void testCreatePrimitiveForAssertionFail() throws Exception {
-        final OsmPrimitive p = TestUtils.createPrimitive("noway name=Foo");
     }
 
@@ -80 +76 @@
             for (final Map.Entry<String, Boolean> i : check.assertions.entrySet()) {
                 final OsmPrimitive p = TestUtils.createPrimitive(i.getKey());
-                if (check.matchesPrimitive(p) != i.getValue()) {
+                final boolean isError = Utils.exists(c.getErrorsForPrimitive(p), new Predicate<TestError>() {
+                    @Override
+                    public boolean evaluate(TestError e) {
+                        //noinspection EqualsBetweenInconvertibleTypes
+                        return e.getTester().equals(check.rule);
+                    }
+                });
+                if (isError != i.getValue()) {
                     final String error = MessageFormat.format("Expecting test ''{0}'' (i.e., {1}) to {2} {3} (i.e., {4})",
-                            check.getMessage(), check.selector, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys());
+                            check.getMessage(), check.rule.selectors, i.getValue() ? "match" : "not match", i.getKey(), p.getKeys());
                     System.err.println(error);
                     assertionErrors.add(error);