refactor isOneWay to properly support bidirectional ways

modules/osm/index.js

@@ -29,7 +29,6 @@ export {
     osmVertexTags,
     osmSetVertexTags,
     osmNodeGeometriesForTags,
-    osmOneWayTags,
     osmPavedTags,
     osmIsInterestingTag,
     osmLifecyclePrefixes,

modules/osm/tags.js

@@ -1,3 +1,5 @@
+import { merge } from 'lodash-es';
+
 export function osmIsInterestingTag(key) {
     return key !== 'attribution' &&
         key !== 'created_by' &&
@@ -124,7 +126,7 @@ export function osmNodeGeometriesForTags(nodeTags) {
     return geometries;
 }
 
-export var osmOneWayTags = {
+export const osmOneWayForwardTags = {
     'aerialway': {
         'chair_lift': true,
         'drag_lift': true,
@@ -138,8 +140,6 @@ export var osmOneWayTags = {
     },
     'conveying': {
         'forward': true,
-        'backward': true,
-        'reversible': true,
     },
     'highway': {
         'motorway': true
@@ -152,6 +152,9 @@ export var osmOneWayTags = {
         'goods_conveyor': true,
         'piste:halfpipe': true
     },
+    'oneway': {
+        'yes': true,
+    },
     'piste:type': {
         'downhill': true,
         'sled': true,
@@ -176,6 +179,28 @@ export var osmOneWayTags = {
         'tidal_channel': true
     }
 };
+export const osmOneWayBackwardTags = {
+    'conveying': {
+        'backward': true,
+    },
+    'oneway': {
+        '-1': true,
+    },
+};
+export const osmOneWayBiDirectionalTags = {
+    'conveying': {
+        'reversible': true,
+    },
+    'oneway': {
+        'alternating': true,
+        'reversible': true,
+    },
+};
+export const osmOneWayTags = merge(
+    osmOneWayForwardTags,
+    osmOneWayBackwardTags,
+    osmOneWayBiDirectionalTags,
+);
 
 // solid and smooth surfaces akin to the assumed default road surface in OSM
 export var osmPavedTags = {

modules/osm/way.js

@@ -3,8 +3,8 @@ import { geoArea as d3_geoArea } from 'd3-geo';
 import { geoExtent, geoVecCross } from '../geo';
 import { osmEntity } from './entity';
 import { osmLanes } from './lanes';
-import { osmTagSuggestingArea, osmOneWayTags, osmRightSideIsInsideTags, osmRemoveLifecyclePrefix } from './tags';
-import { utilArrayUniq } from '../util';
+import { osmTagSuggestingArea, osmRightSideIsInsideTags, osmRemoveLifecyclePrefix, osmOneWayBiDirectionalTags, osmOneWayBackwardTags, osmOneWayForwardTags, osmOneWayTags } from './tags';
+import { utilArrayUniq, utilCheckTagDictionary } from '../util';
 
 
 export function osmWay() {
@@ -138,29 +138,32 @@ Object.assign(osmWay.prototype, {
     },
 
 
-    isOneWay: function() {
-        // explicit oneway tag..
-        var values = {
-            'yes': true,
-            '1': true,
-            '-1': true,
-            'reversible': true,
-            'alternating': true,
-            'no': false,
-            '0': false
-        };
-        if (values[this.tags.oneway] !== undefined) {
-            return values[this.tags.oneway];
-        }
+    /** @returns {boolean} for example, if `oneway=yes` */
+    isOneWayForwards() {
+        if (this.tags.oneway === 'no') return false;
 
-        // implied oneway tag..
-        for (var key in this.tags) {
-            if (key in osmOneWayTags &&
-                (this.tags[key] in osmOneWayTags[key])) {
-                return true;
-            }
-        }
-        return false;
+        return !!utilCheckTagDictionary(this.tags, osmOneWayForwardTags);
+    },
+
+    /** @returns {boolean} for example, if `oneway=-1` */
+    isOneWayBackwards() {
+        if (this.tags.oneway === 'no') return false;
+
+        return !!utilCheckTagDictionary(this.tags, osmOneWayBackwardTags);
+    },
+
+    /** @returns {boolean} for example, if `oneway=alternating` */
+    isBiDirectional() {
+        if (this.tags.oneway === 'no') return false;
+
+        return !!utilCheckTagDictionary(this.tags, osmOneWayBiDirectionalTags);
+    },
+
+    /** @returns {boolean} */
+    isOneWay() {
+        if (this.tags.oneway === 'no') return false;
+
+        return !!utilCheckTagDictionary(this.tags, osmOneWayTags);
     },
 
     // Some identifier for tag that implies that this way is "sided",

modules/svg/lines.js

@@ -265,19 +265,8 @@ export function svgLines(projection, context) {
             var onewayArr = v.filter(function(d) { return d.isOneWay(); });
             var onewaySegments = svgMarkerSegments(
                 projection, graph, 35,
-                function shouldReverse(entity) {
-                    return (
-                        entity.tags.oneway === '-1'
-                        || entity.tags.conveying === 'backward'
-                    );
-                },
-                function bothDirections(entity) {
-                    return (
-                        entity.tags.oneway === 'alternating'
-                        || entity.tags.oneway === 'reversible'
-                        || entity.tags.conveying === 'reversible'
-                    );
-                }
+                entity => entity.isOneWayBackwards(),
+                entity => entity.isBiDirectional(),
             );
             onewaydata[k] = utilArrayFlatten(onewayArr.map(onewaySegments));
 

modules/ui/fields/check.js

@@ -7,7 +7,6 @@ import {
 import { utilRebind } from '../../util/rebind';
 import { t } from '../../core/localizer';
 import { actionReverse } from '../../actions/reverse';
-import { osmOneWayTags } from '../../osm';
 import { svgIcon } from '../../svg/icon';
 
 export { uiFieldCheck as uiFieldDefaultCheck };
@@ -61,12 +60,9 @@ export function uiFieldCheck(field, context) {
         // where implied oneway tag exists (e.g. `junction=roundabout`) #2220, #1841
         if (field.id === 'oneway') {
             var entity = context.entity(_entityIDs[0]);
-            for (var key in entity.tags) {
-                if (key in osmOneWayTags && (entity.tags[key] in osmOneWayTags[key])) {
-                    _impliedYes = true;
-                    texts[0] = t.html('_tagging.presets.fields.oneway_yes.options.undefined');
-                    break;
-                }
+            if (entity.type === 'way' && entity.isOneWay()) {
+                _impliedYes = true;
+                texts[0] = t.html('_tagging.presets.fields.oneway_yes.options.undefined');
             }
         }
     }

modules/util/index.js

@@ -33,7 +33,7 @@ export { utilHashcode } from './util';
 export { utilHighlightEntities } from './util';
 export { utilKeybinding } from './keybinding';
 export { utilNoAuto } from './util';
-export { utilObjectOmit } from './object';
+export { utilObjectOmit, utilCheckTagDictionary } from './object';
 export { utilCompareIDs } from './util';
 export { utilOldestID } from './util';
 export { utilPrefixCSSProperty } from './util';

modules/util/object.js

@@ -7,3 +7,26 @@ export function utilObjectOmit(obj, omitKeys) {
         return result;
     }, {});
 }
+
+/**
+ * @template T
+ * @typedef {{ [key: string]: { [value: string]: T } }} TagDictionary<T>
+ */
+
+/**
+ * searches a dictionary for a match, such as `osmOneWayForwardTags`,
+ * `osmAreaKeysExceptions`, etc.
+ * @template T
+ * @param {Tags} tags
+ * @param {TagDictionary<T>} tagDictionary
+ * @returns {T | undefined}
+ */
+export function utilCheckTagDictionary(tags, tagDictionary) {
+    for (const key in tags) {
+        const value = tags[key];
+        if (tagDictionary[key] && value in tagDictionary[key]) {
+            return tagDictionary[key][value];
+        }
+    }
+    return undefined;
+}

modules/validations/impossible_oneway.js

@@ -2,7 +2,7 @@ import { t, localizer } from '../core/localizer';
 import { modeDrawLine } from '../modes/draw_line';
 import { actionReverse } from '../actions/reverse';
 import { utilDisplayLabel } from '../util/utilDisplayLabel';
-import { osmFlowingWaterwayTagValues, osmOneWayTags, osmRoutableHighwayTagValues } from '../osm/tags';
+import { osmFlowingWaterwayTagValues, osmRoutableHighwayTagValues } from '../osm/tags';
 import { validationIssue, validationIssueFix } from '../core/validation';
 import { services } from '../services';
 
@@ -17,7 +17,7 @@ export function validationImpossibleOneway() {
 
         if (!typeForWay(entity)) return [];
 
-        if (!isOneway(entity)) return [];
+        if (!entity.isOneWay()) return [];
 
         var firstIssues = issuesForNode(entity, entity.first());
         var lastIssues = issuesForNode(entity, entity.last());
@@ -32,18 +32,6 @@ export function validationImpossibleOneway() {
             return null;
         }
 
-        function isOneway(way) {
-            if (way.tags.oneway === 'yes') return true;
-            if (way.tags.oneway) return false;
-
-            for (var key in way.tags) {
-                if (osmOneWayTags[key] && osmOneWayTags[key][way.tags[key]]) {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         function nodeOccursMoreThanOnce(way, nodeID) {
             var occurrences = 0;
             for (var index in way.nodes) {
@@ -129,7 +117,7 @@ export function validationImpossibleOneway() {
             if (wayType === 'waterway' && attachedWaysOfSameType.length === 0) return [];
 
             var attachedOneways = attachedWaysOfSameType.filter(function(attachedWay) {
-                return isOneway(attachedWay);
+                return attachedWay.isOneWay();
             });
 
             // ignore if the way is connected to some non-oneway features

package.json

@@ -85,6 +85,7 @@
     "@types/chai": "^5.0.1",
     "@types/d3": "^7.4.3",
     "@types/happen": "^0.3.0",
+    "@types/lodash-es": "^4.17.12",
     "@types/sinon": "^17.0.3",
     "@types/sinon-chai": "^4.0.0",
     "autoprefixer": "^10.4.20",

test/spec/osm/way.js

@@ -305,7 +305,6 @@ describe('iD.osmWay', function() {
 
         it('returns true when the way has tag oneway=yes', function() {
             expect(iD.osmWay({tags: { oneway: 'yes' }}).isOneWay(), 'oneway yes').to.be.true;
-            expect(iD.osmWay({tags: { oneway: '1' }}).isOneWay(), 'oneway 1').to.be.true;
             expect(iD.osmWay({tags: { oneway: '-1' }}).isOneWay(), 'oneway -1').to.be.true;
         });
 

test/spec/util/object.js

@@ -7,3 +7,16 @@ describe('iD.utilObjectOmit', function() {
     });
 });
 
+
+describe('iD.utilCheckTagDictionary', () => {
+    it('can search a standard tag-dictionary', () => {
+        expect(iD.utilCheckTagDictionary({}, iD.osmPavedTags)).toBeUndefined();
+        expect(iD.utilCheckTagDictionary({ surface: 'asphalt' }, iD.osmPavedTags)).toBe(true);
+    });
+
+    it('works for falsy values', () => {
+        const dictionary = { surface: { paved: 0 } };
+        expect(iD.utilCheckTagDictionary({}, dictionary)).toBeUndefined();
+        expect(iD.utilCheckTagDictionary({ surface: 'paved' }, dictionary)).toBe(0);
+    });
+});