Ticket #19136: 19136.4.patch

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

@@ -73 +73 @@
     public static final int RINGS_SHARE_NODES = 1617;
     /** Incomplete multipolygon was modified */
     public static final int MODIFIED_INCOMPLETE = 1618;
+    /** No further tags for multipolygon */
+    public static final int NO_TAGS = 1619;
+    /** No area tag for multipolygon */
+    public static final int NO_AREA_TAG = 1620;
 
     private static final int FOUND_INSIDE = 1;
     private static final int FOUND_OUTSIDE = 2;
@@ -93 +97 @@
     @Override
     public void visit(Relation r) {
         if (r.isMultipolygon() && !r.isEmpty()) {
+            tagTest(r);
             List<TestError> tmpErrors = new ArrayList<>(30);
             boolean hasUnexpectedWayRoles = checkMembersAndRoles(r, tmpErrors);
             boolean hasRepeatedMembers = checkRepeatedWayMembers(r);
@@ -119 +124 @@
     }
 
     /**
+     * Check if there is any useful tag on a multipolygon relation.
+     * @param r relation
+     */
+    private void tagTest(Relation r) {
+        if (!r.isBoundary()) {
+            if (r.getInterestingTags().size() <= 1) {
+                errors.add(TestError.builder(this, Severity.ERROR, NO_TAGS)
+                        .message(tr("No further tags for multipolygon"))
+                        .primitives(r)
+                        .build());
+            } else if (!r.concernsArea()) {
+                errors.add(TestError.builder(this, Severity.WARNING, NO_AREA_TAG)
+                        .message(tr("No area tag for multipolygon"))
+                        .primitives(r)
+                        .build());
+            }
+        }
+    }
+
+    /**
      * 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 +154 @@
      * @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 +172 @@
                     }
                 }
                 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))
@@ -913 +926 @@
             boolean hasRepeatedMembers = checkRepeatedWayMembers(r);
             if (!hasRepeatedMembers) {
                 polygon = new Multipolygon(r);
-                // don't check style consistency here
+                // don't check style tags or consistency here
                 checkGeometryAndRoles(r, polygon);
             }
             createdRelation = null; // makes sure that repeatCheck is only set once

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

@@ -9 +9 @@
 import java.util.HashSet;
 import java.util.Set;
 
-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.Way;
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test;
@@ -179 +177 @@
         if (!w.isUsable() || w.isArea())
             return;
 
-        for (OsmPrimitive parent: w.getReferrers()) {
-            if (parent instanceof Relation && ((Relation) parent).isMultipolygon())
-                return;
-        }
-
         for (UnclosedWaysCheck c : checks) {
             TestError error = c.getTestError(w, this);
             if (error != null) {

test/unit/org/openstreetmap/josm/data/validation/tests/UnclosedWaysTest.java

@@ -78 +78 @@
     }
 
     /**
-     * Test to make sure the multipolygon ways are ignored
+     * Test to make sure the multipolygon ways are not ignored
+     * See #19136
      * @throws Exception if an exception occurs
      */
     @Test
@@ -95 +96 @@
         ds.addPrimitive(r);
         uwTest.visit(w);
         assertTrue(ElemStyles.hasAreaElemStyle(w, false));
-        assertEquals(0, uwTest.getErrors().size());
+        assertEquals(1, uwTest.getErrors().size());
 
         uwTest.endTest();
     }