Wrong "Area style way is not closed" message for highway=motorway_junction

[GerdP]

What steps will reproduce the problem?

1. Draw a way (unclosed) and tag it highway=motorway_junction
2. Run Validator

What is the expected result?

A warning that the tag is meant for nodes

What happens instead?

A warning that "Area style way is not closed"

Please provide any additional information below. Attach a screenshot if possible.

I failed to find the code that produces this message.

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-07-31 01:02:55 +0200 (Mon, 31 Jul 2017)
Build-Date:2017-07-31 01:53:17
Revision:12545
Relative:URL: ^/trunk

Identification: JOSM/1.5 (12545 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1703 (15063)
Memory Usage: 1377 MB / 5461 MB (1148 MB allocated, but free)
Java version: 1.8.0_121-b13, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (33185)
+ apache-commons (32994)
+ buildings_tools (33004)
+ ejml (32680)
+ geotools (33380)
+ jts (32699)
+ o5m (33243)
+ opendata (33438)
+ pbf (33241)
+ poly (33004)
+ reverter (33088)
+ utilsplugin2 (33328)

Last errors/warnings:
- W: Cannot start IPv4 remotecontrol https server on port 8112: Keystore was tampered with, or password was incorrect
- W: Cannot start IPv6 remotecontrol https server on port 8112: Keystore was tampered with, or password was incorrect

[Klumbumbus]

Can't reproduce with r12691. Please make sure that there are no other tags on the way which may produce the warning. Also please add the parts tagging presets, mappaint styles and validator rules of your status report.

[GerdP]

Okay, please make sure that the new way is not selected before running the validator.
Means, I should have added "press Esc" as step 2)

[GerdP]

Forget the "Press Esc" stuff.
I've also tried with a fresh 12695 and a renamed C:\Users\Gerd\AppData\Roaming\JOSM
The result is a bit different:
1) 1st execution of validator just opens the Window "Prüfergebnisse" (empty)
2) 2nd execution shows the wrong warning

[Klumbumbus]

Hm, strange. I can reproduce but only with fresh preferences.

[GerdP]

OK, I was now able to debug this. The error is added in MultipolygonTest.visit(Way w)
because ElemStyles.hasOnlyAreaOrTextStyleElements(w) returns true. I don't understand the code
in this method. Why does it test if (!(s instanceof AreaElement || s instanceof TextElement)) .
What has TextElement to do with this?

[Don-vip]

@bastiK: we don't understand r11799 ?
By the way the javadoc was not updated ;)

[bastiK]

What does this line in our main style represent?

way|z17-[highway^=motorway][setting("highway_labels")],

If it can be replaced by

way|z17-[highway=motorway][setting("highway_labels")],
way|z17-[highway=motorway_link][setting("highway_labels")],

then this would be more efficient (and cover up the bug introduced with r11799).

[Klumbumbus]

