Ticket #19136: 19136.7.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_MAYBE_NO_AREA = 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 static final Collection<String> NO_AREA_KEY = Arrays.asList("name", "area", "ref", "access", "operator");
+
+    private void checkMultipolygonTags(OsmPrimitive p) {
+        if (p.isAnnotated() || hasAcceptedPrimaryTagForMultipolygon(p))
+            return;
+        if (p.keySet().stream().anyMatch(k -> k.matches("^(abandoned|disused|razed|removed|was).*")))
+            return;
+
+        TestError.Builder builder = null;
+        if (p.hasKey("surface")) {
+            // accept often used tag surface=* as area tag
+            builder = TestError.builder(this, Severity.OTHER, MULTIPOLYGON_INCOMPLETE)
+                    .message(tr("Multipolygon tags"), marktr("only {0} tag"), "surface");
+        } else {
+            Map<String, String> filteredTags = p.getInterestingTags();
+            filteredTags.remove("type");
+            filteredTags.keySet().removeIf(key -> !key.equals(harmonizeKey(key)));
+            NO_AREA_KEY.forEach(filteredTags::remove);
+            if (filteredTags.isEmpty()) {
+                builder = TestError.builder(this, Severity.ERROR, MULTIPOLYGON_NO_AREA)
+                        .message(tr("Multipolygon tags"), marktr("tag describing the area is missing"), new Object());
+
+            }
+        }
+        if (builder == null) {
+            // multipolygon has either no area tag or a rarely used one
+            builder = TestError.builder(this, Severity.WARNING, MULTIPOLYGON_MAYBE_NO_AREA)
+                    .message(tr("Multipolygon tags"), marktr("tag describing the area might be missing"), new Object());
+        }
+        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) {
+        if (p.hasKey("landuse", "amenity", "building", "building:part", "area:highway", "shop", "place", "boundary",
+                "landform", "piste:type", "sport", "golf", "landcover", "aeroway", "office", "healthcare", "craft", "room"))
+            return true;
+        if (p.hasTagDifferent("natural", "tree", "peek", "saddle", "tree_row")
+                || p.hasTagDifferent("man_made", "survey_point", "mast", "flagpole", "manhole", "watertap")
+                || p.hasTagDifferent("highway", "crossing", "bus_stop", "turning_circle", "street_lamp",
+                        "traffic_signals", "stop", "milestone", "mini_roundabout", "motorway_junction", "passing_place",
+                        "speed_camera", "traffic_mirror", "trailhead", "turning_circle", "turning_loop", "toll_gantry")
+                || p.hasTagDifferent("tourism", "attraction", "artwork")
+                || p.hasTagDifferent("leisure", "picnic_table", "slipway", "firepit")
+                || p.hasTagDifferent("historic", "wayside_cross", "milestone"))
+            return true;
+        return p.hasTag("barrier", "hedge", "retaining_wall")
+                || p.hasTag("public_transport", "platform", "station")
+                || p.hasTag("railway", "platform")
+                || p.hasTag("waterway", "riverbank", "dam", "rapids", "dock", "boatyard", "fuel")
+                || p.hasTag("indoor", "corridor", "room", "area")
+                || p.hasTag("power", "substation", "generator", "plant", "switchgear", "converter", "sub_station")
+                || 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("harbour", OsmUtils.TRUE_VALUE)
+                || p.hasTag("flood_prone", OsmUtils.TRUE_VALUE)
+                || p.hasTag("bridge", OsmUtils.TRUE_VALUE)
+                || p.hasTag("ruins", OsmUtils.TRUE_VALUE)
+                || p.hasTag("junction", OsmUtils.TRUE_VALUE);
+
+    }
+
     private void checkSingleTagValueSimple(MultiMap<OsmPrimitive, String> withErrors, OsmPrimitive p, String s, String key, String value) {
         if (!checkValues || value == null)
             return;