Standardize deprecated_tag and missing_tag rule type ids

Run crossing_ways before disconnected_way
Break out crossing_ways issue creation into a separate function

data/core.yaml

@@ -1211,7 +1211,7 @@ en:
         tip: Crossing bridges should use different layers.
       bridge-bridge_connectable:
         tip: Crossing bridges should be connected or use different layers.
-    deprecated_tags:
+    deprecated_tag:
       message: '{feature} has outdated tags: {tags}'
       tip: Some tags become deprecated over time and should be replaced.
     disconnected_way:
@@ -1224,15 +1224,15 @@ en:
     many_deletions:
       message: "Deleting {n} features: {p} nodes, {l} lines, {a} areas, and {r} relations."
       tip:  Only redundant or nonexistent features should be deleted.
+    missing_tag:
+      message: "{feature} has no descriptive tags."
+      tip: Features must have tags that define what they are.
     old_multipolygon:
       message: "{multipolygon} has misplaced tags."
       tip: Multipolygons should be tagged on their relation, not their outer way.
     tag_suggests_area:
       message: 'Feature is a line, but the tag "{tag}" suggests it should be an area.'
       tip: Areas must have connected endpoints.
-    untagged_feature:
-      message: "{feature} has no descriptive tags."
-      tip: Features must have tags that define what they are.
     fix:
       add_connection_vertex:
         title: Connect the features

dist/locales/en.json

@@ -1479,7 +1479,7 @@
                     "tip": "Crossing bridges should be connected or use different layers."
                 }
             },
-            "deprecated_tags": {
+            "deprecated_tag": {
                 "message": "{feature} has outdated tags: {tags}",
                 "tip": "Some tags become deprecated over time and should be replaced."
             },
@@ -1497,6 +1497,10 @@
                 "message": "Deleting {n} features: {p} nodes, {l} lines, {a} areas, and {r} relations.",
                 "tip": "Only redundant or nonexistent features should be deleted."
             },
+            "missing_tag": {
+                "message": "{feature} has no descriptive tags.",
+                "tip": "Features must have tags that define what they are."
+            },
             "old_multipolygon": {
                 "message": "{multipolygon} has misplaced tags.",
                 "tip": "Multipolygons should be tagged on their relation, not their outer way."
@@ -1505,10 +1509,6 @@
                 "message": "Feature is a line, but the tag \"{tag}\" suggests it should be an area.",
                 "tip": "Areas must have connected endpoints."
             },
