Ticket #16998: 16998.patch

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

@@ -38 +38 @@
     protected static final int SOURCE_MAXSPEED_CONTEXT_MISMATCH_VS_MAXSPEED = 2705;
     protected static final int SOURCE_MAXSPEED_CONTEXT_MISMATCH_VS_HIGHWAY = 2706;
     protected static final int SOURCE_WRONG_LINK = 2707;
+    protected static final int ROUNDABOUT_CONNECTIONS_COUNT = 2708;
 
     protected static final String SOURCE_MAXSPEED = "source:maxspeed";
 
@@ -68 +69 @@
     private int pedestrianWays;
     private int cyclistWays;
     private int carsWays;
+    private final Set<Node> connectionNodesComplained = new HashSet<>();
 
     /**
      * Constructs a new {@code Highways} test.
@@ -77 +79 @@
     }
 
     @Override
+    public void initialize() throws Exception {
+        super.initialize();
+        connectionNodesComplained.clear();
+    }
+
+    @Override
+    public void endTest() {
+        connectionNodesComplained.clear();
+        super.endTest();
+    }
+
+    @Override
     public void visit(Node n) {
         if (n.isUsable()) {
             if (!n.hasTag("crossing", "no")
@@ -96 +110 @@
     @Override
     public void visit(Way w) {
         if (w.isUsable()) {
-            if (w.isClosed() && w.hasTag(HIGHWAY, CLASSIFIED_HIGHWAYS) && w.hasTag("junction", "circular", "roundabout")
-                    && IN_DOWNLOADED_AREA_STRICT.test(w)) {
-                // TODO: find out how to handle splitted roundabouts (see #12841)
-                testWrongRoundabout(w);
+            if (isRoundabout(w) && w.hasTag(HIGHWAY, CLASSIFIED_HIGHWAYS) && IN_DOWNLOADED_AREA_STRICT.test(w)) {
+                if (w.isClosed()) {
+                    // TODO: find out how to handle split roundabouts (see #12841)
+                    testWrongRoundabout(w);
+                }
+                testRoundaboutConnections(w, connectionNodesComplained);
             }
             if (w.hasKey(SOURCE_MAXSPEED)) {
                 // Check maxspeed, including context against highway
@@ -109 +125 @@
         }
     }
 
+    /**
+     * Find nodes in roundabout with more than one connected highway
+     * @param w
+     * @param complained
+     */
+    private void testRoundaboutConnections(Way w, Set<Node> complained) {
+        List<OsmPrimitive> primitives = new ArrayList<>();
+        for (Node n : w.getNodes()) {
+            if (complained.contains(n))
+                continue;
+            int countConnectedArcs = 0;
+            primitives.clear();
+            for (Way p : n.getParentWays()) {
+                if (p == w || !p.isUsable() || isRoundabout(p) || !p.hasTag(HIGHWAY, CLASSIFIED_HIGHWAYS))
+                    continue;
+
+                primitives.add(p);
+                if (!p.isFirstLastNode(n)) {
+                    errors.add(TestError.builder(this, Severity.WARNING, ROUNDABOUT_CONNECTIONS_COUNT)
+                            .message(tr("Highway doesn't start or end at roundabout"))
+                            .primitives(Arrays.asList(p,w,n))
+                            .highlight(n)
+                            .build());
+                    complained.add(n);
+                }
+                countConnectedArcs++;
+            }
+            if (countConnectedArcs > 1) {
+                primitives.add(w);
+                primitives.add(n);
+                errors.add(TestError.builder(this, Severity.WARNING, ROUNDABOUT_CONNECTIONS_COUNT)
+                        .message(tr("Multiple highways connected in one node of a roundabout"))
+                        .primitives(new ArrayList<>(primitives))
+                        .highlight(n)
+                        .build());
+                complained.add(n);
+            }
+        }
+    }
+
+    private static boolean isRoundabout(Way w) {
+        return w.hasTag("junction", "circular", "roundabout");
+    }
+
     private void testWrongRoundabout(Way w) {
         Map<String, List<Way>> map = new HashMap<>();
         // Count all highways (per type) connected to this roundabout, except correct links
@@ -190 +250 @@
             // in roundabout designs that physically separate a specific turn from the main roundabout
             // But if we have more than a single adjacent class, and one of them is a roundabout, that's an error
             for (Way w : sameClass) {
-                if (w.hasTag("junction", "circular", "roundabout")) {
+                if (isRoundabout(w)) {
                     return false;
                 }
             }