From 3a933de876649c133f7766d02601bde371d79753 Mon Sep 17 00:00:00 2001
From: Quincy Morgan <quincy.morgan@icloud.com>
Date: Mon, 13 Apr 2020 18:57:26 -0700
Subject: [PATCH] Don't add tags to connection node when connecting crossing
 line/area

---
 modules/validations/crossing_ways.js   | 15 +++++---
 test/spec/validations/crossing_ways.js | 47 ++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js
index 14c9d8448..cf6559219 100644
--- a/modules/validations/crossing_ways.js
+++ b/modules/validations/crossing_ways.js
@@ -135,6 +135,11 @@ export function validationCrossingWays(context) {
     function tagsForConnectionNodeIfAllowed(entity1, entity2, graph) {
         var featureType1 = getFeatureType(entity1, graph);
         var featureType2 = getFeatureType(entity2, graph);
+
+        var geometry1 = entity1.geometry(graph);
+        var geometry2 = entity2.geometry(graph);
+        var bothLines = geometry1 === 'line' && geometry2 === 'line';
+
         if (featureType1 === featureType2) {
             if (featureType1 === 'highway') {
                 var entity1IsPath = osmPathHighwayTagValues[entity1.tags.highway];
@@ -150,10 +155,10 @@ export function validationCrossingWays(context) {
                     var pathFeature = entity1IsPath ? entity1 : entity2;
                     if (['marked', 'unmarked'].indexOf(pathFeature.tags.crossing) !== -1) {
                         // if the path is a crossing, match the crossing type
-                        return { highway: 'crossing', crossing: pathFeature.tags.crossing };
+                        return bothLines ? { highway: 'crossing', crossing: pathFeature.tags.crossing } : {};
                     }
                     // don't add a `crossing` subtag to ambiguous crossings
-                    return { highway: 'crossing' };
+                    return bothLines ? { highway: 'crossing' } : {};
                 }
                 return {};
             }
@@ -167,10 +172,10 @@ export function validationCrossingWays(context) {
                     if (osmPathHighwayTagValues[entity1.tags.highway] ||
                         osmPathHighwayTagValues[entity2.tags.highway]) {
                         // path-rail connections use this tag
-                        return { railway: 'crossing' };
+                        return bothLines ? { railway: 'crossing' } : {};
                     } else {
                         // road-rail connections use this tag
-                        return { railway: 'level_crossing' };
+                        return bothLines ? { railway: 'level_crossing' } : {};
                     }
                 }
 
@@ -184,7 +189,7 @@ export function validationCrossingWays(context) {
                         // do not allow fords on major highways
                         return null;
                     }
-                    return { ford: 'yes' };
+                    return bothLines ? { ford: 'yes' } : {};
                 }
             }
         }
diff --git a/test/spec/validations/crossing_ways.js b/test/spec/validations/crossing_ways.js
index 893072b11..a511b40a0 100644
--- a/test/spec/validations/crossing_ways.js
+++ b/test/spec/validations/crossing_ways.js
@@ -67,7 +67,7 @@ describe('iD.validations.crossing_ways', function () {
     function verifySingleCrossingIssue(issues, connectionTags) {
         // each entity must produce an identical issue
         expect(issues).to.have.lengthOf(2);
-        expect(issues[0].hash).to.eql(issues[1].hash);
+        expect(issues[0].id).to.eql(issues[1].id);
 
         for (var i in issues) {
             var issue = issues[i];
@@ -218,6 +218,11 @@ describe('iD.validations.crossing_ways', function () {
         verifySingleCrossingIssue(validate(), { highway: 'crossing', crossing: 'marked' });
     });
 
+    it('flags road=track crossing footway', function() {
+        createWaysWithOneCrossingPoint({ highway: 'track' }, { highway: 'footway' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
     it('flags cycleway crossing cycleway', function() {
         createWaysWithOneCrossingPoint({ highway: 'cycleway' }, { highway: 'cycleway' });
         verifySingleCrossingIssue(validate(), {});
@@ -283,6 +288,11 @@ describe('iD.validations.crossing_ways', function () {
         verifySingleCrossingIssue(validate(), null);
     });
 
+    it('flags road tunnel crossing waterway tunnel on the same layer', function() {
+        createWaysWithOneCrossingPoint({ highway: 'residential', tunnel: 'yes' }, { waterway: 'canal', tunnel: 'yes' });
+        verifySingleCrossingIssue(validate(), null);
+    });
+
     it('flags railway bridge crossing road bridge on the same layer', function() {
         createWaysWithOneCrossingPoint({ highway: 'residential', bridge: 'yes' }, { railway: 'rail', bridge: 'yes' });
         verifySingleCrossingIssue(validate(), { railway: 'level_crossing' });
@@ -369,14 +379,47 @@ describe('iD.validations.crossing_ways', function () {
         );
     }
 
+    it('ignores road line crossing relation with building=yes without a type', function() {
+        createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { building: 'yes' });
+        var issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores road line crossing type=building relation', function() {
+        createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { building: 'yes', type: 'building' });
+        var issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
+    it('ignores road line crossing waterway multipolygon relation', function() {
+        createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { waterway: 'river', type: 'multipolygon' });
+        var issues = validate();
+        expect(issues).to.have.lengthOf(0);
+    });
+
     it('flags road line crossing building multipolygon relation', function() {
         createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { building: 'yes', type: 'multipolygon' });
         verifySingleCrossingIssue(validate(), null);
     });
 
+    it('flags footway line crossing footway multipolygon relation', function() {
+        createWayAndRelationWithOneCrossingPoint({ highway: 'footway' }, { highway: 'footway', type: 'multipolygon' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
     it('flags road line crossing footway multipolygon relation', function() {
         createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { highway: 'footway', type: 'multipolygon' });
-        verifySingleCrossingIssue(validate(), { highway: 'crossing' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
+    it('flags railway line crossing footway multipolygon relation', function() {
+        createWayAndRelationWithOneCrossingPoint({ railway: 'tram' }, { highway: 'footway', type: 'multipolygon' });
+        verifySingleCrossingIssue(validate(), {});
+    });
+
+    it('flags waterway line crossing footway multipolygon relation', function() {
+        createWayAndRelationWithOneCrossingPoint({ waterway: 'stream' }, { highway: 'footway', type: 'multipolygon' });
+        verifySingleCrossingIssue(validate(), {});
     });
 
 });