diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f6535ce8..3c6f526a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,7 +128,9 @@ _Breaking changes, which may affect downstream projects or sites that embed iD, - Flag unreachable one-way highways and waterways flowing against each other ([#6216]) - Flag disconnected area and multipolygon highways ([#6075]) - Flag new highways disconnected from the larger road network ([#6284], thanks [@Bonkles], [@gaoxm]) +- Flag routing islands - Don't flag highways connected to ferry routes as disconnected ([#6287]) +- Flag disconnected ferry routes - Flag deprecated values among semicolon-delimited tags ([#6038]) - Add quick fixes for setting the `layer` to resolve certain Crossing Ways issues ([#5943]) - Rename "Generic Names" validation rule to "Suspicious Names" diff --git a/data/core.yaml b/data/core.yaml index dbfe60cc7..314b0b5e3 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1365,12 +1365,13 @@ en: reference: "Crossing indoor features should be connected or use different levels." disconnected_way: title: Disconnected Ways - tip: "Find unconnected highways and paths" + tip: "Find unroutable roads, paths, and ferry routes" + routable: + message: + multiple: "{count} routable features are connected only to each other." + reference: "All roads, paths, and ferry routes should connect to form a single routing network." highway: message: "{highway} is disconnected from other roads and paths" - message_new_road: "New {highway} is disconnected from existing road network" - reference: "Highways should connect to other highways or building entrances." - reference_new_road: "New roads should connect to existing road network or building entrances." fixme_tag: title: '"Fix Me" Requests' message: '{feature} has a "Fix Me" request' diff --git a/dist/locales/en.json b/dist/locales/en.json index 5e9fbb897..4d05835f7 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1684,12 +1684,15 @@ }, "disconnected_way": { "title": "Disconnected Ways", - "tip": "Find unconnected highways and paths", + "tip": "Find unroutable roads, paths, and ferry routes", + "routable": { + "message": { + "multiple": "{count} routable features are connected only to each other." + }, + "reference": "All roads, paths, and ferry routes should connect to form a single routing network." + }, "highway": { - "message": "{highway} is disconnected from other roads and paths", - "message_new_road": "New {highway} is disconnected from existing road network", - "reference": "Highways should connect to other highways or building entrances.", - "reference_new_road": "New roads should connect to existing road network or building entrances." + "message": "{highway} is disconnected from other roads and paths" } }, "fixme_tag": { @@ -9103,7 +9106,7 @@ "attribution": { "text": "Terms & Feedback" }, - "description": "DigitalGlobe-Vivid is a mosiac with optimal aesthetics and currency for any area on the globe, 50cm resolution or better, and image currency <20 month average globally.", + "description": "DigitalGlobe-Vivid is a mosaic with optimal aesthetics and currency for any area on the globe, 50cm resolution or better, and image currency <20 month average globally.", "name": "DigitalGlobe Vivid Imagery" }, "EOXAT2018CLOUDLESS": { diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js index 9f0097c6a..5d064a80a 100644 --- a/modules/validations/disconnected_way.js +++ b/modules/validations/disconnected_way.js @@ -13,87 +13,91 @@ export function validationDisconnectedWay() { return osmRoutableHighwayTagValues[entity.tags.highway]; } - function isNewRoad(entityId) { - return entityId[0] === 'w' && entityId[1] === '-'; - } - var validation = function checkDisconnectedWay(entity, context) { var graph = context.graph(); - if (!isTaggedAsHighway(entity)) return []; - - if (!isDisconnectedWay(entity) && - !isDisconnectedMultipolygon(entity) && - !isNewRoadUnreachableFromExistingRoads(entity, graph) - ) { - return []; - } + var routingIslandWays = routingIslandForEntity(entity); + if (!routingIslandWays) return []; var fixes = []; - var entityID = entity.id; - if (entity.type === 'way' && !entity.isClosed()) { - var firstID = entity.first(); - var lastID = entity.last(); + var isSingle = routingIslandWays.size === 1; - var first = context.entity(firstID); - if (first.tags.noexit !== 'yes') { + if (isSingle) { + + if (entity.type === 'way' && !entity.isClosed()) { + var firstID = entity.first(); + var lastID = entity.last(); + + var first = context.entity(firstID); + if (first.tags.noexit !== 'yes') { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-continue-left', + title: t('issues.fix.continue_from_start.title'), + entityIds: [firstID], + onClick: function() { + var wayId = this.issue.entityIds[0]; + var way = context.entity(wayId); + var vertexId = this.entityIds[0]; + var vertex = context.entity(vertexId); + continueDrawing(way, vertex, context); + } + })); + } + var last = context.entity(lastID); + if (last.tags.noexit !== 'yes') { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-continue', + title: t('issues.fix.continue_from_end.title'), + entityIds: [lastID], + onClick: function() { + var wayId = this.issue.entityIds[0]; + var way = context.entity(wayId); + var vertexId = this.entityIds[0]; + var vertex = context.entity(vertexId); + continueDrawing(way, vertex, context); + } + })); + } + + } else { fixes.push(new validationIssueFix({ - icon: 'iD-operation-continue-left', - title: t('issues.fix.continue_from_start.title'), - entityIds: [firstID], - onClick: function() { - var way = context.entity(entityID); - var vertex = context.entity(firstID); - continueDrawing(way, vertex, context); - } - })); - } - var last = context.entity(lastID); - if (last.tags.noexit !== 'yes') { - fixes.push(new validationIssueFix({ - icon: 'iD-operation-continue', - title: t('issues.fix.continue_from_end.title'), - entityIds: [lastID], - onClick: function() { - var way = context.entity(entityID); - var vertex = context.entity(lastID); - continueDrawing(way, vertex, context); - } + title: t('issues.fix.connect_feature.title') })); } + if (!operationDelete([entity.id], context).disabled()) { + fixes.push(new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.delete_feature.title'), + entityIds: [entity.id], + onClick: function() { + var id = this.issue.entityIds[0]; + var operation = operationDelete([id], context); + if (!operation.disabled()) { + operation(); + } + } + })); + } } else { fixes.push(new validationIssueFix({ - title: t('issues.fix.connect_feature.title') + title: t('issues.fix.connect_features.title') })); } - if (!operationDelete([entity.id], context).disabled()) { - fixes.push(new validationIssueFix({ - icon: 'iD-operation-delete', - title: t('issues.fix.delete_feature.title'), - entityIds: [entity.id], - onClick: function() { - var id = this.issue.entityIds[0]; - var operation = operationDelete([id], context); - if (!operation.disabled()) { - operation(); - } - } - })); - } - var suffix = isNewRoad(entity.id) ? '_new_road' : ''; return [new validationIssue({ type: type, severity: 'warning', message: function() { - var entity = context.hasEntity(this.entityIds[0]); - return entity ? t('issues.disconnected_way.highway.message' + suffix, { highway: utilDisplayLabel(entity, context) }) : ''; + if (this.entityIds.length === 1) { + var entity = context.hasEntity(this.entityIds[0]); + return entity ? t('issues.disconnected_way.highway.message', { highway: utilDisplayLabel(entity, context) }) : ''; + } + return t('issues.disconnected_way.routable.message.multiple', { count: this.entityIds.length.toString() }); }, - tooltip: t('issues.disconnected_way.highway.reference' + suffix), reference: showReference, - entityIds: [entity.id], + entityIds: Array.from(routingIslandWays).map(function(way) { return way.id; }), fixes: fixes })]; @@ -104,93 +108,82 @@ export function validationDisconnectedWay() { .enter() .append('div') .attr('class', 'issue-reference') - .text(t('issues.disconnected_way.highway.reference')); + .text(t('issues.disconnected_way.routable.reference')); } + function routingIslandForEntity(entity) { - function vertexIsDisconnected(way, vertex, relation) { - // can not accurately test vertices on tiles not downloaded from osm - #5938 - var osm = context.connection(); - if (osm && !osm.isDataLoaded(vertex.loc)) { - return false; - } + if (entity.type !== 'way') return null; - var parents = graph.parentWays(vertex); + if (!isRoutableWay(entity, true)) return null; - // standalone vertex - if (parents.length === 1) return true; + var waysToCheck = [entity]; + var routingIsland = new Set([entity]); - // entrances are considered connected - if (vertex.tags.entrance && vertex.tags.entrance !== 'no') return false; - if (vertex.tags.amenity === 'parking_entrance') return false; - - return !parents.some(function(parentWay) { - if (parentWay === way) return false; // ignore the way we're testing - // count connections to ferry routes as connected - if (parentWay.tags.route === 'ferry') return true; - if (isTaggedAsHighway(parentWay)) return true; - - return graph.parentRelations(parentWay).some(function(parentRelation) { - // ignore the relation we're testing, if any - if (relation && parentRelation === relation) return false; - - if (parentRelation.tags.type === 'route' && - parentRelation.tags.route === 'ferry') return true; - - return parentRelation.isMultipolygon() && isTaggedAsHighway(parentRelation); - }); - }); - } - - - function isDisconnectedWay(entity) { - if (entity.type !== 'way') return false; - return graph.childNodes(entity).every(function(vertex) { - return vertexIsDisconnected(entity, vertex); - }); - } - - - // check if entity is a new road that cannot eventually connect to any - // existing roads - function isNewRoadUnreachableFromExistingRoads(entity) { - if (!isNewRoad(entity.id) || !isTaggedAsHighway(entity)) return false; - - var visitedWids = new Set(); - return !connectToExistingRoadOrEntrance(entity, visitedWids); - } - - - function connectToExistingRoadOrEntrance(way, visitedWids) { - visitedWids.add(way.id); - for (var i = 0; i < way.nodes.length; i++) { - var vertex = graph.entity(way.nodes[i]); - if (vertex.tags.entrance && vertex.tags.entrance !== 'no') return true; - - var parentWays = graph.parentWays(vertex); - for (var j = 0; j < parentWays.length; j++) { - var parentWay = parentWays[j]; - if (visitedWids.has(parentWay.id)) continue; - if (isTaggedAsHighway(parentWay) && !isNewRoad(parentWay.id)) return true; - if (connectToExistingRoadOrEntrance(parentWay, visitedWids)) return true; + while (waysToCheck.length) { + var wayToCheck = waysToCheck.pop(); + var childNodes = graph.childNodes(wayToCheck); + for (var i in childNodes) { + var vertex = childNodes[i]; + var result = isConnectedVertex(vertex, routingIsland); + if (result === true) { + return null; + } else if (result === false) { + continue; + } + result.forEach(function(connectedWay) { + if (!routingIsland.has(connectedWay)) { + routingIsland.add(connectedWay); + waysToCheck.push(connectedWay); + } + }); } } + + return routingIsland; + } + + function isConnectedVertex(vertex, routingIslandWays) { + // assume ways overlapping unloaded tiles are connected to the wider road network - #5938 + var osm = context.connection(); + if (osm && !osm.isDataLoaded(vertex.loc)) return true; + + // entrances are considered connected + if (vertex.tags.entrance && + vertex.tags.entrance !== 'no') return true; + if (vertex.tags.amenity === 'parking_entrance') return true; + + var parentsWays = graph.parentWays(vertex); + + // standalone vertex + if (parentsWays.length === 1) return false; + + var connectedWays = new Set(); + + for (var i in parentsWays) { + var parentWay = parentsWays[i]; + + // ignore any way we've already accounted for + if (routingIslandWays.has(parentWay)) continue; + + if (isRoutableWay(parentWay, false)) connectedWays.add(parentWay); + } + + if (connectedWays.size) return connectedWays; + return false; } + function isRoutableWay(way, ignoreInnerWays) { + if (isTaggedAsHighway(way) || way.tags.route === 'ferry') return true; - function isDisconnectedMultipolygon(entity) { - if (entity.type !== 'relation' || !entity.isMultipolygon()) return false; + return graph.parentRelations(way).some(function(parentRelation) { + if (parentRelation.tags.type === 'route' && + parentRelation.tags.route === 'ferry') return true; - return entity.members.every(function(member) { - if (member.type !== 'way') return true; - - var way = graph.hasEntity(member.id); - if (!way) return true; - - return graph.childNodes(way).every(function(vertex) { - return vertexIsDisconnected(way, vertex, entity); - }); + if (parentRelation.isMultipolygon() && + isTaggedAsHighway(parentRelation) && + (!ignoreInnerWays || parentRelation.memberById(way.id).role !== 'inner')) return true; }); } diff --git a/modules/validations/impossible_oneway.js b/modules/validations/impossible_oneway.js index 5d5e35c26..7d49ef87e 100644 --- a/modules/validations/impossible_oneway.js +++ b/modules/validations/impossible_oneway.js @@ -207,7 +207,7 @@ export function validationImpossibleOneway() { } } - var validation = function checkDisconnectedWay(entity, context) { + var validation = function checkImpossibleOneway(entity, context) { if (entity.type !== 'way' || entity.geometry(context.graph()) !== 'line') return []; diff --git a/test/spec/validations/disconnected_way.js b/test/spec/validations/disconnected_way.js index dfd757002..00ea24cc4 100644 --- a/test/spec/validations/disconnected_way.js +++ b/test/spec/validations/disconnected_way.js @@ -87,33 +87,6 @@ describe('iD.validations.disconnected_way', function () { expect(issue.entityIds[0]).to.eql('w-1'); }); - it('flags disconnected highway with disconnected entrance vertex', function() { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4], tags: {'entrance': 'yes'}}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2'], tags: {'highway': 'unclassified'}}); - - context.perform( - iD.actionAddEntity(n1), - iD.actionAddEntity(n2), - iD.actionAddEntity(w) - ); - - var issues = validate(); - expect(issues).to.have.lengthOf(1); - var issue = issues[0]; - expect(issue.type).to.eql('disconnected_way'); - expect(issue.severity).to.eql('warning'); - expect(issue.entityIds).to.have.lengthOf(1); - expect(issue.entityIds[0]).to.eql('w-1'); - }); - - it('ignores highways that are connected to existing highways', function() { - createWayAtEndOfExistingOne({'highway': 'secondary'}, {'highway': 'secondary'}); - - var issues = validate(); - expect(issues).to.have.lengthOf(0); - }); - it('ignores highway with connected entrance vertex', function() { var n1 = iD.osmNode({id: 'n-1', loc: [4,4], tags: {'entrance': 'yes'}});