Raise "Unclosed way - landuse" warnings to errors

[mkoniecz]

What steps will reproduce the problem?

1. Create unclosed landuse=grass way
2. Run validator

What is the expected result?

I get error, as it is a clear mistake that may lead to confusing data loss.

What happens instead?

I get warning.

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

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-01-21 23:33:21 +0100 (Thu, 21 Jan 2021)
Revision:17474
Build-Date:2021-01-22 02:30:49
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (17474 en) Linux Ubuntu 20.04.2 LTS
Memory Usage: 2104 MB / 3974 MB (235 MB allocated, but free)
Java version: 11.0.9.1+1-Ubuntu-0ubuntu1.20.04, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Desktop environment: LXQt
Java package: openjdk-11-jre:amd64-11.0.9.1+1-0ubuntu1~20.04
Java ATK Wrapper package: libatk-wrapper-java:all-0.37.1-1
Environment variable LANG: en_US.UTF-8
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:-
Dataset consistency test: No problems found

Plugins:
+ buildings_tools (35669)
+ reverter (35688)
+ todo (30306)

Validator rules:
+ https://josm.openstreetmap.de/josmfile?page=Rules/OSMLint&zip=1
+ ${HOME}/Documents/install_moje/OSM software/manual editing and discussions/josm/resources/data/validator/deprecated.mapcss

Last errors/warnings:
- 00836.295 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 00891.651 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 00925.747 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 00931.892 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 00941.583 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 01009.990 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 01010.570 W: java.net.SocketException: Connection reset. Cause: java.net.SocketException: Connection reset
- 01128.302 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 01134.416 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server
- 01136.863 W: java.net.SocketException: Unexpected end of file from server. Cause: java.net.SocketException: Unexpected end of file from server

[GerdP]

I agree if we change it for all landuse=*

[mkoniecz]

Definitely, my title was overly specific.

[GerdP]

Hmm, so far the severity is the same for all warnings produced by this test. It tests lots of tag keys. For some the result depends on the tag value, e.g. natural=* produces if warning if the value is not one that is known to be an exception:

"arete", "bay", "cave", "cliff", "coastline", "earth_bank", "gorge", "gully",
"mountain_range", "peak", "ridge", "saddle", "strait", "tree", "tree_row", "valley", "volcano"

The problem: We may show an error for a natural=xyz if I simply change the severity for all checks.
Possible solution: Show error for all those tag keys which don't have exceptions.

[mkoniecz]

For natural I would worry about exceptions (one new linear natural tag just got proposed).

Possible solution: Show error for all those tag keys which don't have exceptions.

+1

Or maybe simply split this check in two?

[GerdP]

Thought about using method OsmPrimitive.hasAreaTags() to decide this, but that also uses an exception list for key "leisure" :(

[GerdP]

Raise warning to error for all checks when we know that the tag must describe an area

[GerdP]

I am not sure why building=no is ignored in UnclosedWay, but shop=no or landuse=no isn't. I found only 11 unclosed ways with building=no and they all look suspicious.

[GerdP]

In 17489/josm:

fix #20455: Raise "Unclosed way - landuse" warnings to errors

• Raise warning to error for those checks where we know that the tag must describe an area

[mkoniecz]

Thanks! It will make easier to spot things that are definitely serious issues.

[stoecker]

Milestone renamed

[GerdP]

I think I have to lower the severity for boundary=* ways. If you download a route relation like r2554492 with members you get 10 errors "Unclosed way - boundary type administrative"

[mkoniecz]

That would be fine.

[GerdP]

In 17491/josm:

see #20455: Raise "Unclosed way - landuse" warnings to errors

• lower the severity for boundary=* ways to warning again

[Don-vip]

@Gerd Newest version of Error-Prone complains about r17491:

    [javac] C:\SVN\josm\core\src\org\openstreetmap\josm\data\validation\tests\UnclosedWays.java:100: warning: [OperatorPrecedence] Use grouping parenthesis to make the operator precedence explicit
    [javac]                 if (ignore && !specialValues.isEmpty() || "boundary".equals(key)) {
    [javac]                            ^
    [javac]     (see https://errorprone.info/bugpattern/OperatorPrecedence)
    [javac]   Did you mean 'if ((ignore && !specialValues.isEmpty()) || "boundary".equals(key)) {'?

I agree this is not clear, can you please make the operator precedence explicit? Not sure what is really wanted here.

[GerdP]

Done with r17514.