diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js index 48040e9a7..d8cbdb9e7 100644 --- a/modules/validations/almost_junction.js +++ b/modules/validations/almost_junction.js @@ -9,19 +9,11 @@ import { geoHasSelfIntersections, geoSphericalClosestNode } from '../geo'; -import { - utilDisplayLabel -} from '../util'; -import { - actionAddMidpoint, - actionChangeTags, - actionMergeNodes -} from '../actions'; + +import { actionAddMidpoint, actionChangeTags, actionMergeNodes } from '../actions'; import { t } from '../util/locale'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; +import { utilDisplayLabel } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; /** @@ -32,16 +24,17 @@ export function validationAlmostJunction() { function isHighway(entity) { return entity.type === 'way' && entity.tags.highway && entity.tags.highway !== 'no'; } + function isNoexit(node) { return node.tags.noexit && node.tags.noexit === 'yes'; } function findConnectableEndNodesByExtension(way, graph, tree) { - var results = [], - nidFirst = way.nodes[0], - nidLast = way.nodes[way.nodes.length - 1], - nodeFirst = graph.entity(nidFirst), - nodeLast = graph.entity(nidLast); + var results = []; + var nidFirst = way.nodes[0]; + var nidLast = way.nodes[way.nodes.length - 1]; + var nodeFirst = graph.entity(nidFirst); + var nodeLast = graph.entity(nidLast); if (nidFirst === nidLast) return results; @@ -63,6 +56,7 @@ export function validationAlmostJunction() { } } } + if (!isNoexit(nodeLast) && graph.parentWays(nodeLast).length === 1) { var connNearLast = canConnectByExtend(way, way.nodes.length - 1, graph, tree); if (connNearLast !== null) { @@ -82,35 +76,37 @@ export function validationAlmostJunction() { return results; } + function canConnectByExtend(way, endNodeIdx, graph, tree) { - var EXTEND_TH_METERS = 5, - tipNid = way.nodes[endNodeIdx], // the 'tip' node for extension point - midNid = endNodeIdx === 0 ? way.nodes[1] : way.nodes[way.nodes.length - 2], // the other node of the edge - tipNode = graph.entity(tipNid), - midNode = graph.entity(midNid), - lon = tipNode.loc[0], - lat = tipNode.loc[1], - lon_range = geoMetersToLon(EXTEND_TH_METERS, lat) / 2, - lat_range = geoMetersToLat(EXTEND_TH_METERS) / 2, - queryExtent = geoExtent([ - [lon - lon_range, lat - lat_range], - [lon + lon_range, lat + lat_range] - ]); + var EXTEND_TH_METERS = 5; + var tipNid = way.nodes[endNodeIdx]; // the 'tip' node for extension point + var midNid = endNodeIdx === 0 ? way.nodes[1] : way.nodes[way.nodes.length - 2]; // the other node of the edge + var tipNode = graph.entity(tipNid); + var midNode = graph.entity(midNid); + var lon = tipNode.loc[0]; + var lat = tipNode.loc[1]; + var lon_range = geoMetersToLon(EXTEND_TH_METERS, lat) / 2; + var lat_range = geoMetersToLat(EXTEND_TH_METERS) / 2; + var queryExtent = geoExtent([ + [lon - lon_range, lat - lat_range], + [lon + lon_range, lat + lat_range] + ]); // first, extend the edge of [midNode -> tipNode] by EXTEND_TH_METERS and find the "extended tip" location - var edgeLen = geoSphericalDistance(midNode.loc, tipNode.loc), - t = EXTEND_TH_METERS / edgeLen + 1.0, - extTipLoc = geoVecInterp(midNode.loc, tipNode.loc, t); + var edgeLen = geoSphericalDistance(midNode.loc, tipNode.loc); + var t = EXTEND_TH_METERS / edgeLen + 1.0; + var extTipLoc = geoVecInterp(midNode.loc, tipNode.loc, t); // then, check if the extension part [tipNode.loc -> extTipLoc] intersects any other ways var intersected = tree.intersects(queryExtent, graph); for (var i = 0; i < intersected.length; i++) { if (!isHighway(intersected[i]) || intersected[i].id === way.id) continue; + var way2 = intersected[i]; for (var j = 0; j < way2.nodes.length - 1; j++) { - var nA = graph.entity(way2.nodes[j]), - nB = graph.entity(way2.nodes[j + 1]), - crossLoc = geoLineIntersection([tipNode.loc, extTipLoc], [nA.loc, nB.loc]); + var nA = graph.entity(way2.nodes[j]); + var nB = graph.entity(way2.nodes[j + 1]); + var crossLoc = geoLineIntersection([tipNode.loc, extTipLoc], [nA.loc, nB.loc]); if (crossLoc !== null) { return { wid: way2.id, @@ -123,29 +119,32 @@ export function validationAlmostJunction() { return null; } + var type = 'almost_junction'; - var validation = function(endHighway, context) { + var validation = function(endHighway, context) { if (!isHighway(endHighway)) return []; var graph = context.graph(); var tree = context.history().tree(); - var issues = []; + var extendableNodeInfos = findConnectableEndNodesByExtension(endHighway, graph, tree); extendableNodeInfos.forEach(function(extendableNodeInfo) { var node = extendableNodeInfo.node; var edgeHighway = graph.entity(extendableNodeInfo.wid); + var fixes = [new validationIssueFix({ title: t('issues.fix.connect_almost_junction.title'), onClick: function() { - var endNode = this.issue.entities[1], - targetEdge = this.issue.info.edge, - crossLoc = this.issue.info.cross_loc; + var endNode = this.issue.entities[1]; + var targetEdge = this.issue.info.edge; + var crossLoc = this.issue.info.cross_loc; var edgeNodes = [context.graph().entity(targetEdge[0]), context.graph().entity(targetEdge[1])]; var closestNodeInfo = geoSphericalClosestNode(edgeNodes, crossLoc); - // if there is already a point nearby, just connect to that + + // already a point nearby, just connect to that if (closestNodeInfo.distance < 0.75) { context.perform( actionMergeNodes([closestNodeInfo.node.id, endNode.id], closestNodeInfo.node.loc), @@ -160,6 +159,7 @@ export function validationAlmostJunction() { } } })]; + if (Object.keys(node.tags).length === 0) { // node has no tags, suggest noexit fix fixes.push(new validationIssueFix({ @@ -173,6 +173,7 @@ export function validationAlmostJunction() { } })); } + issues.push(new validationIssue({ type: type, severity: 'warning', diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js index 44b4f9df3..e0cdc672b 100644 --- a/modules/validations/crossing_ways.js +++ b/modules/validations/crossing_ways.js @@ -1,22 +1,13 @@ import _clone from 'lodash-es/clone'; import _map from 'lodash-es/map'; import _flattenDeep from 'lodash-es/flatten'; -import { - geoExtent, - geoLineIntersection, - geoSphericalClosestNode -} from '../geo'; -import { utilDisplayLabel } from '../util'; -import { t } from '../util/locale'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; + +import { actionAddMidpoint, actionMergeNodes } from '../actions'; +import { geoExtent, geoLineIntersection, geoSphericalClosestNode } from '../geo'; import { osmNode } from '../osm'; -import { - actionAddMidpoint, - actionMergeNodes -} from '../actions'; +import { t } from '../util/locale'; +import { utilDisplayLabel } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; export function validationCrossingWays() { @@ -25,7 +16,9 @@ export function validationCrossingWays() { function findEdgeToWayCrossCoords(n1, n2, way, graph) { var crossCoords = []; var nA, nB; - var segment1 = [n1.loc, n2.loc], segment2; + var segment1 = [n1.loc, n2.loc]; + var segment2; + var nodes = graph.childNodes(way); for (var j = 0; j < nodes.length - 1; j++) { nA = nodes[j]; @@ -44,6 +37,7 @@ export function validationCrossingWays() { return crossCoords; } + // returns the way or its parent relation, whichever has a useful feature type function getFeatureWithFeatureTypeTagsForWay(way, graph) { if (getFeatureTypeForTags(way.tags) === null) { @@ -59,21 +53,25 @@ export function validationCrossingWays() { return way; } + function hasTag(tags, key) { return tags[key] !== undefined && tags[key] !== 'no'; } + function getFeatureTypeForCrossingCheck(way, graph) { var tags = getFeatureWithFeatureTypeTagsForWay(way, graph).tags; return getFeatureTypeForTags(tags); } + // only validate certain waterway features var waterways = ['canal', 'ditch', 'drain', 'river', 'stream']; // ignore certain highway and railway features var ignoredHighways = ['rest_area', 'services']; var ignoredRailways = ['train_wash']; + function getFeatureTypeForTags(tags) { if (hasTag(tags, 'building')) return 'building'; @@ -87,6 +85,7 @@ export function validationCrossingWays() { return null; } + function extendTagsByInferredLayer(tags, way) { if (!hasTag(tags, 'layer')) { tags.layer = way.layer().toString(); @@ -94,9 +93,10 @@ export function validationCrossingWays() { return tags; } + function isLegitCrossing(way1, featureType1, way2, featureType2, graph) { - var tags1 = _clone(getFeatureWithFeatureTypeTagsForWay(way1, graph).tags), - tags2 = _clone(getFeatureWithFeatureTypeTagsForWay(way2, graph).tags); + var tags1 = _clone(getFeatureWithFeatureTypeTagsForWay(way1, graph).tags); + var tags2 = _clone(getFeatureWithFeatureTypeTagsForWay(way2, graph).tags); tags1 = extendTagsByInferredLayer(tags1, way1); tags2 = extendTagsByInferredLayer(tags2, way2); @@ -153,6 +153,7 @@ export function validationCrossingWays() { return false; } + // highway values for which we shouldn't recommend connecting to waterways var highwaysDisallowingFords = [ 'motorway', 'motorway_link', 'trunk', 'trunk_link', @@ -185,6 +186,7 @@ export function validationCrossingWays() { } if (featureType1 === 'waterway') return {}; if (featureType1 === 'railway') return {}; + } else { var featureTypes = [featureType1, featureType2]; if (featureTypes.indexOf('highway') !== -1) { @@ -199,6 +201,7 @@ export function validationCrossingWays() { return { railway: 'level_crossing' }; } } + if (featureTypes.indexOf('waterway') !== -1) { // do not allow fords on structures if (hasTag(entity1.tags, 'tunnel') && hasTag(entity2.tags, 'tunnel')) return null; @@ -216,6 +219,7 @@ export function validationCrossingWays() { return null; } + function findCrossingsByWay(primaryWay, graph, tree) { var edgeCrossInfos = []; if (primaryWay.type !== 'way') return edgeCrossInfos; @@ -224,27 +228,28 @@ export function validationCrossingWays() { if (primaryFeatureType === null) return edgeCrossInfos; for (var i = 0; i < primaryWay.nodes.length - 1; i++) { - var nid1 = primaryWay.nodes[i], - nid2 = primaryWay.nodes[i + 1], - n1 = graph.entity(nid1), - n2 = graph.entity(nid2), - extent = geoExtent([ - [ - Math.min(n1.loc[0], n2.loc[0]), - Math.min(n1.loc[1], n2.loc[1]) - ], - [ - Math.max(n1.loc[0], n2.loc[0]), - Math.max(n1.loc[1], n2.loc[1]) - ] - ]), - intersected = tree.intersects(extent, graph); + var nid1 = primaryWay.nodes[i]; + var nid2 = primaryWay.nodes[i + 1]; + var n1 = graph.entity(nid1); + var n2 = graph.entity(nid2); + var extent = geoExtent([ + [ + Math.min(n1.loc[0], n2.loc[0]), + Math.min(n1.loc[1], n2.loc[1]) + ], + [ + Math.max(n1.loc[0], n2.loc[0]), + Math.max(n1.loc[1], n2.loc[1]) + ] + ]); + + var intersected = tree.intersects(extent, graph); for (var j = 0; j < intersected.length; j++) { if (intersected[j].type !== 'way') continue; // only check crossing highway, waterway, building, and railway - var way = intersected[j], - wayFeatureType = getFeatureTypeForCrossingCheck(way, graph); + var way = intersected[j]; + var wayFeatureType = getFeatureTypeForCrossingCheck(way, graph); if (wayFeatureType === null || isLegitCrossing(primaryWay, primaryFeatureType, way, wayFeatureType, graph) || isLegitCrossing(way, wayFeatureType, primaryWay, primaryFeatureType, graph)) { @@ -268,8 +273,8 @@ export function validationCrossingWays() { var type = 'crossing_ways'; - var validation = function(entity, context) { + var validation = function(entity, context) { var graph = context.graph(); var tree = context.history().tree(); @@ -307,8 +312,8 @@ export function validationCrossingWays() { return issues; }; - function createIssue(crossing, context) { + function createIssue(crossing, context) { var graph = context.graph(); // use the entities with the tags that define the feature type @@ -336,14 +341,12 @@ export function validationCrossingWays() { if (connectionTags) { crossingTypeID += '_connectable'; } - } - else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) { + } else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) { crossingTypeID = 'bridge-bridge'; if (connectionTags) { crossingTypeID += '_connectable'; } - } - else { + } else { crossingTypeID = crossing.featureTypes.sort().join('-'); } @@ -357,9 +360,9 @@ export function validationCrossingWays() { fixes.push(new validationIssueFix({ title: t('issues.fix.add_connection_vertex.title'), onClick: function() { - var loc = this.issue.coordinates, - connectionTags = this.issue.info.connectionTags, - edges = this.issue.info.edges; + var loc = this.issue.coordinates; + var connectionTags = this.issue.info.connectionTags; + var edges = this.issue.info.edges; context.perform( function actionConnectCrossingWays(graph) { diff --git a/modules/validations/deprecated_tag.js b/modules/validations/deprecated_tag.js index 8940db34c..e12cbc380 100644 --- a/modules/validations/deprecated_tag.js +++ b/modules/validations/deprecated_tag.js @@ -1,17 +1,9 @@ import _clone from 'lodash-es/clone'; -import { t } from '../util/locale'; -import { - utilDisplayLabel, - utilTagText -} from '../util'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; -import { - actionChangeTags -} from '../actions'; +import { t } from '../util/locale'; +import { actionChangeTags } from '../actions'; +import { utilDisplayLabel, utilTagText } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; export function validationDeprecatedTag() { diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js index ca567477f..2104b2d26 100644 --- a/modules/validations/disconnected_way.js +++ b/modules/validations/disconnected_way.js @@ -1,13 +1,9 @@ import { t } from '../util/locale'; -import { - utilDisplayLabel -} from '../util'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; -import { operationDelete } from '../operations/index'; import { modeDrawLine } from '../modes'; +import { operationDelete } from '../operations/index'; +import { utilDisplayLabel } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; + export function validationDisconnectedWay() { diff --git a/modules/validations/generic_name.js b/modules/validations/generic_name.js index c3aece897..5212d8391 100644 --- a/modules/validations/generic_name.js +++ b/modules/validations/generic_name.js @@ -1,15 +1,8 @@ import _clone from 'lodash-es/clone'; import { t } from '../util/locale'; -import { - utilPreset -} from '../util'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; -import { - actionChangeTags -} from '../actions'; +import { utilPreset } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; +import { actionChangeTags } from '../actions'; import { discardNames } from '../../node_modules/name-suggestion-index/config/filters.json'; diff --git a/modules/validations/index.js b/modules/validations/index.js index 6dca7c307..235f2e540 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -1,8 +1,8 @@ +export { validationAlmostJunction } from './almost_junction'; +export { validationCrossingWays } from './crossing_ways'; export { validationDeprecatedTag } from './deprecated_tag'; export { validationDisconnectedWay } from './disconnected_way'; -export { validationCrossingWays } from './crossing_ways'; -export { validationAlmostJunction } from './almost_junction'; -export { validationGenericName } from './generic_name.js'; +export { validationGenericName } from './generic_name'; export { validationManyDeletions } from './many_deletions'; export { validationMaprules } from './maprules'; export { validationMissingTag } from './missing_tag'; diff --git a/modules/validations/many_deletions.js b/modules/validations/many_deletions.js index 0e4d1753e..676ce73a5 100644 --- a/modules/validations/many_deletions.js +++ b/modules/validations/many_deletions.js @@ -1,14 +1,10 @@ import { t } from '../util/locale'; -import { - validationIssue -} from '../core/validator'; +import { validationIssue } from '../core/validator'; + export function validationManyDeletions() { - var totalOtherGeomThreshold = 50; - - // relations are less common so use a lower threshold - var relationThreshold = 10; + var relationThreshold = 10; // relations are less common so use a lower threshold var type = 'many_deletions'; @@ -17,6 +13,7 @@ export function validationManyDeletions() { var points = 0, lines = 0, areas = 0, relations = 0; var base = context.history().base(); var geometry; + changes.deleted.forEach(function(entity) { if (entity.type === 'node' && entity.geometry(base) === 'point') { points++; @@ -31,9 +28,8 @@ export function validationManyDeletions() { relations++; } }); - if (points + lines + areas >= totalOtherGeomThreshold || - relations >= relationThreshold) { + if (points + lines + areas >= totalOtherGeomThreshold || relations >= relationThreshold) { var totalFeatures = points + lines + areas + relations; var messageType = 'points-lines-areas'; diff --git a/modules/validations/maprules.js b/modules/validations/maprules.js index 900d56b61..2b106b6a6 100644 --- a/modules/validations/maprules.js +++ b/modules/validations/maprules.js @@ -1,6 +1,7 @@ import { services } from '../services'; -export function validationMaprules() { + +export function validationMaprules() { var validation = function(entity, context) { if (!services.maprules) return []; diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index fff0d0811..7015f392e 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -1,15 +1,12 @@ import _without from 'lodash-es/without'; import _isEmpty from 'lodash-es/isEmpty'; + +import { operationDelete } from '../operations/index'; import { osmIsInterestingTag } from '../osm/tags'; import { t } from '../util/locale'; -import { - utilDisplayLabel -} from '../util'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; -import { operationDelete } from '../operations/index'; +import { utilDisplayLabel } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; + export function validationMissingTag() { @@ -24,7 +21,6 @@ export function validationMissingTag() { var type = 'missing_tag'; var validation = function(entity, context) { - var graph = context.graph(); // ignore vertex features and relation members @@ -32,14 +28,13 @@ export function validationMissingTag() { return []; } - var messageObj = {}, missingTagType; + var messageObj = {}; + var missingTagType; if (_isEmpty(entity.tags)) { missingTagType = 'any'; - } else if (!hasDescriptiveTags(entity)) { missingTagType = 'descriptive'; - } else if (entity.type === 'relation' && !entity.tags.type) { missingTagType = 'specific'; messageObj.tag = 'type'; diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js index f36cfab93..67c690e59 100644 --- a/modules/validations/old_multipolygon.js +++ b/modules/validations/old_multipolygon.js @@ -1,23 +1,16 @@ import { t } from '../util/locale'; -import { - osmIsOldMultipolygonOuterMember, - osmOldMultipolygonOuterMemberOfRelation -} from '../osm'; + +import { actionChangeTags } from '../actions'; +import { osmIsOldMultipolygonOuterMember, osmOldMultipolygonOuterMemberOfRelation } from '../osm'; import { utilDisplayLabel } from '../util'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; -import { - actionChangeTags -} from '../actions'; +import { validationIssue, validationIssueFix } from '../core/validator'; + export function validationOldMultipolygon() { - var type = 'old_multipolygon'; - var validation = function(entity, context) { + var validation = function(entity, context) { var issues = []; var graph = context.graph(); diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js index d1ee4771c..926bacbe9 100644 --- a/modules/validations/tag_suggests_area.js +++ b/modules/validations/tag_suggests_area.js @@ -1,30 +1,17 @@ import _clone from 'lodash-es/clone'; -import { t } from '../util/locale'; -import { - utilDisplayLabel, - utilTagText -} from '../util'; -import { - validationIssue, - validationIssueFix -} from '../core/validator'; -import { - actionAddVertex, - actionChangeTags, - actionMergeNodes -} from '../actions'; + +import { actionAddVertex, actionChangeTags, actionMergeNodes } from '../actions'; import { geoHasSelfIntersections, geoSphericalDistance } from '../geo'; +import { t } from '../util/locale'; +import { utilDisplayLabel, utilTagText } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validator'; export function validationTagSuggestsArea() { - var type = 'tag_suggests_area'; var validation = function(entity, context) { - - if (entity.type !== 'way') { - return []; - } + if (entity.type !== 'way') return []; var issues = []; var graph = context.graph(); @@ -35,9 +22,9 @@ export function validationTagSuggestsArea() { var tagText = utilTagText({ tags: tagSuggestingArea }); var fixes = []; var nodes = graph.childNodes(entity), testNodes; - var firstToLastDistanceMeters = geoSphericalDistance(nodes[0].loc, nodes[nodes.length-1].loc); var connectEndpointsOnClick; + // if the distance is very small, attempt to merge the endpoints if (firstToLastDistanceMeters < 0.75) { testNodes = _clone(nodes); @@ -54,6 +41,7 @@ export function validationTagSuggestsArea() { }; } } + if (!connectEndpointsOnClick) { // if the points were not merged, attempt to close the way testNodes = _clone(nodes); @@ -71,12 +59,14 @@ export function validationTagSuggestsArea() { }; } } + if (connectEndpointsOnClick) { fixes.push(new validationIssueFix({ title: t('issues.fix.connect_endpoints.title'), onClick: connectEndpointsOnClick })); } + fixes.push(new validationIssueFix({ title: t('issues.fix.remove_tag.title'), onClick: function() { @@ -91,6 +81,7 @@ export function validationTagSuggestsArea() { ); } })); + var featureLabel = utilDisplayLabel(entity, context); issues.push(new validationIssue({ type: type,