Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 17399) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 17400) @@ -114,30 +114,19 @@ boolean ignoreWaySegmentCombination(Way w1, Way w2) { if (w1 == w2) - return false; - if (areLayerOrLevelDifferent(w1, w2)) { return true; - } + if (areLayerOrLevelDifferent(w1, w2)) + return true; if (isBuilding(w1) && isBuilding(w2)) - return true; - if (w1.hasKey(HIGHWAY) && w2.hasKey(HIGHWAY) && !Objects.equals(w1.get("level"), w2.get("level"))) { - return true; - } + return true; // handled by mapcss tests if (((isResidentialArea(w1) || w1.hasKey(BARRIER, HIGHWAY, RAILWAY, WATERWAY)) && isResidentialArea(w2)) || ((isResidentialArea(w2) || w2.hasKey(BARRIER, HIGHWAY, RAILWAY, WATERWAY)) && isResidentialArea(w1))) return true; - if (isSubwayOrTramOrRazed(w2)) { - return true; - } + if (isWaterArea(w1) && isWaterArea(w2)) + return true; // handled by mapcss tests if (w1.hasKey(RAILWAY) && w2.hasKey(RAILWAY) && w1.hasTag(RAILWAY, "yard") != w2.hasTag(RAILWAY, "yard")) { return true; // see #20089 } - if (isCoastline(w1) != isCoastline(w2)) { - return true; - } - if ((w1.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w2.hasTag(WATERWAY, "riverbank")) - || (w2.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w1.hasTag(WATERWAY, "riverbank"))) { - return true; - } - return isProposedOrAbandoned(w2); + return (w1.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w2.hasTag(WATERWAY, "riverbank")) + || (w2.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w1.hasTag(WATERWAY, "riverbank")); } @@ -274,12 +263,9 @@ /** - * Self crossing ways test (for all the rest) + * Self crossing ways test (for all ways) */ public static class SelfCrossing extends CrossingWays { protected static final int CROSSING_SELF = 604; - - CrossingWays.Ways normalTest = new Ways(); - CrossingWays.Boundaries boundariesTest = new Boundaries(); /** @@ -291,12 +277,6 @@ @Override - public boolean isPrimitiveUsable(OsmPrimitive p) { - return super.isPrimitiveUsable(p) && !(normalTest.isPrimitiveUsable(p) - || boundariesTest.isPrimitiveUsable(p)); - } - - @Override boolean ignoreWaySegmentCombination(Way w1, Way w2) { - return w1 != w2; // should not happen + return false; // we should not get here } } @@ -332,4 +312,8 @@ } + static boolean isWaterArea(OsmPrimitive w) { + return w.hasTag("natural", "water") || w.hasTag(LANDUSE, "reservoir"); + } + static boolean isHighway(OsmPrimitive w) { return w.hasTagDifferent(HIGHWAY, "rest_area", "services", "bus_stop", "platform"); @@ -358,5 +342,6 @@ @Override public void visit(Way w) { - if (this instanceof SelfCrossing) { + boolean findSelfCrossingOnly = this instanceof SelfCrossing; + if (findSelfCrossingOnly) { // free memory, we are not interested in previous ways cellSegments.clear(); @@ -378,5 +363,6 @@ List highlight; - if (!es1.intersects(es2) || ignoreWaySegmentCombination(es1.way, es2.way)) { + if (!es1.intersects(es2) + || (!findSelfCrossingOnly && ignoreWaySegmentCombination(es1.way, es2.way))) { continue; } @@ -475,5 +461,5 @@ * @return {@code true} if one or more segments of the way are crossing * @see SelfIntersectingWay - * @since xxx + * @since 17393 */ public static boolean isSelfCrossing(Way way) { Index: /trunk/test/data/crossingWays.osm =================================================================== --- /trunk/test/data/crossingWays.osm (revision 17400) +++ /trunk/test/data/crossingWays.osm (revision 17400) @@ -0,0 +1,474 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Index: /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CrossingWaysTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CrossingWaysTest.java (revision 17399) +++ /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CrossingWaysTest.java (revision 17400) @@ -6,18 +6,23 @@ import static org.junit.jupiter.api.Assertions.assertTrue; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.HashMap; import java.util.List; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import org.junit.jupiter.api.Test; import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.coor.EastNorth; import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.WaySegment; +import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.tests.CrossingWays.Boundaries; import org.openstreetmap.josm.data.validation.tests.CrossingWays.SelfCrossing; import org.openstreetmap.josm.data.validation.tests.CrossingWays.Ways; +import org.openstreetmap.josm.io.OsmReader; import org.openstreetmap.josm.testutils.JOSMTestRules; @@ -34,5 +39,5 @@ @RegisterExtension @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules rule = new JOSMTestRules().preferences(); + public JOSMTestRules rule = new JOSMTestRules().preferences().projection(); private static Way newUsableWay(String tags) { @@ -127,4 +132,5 @@ testMessage(601, test, "amenity=restaurant", "amenity=restaurant"); testMessage(611, test, "building=yes", "amenity=restaurant"); + testMessage(611, test, "building=yes", "natural=water"); testMessage(612, test, "building=yes", "highway=road"); testMessage(613, test, "building=yes", "railway=rail"); @@ -133,8 +139,10 @@ testMessage(620, test, "highway=road", "highway=road"); testMessage(621, test, "highway=road", "amenity=restaurant"); + testMessage(621, test, "highway=road", "natural=water"); testMessage(622, test, "highway=road", "railway=rail"); testMessage(623, test, "highway=road", "waterway=river"); testMessage(630, test, "railway=rail", "railway=rail"); testMessage(631, test, "railway=rail", "amenity=restaurant"); + testMessage(631, test, "railway=rail", "natural=water"); testMessage(632, test, "railway=rail", "waterway=river"); testMessage(641, test, "landuse=residential", "amenity=restaurant"); @@ -146,4 +154,5 @@ testMessage(663, test, "barrier=hedge", "railway=rail"); testMessage(664, test, "barrier=hedge", "waterway=river"); + testMessage(665, test, "barrier=hedge", "natural=water"); assertFalse(test.isPrimitiveUsable(newUsableWay("amenity=restaurant"))); @@ -174,9 +183,39 @@ SelfCrossing test = new CrossingWays.SelfCrossing(); // isPrimitiveUsable - assertFalse(test.isPrimitiveUsable(newUsableWay("highway=motorway"))); - assertFalse(test.isPrimitiveUsable(newUsableWay("barrier=yes"))); - assertFalse(test.isPrimitiveUsable(newUsableWay("boundary=administrative"))); assertFalse(test.isPrimitiveUsable(TestUtils.newWay("amenity=restaurant"))); // Unusable (0 node) - assertTrue(test.isPrimitiveUsable(newUsableWay("amenity=restaurant"))); // Usable (2 nodes) - } + } + + /** + * Various cases of crossing linear ways and areas, mainly for coverage + * @throws Exception if an error occurs + */ + @Test + void testCoverage() throws Exception { + DataSet ds = OsmReader.parseDataSet( + Files.newInputStream(Paths.get(TestUtils.getTestDataRoot(), "crossingWays.osm")), null); + CrossingWays crossingWays = new CrossingWays.Ways(); + crossingWays.startTest(null); + crossingWays.visit(ds.allPrimitives()); + crossingWays.endTest(); + + for (TestError e : crossingWays.getErrors()) { + // we don't report self crossing ways in this test + assertEquals(2, e.getPrimitives().size(), e.getPrimitives().toString()); + // see #20121: crossing water areas should not be reported + assertFalse(e.getPrimitives().stream().filter(Way.class::isInstance).allMatch(CrossingWays::isWaterArea)); + } + + CrossingWays selfCrossing = new CrossingWays.SelfCrossing(); + selfCrossing.startTest(null); + selfCrossing.visit(ds.allPrimitives()); + selfCrossing.endTest(); + assertEquals(1, selfCrossing.getErrors().size()); + + CrossingWays crossingBoundaries = new CrossingWays.Boundaries(); + crossingBoundaries.startTest(null); + crossingBoundaries.visit(ds.allPrimitives()); + crossingBoundaries.endTest(); + assertEquals(2, crossingBoundaries.getErrors().size()); + } + }