[Patch] tunnel=culvert should have waterway=* as well

[ivanbranco]

Hi,

they redirect me here from the ​Osmose github since the error comes from this JOSM validator rule​. I copy and paste my proposal:

"there's already a general check for the tunnel=* tag (item 9001 class 9001002). I would like to propose to make a more specific rule for one of its values which is tunnel=culvert. The Wiki says "A culvert is a device used to channel water", so I would expect this value to be matched with waterway=* only. At the moment we have 3914 highways tagged with tunnel=culvert (​overpass query).

I would just flag every way tagged with tunnel=culvert which is not tagged with waterway=* as well, so I would expect all these ways to be flagged as an error: ​overpass query"

[anonymous]

And of course excluding man_made=tunnel + tunnel=culvert for tunnels mapped separately

[skyper]

tunnel relations with type=tunnel should be allowed, too.

[anonymous]

Replying to skyper:

tunnel relations with type=tunnel should be allowed, too.

In this case the check could be done on the way members without a role or with "across" or "through" role, so we could find stuff like this culvert on a railway: ​https://www.openstreetmap.org/way/232574325

[Famlam]

Or just limiting it to way?

[ivanbranco]

Replying to Famlam:

Or just limiting it to way?

That's where most of errors are anyway. type=tunnel is not that used, and there are only 4 type=tunnel+tunnel=culvert. Of them 1 is correct and 3 wrong but all from the same user.

[skyper]

way[tunnel=culvert][!waterway][man_made!=tunnel] {
  throwWarning: tr("{0} on suspicious object", "{0.key}");
  group: tr("suspicious tag combination");
}

Maybe, the general check at trunk/resources/data/validator/combinations.mapcss?rev=18546#L490​ should be moved into a single block to ignore matches from this rule and in order to not get two warnings about one problem.

[skyper]

patch

[skyper]

Find attached patch​
It only warns about ways and excludes the matches from the general warning about suspicious object.

[taylor.smock]

In 18659/josm:

Fix #22547: tunnel=culvert should have waterway=* or man_made=tunnel (patch by skyper)

[stoecker]

I understand to mention the ticket, but why for the rule above? That makes no sense.

[taylor.smock]

Replying to stoecker:

I understand to mention the ticket, but why for the rule above? That makes no sense.

I assume you are talking about the

/* {0.key} without {1.key} or {2.tag}, #22547 */

line.

I was going to ask skyper about it, then looked at some of the other rules nearby, which usually had ticket references.

[stoecker]

Right, but shouldn't there be a comment above the new rule with the reference instead of adding that one rule above?

[taylor.smock]

I guess I was looking at it as a section header.

/* {0.tag} without {1.key} (info level), #15107 */
[...snip...]

/* {0.key} without {1.key} or {2.tag}, #22547 */
way[bridge:structure           ][!bridge][man_made!=bridge],
*[segregated                   ][!highway][railway!=crossing] {
  throwWarning: tr("{0} without {1} or {2}", "{0.key}", "{1.key}", "{2.tag}");
  group: tr("missing tag");
}
way[tunnel=culvert][man_made!=tunnel][!waterway] {
  throwWarning: tr("{0} without {1} or {2}", "{0.tag}", "{1.tag}", "{2.key}");
  group: tr("suspicious tag combination");
  set TunnelCulvertWithoutWaterway;
}

/* {0.tag} without {1.tag} (info level) see #11600 #11393 #11850 */
[...snip...]

/* {0.tag} without {1.tag} */
[...snip...]

I probably should have copied that line to right above the culvert check, and then dropped the #22547 reference in the original line.

[taylor.smock]

In 18660/josm:

See #22547: move ticket reference to just above rule

[skyper]

Replying to taylor.smock:

I guess I was looking at it as a section header.

Yes, that was the thought behind it and the reason to not add an empty line in between the rules. Probably, I should have placed the ticket reference as separate comment just above the new line.