Ticket #19136: 19136.5.patch

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

@@ -120 +120 @@
 
     /**
      * Various style-related checks:<ul>
+     * <li>{@link #NO_STYLE}: No area style for multipolygon</li>
      * <li>{@link #INNER_STYLE_MISMATCH}: With the currently used mappaint style the style for inner way equals the multipolygon style</li>
-     * <li>{@link #OUTER_STYLE_MISMATCH}: Style for outer way mismatches</li>
+     * <li>{@link #OUTER_STYLE_MISMATCH}: With the currently used mappaint style the style for outer way mismatches the area style</li>
      * <li>{@link #OUTER_STYLE}: Area style on outer way</li>
      * </ul>
      * @param r relation
@@ -128 +129 @@
      * @param polygon multipolygon
      */
     private void checkStyleConsistency(Relation r, Multipolygon polygon) {
-        ElemStyles styles = MapPaintStyles.getStyles();
-        if (styles != null && !r.isBoundary()) {
+        if (MapPaintStyles.getStyles() != null && !r.isBoundary()) {
             AreaElement area = ElemStyles.getAreaElemStyle(r, false);
-            boolean areaStyle = area != null;
-            // If area style was not found for relation then use style of ways
             if (area == null) {
-                for (Way w : polygon.getOuterWays()) {
-                    area = ElemStyles.getAreaElemStyle(w, true);
-                    if (area != null) {
-                        break;
-                    }
-                }
-                if (area == null) {
-                    errors.add(TestError.builder(this, Severity.OTHER, NO_STYLE)
-                            .message(tr("No area style for multipolygon"))
-                            .primitives(r)
-                            .build());
-                }
-            }
-
-            if (area != null) {
+                errors.add(TestError.builder(this, Severity.OTHER, NO_STYLE)
+                        .message(tr("No area style for multipolygon"))
+                        .primitives(r)
+                        .build());
+            } else {
                 for (Way wInner : polygon.getInnerWays()) {
-                    if (area.equals(ElemStyles.getAreaElemStyle(wInner, false))) {
+                    if (wInner.isClosed() && area.equals(ElemStyles.getAreaElemStyle(wInner, false))) {
                         errors.add(TestError.builder(this, Severity.OTHER, INNER_STYLE_MISMATCH)
                                 .message(tr("With the currently used mappaint style the style for inner way equals the multipolygon style"))
                                 .primitives(Arrays.asList(r, wInner))
@@ -159 +147 @@
                     }
                 }
                 for (Way wOuter : polygon.getOuterWays()) {
+                    if (!wOuter.isArea())
+                        continue;
                     AreaElement areaOuter = ElemStyles.getAreaElemStyle(wOuter, false);
                     if (areaOuter != null) {
                         if (!area.equals(areaOuter)) {
-                            String message = !areaStyle ? tr("Style for outer way mismatches")
-                                    : tr("With the currently used mappaint style the style for outer way mismatches the area style");
                             errors.add(TestError.builder(this, Severity.OTHER, OUTER_STYLE_MISMATCH)
-                                    .message(message)
+                                    .message(tr("With the currently used mappaint style the style for outer way mismatches the area style"))
                                     .primitives(Arrays.asList(r, wOuter))
                                     .highlight(wOuter)
                                     .build());
-                        } else if (areaStyle) { /* style on outer way of multipolygon, but equal to polygon */
+                        } else { /* style on outer way of multipolygon, but equal to polygon */
                             errors.add(TestError.builder(this, Severity.WARNING, OUTER_STYLE)
                                     .message(tr("Area style on outer way"))
                                     .primitives(Arrays.asList(r, wOuter))

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

@@ -38 +38 @@
 import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.osm.AbstractPrimitive;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.OsmUtils;
+import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Tag;
 import org.openstreetmap.josm.data.osm.TagMap;
 import org.openstreetmap.josm.data.osm.Tagged;
@@ -188 +190 @@
     protected static final int MISSPELLED_VALUE_NO_FIX  = 1215;
     protected static final int UNUSUAL_UNICODE_CHAR_VALUE = 1216;
     protected static final int INVALID_PRESETS_TYPE     = 1217;
+    protected static final int MULTIPOLYGON_NO_AREA     = 1218;
+    protected static final int MULTIPOLYGON_INCOMPLETE  = 1219;
+    protected static final int MULTIPOLYGON_NO_TAG      = 1220;
     // CHECKSTYLE.ON: SingleSpaceSeparator
 
     protected EditableList sourcesList;
@@ -622 +627 @@
             }
         }
 
+        if (p instanceof Relation && p.hasTag("type", "multipolygon")) {
+            checkMultipolygonTags(p);
+        }
+
         if (checkPresetsTypes) {
             TagMap tags = p.getKeys();
             TaggingPresetType presetType = TaggingPresetType.forPrimitive(p);
@@ -659 +668 @@
         }
     }
 
+    private void checkMultipolygonTags(OsmPrimitive p) {
+        if (p.isAnnotated() || hasAcceptedPrimaryTagForMultipolygon(p))
+            return;
+        Map<String, String> filteredTags = p.getInterestingTags();
+        TestError.Builder builder = null;
+        if (filteredTags.size() == 1) {
+            builder = TestError.builder(this, Severity.ERROR, MULTIPOLYGON_NO_TAG)
+                    .message(tr("multipolygon relation: no further tags"));
+        }
+        if (filteredTags.size() == 2) {
+            if (filteredTags.containsKey("name")) {
+                builder = TestError.builder(this, Severity.ERROR, MULTIPOLYGON_INCOMPLETE)
+                        .message(tr("multipolygon relation: only {0} tag", "name"));
+            }
+            if (filteredTags.containsKey("area")) {
+                builder = TestError.builder(this, Severity.WARNING, MULTIPOLYGON_INCOMPLETE)
+                        .message(tr("multipolygon relation: only {0} tag", "area"));
+
+            }
+        }
+        if (builder == null && filteredTags.containsKey("surface")) {
+            builder = TestError.builder(this, Severity.OTHER, MULTIPOLYGON_INCOMPLETE)
+                    .message(tr("multipolygon relation: only {0} tag", "surface"));
+        }
+        if (builder == null) {
+            builder = TestError.builder(this, Severity.WARNING, MULTIPOLYGON_NO_AREA)
+                    .message(tr("multipolygon relation: found no area type tag"));
+        }
+        errors.add(builder.primitives(p).build());
+    }
+
+    /**
+     * Check if a multipolygon has a main tag that describes the type of area. Accepts also some deprecated tags and typos.
+     * @param p the multipolygon
+     * @return true if the multipolygon has a main tag that (likely) describes the type of area.
+     */
+    private static boolean hasAcceptedPrimaryTagForMultipolygon(OsmPrimitive p) {
+        return p.hasKey("landuse", "amenity", "building", "building:part", "area:highway", "shop", "place", "landform",
+                "piste:type", "sport", "golf", "landcover", "aeroway", "office", "healthcare", "disused:amenity",
+                "disused:landuse", "disused:building", "disused:leisure")
+                || p.hasTagDifferent("natural", "tree", "peek", "saddle", "tree_row")
+                || p.hasTagDifferent("man_made", "survey_point", "mast", "flagpole", "manhole", "watertap")
+                || p.hasTag("highway", "pedestrian", "services", "rest_area", "platform", "footway")
+                || (p.hasTag("highway", "residential", "footway") && p.hasTag("area", OsmUtils.TRUE_VALUE)) // alternative to place=square
+                || p.hasTag("barrier", "hedge", "retaining_wall")
+                || p.hasTag("boundary", "national_park", "protected_area")
+                || p.hasTag("public_transport", "platform", "station")
+                || p.hasTag("railway", "platform")
+                || p.hasTag("waterway", "riverbank")
+                || p.hasTagDifferent("tourism", "attraction", "artwork")
+                || p.hasTagDifferent("leisure", "picnic_table", "slipway", "firepit")
+                || p.hasTag("indoor", "corridor", "room", "area")
+                || p.hasTag("power", "substation", "generator", "plant", "switchgear", "converter", "sub_station")
+                || p.hasTagDifferent("historic", "wayside_cross", "milestone")
+                || p.hasTag("seamark:type", "harbour", "fairway", "anchorage", "landmark", "berth", "harbour_basin",
+                        "separation_zone")
+                || (p.get("seamark:type") != null && p.get("seamark:type").matches(".*\\_(area|zone)$"))
+                || p.hasTag("junction", "yes");
+
+    }
+
     private void checkSingleTagValueSimple(MultiMap<OsmPrimitive, String> withErrors, OsmPrimitive p, String s, String key, String value) {
         if (!checkValues || value == null)
             return;