Flag points as areas or lines, areas or lines as points, and lines as areas (close #8231)

ARCHITECTURE.md

@@ -457,8 +457,17 @@ An issue with the active [MapRules](https://github.com/radiant-maxar/maprules) v
 A feature's tags indicate it should have a different geometry than it currently does.
 
 * `area_as_line`: an unclosed way has tags implying it should be a closed area (e.g. `area=yes` or `building=yes`)
-* `vertex_as_point`: a detached node has tags implying it should be part of a way (e.g. `highway=stop`)
+* `area_as_point`
+* `area_as_vertex`
+* `line_as_area`
+* `line_as_point`
+* `line_as_vertex`: a detached node has tags implying it should be a line (e.g. `highway=motorway`)
+* `point_as_area`
+* `point_as_line`
 * `point_as_vertex`: a vertex node has tags implying it should be detached from ways (e.g. `amenity=cafe`)
+* `vertex_as_area`
+* `vertex_as_line`
+* `vertex_as_point`: a detached node has tags implying it should be part of a way (e.g. `highway=stop`)
 * `unclosed_multipolygon_part`: a relation is tagged as a multipolygon but not all of its member ways form closed rings
 
 ##### `missing_role`

data/core.yaml

@@ -1650,6 +1650,8 @@ en:
         message: "{feature} ends very close to itself but does not reconnect"
       highway-highway:
         reference: Intersecting highways should share a junction vertex.
+    area_as_point:
+      message: '{feature} should be an area, not a point'
     close_nodes:
       title: "Very Close Points"
       tip: "Find redundant and crowded points"
@@ -1729,9 +1731,14 @@ en:
         message: '{feature} has an invalid email address'
         message_multi: '{feature} has multiple invalid email addresses'
         reference: 'Email addresses must look like "user@example.com".'
+    line_as_area:
+      message: '{feature} should be a line, not an area'
+    line_as_point:
+      message: '{feature} should be a line, not a point'
     mismatched_geometry:
       title: Mismatched Geometry
       tip: "Find features with conflicting tags and geometry"
+      reference: Most features are limited to certain geometry types.
     missing_role:
       title: Missing Roles
       message: "{member} has no role within {relation}"
@@ -1763,6 +1770,10 @@ en:
         message: "{feature} looks like a brand with nonstandard tags"
         message_incomplete: "{feature} looks like a brand with incomplete tags"
         reference: "All features of the same brand should be tagged the same way."
+    point_as_area:
+      message: '{feature} should be a point, not an area'
+    point_as_line:
+      message: '{feature} should be a point, not a line'
     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."

dist/locales/en.json

@@ -2067,6 +2067,9 @@
                     "reference": "Intersecting highways should share a junction vertex."
                 }
             },
+            "area_as_point": {
+                "message": "{feature} should be an area, not a point"
+            },
             "close_nodes": {
                 "title": "Very Close Points",
                 "tip": "Find redundant and crowded points",
@@ -2177,9 +2180,16 @@
                     "reference": "Email addresses must look like \"user@example.com\"."
                 }
             },
+            "line_as_area": {
+                "message": "{feature} should be a line, not an area"
+            },
+            "line_as_point": {
+                "message": "{feature} should be a line, not a point"
+            },
             "mismatched_geometry": {
                 "title": "Mismatched Geometry",
-                "tip": "Find features with conflicting tags and geometry"
+                "tip": "Find features with conflicting tags and geometry",
+                "reference": "Most features are limited to certain geometry types."
             },
             "missing_role": {
                 "title": "Missing Roles",
@@ -2222,6 +2232,12 @@
                     "reference": "All features of the same brand should be tagged the same way."
                 }
             },
+            "point_as_area": {
+                "message": "{feature} should be a point, not an area"
+            },
+            "point_as_line": {
+                "message": "{feature} should be a point, not a line"
+            },
             "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."

modules/validations/mismatched_geometry.js

@@ -1,3 +1,4 @@
+import deepEqual from 'fast-deep-equal';
 import { actionAddVertex } from '../actions/add_vertex';
 import { actionChangeTags } from '../actions/change_tags';
 import { actionMergeNodes } from '../actions/merge_nodes';
@@ -139,7 +140,7 @@ export function validationMismatchedGeometry() {
         }
     }
 
-    function vertexTaggedAsPointIssue(entity, graph) {
+    function vertexPointIssue(entity, graph) {
         // we only care about nodes
         if (entity.type !== 'node') return null;
 
@@ -196,46 +197,115 @@ export function validationMismatchedGeometry() {
                         .html(t.html('issues.point_as_vertex.reference'));
                 },
                 entityIds: [entity.id],
-                dynamicFixes: function(context) {
-
-                    var entityId = this.entityIds[0];
-
-                    var extractOnClick = null;
-                    if (!context.hasHiddenConnections(entityId)) {
-
-                        extractOnClick = function(context) {
-                            var entityId = this.issue.entityIds[0];
-                            var action = actionExtract(entityId);
-                            context.perform(
-                                action,
-                                t('operations.extract.annotation', { n: 1 })
-                            );
-                            // re-enter mode to trigger updates
-                            context.enter(modeSelect(context, [action.getExtractedNodeID()]));
-                        };
-                    }
-
-                    return [
-                        new validationIssueFix({
-                            icon: 'iD-operation-extract',
-                            title: t.html('issues.fix.extract_point.title'),
-                            onClick: extractOnClick
-                        })
-                    ];
-                }
+                dynamicFixes: dynamicExtractFixes
             });
         }
 
         return null;
     }
 
