fix false positive "unreachable oneway" validation cased by oneway=-1

fixes #11068
This commit is contained in:
Martin Raifer
2025-05-27 14:43:37 +02:00
parent ca98ef6f1f
commit bd71ccba17
3 changed files with 216 additions and 30 deletions
+2
View File
@@ -49,6 +49,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
#### :bug: Bugfixes
* Refresh numeric input fields after leaving focus with the value that is stored in the tag ([#11027])
* Fix oneway field falsely showing "Assumed to be Yes" if cycled through all options back to the default state
* Fix false positives in "unreachable oneway" validation when `oneway=-1` tag is present ([#11068])
#### :earth_asia: Localization
#### :hourglass: Performance
#### :mortar_board: Walkthrough / Help
@@ -60,6 +61,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
[#11014]: https://github.com/openstreetmap/iD/pull/11014
[#11027]: https://github.com/openstreetmap/iD/pull/11027
[#11054]: https://github.com/openstreetmap/iD/issues/1104
[#11068]: https://github.com/openstreetmap/iD/issues/11068
[@keiffer213]: https://github.com/keiffer213
[@haipq07]: https://github.com/haipq07
+27 -30
View File
@@ -7,22 +7,18 @@ import { validationIssue, validationIssueFix } from '../core/validation';
import { services } from '../services';
export function validationImpossibleOneway() {
var type = 'impossible_oneway';
var validation = function checkImpossibleOneway(entity, graph) {
const type = 'impossible_oneway';
const validation = function checkImpossibleOneway(entity, graph) {
if (entity.type !== 'way' || entity.geometry(graph) !== 'line') return [];
if (entity.isClosed()) return [];
if (!typeForWay(entity)) return [];
if (!entity.isOneWay()) return [];
var firstIssues = issuesForNode(entity, entity.first());
var lastIssues = issuesForNode(entity, entity.last());
return firstIssues.concat(lastIssues);
return [
...issuesForNode(entity, entity.first()),
...issuesForNode(entity, entity.last())
];
function typeForWay(way) {
if (way.geometry(graph) !== 'line') return null;
@@ -33,10 +29,10 @@ export function validationImpossibleOneway() {
}
function nodeOccursMoreThanOnce(way, nodeID) {
var occurrences = 0;
for (var index in way.nodes) {
let occurrences = 0;
for (const index in way.nodes) {
if (way.nodes[index] === nodeID) {
occurrences += 1;
occurrences++;
if (occurrences > 1) return true;
}
}
@@ -90,25 +86,21 @@ export function validationImpossibleOneway() {
}
function issuesForNode(way, nodeID) {
var isFirst = nodeID === way.first();
var wayType = typeForWay(way);
const isFirst = (nodeID === way.first()) ^ way.isOneWayBackwards();
const wayType = typeForWay(way);
// ignore if this way is self-connected at this node
if (nodeOccursMoreThanOnce(way, nodeID)) return [];
var osm = services.osm;
const osm = services.osm;
if (!osm) return [];
var node = graph.hasEntity(nodeID);
const node = graph.hasEntity(nodeID);
// ignore if this node or its tile are unloaded
if (!node || !osm.isDataLoaded(node.loc)) return [];
if (isConnectedViaOtherTypes(way, node)) return [];
var attachedWaysOfSameType = graph.parentWays(node).filter(function(parentWay) {
const attachedWaysOfSameType = graph.parentWays(node).filter(parentWay => {
if (parentWay.id === way.id) return false;
return typeForWay(parentWay) === wayType;
});
@@ -116,25 +108,30 @@ export function validationImpossibleOneway() {
// assume it's okay for waterways to start or end disconnected for now
if (wayType === 'waterway' && attachedWaysOfSameType.length === 0) return [];
var attachedOneways = attachedWaysOfSameType.filter(function(attachedWay) {
return attachedWay.isOneWay();
});
const attachedOneways = attachedWaysOfSameType
.filter(attachedWay => attachedWay.isOneWay());
// ignore if the way is connected to some non-oneway features
if (attachedOneways.length < attachedWaysOfSameType.length) return [];
if (attachedOneways.length) {
var connectedEndpointsOkay = attachedOneways.some(function(attachedOneway) {
if ((isFirst ? attachedOneway.first() : attachedOneway.last()) !== nodeID) return true;
const connectedEndpointsOkay = attachedOneways.some(attachedOneway => {
const isAttachedBackwards = attachedOneway.isOneWayBackwards();
if ((isFirst ^ isAttachedBackwards
? attachedOneway.first()
: attachedOneway.last()
) !== nodeID) {
return true;
}
if (nodeOccursMoreThanOnce(attachedOneway, nodeID)) return true;
return false;
});
if (connectedEndpointsOkay) return [];
}
var placement = isFirst ? 'start' : 'end',
messageID = wayType + '.',
referenceID = wayType + '.';
const placement = isFirst ? 'start' : 'end';
let messageID = wayType + '.';
let referenceID = wayType + '.';
if (wayType === 'waterway') {
messageID += 'connected.' + placement;
+187
View File
@@ -0,0 +1,187 @@
describe('iD.validations.impossible_oneway', function() {
let context;
beforeEach(function() {
context = iD.coreContext().assetPath('../dist/').init();
iD.services.osm = { isDataLoaded: () => true };
});
function validate() {
const validator = iD.validationImpossibleOneway(context);
const changes = context.history().changes();
const entities = changes.modified.concat(changes.created);
const issues = entities.flatMap(entity =>
validator(entity, context.graph()));
return issues;
}
it('has no errors on init', function() {
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
describe('highways', function() {
it('does not flag properly connecting oneway roads', function() {
context.perform(...[
iD.osmNode({ id: 'n-0', loc: [2, 1] }),
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmNode({ id: 'n-3', loc: [3, 0] }),
iD.osmWay({ id: 'w-0', nodes: ['n-1', 'n-0', 'n-3'], tags: {
'highway': 'unclassified'
}}),
iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}}),
iD.osmWay({ id: 'w-2', nodes: ['n-2', 'n-3'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('flags dangling oneway end', function() {
context.perform(...[
iD.osmNode({ id: 'n-0', loc: [0, 0] }),
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmWay({ id: 'w-0', nodes: ['n-0', 'n-1'], tags: {
'highway': 'unclassified'
}}),
iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(1);
const issue1 = issues[0];
expect(issue1.type).to.eql('impossible_oneway');
expect(issue1.subtype).to.eql('highway');
expect(issue1.severity).to.eql('warning');
expect(issue1.entityIds).to.eql(['w-1', 'n-2']);
});
it('flags unconnected oneway start', function() {
context.perform(...[
iD.osmNode({ id: 'n-0', loc: [0, 0] }),
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmWay({ id: 'w-0', nodes: ['n-0', 'n-1'], tags: {
'highway': 'unclassified'
}}),
iD.osmWay({ id: 'w-1', nodes: ['n-2', 'n-1'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(1);
const issue1 = issues[0];
expect(issue1.type).to.eql('impossible_oneway');
expect(issue1.subtype).to.eql('highway');
expect(issue1.severity).to.eql('warning');
expect(issue1.entityIds).to.eql(['w-1', 'n-2']);
});
it('flags oneway pointing to each other', function() {
context.perform(...[
iD.osmNode({ id: 'n-0', loc: [2, 1] }),
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmNode({ id: 'n-3', loc: [3, 0] }),
iD.osmWay({ id: 'w-0', nodes: ['n-1', 'n-0', 'n-3'], tags: {
'highway': 'unclassified'
}}),
iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}}),
iD.osmWay({ id: 'w-2', nodes: ['n-3', 'n-2'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(2);
const issue1 = issues[0];
expect(issue1.type).to.eql('impossible_oneway');
expect(issue1.subtype).to.eql('highway');
expect(issue1.severity).to.eql('warning');
expect(issue1.entityIds).to.eql(['w-1', 'n-2']);
const issue2 = issues[1];
expect(issue2.type).to.eql('impossible_oneway');
expect(issue2.entityIds).to.eql(['w-2', 'n-2']);
});
it('does not flags oneway with reverse "-1" oneway direction', function() {
context.perform(...[
iD.osmNode({ id: 'n-0', loc: [2, 1] }),
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmNode({ id: 'n-3', loc: [3, 0] }),
iD.osmWay({ id: 'w-0', nodes: ['n-1', 'n-0', 'n-3'], tags: {
'highway': 'unclassified'
}}),
iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: {
'highway': 'unclassified',
'oneway' : 'yes'
}}),
iD.osmWay({ id: 'w-2', nodes: ['n-3', 'n-2'], tags: {
'highway': 'unclassified',
'oneway' : '-1'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(0);
});
});
describe('waterways', function() {
it('does not flag unconnected start or end points', function() {
context.perform(...[
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: {
'waterway': 'stream'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('flags waterways pointing to each other', function() {
context.perform(...[
iD.osmNode({ id: 'n-1', loc: [1, 0] }),
iD.osmNode({ id: 'n-2', loc: [2, 0] }),
iD.osmNode({ id: 'n-3', loc: [3, 0] }),
iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: {
'waterway': 'stream'
}}),
iD.osmWay({ id: 'w-2', nodes: ['n-3', 'n-2'], tags: {
'waterway': 'stream'
}})
].map(iD.actionAddEntity));
const issues = validate();
expect(issues).to.have.lengthOf(2);
const issue1 = issues[0];
expect(issue1.type).to.eql('impossible_oneway');
expect(issue1.subtype).to.eql('waterway');
expect(issue1.severity).to.eql('warning');
expect(issue1.entityIds).to.eql(['w-1', 'n-2']);
const issue2 = issues[1];
expect(issue2.type).to.eql('impossible_oneway');
expect(issue2.entityIds).to.eql(['w-2', 'n-2']);
});
});
});