Skip to content

Commit 277b5d4

Browse files
mtmailmtmail
committed
new validator: mutually exclusive tags
1 parent 85f4cc0 commit 277b5d4

File tree

7 files changed

+211
-101
lines changed

7 files changed

+211
-101
lines changed

‎data/core.yaml‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,9 +1839,6 @@ en:
18391839
amap: "Amap products are proprietary and must not be used as references."
18401840
baidu: "Baidu products are proprietary and must not be used as references."
18411841
google: "Google products are proprietary and must not be used as references."
1842-
incorrect_name:
1843-
message: '{feature} has the mistaken name "{name}"'
1844-
message_language: '{feature} has the mistaken name "{name}" in {language}'
18451842
invalid_format:
18461843
title: Invalid Formatting
18471844
tip: Find tags with unexpected formats
@@ -1873,6 +1870,15 @@ en:
18731870
message: "{feature} has no descriptive tags"
18741871
relation_type:
18751872
message: "{feature} is a relation without a type"
1873+
mutually_exclusive_tags:
1874+
title: Contradictory Tags
1875+
tip: "Find features that have contradictory tags."
1876+
default:
1877+
message: "{feature} has both {tag1} and {tag2}"
1878+
reference: 'The tags "{tag1}" and "{tag2}" contradict each other. Only one tag should be set.'
1879+
same_value:
1880+
message: "{feature}: {tag1} and {tag2} have same value"
1881+
reference: 'The tags "{tag1}" and "{tag2}" have the same value. Only one tag should be set.'
18761882
old_multipolygon:
18771883
message: "{multipolygon} has misplaced tags"
18781884
reference: "Multipolygons should be tagged on their relation, not their outer way."
@@ -1998,6 +2004,9 @@ en:
19982004
remove_tag:
19992005
title: Remove the tag
20002006
annotation: Removed tag.
2007+
remove_named_tag:
2008+
title: 'Remove the "{tag}" tag'
2009+
annotation: 'Removed "{tag}" tag.'
20012010
remove_tags:
20022011
title: Remove the tags
20032012
remove_the_name:

‎modules/osm/tags.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,15 @@ export function isColourValid(value) {
255255
}
256256
return true;
257257
}
258+
259+
// https://wiki.openstreetmap.org/wiki/Special:WhatLinksHere/Property:P44
260+
export var osmMutuallyExclusiveTagPairs = [
261+
['noname', 'name'],
262+
['noref', 'ref'],
263+
['nohousenumber', 'addr:housenumber'],
264+
['noaddress', 'addr:housenumber'],
265+
['noaddress', 'addr:housename'],
266+
['noaddress', 'addr:unit'],
267+
['addr:nostreet', 'addr:street']
268+
];
269+

‎modules/validations/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export { validationMaprules } from './maprules';
1010
export { validationMismatchedGeometry } from './mismatched_geometry';
1111
export { validationMissingRole } from './missing_role';
1212
export { validationMissingTag } from './missing_tag';
13+
export { validationMutuallyExclusiveTags } from './mutually_exclusive_tags';
1314
export { validationOutdatedTags } from './outdated_tags';
1415
export { validationPrivateData } from './private_data';
1516
export { validationSuspiciousName } from './suspicious_name';