-            "untagged_feature": {
-                "message": "{feature} has no descriptive tags.",
-                "tip": "Features must have tags that define what they are."
-            },
             "fix": {
                 "add_connection_vertex": {
                     "title": "Connect the features",

modules/core/validator.js

@@ -100,13 +100,16 @@ export function coreValidator(context) {
         // other validations require feature to be tagged
         if (!runValidation('missing_tag')) return issues;
         if (entity.type === 'way') {
+
+            runValidation('crossing_ways');
+
             // only check for disconnected way if no almost junctions
             if (runValidation('almost_junction')) {
                 runValidation('disconnected_way');
             } else {
                 ranValidations.add('disconnected_way');
             }
-            runValidation('crossing_ways');
+
             runValidation('tag_suggests_area');
         }
         // run all validations not yet run manually

modules/validations/almost_junction.js

@@ -106,6 +106,8 @@ export function validationAlmostJunction() {
         return null;
     }
 
+    var type = 'almost_junction';
+
     var validation = function(endHighway, context) {
         var graph = context.graph();
         var tree = context.history().tree();
@@ -144,7 +146,7 @@ export function validationAlmostJunction() {
                 }));
             }
             issues.push(new validationIssue({
-                type: 'almost_junction',
+                type: type,
                 severity: 'warning',
                 message: t('issues.almost_junction.message', {
                     feature: utilDisplayLabel(endHighway, context),
@@ -164,7 +166,7 @@ export function validationAlmostJunction() {
         return issues;
     };
 
-    validation.type = 'almost_junction';
+    validation.type = type;
 
     return validation;
 }

modules/validations/crossing_ways.js

@@ -240,12 +240,13 @@ export function validationCrossingWays() {
         return edgeCrossInfos;
     }
 
+    var type = 'crossing_ways';
+
     var validation = function(entity, context) {
 
         var graph = context.graph();
         var tree = context.history().tree();
-        // create one issue per crossing point
-        var issues = [];
+
         var waysToCheck = _flattenDeep(_map([entity], function(entity) {
             if (!getFeatureTypeForTags(entity.tags)) {
                 return [];
@@ -268,103 +269,110 @@ export function validationCrossingWays() {
             }
             return [];
         }));
+
         var crossings = waysToCheck.reduce(function(array, way) {
             return array.concat(findCrossingsByWay(way, graph, tree));
         }, []);
-        for (var j in crossings) {
-            var crossing = crossings[j];
-
-            // use the entities with the tags that define the feature type
-            var entities = crossing.ways.sort(function(entity1, entity2) {
-                var type1 = getFeatureTypeForCrossingCheck(entity1, graph);
-                var type2 = getFeatureTypeForCrossingCheck(entity2, graph);
-                if (type1 === type2) {
-                    return utilDisplayLabel(entity1, context) > utilDisplayLabel(entity2, context);
-                } else if (type1 === 'waterway') {
-                    return true;
-                } else if (type2 === 'waterway') {
-                    return false;
-                }
-                return type1 < type2;
-            });
-            entities = _map(entities, function(way) {
-                return getFeatureWithFeatureTypeTagsForWay(way, graph);
-            });
-
-            var connectionTags = tagsForConnectionNodeIfAllowed(entities[0], entities[1]);
-
-            var crossingTypeID;
-            if (hasTag(entities[0].tags, 'tunnel') && hasTag(entities[1].tags, 'tunnel')) {
-                crossingTypeID = 'tunnel-tunnel';
-                if (connectionTags) {
-                    crossingTypeID += '_connectable';
-                }
-            }
-            else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) {
-                crossingTypeID = 'bridge-bridge';
-                if (connectionTags) {
-                    crossingTypeID += '_connectable';
-                }
-            }
-            else {
-                crossingTypeID = crossing.featureTypes.sort().join('-');
-            }
-
-            var messageDict = {
-                feature: utilDisplayLabel(entities[0], context),
-                feature2: utilDisplayLabel(entities[1], context)
-            };
-
-            var fixes = [];
-            if (connectionTags) {
-                fixes.push(new validationIssueFix({
-                    title: t('issues.fix.add_connection_vertex.title'),
-                    onClick: function() {
-                        var loc = this.issue.coordinates;
-                        var ways = this.issue.info.ways;
-                        var connectionTags = this.issue.info.connectionTags;
-
-                        context.perform(
-                            function actionConnectCrossingWays(graph) {
-                                var projection = context.projection;
-
-                                var node = osmNode({ loc: loc, tags: connectionTags });
-                                graph = graph.replace(node);
-
-                                var way0 = graph.entity(ways[0].id);
-                                var choice0 = geoChooseEdge(graph.childNodes(way0), projection(loc), projection);
-                                var edge0 = [way0.nodes[choice0.index - 1], way0.nodes[choice0.index]];
-                                graph = actionAddMidpoint({loc: loc, edge: edge0}, node)(graph);
-
-                                var way1 = graph.entity(ways[1].id);
-                                var choice1 = geoChooseEdge(graph.childNodes(way1), projection(loc), projection);
-                                var edge1 = [way1.nodes[choice1.index - 1], way1.nodes[choice1.index]];
-                                graph = actionAddMidpoint({loc: loc, edge: edge1}, node)(graph);
-
-                                return graph;
-                            },
-                            t('issues.fix.add_connection_vertex.undo_redo')
-                        );
-                    }
-                }));
-            }
-
-            issues.push(new validationIssue({
-                type: 'crossing_ways',
-                severity: 'warning',
-                message: t('issues.crossing_ways.message', messageDict),
-                tooltip: t('issues.crossing_ways.'+crossingTypeID+'.tip'),
-                entities: entities,
-                info: { ways: crossing.ways, connectionTags: connectionTags },
-                coordinates: crossing.cross_point,
-                fixes: fixes
-            }));
-        }
 
+        var issues = [];
+        crossings.forEach(function(crossing) {
+            issues.push(createIssue(crossing, context));
+        });
         return issues;
     };
 
-    validation.type = 'crossing_ways';
+    function createIssue(crossing, context) {
+
+        var graph = context.graph();
+
+        // use the entities with the tags that define the feature type
+        var entities = crossing.ways.sort(function(entity1, entity2) {
+            var type1 = getFeatureTypeForCrossingCheck(entity1, graph);
+            var type2 = getFeatureTypeForCrossingCheck(entity2, graph);
+            if (type1 === type2) {
+                return utilDisplayLabel(entity1, context) > utilDisplayLabel(entity2, context);
+            } else if (type1 === 'waterway') {
+                return true;
+            } else if (type2 === 'waterway') {
+                return false;
+            }
+            return type1 < type2;
+        });
+        entities = _map(entities, function(way) {
+            return getFeatureWithFeatureTypeTagsForWay(way, graph);
+        });
+
+        var connectionTags = tagsForConnectionNodeIfAllowed(entities[0], entities[1]);
+
+        var crossingTypeID;
+        if (hasTag(entities[0].tags, 'tunnel') && hasTag(entities[1].tags, 'tunnel')) {
+            crossingTypeID = 'tunnel-tunnel';
+            if (connectionTags) {
+                crossingTypeID += '_connectable';
+            }
+        }
+        else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) {
+            crossingTypeID = 'bridge-bridge';
+            if (connectionTags) {
+                crossingTypeID += '_connectable';
+            }
+        }
+        else {
+            crossingTypeID = crossing.featureTypes.sort().join('-');
+        }
+
+        var messageDict = {
+            feature: utilDisplayLabel(entities[0], context),
+            feature2: utilDisplayLabel(entities[1], context)
+        };
+
+        var fixes = [];
+        if (connectionTags) {
+            fixes.push(new validationIssueFix({
+                title: t('issues.fix.add_connection_vertex.title'),
+                onClick: function() {
+                    var loc = this.issue.coordinates;
+                    var ways = this.issue.info.ways;
+                    var connectionTags = this.issue.info.connectionTags;
+
+                    context.perform(
+                        function actionConnectCrossingWays(graph) {
+                            var projection = context.projection;
+
+                            var node = osmNode({ loc: loc, tags: connectionTags });
+                            graph = graph.replace(node);
+
+                            var way0 = graph.entity(ways[0].id);
+                            var choice0 = geoChooseEdge(graph.childNodes(way0), projection(loc), projection);
+                            var edge0 = [way0.nodes[choice0.index - 1], way0.nodes[choice0.index]];
+                            graph = actionAddMidpoint({loc: loc, edge: edge0}, node)(graph);
+
+                            var way1 = graph.entity(ways[1].id);
+                            var choice1 = geoChooseEdge(graph.childNodes(way1), projection(loc), projection);
+                            var edge1 = [way1.nodes[choice1.index - 1], way1.nodes[choice1.index]];
+                            graph = actionAddMidpoint({loc: loc, edge: edge1}, node)(graph);
+
+                            return graph;
+                        },
+                        t('issues.fix.add_connection_vertex.undo_redo')
+                    );
+                }
+            }));
+        }
+
+        return new validationIssue({
+            type: type,
+            severity: 'warning',
+            message: t('issues.crossing_ways.message', messageDict),
+            tooltip: t('issues.crossing_ways.'+crossingTypeID+'.tip'),
+            entities: entities,
+            info: { ways: crossing.ways, connectionTags: connectionTags },
+            coordinates: crossing.cross_point,
+            fixes: fixes
+        });
+    }
+
+    validation.type = type;
 
     return validation;
 }

modules/validations/deprecated_tag.js

@@ -15,6 +15,8 @@ import {
 
 export function validationDeprecatedTag() {
 
+    var type = 'deprecated_tag';
+
     var validation = function(change, context) {
         var issues = [];
         var deprecatedTagsArray = change.deprecatedTags();
@@ -24,10 +26,10 @@ export function validationDeprecatedTag() {
                 var tagsLabel = utilTagText({ tags: deprecatedTags.old });
                 var featureLabel = utilDisplayLabel(change, context);
                 issues.push(new validationIssue({
-                    type: 'deprecated_tags',
+                    type: type,
                     severity: 'warning',
-                    message: t('issues.deprecated_tags.message', { feature: featureLabel, tags: tagsLabel }),
-                    tooltip: t('issues.deprecated_tags.tip'),
+                    message: t('issues.deprecated_tag.message', { feature: featureLabel, tags: tagsLabel }),
+                    tooltip: t('issues.deprecated_tag.tip'),
                     entities: [change],
                     hash: tagsLabel,
                     info: {
@@ -80,7 +82,7 @@ export function validationDeprecatedTag() {
         return issues;
     };
 
-    validation.type = 'deprecated_tag';
+    validation.type = type;
 
     return validation;
 }

modules/validations/disconnected_way.js

@@ -29,6 +29,7 @@ export function validationDisconnectedWay() {
             });
     }
 
+    var type = 'disconnected_way';
 
     var validation = function(entity, context) {
         var issues = [];
@@ -37,7 +38,7 @@ export function validationDisconnectedWay() {
             var entityLabel = utilDisplayLabel(entity, context);
 
             issues.push(new validationIssue({
-                type: 'disconnected_way',
+                type: type,
                 severity: 'warning',
                 message: t('issues.disconnected_way.highway.message', { highway: entityLabel }),
                 tooltip: t('issues.disconnected_way.highway.tip'),
@@ -81,7 +82,7 @@ export function validationDisconnectedWay() {
         return issues;
     };
 
-    validation.type = 'disconnected_way';
+    validation.type = type;
 
     return validation;
 }

modules/validations/generic_name.js

@@ -41,6 +41,7 @@ export function validationGenericName(context) {
         return false;
     }
 
+    var type = 'generic_name';
 
     var validation = function(entity) {
         var issues = [];
@@ -48,7 +49,7 @@ export function validationGenericName(context) {
         if (generic) {
             var preset = utilPreset(entity, context);
             issues.push(new validationIssue({
-                type: 'generic_name',
+                type: type,
                 severity: 'warning',
                 message: t('issues.generic_name.message', {feature: preset.name(), name: generic}),
                 tooltip: t('issues.generic_name.tip'),
@@ -73,7 +74,7 @@ export function validationGenericName(context) {
         return issues;
     };
 
-    validation.type = 'generic_name';
+    validation.type = type;
 
     return validation;
 }

modules/validations/many_deletions.js

@@ -7,6 +7,8 @@ export function validationManyDeletions() {
 
     var threshold = 100;
 
+    var type = 'many_deletions';
+
     var validation = function(changes, context) {
         var issues = [];
         var nodes = 0, ways = 0, areas = 0, relations = 0;
@@ -19,7 +21,7 @@ export function validationManyDeletions() {
         });
         if (changes.deleted.length > threshold) {
             issues.push(new validationIssue({
-                type: 'many_deletions',
+                type: type,
                 severity: 'warning',
                 message: t(
                     'issues.many_deletions.message',
@@ -33,7 +35,7 @@ export function validationManyDeletions() {
         return issues;
     };
 
-    validation.type = 'many_deletions';
+    validation.type = type;
     validation.inputType = 'changes';
 
     return validation;

modules/validations/missing_tag.js

@@ -20,6 +20,8 @@ export function validationMissingTag() {
         return keys.length > 0;
     }
 
+    var type = 'missing_tag';
+
     var validation = function(entity, context) {
         var types = ['point', 'line', 'area', 'relation'];
         var issues = [];
@@ -30,10 +32,10 @@ export function validationMissingTag() {
             !(hasDescriptiveTags(entity) || entity.hasParentRelations(graph))) {
             var entityLabel = utilDisplayLabel(entity, context);
             issues.push(new validationIssue({
-                type: 'missing_tag',
+                type: type,
                 severity: 'error',
-                message: t('issues.untagged_feature.message', {feature: entityLabel}),
-                tooltip: t('issues.untagged_feature.tip'),
+                message: t('issues.missing_tag.message', {feature: entityLabel}),
+                tooltip: t('issues.missing_tag.tip'),
                 entities: [entity],
                 fixes: [
                     new validationIssueFix({
@@ -56,7 +58,7 @@ export function validationMissingTag() {
         return issues;
     };
 
-    validation.type = 'missing_tag';
+    validation.type = type;
 
     return validation;
 }

modules/validations/old_multipolygon.js

@@ -14,6 +14,8 @@ import {
 
 export function validationOldMultipolygon() {
 
+    var type = 'old_multipolygon';
+
     var validation = function(entity, context) {
 
         var issues = [];
@@ -33,7 +35,7 @@ export function validationOldMultipolygon() {
         if (multipolygon && outerWay) {
             var multipolygonLabel = utilDisplayLabel(multipolygon, context);
             issues.push(new validationIssue({
-                type: 'old_multipolygon',
+                type: type,
                 severity: 'warning',
                 message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }),
                 tooltip: t('issues.old_multipolygon.tip'),
@@ -61,7 +63,7 @@ export function validationOldMultipolygon() {
         return issues;
     };
 
-    validation.type = 'old_multipolygon';
+    validation.type = type;
 
     return validation;
 }

modules/validations/tag_suggests_area.js

@@ -37,6 +37,8 @@ export function validationTagSuggestsArea() {
         return false;
     }
 
+    var type = 'tag_suggests_area';
+
     var validation = function(entity, context) {
         var issues = [];
         var graph = context.graph();
@@ -46,7 +48,7 @@ export function validationTagSuggestsArea() {
         if (suggestingTags) {
             var tagText = utilTagText({ tags: suggestingTags });
             issues.push(new validationIssue({
-                type: 'tag_suggests_area',
+                type: type,
                 severity: 'warning',
                 message: t('issues.tag_suggests_area.message', { tag: tagText }),
                 tooltip: t('issues.tag_suggests_area.tip'),
@@ -73,7 +75,7 @@ export function validationTagSuggestsArea() {
         return issues;
     };
 
-    validation.type = 'tag_suggests_area';
+    validation.type = type;
 
     return validation;
 }

test/spec/validations/deprecated_tag.js

@@ -45,7 +45,7 @@ describe('iD.validations.deprecated_tag', function () {
         var issues = validate();
         expect(issues).to.have.lengthOf(1);
         var issue = issues[0];
-        expect(issue.type).to.eql('deprecated_tags');
+        expect(issue.type).to.eql('deprecated_tag');
         expect(issue.severity).to.eql('warning');
         expect(issue.entities).to.have.lengthOf(1);
         expect(issue.entities[0].id).to.eql('w-1');