Warn about main tag with incorrect object type (steps)

[skyper]

What steps will reproduce the problem?

1. have a closed way tagged with highway=steps (​254486816)
2. run validator

What is the expected result?

A warning about an incorrect/unusual object type (closed way).

What happens instead?

No warning

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

The object types are set correctly in presets as I get a warning using it and I have to manually add highway=steps.

In general, it would be nice if you get a warning if presets do not allow the tag on the object type.
Thanks

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-12-21 15:53:55 +0100 (Sat, 21 Dec 2019)
Revision:15607
Build-Date:2019-12-22 02:30:55
URL:https://josm.openstreetmap.de/svn/trunk

Last errors/warnings:
- W: No configuration settings found.  Using hardcoded default values for all pools.
- W: Region [TMS_BLOCK_v2] Resetting cache

[Don-vip]

In 15640/josm:

fix #18455 - detect objects not matching their presets object type (info level)

[GerdP]

I think the error message "Wrong presets types" is missleading. Seems to say that the preset is wrong?

[Don-vip]

Feel free to improve the error message.

[GerdP]

What do you think about "Type not in preset" ?

[Don-vip]

Yep, ok :)

[GerdP]

In 15642/josm:

see #18455: Improve error group string "Wrong presets types" -> "Type not in preset"

[skyper]

Replying to GerdP:

What do you think about "Type not in preset" ?

How about "Object type not in preset" and "Object type {0} is not supported by tagging preset: {1}" ?

[GerdP]

Yes, better.

[skyper]

wording patch

[skyper]

added patch​ for better wording.

[GerdP]

In 15645/josm:

fix #18455: Improve error messages again
Patch by skyper, modified formatting

[Klumbumbus]

Nice test! Thanks.

[skyper]

Thank too,

found a little problem with relations. (​1970390)

• Object type relation is not supported by tagging preset: General attributes (oneway) (1)
• Object type relation is not supported by tagging preset: River (1)

[skyper]

Found one more one a building mapped as mulipolygon:

• Object type multipolygon is not supported by tagging preset: Simple 3D buildings outline (1)

Maybe do not check all relations.

[skyper]

Replying to skyper:

Found one more one a building mapped as mulipolygon:

• Object type multipolygon is not supported by tagging preset: Simple 3D buildings outline (1)

Forget this one. Its from an additional preset.

[Don-vip]

In 15667/josm:

fix #18455 - smarter error detection, should lead to less false positives

[GerdP]

It seems this additional test requires a lot of CPU power. TagChecker takes much longer compared to r15628.
According to Java Mission Control the "hot" methods are

Stack Trace	                                                                        Sample Count	Percentage(%)
org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItem.matches(Iterable, Map)	121	        9,967
org.openstreetmap.josm.gui.tagging.presets.items.KeyedItem.matches(Map) 	        70	        5,766

[Don-vip]

Replying to GerdP:

It seems this additional test requires a lot of CPU power.

Yes, that was also one good reason to make it an INFO level. Do you see a possible optimization?

[GerdP]

The test is executed even if info level is not selected. A simple improvement would be to check this first.

[Don-vip]

You're right, I thought I checked it and forgot to code it.

[Don-vip]

In 15682/josm:

see #18455 - don't perform INFO level tests in TagChecker if not enabled

[GerdP]

Maybe TaggingPresetItem.matches(Iterable<? extends TaggingPresetItem> data, Map<String, String> tags)
could be improved. Current loop iterates over all items. This is needed since the check item.matches(tags) may return false for the last item. If the list of items could be sorted so that we can first check if a mandantory tag is missing we might save some iterations, esp. those for link, space etc..

[Don-vip]

In 15683/josm:

see #18455 - fix unit tests

[GerdP]

POC: filter TaggingPreset.data to avoid tests which never match. Reduces runtime from ~10 secs to ~2 secs with my test data.

[GerdP]

This is the careful version which only adds the logic in TagChecker. I'd prefer to use the index in TaggingPresets.
Would probably also improve reaction time when mouse moves over dialogs.

[GerdP]

@Vincent: I tried to code this approach in TaggingPresets but the performance improvement is much smaller compared to the solution in 18455-performance.patch as we would still iterate over many presets which never match.
Should I commit the patch?

[Don-vip]

Replying to GerdP:

Should I commit the patch?

Looks fine, go ahead.

[GerdP]

In 15788/josm:

see #18455: improve performance

• filter TaggingPreset.data to avoid tests which never match. This reduces significantly the number of iterations.

[Don-vip]

In 16154/josm:

see #17533 - see #18455 - partial revert of r15640, causing untagged nodes to be skipped entirely