NPE in UnconnectedWays

[taylor.smock]

What steps will reproduce the problem?

1. Open the attached file
2. Run the validator
3. See the crash

What is the expected result?

No crash

What happens instead?

Crash (NPE)

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

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-09-16 01:50:12 +0200 (Mon, 16 Sep 2019)
Revision:15353
Build-Date:2019-09-16 01:30:59
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (15353 en) Mac OS X 10.14.6
OS Build number: Mac OS X 10.14.6 (18G95)
Memory Usage: 1289 MB / 2048 MB (416 MB allocated, but free)
Java version: 12+33, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: Display 1007460930 1920x1080, Display 69955572 2048x1152
Maximum Screen Size: 2048x1152
Dataset consistency test: No problems found

Plugins:
+ Mapillary (1.5.18)
+ MicrosoftStreetside (34977)
+ OpenStreetCam (256)
+ apache-commons (34908)
+ apache-http (34908)
+ auto_tools (73)
+ buildings_tools (34982)
+ conflation (0.6.3)
+ geojson (124)
+ graphview (34977)
+ gson (34908)
+ highwayNameModification (1557754861)
+ imagery_offset_db (34908)
+ javafx-osx (34908)
+ jna (34908)
+ jts (35064)
+ junctionchecking (34977)
+ kaartvalidation (1554390132)
+ mapdust (35039)
+ markseen (13)
+ openqa (1557250801)
+ osm-obj-info (56)
+ project_creation (UNKNOWN)
+ reverter (35084)
+ rex (53)
+ tageditor (34977)
+ todo (30306)
+ tofix (487)
+ turnlanes (34994)
+ turnlanes-tagging (281)
+ utilsplugin2 (35098)

Tagging presets:
+ https://raw.githubusercontent.com/osmlab/name-suggestion-index/master/dist/name-suggestions.presets.xml

Map paint styles:
- ${HOME}/workspace/osm/JOSM Paint Styles and Presets/Kaart-Styles.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1
+ https://raw.githubusercontent.com/species/josm-preset-traffic_sign_direction/master/direction.mapcss
+ https://github.com/osmlab/appledata/archive/josm_paint_inline_validation.zip
+ https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ParkingLanes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Sidewalks&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/DestinationSignRelation&zip=1

Validator rules:
+ ${HOME}/workspace/osm/kaart.validator.mapcss
- ${HOME}/workspace/osm/kaart.processing.validator.mapcss

Last errors/warnings:
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 6
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 5
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 5
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 5
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 6
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 6
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 6
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 5
- E: java.lang.reflect.InvocationTargetException. Cause: java.util.regex.PatternSyntaxException: Illegal character range near index 6
- E: Handled by bug report queue: java.lang.NullPointerException


=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-0 (19) of main
java.lang.NullPointerException
	at org.openstreetmap.josm.data.coor.LatLon.greatCircleDistance(LatLon.java:228)
	at org.openstreetmap.josm.data.validation.tests.UnconnectedWays$MyWaySegment.isConnectedTo(UnconnectedWays.java:460)
	at org.openstreetmap.josm.data.validation.tests.UnconnectedWays$MyWaySegment.isConnectedTo(UnconnectedWays.java:426)
	at org.openstreetmap.josm.data.validation.tests.UnconnectedWays.getHighwayEndNodesNearOtherHighway(UnconnectedWays.java:251)
	at org.openstreetmap.josm.data.validation.tests.UnconnectedWays.endTest(UnconnectedWays.java:372)
	at org.openstreetmap.josm.actions.ValidateAction$ValidationTask.realRun(ValidateAction.java:169)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.doRealRun(PleaseWaitRunnable.java:94)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.run(PleaseWaitRunnable.java:142)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:835)

[taylor.smock]

Sample file where NPE occurs

[taylor.smock]

Very simple workaround (checks if we know the latlon of the next Node).

[taylor.smock]

Check if we know if the latlon exists before we add it to the nextNodes variable, also modify if statements for if else if else.

[GerdP]

I think with your patch the algo will not find both possible next nodes.

[taylor.smock]

Get both nodes to compare to instead of just one node, some code duplication in the checks for coordinates has occurred

[taylor.smock]

Replying to GerdP:

I think with your patch the algo will not find both possible next nodes.

Having reread the patch, I'm pretty certain you are right.
I've modified it to fix that issue, but it has brought in code duplication. For those three lines, is it worth it to create a new function? Or to rewrite that short section to be in a for loop?

if (temporaryNode != null && temporaryNode.isLatLonKnown()) {
    nextNodes.add(temporaryNode);
}

EDIT: If that is the case, then the first patch (18137.patch​) would be better.

[GerdP]

I am not yet sure what the algo should do when a node has no latlon. I think it should try to find the next valid node in each direction.

[taylor.smock]

Try to find the next node in each direction when one is null or we don't know the LatLon (needs testing)

[taylor.smock]

Iterate through nodes until we find one that has a good LatLon, needs a check in the for (Node next : nextNodes) block since we may only find nodes with no known latlon (and that will be what is assigned to the array) or no nodes (in which case we have null in the array).

[GerdP]

Don't try to follow unsuable ways

[GerdP]

I'll have to do some checks and and add a unit test but I think we can ignore ways with incomplete nodes.

[GerdP]

In 15354/josm:

fix #18137: NPE in UnconnectedWays