‎modules/validations/mutually_exclusive_tags.js‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { actionChangeTags } from '../actions/change_tags';
2+
import { t } from '../core/localizer';
3+
import { utilDisplayLabel } from '../util';
4+
import { validationIssue, validationIssueFix } from '../core/validation';
5+
import { osmMutuallyExclusiveTagPairs } from '../osm/tags';
6+
7+
export function validationMutuallyExclusiveTags(/* context */) {
8+
const type = 'mutually_exclusive_tags';
9+
10+
// https://wiki.openstreetmap.org/wiki/Special:WhatLinksHere/Property:P44
11+
const tagKeyPairs = osmMutuallyExclusiveTagPairs;
12+
13+
const validation = function checkMutuallyExclusiveTags(entity /*, graph */) {
14+
15+
let pairsFounds = tagKeyPairs.filter((pair) => {
16+
return (pair[0] in entity.tags && pair[1] in entity.tags);
17+
}).filter((pair) => {
18+
// noname=no is double-negation, thus positive and not conflicting. We'll ignore those
19+
return !((pair[0].match(/^(addr:)?no[a-z]/) && entity.tags[pair[0]] === 'no') ||
20+
(pair[1].match(/^(addr:)?no[a-z]/) && entity.tags[pair[1]] === 'no'));
21+
});
22+
23+
// Additional:
24+
// Check if name and not:name (and similar) are set and both have the same value
25+
// not:name can actually have multiple values, separate by ;
26+
// https://taginfo.openstreetmap.org/search?q=not%3A#keys
27+
Object.keys(entity.tags).forEach((key) => {
28+
let negative_key = 'not:' + key;
29+
if (negative_key in entity.tags && entity.tags[negative_key].split(';').includes(entity.tags[key])) {
30+
pairsFounds.push([negative_key, key, 'same_value']);
31+
}
32+
// For name:xx we also compare against the not:name tag
33+
if (key.match(/^name:[a-z]+/)) {
34+
negative_key = 'not:name';
35+
if (negative_key in entity.tags && entity.tags[negative_key].split(';').includes(entity.tags[key])) {
36+
pairsFounds.push([negative_key, key, 'same_value']);
37+
}
38+
}
39+
});
40+
41+
let issues = pairsFounds.map((pair) => {
42+
const subtype = pair[2] || 'default';
43+
return new validationIssue({
44+
type: type,
45+
subtype: subtype,
46+
severity: 'warning',
47+
message: function(context) {
48+
let entity = context.hasEntity(this.entityIds[0]);
49+
return entity ? t.append(`issues.${type}.${subtype}.message`, {
50+
feature: utilDisplayLabel(entity, context.graph()),
51+
tag1: pair[0],
52+
tag2: pair[1]
53+
}) : '';
54+
},
55+
reference: (selection) => showReference(selection, pair, subtype),
56+
entityIds: [entity.id],
57+
dynamicFixes: () => pair.slice(0,2).map((tagToRemove) => createIssueFix(tagToRemove))
58+
});
59+
});
60+
61+
function createIssueFix(tagToRemove) {
62+
return new validationIssueFix({
63+
icon: 'iD-operation-delete',
64+
title: t.append('issues.fix.remove_named_tag.title', { tag: tagToRemove }),
65+
onClick: function(context) {
66+
const entityId = this.issue.entityIds[0];
67+
const entity = context.entity(entityId);
68+
let tags = Object.assign({}, entity.tags); // shallow copy
69+
delete tags[tagToRemove];
70+
context.perform(
71+
actionChangeTags(entityId, tags),
72+
t('issues.fix.remove_named_tag.annotation', { tag: tagToRemove })
73+
);
74+
}
75+
});
76+
}
77+
78+
function showReference(selection, pair, subtype) {
79+
selection.selectAll('.issue-reference')
80+
.data([0])
81+
.enter()
82+
.append('div')
83+
.attr('class', 'issue-reference')
84+
.call(t.append(`issues.${type}.${subtype}.reference`, { tag1: pair[0], tag2: pair[1] }));
85+
}
86+
87+
return issues;
88+
};
89+
90+
validation.type = type;
91+
92+
return validation;
93+
}

‎modules/validations/suspicious_name.js‎

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -96,53 +96,6 @@ export function validationSuspiciousName() {
9696
}
9797
}
9898

