diff --git a/data/core.yaml b/data/core.yaml index 59d33f9e1..4096936cc 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1436,6 +1436,9 @@ en: message: '{feature} has an invalid website.' message_multi: '{feature} has multiple invalid websites.' reference: 'Websites should start with "http" or "https".' + mismatched_geometry: + title: Mismatched Geometry + tip: "Find features with conflicting tags and geometry" missing_role: title: Missing Roles message: "{member} has no role within {relation}" @@ -1466,6 +1469,9 @@ en: noncanonical_brand: message: "{feature} looks like a brand with nonstandard tags" reference: "All features of the same brand should be tagged the same way." + point_as_vertex: + message: '{feature} should be a standalone point based on its tags' + reference: "Some features shouldn't be part of lines or areas." private_data: title: Private Information tip: "Find features that may contain private information" @@ -1473,9 +1479,7 @@ en: contact: message: '{feature} might be tagged with private contact information' tag_suggests_area: - title: Lines Tagged as Areas message: '{feature} should be a closed area based on the tag "{tag}"' - tip: "Find features that are tagged as lines and should possibly be tagged as areas" reference: "Areas must have connected endpoints." unknown_road: message: "{feature} has no classification" @@ -1503,6 +1507,9 @@ en: tip: "Find features with unsquare corners that can be drawn better" buildings: reference: "Buildings with unsquare corners can often be drawn more accurately." + vertex_as_point: + message: '{feature} should be part of a line or area based on its tags' + reference: "Some features shouldn't be standalone points." fix: connect_almost_junction: annotation: Connected very close features. diff --git a/dist/locales/en.json b/dist/locales/en.json index 333f96397..ea4928f64 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1777,6 +1777,10 @@ "reference": "Websites should start with \"http\" or \"https\"." } }, + "mismatched_geometry": { + "title": "Mismatched Geometry", + "tip": "Find features with conflicting tags and geometry" + }, "missing_role": { "title": "Missing Roles", "message": "{member} has no role within {relation}", @@ -1817,6 +1821,10 @@ "reference": "All features of the same brand should be tagged the same way." } }, + "point_as_vertex": { + "message": "{feature} should be a standalone point based on its tags", + "reference": "Some features shouldn't be part of lines or areas." + }, "private_data": { "title": "Private Information", "tip": "Find features that may contain private information", @@ -1826,9 +1834,7 @@ } }, "tag_suggests_area": { - "title": "Lines Tagged as Areas", "message": "{feature} should be a closed area based on the tag \"{tag}\"", - "tip": "Find features that are tagged as lines and should possibly be tagged as areas", "reference": "Areas must have connected endpoints." }, "unknown_road": { @@ -1868,6 +1874,10 @@ "reference": "Buildings with unsquare corners can often be drawn more accurately." } }, + "vertex_as_point": { + "message": "{feature} should be part of a line or area based on its tags", + "reference": "Some features shouldn't be standalone points." + }, "fix": { "connect_almost_junction": { "annotation": "Connected very close features." diff --git a/modules/core/validator.js b/modules/core/validator.js index e01a207a5..6a30221e8 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -349,7 +349,7 @@ export function coreValidator(context) { ran.impossible_oneway = true; } - runValidation('tag_suggests_area'); + runValidation('mismatched_geometry'); // run all rules not yet run Object.keys(_rules).forEach(runValidation); diff --git a/modules/validations/index.js b/modules/validations/index.js index bf3bd8abc..224f349ef 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -8,9 +8,9 @@ export { validationImpossibleOneway } from './impossible_oneway'; export { validationIncompatibleSource } from './incompatible_source'; export { validationFormatting } from './invalid_format'; export { validationMaprules } from './maprules'; +export { validationMismatchedGeometry } from './mismatched_geometry'; export { validationMissingRole } from './missing_role'; export { validationMissingTag } from './missing_tag'; export { validationOutdatedTags } from './outdated_tags'; export { validationPrivateData } from './private_data'; -export { validationTagSuggestsArea } from './tag_suggests_area'; -export { validationUnsquareWay } from './unsquare_way'; \ No newline at end of file +export { validationUnsquareWay } from './unsquare_way'; diff --git a/modules/validations/mismatched_geometry.js b/modules/validations/mismatched_geometry.js new file mode 100644 index 000000000..17163c00c --- /dev/null +++ b/modules/validations/mismatched_geometry.js @@ -0,0 +1,208 @@ +import { actionAddVertex } from '../actions/add_vertex'; +import { actionChangeTags } from '../actions/change_tags'; +import { actionMergeNodes } from '../actions/merge_nodes'; +import { osmNodeGeometriesForTags } from '../osm/tags'; +import { geoHasSelfIntersections, geoSphericalDistance } from '../geo'; +import { t } from '../util/locale'; +import { utilDisplayLabel, utilTagText } from '../util'; +import { validationIssue, validationIssueFix } from '../core/validation'; + + +export function validationMismatchedGeometry(context) { + var type = 'mismatched_geometry'; + + function tagSuggestingLineIsArea(entity) { + if (entity.type !== 'way' || entity.isClosed()) return null; + + var tagSuggestingArea = entity.tagSuggestingArea(); + if (!tagSuggestingArea) { + return null; + } + + if (context.presets().matchTags(tagSuggestingArea, 'line') === + context.presets().matchTags(tagSuggestingArea, 'area')) { + // these tags also allow lines and making this an area wouldn't matter + return null; + } + + return tagSuggestingArea; + } + + function makeConnectEndpointsFixOnClick(way, graph) { + // must have at least three nodes to close this automatically + if (way.nodes.length < 3) return null; + + var nodes = graph.childNodes(way), testNodes; + var firstToLastDistanceMeters = geoSphericalDistance(nodes[0].loc, nodes[nodes.length-1].loc); + + // if the distance is very small, attempt to merge the endpoints + if (firstToLastDistanceMeters < 0.75) { + testNodes = nodes.slice(); // shallow copy + testNodes.pop(); + testNodes.push(testNodes[0]); + // make sure this will not create a self-intersection + if (!geoHasSelfIntersections(testNodes, testNodes[0].id)) { + return function(context) { + var way = context.entity(this.issue.entityIds[0]); + context.perform( + actionMergeNodes([way.nodes[0], way.nodes[way.nodes.length-1]], nodes[0].loc), + t('issues.fix.connect_endpoints.annotation') + ); + }; + } + } + + // if the points were not merged, attempt to close the way + testNodes = nodes.slice(); // shallow copy + testNodes.push(testNodes[0]); + // make sure this will not create a self-intersection + if (!geoHasSelfIntersections(testNodes, testNodes[0].id)) { + return function(context) { + var wayId = this.issue.entityIds[0]; + var way = context.entity(wayId); + var nodeId = way.nodes[0]; + var index = way.nodes.length; + context.perform( + actionAddVertex(wayId, nodeId, index), + t('issues.fix.connect_endpoints.annotation') + ); + }; + } + } + + function lineTaggedAsAreaIssue(entity, graph) { + + var tagSuggestingArea = tagSuggestingLineIsArea(entity); + if (!tagSuggestingArea) return null; + + var fixes = []; + + var connectEndsOnClick = makeConnectEndpointsFixOnClick(entity, graph); + + fixes.push(new validationIssueFix({ + title: t('issues.fix.connect_endpoints.title'), + onClick: connectEndsOnClick + })); + + fixes.push(new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.remove_tag.title'), + onClick: function(context) { + var entityId = this.issue.entityIds[0]; + var entity = context.entity(entityId); + var tags = Object.assign({}, entity.tags); // shallow copy + for (var key in tagSuggestingArea) { + delete tags[key]; + } + context.perform( + actionChangeTags(entityId, tags), + t('issues.fix.remove_tag.annotation') + ); + } + })); + + return new validationIssue({ + type: type, + subtype: 'area_as_line', + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]); + return entity ? t('issues.tag_suggests_area.message', { + feature: utilDisplayLabel(entity, context), + tag: utilTagText({ tags: tagSuggestingArea }) + }) : ''; + }, + reference: showReference, + entityIds: [entity.id], + hash: JSON.stringify(tagSuggestingArea) + + // avoid stale "connect endpoints" fix + (typeof connectEndsOnClick), + fixes: fixes + }); + + + function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(t('issues.tag_suggests_area.reference')); + } + } + + function vertexTaggedAsPointIssue(entity, graph) { + // we only care about nodes + if (entity.type !== 'node') return null; + + // ignore tagless points + if (Object.keys(entity.tags).length === 0) return null; + + // address lines are special so just ignore them + if (entity.isOnAddressLine(graph)) return null; + + var geometry = entity.geometry(graph); + var allowedGeometries = osmNodeGeometriesForTags(entity.tags); + + if (geometry === 'point' && !allowedGeometries.point && allowedGeometries.vertex) { + + return new validationIssue({ + type: type, + subtype: 'vertex_as_point', + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]); + return entity ? t('issues.vertex_as_point.message', { + feature: utilDisplayLabel(entity, context) + }) : ''; + }, + reference: function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(t('issues.vertex_as_point.reference')); + }, + entityIds: [entity.id] + }); + + } else if (geometry === 'vertex' && !allowedGeometries.vertex && allowedGeometries.point) { + + return new validationIssue({ + type: type, + subtype: 'point_as_vertex', + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]); + return entity ? t('issues.point_as_vertex.message', { + feature: utilDisplayLabel(entity, context) + }) : ''; + }, + reference: function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(t('issues.point_as_vertex.reference')); + }, + entityIds: [entity.id] + }); + } + + return null; + } + + var validation = function checkMismatchedGeometry(entity, graph) { + var issues = [ + vertexTaggedAsPointIssue(entity, graph), + lineTaggedAsAreaIssue(entity, graph) + ]; + return issues.filter(Boolean); + }; + + validation.type = type; + + return validation; +} diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js deleted file mode 100644 index cfd92772e..000000000 --- a/modules/validations/tag_suggests_area.js +++ /dev/null @@ -1,129 +0,0 @@ -import { actionAddVertex } from '../actions/add_vertex'; -import { actionChangeTags } from '../actions/change_tags'; -import { actionMergeNodes } from '../actions/merge_nodes'; -import { geoHasSelfIntersections, geoSphericalDistance } from '../geo'; -import { t } from '../util/locale'; -import { utilDisplayLabel, utilTagText } from '../util'; -import { validationIssue, validationIssueFix } from '../core/validation'; - - -export function validationTagSuggestsArea(context) { - var type = 'tag_suggests_area'; - - - var validation = function checkTagSuggestsArea(entity, graph) { - if (entity.type !== 'way' || entity.isClosed()) return []; - - var tagSuggestingArea = entity.tagSuggestingArea(); - if (!tagSuggestingArea) { - return []; - } - - if (context.presets().matchTags(tagSuggestingArea, 'line') === - context.presets().matchTags(tagSuggestingArea, 'area')) { - // these tags also allow lines and making this an area wouldn't matter - return []; - } - - var tagText = utilTagText({ tags: tagSuggestingArea }); - var fixes = []; - - var connectEndpointsOnClick; - - // must have at least three nodes to close this automatically - if (entity.nodes.length >= 3) { - var nodes = graph.childNodes(entity), testNodes; - var firstToLastDistanceMeters = geoSphericalDistance(nodes[0].loc, nodes[nodes.length-1].loc); - - // if the distance is very small, attempt to merge the endpoints - if (firstToLastDistanceMeters < 0.75) { - testNodes = nodes.slice(); // shallow copy - testNodes.pop(); - testNodes.push(testNodes[0]); - // make sure this will not create a self-intersection - if (!geoHasSelfIntersections(testNodes, testNodes[0].id)) { - connectEndpointsOnClick = function(context) { - var way = context.entity(this.issue.entityIds[0]); - context.perform( - actionMergeNodes([way.nodes[0], way.nodes[way.nodes.length-1]], nodes[0].loc), - t('issues.fix.connect_endpoints.annotation') - ); - }; - } - } - - if (!connectEndpointsOnClick) { - // if the points were not merged, attempt to close the way - testNodes = nodes.slice(); // shallow copy - testNodes.push(testNodes[0]); - // make sure this will not create a self-intersection - if (!geoHasSelfIntersections(testNodes, testNodes[0].id)) { - connectEndpointsOnClick = function(context) { - var wayId = this.issue.entityIds[0]; - var way = context.entity(wayId); - var nodeId = way.nodes[0]; - var index = way.nodes.length; - context.perform( - actionAddVertex(wayId, nodeId, index), - t('issues.fix.connect_endpoints.annotation') - ); - }; - } - } - } - - fixes.push(new validationIssueFix({ - title: t('issues.fix.connect_endpoints.title'), - onClick: connectEndpointsOnClick - })); - - fixes.push(new validationIssueFix({ - icon: 'iD-operation-delete', - title: t('issues.fix.remove_tag.title'), - onClick: function(context) { - var entityId = this.issue.entityIds[0]; - var entity = context.entity(entityId); - var tags = Object.assign({}, entity.tags); // shallow copy - for (var key in tagSuggestingArea) { - delete tags[key]; - } - context.perform( - actionChangeTags(entityId, tags), - t('issues.fix.remove_tag.annotation') - ); - } - })); - - return [new validationIssue({ - type: type, - severity: 'warning', - message: function(context) { - var entity = context.hasEntity(this.entityIds[0]); - return entity ? t('issues.tag_suggests_area.message', { - feature: utilDisplayLabel(entity, context), - tag: tagText - }) : ''; - }, - reference: showReference, - entityIds: [entity.id], - hash: JSON.stringify(tagSuggestingArea) + - // avoid stale "connect endpoints" fix - (typeof connectEndpointsOnClick), - fixes: fixes - })]; - - - function showReference(selection) { - selection.selectAll('.issue-reference') - .data([0]) - .enter() - .append('div') - .attr('class', 'issue-reference') - .text(t('issues.tag_suggests_area.reference')); - } - }; - - validation.type = type; - - return validation; -} diff --git a/test/index.html b/test/index.html index 6587d173f..571ca9107 100644 --- a/test/index.html +++ b/test/index.html @@ -157,12 +157,12 @@ + -