Index: trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 6388) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 6390) @@ -9,6 +9,8 @@ import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Set; import org.openstreetmap.josm.Main; @@ -22,4 +24,5 @@ import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.PolyData.Intersection; import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache; +import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; @@ -28,4 +31,5 @@ import org.openstreetmap.josm.gui.mappaint.ElemStyles; import org.openstreetmap.josm.gui.mappaint.MapPaintStyles; +import org.openstreetmap.josm.gui.progress.ProgressMonitor; /** @@ -50,4 +54,5 @@ private final List> nonClosedWays = new ArrayList>(); + private final Set keysCheckedByAnotherTest = new HashSet(); /** @@ -62,4 +67,16 @@ public void initialize() { styles = MapPaintStyles.getStyles(); + } + + @Override + public void startTest(ProgressMonitor progressMonitor) { + super.startTest(progressMonitor); + keysCheckedByAnotherTest.clear(); + for (Test t : OsmValidator.getEnabledTests(false)) { + if (t instanceof UnclosedWays) { + keysCheckedByAnotherTest.addAll(((UnclosedWays)t).getCheckedKeys()); + break; + } + } } @@ -124,9 +141,11 @@ List nodes = w.getNodes(); if (nodes.size()<1) return; // fix zero nodes bug - // Fix #9291 - Do not raise warning for sport=climbing + natural=cliff - if (!(w.hasTag("sport", "climbing") && w.hasTag("natural", "cliff"))) { - errors.add(new TestError(this, Severity.WARNING, tr("Area style way is not closed"), NOT_CLOSED, - Collections.singletonList(w), Arrays.asList(nodes.get(0), nodes.get(nodes.size() - 1)))); - } + for (String key : keysCheckedByAnotherTest) { + if (w.hasKey(key)) { + return; + } + } + errors.add(new TestError(this, Severity.WARNING, tr("Area style way is not closed"), NOT_CLOSED, + Collections.singletonList(w), Arrays.asList(nodes.get(0), nodes.get(nodes.size() - 1)))); } } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/UnclosedWays.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/UnclosedWays.java (revision 6388) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/UnclosedWays.java (revision 6390) @@ -7,4 +7,8 @@ import java.text.MessageFormat; import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import org.openstreetmap.josm.data.osm.OsmPrimitive; @@ -20,9 +24,10 @@ * * @author stoecker + * @since 3669 */ public class UnclosedWays extends Test { /** - * Constructor + * Constructs a new {@code UnclosedWays} test. */ public UnclosedWays() { @@ -30,82 +35,149 @@ } - private String type; - private String etype; - private int mode; - - public void set(int m, String text, String desc) { - etype = MessageFormat.format(text, desc); - type = tr(text, tr(desc)); - mode = m; + /** + * A check performed by UnclosedWays test. + * @since 6390 + */ + private class UnclosedWaysCheck { + /** The unique numeric code for this check */ + public final int code; + /** The OSM key checked */ + public final String key; + /** The English message */ + private final String engMessage; + /** The special values, to be ignored if ignore is set to true; to be considered only if ignore is set to false */ + private final List specialValues; + /** The boolean indicating if special values must be ignored or considered only */ + private final boolean ignore; + + /** + * Constructs a new {@code UnclosedWaysCheck}. + * @param code The unique numeric code for this check + * @param key The OSM key checked + * @param engMessage The English message + */ + @SuppressWarnings("unchecked") + public UnclosedWaysCheck(int code, String key, String engMessage) { + this(code, key, engMessage, (List) Collections.EMPTY_LIST); + } + + /** + * Constructs a new {@code UnclosedWaysCheck}. + * @param code The unique numeric code for this check + * @param key The OSM key checked + * @param engMessage The English message + * @param ignoredValues The ignored values. + */ + public UnclosedWaysCheck(int code, String key, String engMessage, List ignoredValues) { + this(code, key, engMessage, ignoredValues, true); + } + + /** + * Constructs a new {@code UnclosedWaysCheck}. + * @param code The unique numeric code for this check + * @param key The OSM key checked + * @param engMessage The English message + * @param specialValues The special values, to be ignored if ignore is set to true; to be considered only if ignore is set to false + * @param ignore indicates if special values must be ignored or considered only + */ + public UnclosedWaysCheck(int code, String key, String engMessage, List specialValues, boolean ignore) { + this.code = code; + this.key = key; + this.engMessage = engMessage; + this.specialValues = specialValues; + this.ignore = ignore; + } + + /** + * Returns the test error of the given way, if any. + * @param w The way to check + * @return The test error if the way is erroneous, {@code null} otherwise + */ + public final TestError getTestError(Way w) { + String value = w.get(key); + if (isValueErroneous(value)) { + String type = engMessage.contains("{0}") ? tr(engMessage, tr(value)) : tr(engMessage); + String etype = engMessage.contains("{0}") ? MessageFormat.format(engMessage, value) : engMessage; + return new TestError(UnclosedWays.this, Severity.WARNING, tr("Unclosed way"), + type, etype, code, Arrays.asList(w), + // The important parts of an unclosed way are the first and + // the last node which should be connected, therefore we highlight them + Arrays.asList(w.firstNode(), w.lastNode())); + } + return null; + } + + protected boolean isValueErroneous(String value) { + return value != null && ignore != specialValues.contains(value); + } } - public void set(int m, String text) { - etype = text; - type = tr(text); - mode = m; + /** + * A check performed by UnclosedWays test where the key is treated as boolean. + * @since 6390 + */ + private final class UnclosedWaysBooleanCheck extends UnclosedWaysCheck { + + /** + * Constructs a new {@code UnclosedWaysBooleanCheck}. + * @param code The unique numeric code for this check + * @param key The OSM key checked + * @param engMessage The English message + */ + public UnclosedWaysBooleanCheck(int code, String key, String engMessage) { + super(code, key, engMessage); + } + + @Override + protected boolean isValueErroneous(String value) { + Boolean btest = OsmUtils.getOsmBoolean(value); + // Not a strict boolean comparison to handle building=house like a building=yes + return (btest != null && btest) || (btest == null && value != null); + } + } + + private final UnclosedWaysCheck[] checks = { + new UnclosedWaysCheck(1101, "natural", marktr("natural type {0}"), Arrays.asList("coastline", "cliff", "tree_row")), + new UnclosedWaysCheck(1102, "landuse", marktr("landuse type {0}")), + new UnclosedWaysCheck(1103, "amenities", marktr("amenities type {0}")), + new UnclosedWaysCheck(1104, "sport", marktr("sport type {0}"), Arrays.asList("water_slide", "climbing")), + new UnclosedWaysCheck(1105, "tourism", marktr("tourism type {0}")), + new UnclosedWaysCheck(1106, "shop", marktr("shop type {0}")), + new UnclosedWaysCheck(1107, "leisure", marktr("leisure type {0}"), Arrays.asList("track")), + new UnclosedWaysCheck(1108, "waterway", marktr("waterway type {0}"), Arrays.asList("riverbank"), false), + new UnclosedWaysBooleanCheck(1120, "building", marktr("building")), + new UnclosedWaysBooleanCheck(1130, "area", marktr("area")), + }; + + /** + * Returns the set of checked OSM keys. + * @return The set of checked OSM keys. + * @since 6390 + */ + public Set getCheckedKeys() { + Set keys = new HashSet(); + for (UnclosedWaysCheck c : checks) { + keys.add(c.key); + } + return keys; } @Override public void visit(Way w) { - String test; - type = etype = null; - mode = 0; - if (!w.isUsable()) + if (!w.isUsable() || w.isArea()) return; - test = w.get("natural"); - if (test != null && !"coastline".equals(test) && !"cliff".equals(test) && !"tree_row".equals(test)) { - set(1101, marktr("natural type {0}"), test); - } - test = w.get("landuse"); - if (test != null) { - set(1102, marktr("landuse type {0}"), test); - } - test = w.get("amenities"); - if (test != null) { - set(1103, marktr("amenities type {0}"), test); - } - test = w.get("sport"); - if (test != null && !test.equals("water_slide") && !test.equals("climbing")) { - set(1104, marktr("sport type {0}"), test); - } - test = w.get("tourism"); - if (test != null) { - set(1105, marktr("tourism type {0}"), test); - } - test = w.get("shop"); - if (test != null) { - set(1106, marktr("shop type {0}"), test); - } - test = w.get("leisure"); - if (test != null && !"track".contains(test)) { - set(1107, marktr("leisure type {0}"), test); - } - test = w.get("waterway"); - if ("riverbank".equals(test)) { - set(1108, marktr("waterway type {0}"), test); - } - Boolean btest = OsmUtils.getOsmBoolean(w.get("building")); - if (btest != null && btest) { - set(1120, marktr("building")); - } - btest = OsmUtils.getOsmBoolean(w.get("area")); - if (btest != null && btest) { - set(1130, marktr("area")); + for (OsmPrimitive parent: w.getReferrers()) { + if (parent instanceof Relation && ((Relation)parent).isMultipolygon()) + return; } - if (type != null && !w.isArea()) { - for (OsmPrimitive parent: w.getReferrers()) { - if (parent instanceof Relation && ((Relation)parent).isMultipolygon()) - return; + for (UnclosedWaysCheck c : checks) { + TestError error = c.getTestError(w); + if (error != null) { + errors.add(error); + return; } - - errors.add(new TestError(this, Severity.WARNING, tr("Unclosed way"), - type, etype, mode, - Arrays.asList(w), - // The important parts of an unclosed way are the first and - // the last node which should be connected, therefore we highlight them - Arrays.asList(w.firstNode(), w.lastNode()))); } } Index: trunk/src/org/openstreetmap/josm/gui/mappaint/ElemStyles.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/ElemStyles.java (revision 6388) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/ElemStyles.java (revision 6390) @@ -450,5 +450,5 @@ * we pretend it is. This is useful for generating area styles from the (segmented) * outer ways of a multipolygon. - * @return {@code true} iff primitive has an AreaElemStyle + * @return {@code true} if primitive has an AreaElemStyle */ public static boolean hasAreaElemStyle(OsmPrimitive p, boolean pretendWayIsClosed) {