+
+    function otherMismatchIssue(entity, graph) {
+        // ignore boring features
+        if (!entity.hasInterestingTags()) return null;
+
+        if (entity.type !== 'node' && entity.type !== 'way') return null;
+
+        var sourceGeom = entity.geometry(graph);
+
+        var targetGeoms = entity.type === 'way' ? ['point', 'vertex'] : ['line', 'area'];
+
+        if (sourceGeom === 'area') targetGeoms.unshift('line');
+
+        var targetGeom = targetGeoms.find(nodeGeom => {
+            var asSource = presetManager.matchTags(entity.tags, sourceGeom);
+            var asTarget = presetManager.matchTags(entity.tags, nodeGeom);
+            if (!asSource || !asTarget ||
+                asSource === asTarget ||
+                // sometimes there are two presets with the same tags for different geometries
+                deepEqual(asSource.tags, asTarget.tags)) return false;
+
+            if (asTarget.isFallback()) return false;
+
+            var primaryKey = Object.keys(asTarget.tags)[0];
+
+            // special case: buildings-as-points are discouraged by iD, but common in OSM, so ignore them
+            if (primaryKey === 'building') return false;
+
+            if (asTarget.tags[primaryKey] === '*') return false;
+
+            return asSource.isFallback() || asSource.tags[primaryKey] === '*';
+        });
+
+        if (!targetGeom) return null;
+
+        var subtype = targetGeom + '_as_' + sourceGeom;
+
+        if (targetGeom === 'vertex') targetGeom = 'point';
+        if (sourceGeom === 'vertex') sourceGeom = 'point';
+
+        var referenceId = targetGeom + '_as_' + sourceGeom;
+
+        var dynamicFixes = targetGeom === 'point' ? dynamicExtractFixes : null;
+
+        return new validationIssue({
+            type: type,
+            subtype: subtype,
+            severity: 'warning',
+            message: function(context) {
+                var entity = context.hasEntity(this.entityIds[0]);
+                return entity ? t.html('issues.' + referenceId + '.message', {
+                    feature: utilDisplayLabel(entity, targetGeom)
+                }) : '';
+            },
+            reference: function showReference(selection) {
+                selection.selectAll('.issue-reference')
+                    .data([0])
+                    .enter()
+                    .append('div')
+                    .attr('class', 'issue-reference')
+                    .html(t.html('issues.mismatched_geometry.reference'));
+            },
+            entityIds: [entity.id],
+            dynamicFixes: dynamicFixes
+        });
+    }
+
+    function dynamicExtractFixes(context) {
+
+        var entityId = this.entityIds[0];
+
+        var extractOnClick = null;
+        if (!context.hasHiddenConnections(entityId)) {
+
+            extractOnClick = function(context) {
+                var entityId = this.issue.entityIds[0];
+                var action = actionExtract(entityId);
+                context.perform(
+                    action,
+                    t('operations.extract.annotation', { n: 1 })
+                );
+                // re-enter mode to trigger updates
+                context.enter(modeSelect(context, [action.getExtractedNodeID()]));
+            };
+        }
+
+        return [
+            new validationIssueFix({
+                icon: 'iD-operation-extract',
+                title: t.html('issues.fix.extract_point.title'),
+                onClick: extractOnClick
+            })
+        ];
+    }
+
     function unclosedMultipolygonPartIssues(entity, graph) {
 
         if (entity.type !== 'relation' ||
             !entity.isMultipolygon() ||
             entity.isDegenerate() ||
             // cannot determine issues for incompletely-downloaded relations
-            !entity.isComplete(graph)) return null;
+            !entity.isComplete(graph)) return [];
 
         var sequences = osmJoinWays(entity.members, graph);
 
@@ -285,12 +355,16 @@ export function validationMismatchedGeometry() {
     }
 
     var validation = function checkMismatchedGeometry(entity, graph) {
-        var issues = [
-            vertexTaggedAsPointIssue(entity, graph),
-            lineTaggedAsAreaIssue(entity)
-        ];
-        issues = issues.concat(unclosedMultipolygonPartIssues(entity, graph));
-        return issues.filter(Boolean);
+        var vertexPoint = vertexPointIssue(entity, graph);
+        if (vertexPoint) return [vertexPoint];
+
+        var lineAsArea = lineTaggedAsAreaIssue(entity);
+        if (lineAsArea) return [lineAsArea];
+
+        var mismatch = otherMismatchIssue(entity, graph);
+        if (mismatch) return [mismatch];
+
+        return unclosedMultipolygonPartIssues(entity, graph);
     };
 
     validation.type = type;