99-
function makeIncorrectNameIssue(entityId, nameKey, incorrectName, langCode) {
100-
return new validationIssue({
101-
type: type,
102-
subtype: 'not_name',
103-
severity: 'warning',
104-
message: function(context) {
105-
const entity = context.hasEntity(this.entityIds[0]);
106-
if (!entity) return '';
107-
const preset = presetManager.match(entity, context.graph());
108-
const langName = langCode && localizer.languageName(langCode);
109-
return t.append('issues.incorrect_name.message' + (langName ? '_language' : ''),
110-
{ feature: preset.name(), name: incorrectName, language: langName }
111-
);
112-
},
113-
reference: showReference,
114-
entityIds: [entityId],
115-
hash: `${nameKey}=${incorrectName}`,
116-
dynamicFixes: function() {
117-
return [
118-
new validationIssueFix({
119-
icon: 'iD-operation-delete',
120-
title: t.append('issues.fix.remove_the_name.title'),
121-
onClick: function(context) {
122-
const entityId = this.issue.entityIds[0];
123-
const entity = context.entity(entityId);
124-
let tags = Object.assign({}, entity.tags); // shallow copy
125-
delete tags[nameKey];
126-
context.perform(
127-
actionChangeTags(entityId, tags), t('issues.fix.remove_mistaken_name.annotation')
128-
);
129-
}
130-
})
131-
];
132-
}
133-
});
134-
135-
function showReference(selection) {
136-
selection.selectAll('.issue-reference')
137-
.data([0])
138-
.enter()
139-
.append('div')
140-
.attr('class', 'issue-reference')
141-
.call(t.append('issues.generic_name.reference'));
142-
}
143-
}
144-
145-
14699
let validation = function checkGenericName(entity) {
147100
const tags = entity.tags;
148101

@@ -151,23 +104,14 @@ export function validationSuspiciousName() {
151104
if (hasWikidata) return [];
152105

153106
let issues = [];
154-
const notNames = (tags['not:name'] || '').split(';');
155107

156108
for (let key in tags) {
157109
const m = key.match(/^name(?:(?::)([a-zA-Z_-]+))?$/);
158110
if (!m) continue;
159111

160112
const langCode = m.length >= 2 ? m[1] : null;
161113
const value = tags[key];
162-
if (notNames.length) {
163-
for (let i in notNames) {
164-
const notName = notNames[i];
165-
if (notName && value === notName) {
166-
issues.push(makeIncorrectNameIssue(entity.id, key, value, langCode));
167-
continue;
168-
}
169-
}
170-
}
114+
171115
if (isGenericName(value, tags)) {
172116
issues.provisional = _waitingForNsi; // retry later if we are waiting on NSI to finish loading
173117
issues.push(makeGenericNameIssue(entity.id, key, value, langCode));

‎test/spec/validations/mutually_exclusive_tags.js‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
describe('iD.validations.mutually_exclusive_tags', function () {
2+
var context;
3+
4+
beforeEach(function() {
5+
context = iD.coreContext().init();
6+
});
7+
8+
function createNode(tags) {
9+
context.perform(
10+
iD.actionAddEntity({id: 'n-1', loc: [4,4], tags: tags})
11+
);
12+
}
13+
14+
function validate(validator) {
15+
var changes = context.history().changes();
16+
var entities = changes.modified.concat(changes.created);
17+
var issues = [];
18+
entities.forEach(function(entity) {
19+
issues = issues.concat(validator(entity, context.graph()));
20+
});
21+
return issues;
22+
}
23+
24+
25+
it('has no errors on init', function(done) {
26+
var validator = iD.validationMutuallyExclusiveTags(context);
27+
window.setTimeout(function() { // async, so data will be available
28+
var issues = validate(validator);
29+
expect(issues).to.have.lengthOf(0);
30+
done();
31+
}, 20);
32+
});
33+
34+
it('has no errors on good tags', function(done) {
35+
createNode({'name': 'Trader Joe', 'not:name': 'Trader Jane'});
36+
var validator = iD.validationMutuallyExclusiveTags(context);
37+
window.setTimeout(function() { // async, so data will be available
38+
var issues = validate(validator);
39+
expect(issues).to.have.lengthOf(0);
40+
done();
41+
}, 20);
42+
});
43+
44+
it('flags mutually exclusive tags', function(done) {
45+
createNode({'name': 'Trader Joe', 'noname': 'yes'});
46+
var validator = iD.validationMutuallyExclusiveTags(context);
47+
window.setTimeout(function() { // async, so data will be available
48+
var issues = validate(validator);
49+
expect(issues).to.have.lengthOf(1);
50+
var issue = issues[0];
51+
expect(issue.type).to.eql('mutually_exclusive_tags');
52+
expect(issue.subtype).to.eql('default');
53+
expect(issue.severity).to.eql('warning');
54+
expect(issue.entityIds).to.have.lengthOf(1);
55+
expect(issue.entityIds[0]).to.eql('n-1');
56+
done();
57+
}, 20);
58+
});
59+
60+
it('flags feature with a mutually exclusive `not:name` value', function(done) {
61+
createNode({ shop: 'supermarket', name: 'Lous', 'not:name': 'Lous' });
62+
var validator = iD.validationMutuallyExclusiveTags(context);
63+
window.setTimeout(function() { // async, so data will be available
64+
var issues = validate(validator);
65+
expect(issues).to.have.lengthOf(1);
66+
var issue = issues[0];
67+
expect(issue.type).to.eql('mutually_exclusive_tags');
68+
expect(issue.subtype).to.eql('same_value');
69+
expect(issue.severity).to.eql('warning');
70+
expect(issue.entityIds).to.have.lengthOf(1);
71+
expect(issue.entityIds[0]).to.eql('n-1');
72+
done();
73+
}, 20);
74+
});
75+
76+
77+
it('flags feature with a mutually exclusive semicolon-separated `not:name` value', function(done) {
78+
createNode({ shop: 'supermarket', name: 'Lous', 'not:name': 'Louis\';Lous;Louis\'s' });
79+
var validator = iD.validationMutuallyExclusiveTags(context);
80+
window.setTimeout(function() { // async, so data will be available
81+
var issues = validate(validator);
82+
expect(issues).to.have.lengthOf(1);
83+
var issue = issues[0];
84+
expect(issue.type).to.eql('mutually_exclusive_tags');
85+
expect(issue.subtype).to.eql('same_value');
86+
expect(issue.severity).to.eql('warning');
87+
expect(issue.entityIds).to.have.lengthOf(1);
88+
expect(issue.entityIds[0]).to.eql('n-1');
89+
done();
90+
}, 20);
91+
});
92+
});

‎test/spec/validations/suspicious_name.js‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -183,45 +183,4 @@ describe('iD.validations.suspicious_name', function () {
183183
done();
184184
}, 20);
185185
});
186-
187-
it('ignores feature with a non-matching `not:name` tag', function(done) {
188-
createWay({ shop: 'supermarket', name: 'Lou\'s', 'not:name': 'Lous' });
189-
var validator = iD.validationSuspiciousName(context);
190-
window.setTimeout(function() { // async, so data will be available
191-
var issues = validate(validator);
192-
expect(issues).to.have.lengthOf(0);
193-
done();
194-
}, 20);
195-
});
196-
197-
it('flags feature with a matching `not:name` tag', function(done) {
198-
createWay({ shop: 'supermarket', name: 'Lous', 'not:name': 'Lous' });
199-
var validator = iD.validationSuspiciousName(context);
200-
window.setTimeout(function() { // async, so data will be available
201-
var issues = validate(validator);
202-
expect(issues).to.have.lengthOf(1);
203-
var issue = issues[0];
204-
expect(issue.type).to.eql('suspicious_name');
205-
expect(issue.subtype).to.eql('not_name');
206-
expect(issue.entityIds).to.have.lengthOf(1);
207-
expect(issue.entityIds[0]).to.eql('w-1');
208-
done();
209-
}, 20);
210-
});
211-
212-
it('flags feature with a matching a semicolon-separated `not:name` tag', function(done) {
213-
createWay({ shop: 'supermarket', name: 'Lous', 'not:name': 'Louis\';Lous;Louis\'s' });
214-
window.setTimeout(function() { // async, so data will be available
215-
var validator = iD.validationSuspiciousName(context);
216-
var issues = validate(validator);
217-
expect(issues).to.have.lengthOf(1);
218-
var issue = issues[0];
219-
expect(issue.type).to.eql('suspicious_name');
220-
expect(issue.subtype).to.eql('not_name');
221-
expect(issue.entityIds).to.have.lengthOf(1);
222-
expect(issue.entityIds[0]).to.eql('w-1');
223-
done();
224-
}, 20);
225-
});
226-
227186
});

0 commit comments

Comments
 (0)
⚡