Yes, can be replaced. (Fixing the real bug would be good too so we don't run into it again accidentally as this this connection is not obvious.)

[bastiK]

In 12700/josm:

fixed #15203 - Wrong "Area style way is not closed" message for highway=motorway_junction

[bastiK]

In 12701/josm:

see #15203 - avoid ^{= operator in default style}

improves efficiency; makes sure, it is not applied to unwanted tags (like motorway_junction)

[Klumbumbus]

In 12702/josm:

see #15203 - typo

[GerdP]

The wrong warning is gone, but the wanted warning is not produced.

[bastiK]

In 12708/josm:

fixed #15203 - add validator warning for highway=motorway_junction used on a way

[GerdP]

There are a few more highway values which should only be used on nodes:
give_way
milestone
mini_roundabout
stop
street_lamp
traffic_signals
turning_loop
turning_circle

[bastiK]

Klumbumbus, do you like to check this and make the commit?

[Klumbumbus]

For performance reasons I don't like checks for extremly rare cases. Current taginfo numbers for these tags on ways:

3 - motorway_junction
0 - give_way
23 - milestone
20 - mini_roundabout
0 - stop
0 - street_lamp
31 - traffic_signals
55 - turning_loop
48 - turning_circle

Judging only from these numbers I would say for none of these tags a check is useful.

Looking at taghistory:

I would say a check for turning_circle is useful as this one rises much faster

[GerdP]

I've recently corrected many mistagged ways with highway=* key. What I don't understand is
that JOSM warns when I use highway=road while it completely ignores many typical typos.

[Don-vip]

Replying to Klumbumbus:

Looking at taghistory:

what the hell happened end of 2015?

[GerdP]

That was probably also my work. I stopped that in march 2016 and started again a few days ago.

[Don-vip]

Ah it's only ways, didn't read the legend correctly, sorry.

[naoliv]

Gerd, maybe you can use a local rule for this?

way[highway =~ /^(give_way|milestone|mini_roundabout|motorway_junction|stop|street_lamp|traffic_signals|turning_circle|turning_loop)$/],
relation[highway =~ /^(give_way|milestone|mini_roundabout|motorway_junction|stop|street_lamp|traffic_signals|turning_circle|turning_loop)$/] {
        throwError: tr("{0} should only be used on nodes", concat("highway=", tag(highway)));
}

Or even create a rule at https://josm.openstreetmap.de/wiki/Rules with "deeper" QA rules? (which could include this and everything else that won't be included in JOSM by default)

[GerdP]

Well, I don't need those rules for me. Anyhow, if you don't like the idea I can live with that.

[Klumbumbus]

Replying to GerdP:

That was probably also my work. I stopped that in march 2016 and started again a few days ago.

If this is a regular clean up work for some users then we should add the warnings to reduce this. I'll add the warnings in the next milestone.

The main intention of this ticket (false positive warning) was fixed.

[Klumbumbus]

Replying to GerdP:

What I don't understand is that JOSM warns when I use highway=road while it completely ignores many typical typos.

• We have a test for typos in keys and values, tag checker ("... not in presets") (as long as it is not on the ignore list)
• We have words.cfg (However this list is not really maintained.)
• We have the similar named test

Well, all not perfect, but it helps to detect typos. Can you give examples of typos, which are not detected?

[Klumbumbus]

In 12775/josm:

see #15203 - warn about some highway tags used on ways instead of nodes

[GerdP]

Replying to Klumbumbus:

In 12775/josm:

see #15203 - warn about some highway tags used on ways instead of nodes

Thanks, I've somehow missed comment 28, will do some testing tomorrow.

[GerdP]

Replying to Klumbumbus:

Replying to GerdP:

What I don't understand is that JOSM warns when I use highway=road while it completely ignores many typical typos.

• We have a test for typos in keys and values, tag checker ("... not in presets") (as long as it is not on the ignore list)
• We have words.cfg (However this list is not really maintained.)
• We have the similar named test

Well, all not perfect, but it helps to detect typos. Can you give examples of typos, which are not detected?

OK, I found out that typical typos like highway=unclassfied (missing i) are detected as "Presets do not contain property value" with Severity.OTHER while the correct highway=road gives "Unspecific highway type" with Severity.WARNING
because highway.mapcc contains this rule:
way[highway=road] {

throwWarning: tr("Unspecific highway type");
assertMatch: "way highway=road";
assertNoMatch: "way highway=residential";

}

I think it should be the other way round, means, if the highway key is used with an unknown value it should give a warning while
highway=road should result in an Severity.OTHER message.

If I got that right, the file words.cfg is only used for tag keys, typos like higway instead of highway also give a warning with
a useful message like "Misspelled property key - Key 'higway' looks like 'highway'. ".
The content of words.cfg really looks a bit out-aged.

Maybe we can add a similar file for well known typos in often used keys, so that
highway=unclassfied gives a warning like "Misspelled property value - Value 'unclassfied' looks like 'unclassified'. ".

[Klumbumbus]

highway=road is fine as Severity.WARNING as highway=road should be fixed to another value.

The reason that "Presets do not contain property value" is Severity.OTHER is that this is a pretty general test. There are a lot of tags, which are fine but not in the presets (e.g. because they are less used, see also DevelopersGuide/DefaultPresets). So this test is just an information, that an tag is not in the presets and may be "wrong" but not for sure. The ignore list helps a bit to clean up the results of this test. However the test is still not appropriate as Severity.WARNING

[GerdP]

Okay, I agree that we should not simply change the severity levels. What do you think about the new file for typical typos?
I think I'll try to implement that.