Index: trunk/data/validator/highway.mapcss =================================================================== --- trunk/data/validator/highway.mapcss (revision 6600) +++ trunk/data/validator/highway.mapcss (revision 6601) @@ -1,2 +1,9 @@ +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,5 +47,5 @@ } -way[highway =~ /motorway|trunk|primary|secondary|tertiary/][!ref] { +way.major_road[!ref] { throwOther: tr("highway without a reference"); assertMatch: "way highway=primary"; @@ -46,8 +53,8 @@ } -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"; Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 6600) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 6601) @@ -23,9 +23,6 @@ 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,4 +31,5 @@ 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,5 +66,5 @@ static class TagCheck implements Predicate { - protected final List selector; + protected final MapCSSRule rule; protected final List change = new ArrayList(); protected final Map keyChange = new LinkedHashMap(); @@ -75,6 +73,6 @@ protected final Map assertions = new HashMap(); - TagCheck(List selector) { - this.selector = selector; + TagCheck(MapCSSRule rule) { + this.rule = rule; } @@ -113,5 +111,6 @@ 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,4 +141,6 @@ } 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,5 +148,5 @@ } } - 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,5 +184,7 @@ * @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,6 +192,10 @@ 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,6 +325,9 @@ */ 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,4 +341,39 @@ } } + } + + 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 getErrorsForPrimitive(OsmPrimitive p) { + final ArrayList r = new ArrayList(); + 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,11 +385,5 @@ @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)); } Index: trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java (revision 6600) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java (revision 6601) @@ -21,4 +21,5 @@ public StyleSource source; private Context context = Context.PRIMITIVE; + public static final String DEFAULT_LAYER = "default"; /** Index: trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java (revision 6600) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Condition.java (revision 6601) @@ -291,5 +291,5 @@ @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); } Index: trunk/test/unit/org/openstreetmap/TestUtils.java =================================================================== --- trunk/test/unit/org/openstreetmap/TestUtils.java (revision 6600) +++ trunk/test/unit/org/openstreetmap/TestUtils.java (revision 6601) @@ -60,3 +60,9 @@ assertThat(p.get("railway"), is("rail")); } + + @Test(expected = IllegalArgumentException.class) + public void testCreatePrimitiveFail() throws Exception { + TestUtils.createPrimitive("noway name=Foo"); + } + } Index: trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (revision 6600) +++ trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (revision 6601) @@ -9,9 +9,10 @@ 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,11 +63,6 @@ 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,7 +76,14 @@ for (final Map.Entry 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() { + @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);