Index: trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (revision 14965) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java (revision 14966) @@ -64,4 +64,5 @@ public static final String ROLE_VERIF_PROBLEM_MSG = tr("Role verification problem"); private boolean ignoreMultiPolygons; + private boolean ignoreTurnRestrictions; /** @@ -109,5 +110,7 @@ if (t instanceof MultipolygonTest) { ignoreMultiPolygons = true; - break; + } + if (t instanceof TurnrestrictionTest) { + ignoreTurnRestrictions = true; } } @@ -125,4 +128,8 @@ if (ignoreMultiPolygons && n.isMultipolygon()) { // see #17010: don't report same problem twice + return; + } + if (ignoreTurnRestrictions && n.hasTag("type","restriction")) { + // see #17561: don't report same problem twice return; } @@ -321,5 +328,5 @@ if (!key.isEmpty()) { errors.add(TestError.builder(this, Severity.WARNING, ROLE_UNKNOWN) - .message(ROLE_VERIF_PROBLEM_MSG, marktr("Role ''{0}'' unknown in templates ''{1}''"), key, templates) + .message(ROLE_VERIF_PROBLEM_MSG, marktr("Role ''{0}'' is not in templates ''{1}''"), key, templates) .primitives(n) .build()); Index: trunk/src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java (revision 14965) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java (revision 14966) @@ -5,4 +5,5 @@ import java.util.ArrayList; +import java.util.Arrays; import java.util.List; @@ -38,4 +39,13 @@ protected static final int SUPERFLUOUS = 1815; protected static final int FROM_EQUALS_TO = 1816; + protected static final int UNKNOWN_RESTRICTION = 1817; + protected static final int TO_CLOSED_WAY = 1818; + protected static final int FROM_CLOSED_WAY = 1819; + + private static final List SUPPORTED_RESTRICTIONS = Arrays.asList( + "no_right_turn", "no_left_turn", "no_u_turn", "no_straight_on", + "only_right_turn", "only_left_turn", "only_straight_on", + "no_entry", "no_exit" + ); /** @@ -50,4 +60,12 @@ if (!r.hasTag("type", "restriction")) return; + + if (!r.hasTag("restriction", SUPPORTED_RESTRICTIONS)) { + errors.add(TestError.builder(this, Severity.ERROR, UNKNOWN_RESTRICTION) + .message(tr("Unknown restriction")) + .primitives(r) + .build()); + return; + } Way fromWay = null; @@ -98,5 +116,5 @@ default: errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_ROLE) - .message(tr("Unknown role")) + .message(tr("Unknown role in restriction")) .primitives(l) .highlight(m.getMember()) @@ -105,5 +123,6 @@ } else if (m.isNode()) { Node n = m.getNode(); - if ("via".equals(m.getRole())) { + switch (m.getRole()) { + case "via": if (!via.isEmpty()) { if (via.get(0) instanceof Node) { @@ -115,7 +134,15 @@ via.add(n); } - } else { + break; + case "location_hint": errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_ROLE) - .message(tr("Unknown role")) + .message(tr("Role location_hint in not in templates")) + .primitives(l) + .highlight(m.getMember()) + .build()); + break; + default: + errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_ROLE) + .message(tr("Unknown role in restriction")) .primitives(l) .highlight(m.getMember()) @@ -135,4 +162,5 @@ .primitives(r) .build()); + return; } if (moreto) { @@ -141,4 +169,5 @@ .primitives(r) .build()); + return; } if (morevia) { @@ -147,4 +176,5 @@ .primitives(r) .build()); + return; } if (mixvia) { @@ -153,4 +183,5 @@ .primitives(r) .build()); + return; } @@ -161,9 +192,24 @@ .build()); return; - } + } else if (fromWay.isClosed()) { + errors.add(TestError.builder(this, Severity.ERROR, FROM_CLOSED_WAY) + .message(tr("\"from\" way is a closed way")) + .primitives(r) + .highlight(fromWay) + .build()); + return; + } + if (toWay == null) { errors.add(TestError.builder(this, Severity.ERROR, NO_TO) .message(tr("No \"to\" way found")) .primitives(r) + .build()); + return; + } else if (toWay.isClosed()) { + errors.add(TestError.builder(this, Severity.ERROR, TO_CLOSED_WAY) + .message(tr("\"to\" way is a closed way")) + .primitives(r) + .highlight(toWay) .build()); return; @@ -186,20 +232,45 @@ if (via.get(0) instanceof Node) { final Node viaNode = (Node) via.get(0); - final Way viaPseudoWay = new Way(); - viaPseudoWay.addNode(viaNode); - checkIfConnected(fromWay, viaPseudoWay, - tr("The \"from\" way does not start or end at a \"via\" node."), FROM_VIA_NODE); - if (toWay.isOneway() != 0 && viaNode.equals(toWay.lastNode(true))) { + if (isFullOneway(toWay) && viaNode.equals(toWay.lastNode(true))) { errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) .message(tr("Superfluous turnrestriction as \"to\" way is oneway")) .primitives(r) - .build()); - return; - } - checkIfConnected(viaPseudoWay, toWay, + .highlight(toWay) + .build()); + return; + } + if (isFullOneway(fromWay) && viaNode.equals(fromWay.firstNode(true))) { + errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) + .message(tr("Superfluous turnrestriction as \"from\" way is oneway")) + .primitives(r) + .highlight(fromWay) + .build()); + return; + } + final Way viaPseudoWay = new Way(); + viaPseudoWay.addNode(viaNode); + checkIfConnected(r, fromWay, viaPseudoWay, + tr("The \"from\" way does not start or end at a \"via\" node."), FROM_VIA_NODE); + checkIfConnected(r, viaPseudoWay, toWay, tr("The \"to\" way does not start or end at a \"via\" node."), TO_VIA_NODE); } else { + if (isFullOneway(toWay) && ((Way) via.get(via.size() - 1)).isFirstLastNode(toWay.lastNode(true))) { + errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) + .message(tr("Superfluous turnrestriction as \"to\" way is oneway")) + .primitives(r) + .highlight(toWay) + .build()); + return; + } + if (isFullOneway(fromWay) && ((Way) via.get(0)).isFirstLastNode(fromWay.firstNode(true))) { + errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) + .message(tr("Superfluous turnrestriction as \"from\" way is oneway")) + .primitives(r) + .highlight(fromWay) + .build()); + return; + } // check if consecutive ways are connected: from/via[0], via[i-1]/via[i], via[last]/to - checkIfConnected(fromWay, (Way) via.get(0), + checkIfConnected(r, fromWay, (Way) via.get(0), tr("The \"from\" and the first \"via\" way are not connected."), FROM_VIA_WAY); if (via.size() > 1) { @@ -207,16 +278,9 @@ Way previous = (Way) via.get(i - 1); Way current = (Way) via.get(i); - checkIfConnected(previous, current, + checkIfConnected(r, previous, current, tr("The \"via\" ways are not connected."), UNCONNECTED_VIA); } } - if (toWay.isOneway() != 0 && ((Way) via.get(via.size() - 1)).isFirstLastNode(toWay.lastNode(true))) { - errors.add(TestError.builder(this, Severity.WARNING, SUPERFLUOUS) - .message(tr("Superfluous turnrestriction as \"to\" way is oneway")) - .primitives(r) - .build()); - return; - } - checkIfConnected((Way) via.get(via.size() - 1), toWay, + checkIfConnected(r, (Way) via.get(via.size() - 1), toWay, tr("The last \"via\" and the \"to\" way are not connected."), TO_VIA_WAY); } @@ -227,5 +291,5 @@ } - private void checkIfConnected(Way previous, Way current, String msg, int code) { + private void checkIfConnected(Relation r, Way previous, Way current, String msg, int code) { boolean c; if (isFullOneway(previous) && isFullOneway(current)) { @@ -243,7 +307,20 @@ } if (!c) { + List hilite = new ArrayList<>(); + if (previous.getNodesCount() == 1 && previous.isNew()) + hilite.add(previous.firstNode()); + else + hilite.add(previous); + if (current.getNodesCount() == 1 && current.isNew()) + hilite.add(current.firstNode()); + else + hilite.add(current); + List primitives = new ArrayList<>(); + primitives.add(r); + primitives.addAll(hilite); errors.add(TestError.builder(this, Severity.ERROR, code) .message(msg) - .primitives(previous, current) + .primitives(primitives) + .highlight(hilite) .build()); }