Modify

Opened 5 weeks ago

Closed 4 weeks ago

Last modified 2 weeks ago

#20455 closed enhancement (fixed)

Raise "Unclosed way - landuse" warnings to errors

Reported by: mkoniecz Owned by: team
Priority: normal Milestone: 21.02
Component: Core validator Version:
Keywords: template_report Cc:

Description

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

Attachments (1)

20455.patch (1.2 KB) - added by GerdP 4 weeks ago.
Raise warning to error for all checks when we know that the tag must describe an area

Download all attachments as: .zip

Change History (17)

comment:1 Changed 5 weeks ago by GerdP

I agree if we change it for all landuse=*

comment:2 Changed 5 weeks ago by mkoniecz

Summary: Raise "Unclosed way - landuse type grass" warnings to errorsRaise "Unclosed way - landuse" warnings to errors

Definitely, my title was overly specific.

Last edited 5 weeks ago by mkoniecz (previous) (diff)

comment:3 Changed 5 weeks ago by 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.

comment:4 Changed 4 weeks ago by 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?

comment:5 Changed 4 weeks ago by GerdP

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

Changed 4 weeks ago by GerdP

Attachment: 20455.patch added

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

comment:6 Changed 4 weeks ago by 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.

comment:7 Changed 4 weeks ago by GerdP

Resolution: fixed
Status: newclosed

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

comment:8 Changed 4 weeks ago by GerdP

Milestone: 21.02

comment:9 Changed 4 weeks ago by mkoniecz

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

comment:10 Changed 4 weeks ago by stoecker

Milestone: 21.0221.03

Milestone renamed

comment:11 Changed 4 weeks ago by 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"

comment:12 Changed 4 weeks ago by mkoniecz

That would be fine.

comment:13 Changed 4 weeks ago by GerdP

In 17491/josm:

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

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

comment:14 Changed 2 weeks ago by 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.

comment:15 Changed 2 weeks ago by GerdP

Done with r17514.

comment:16 Changed 2 weeks ago by Don-vip

Milestone: 21.0321.02

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.