Context Navigation

Modify ↓

#14176 closed enhancement (fixed)

Use Java 8 Date API (JSR 310): replace Date by Instant?

Reported by:	Don-vip	Owned by:	simon04
Priority:	normal	Milestone:	21.05
Component:	Core	Version:
Keywords:	date instant java8	Cc:	simon04

Description

Follow-up from #13376:

Left to be done/discussed: replace 257 occurrences of Date by Instant, or leave it as it is :)

Given the number of deprecated methods in Date it's almost if the class is now deprecated itself. I'd say we can change everything that does not impact plugin API.

Compatibility will is a major concern since methods like AbstractPrimitive#getTimestamp, AbstractPrimitive#setTimestamp, Note#getCreatedAt, Note#getClosedAt all take/return Dates.

Attachments (1)

14176-notes.patch (16.4 KB) - added by simon04 14 months ago.

Download all attachments as: .zip

Change History (40)

comment:1 Changed 14 months ago by simon04

In 17653/josm:

see #14176 - DateUtils.parseInstant

Changed 14 months ago by simon04

Attachment:	14176-notes.patch added

comment:2 Changed 14 months ago by simon04

Changing the Note and NoteComment data objects might be a good starting point, since notes are not used by plugins -> attachment:14176-notes.patch

comment:3 Changed 14 months ago by Don-vip

Coverity found an issue:

*** CID 1451346:  Null pointer dereferences  (NULL_RETURNS)
/src/org/openstreetmap/josm/tools/date/DateUtils.java: 142 in org.openstreetmap.josm.tools.date.DateUtils.parseInstant(java.lang.String)()
136                 if (d != null)
137                     return d.toInstant();
138             }
139     
140             try {
141                 // slow path for fractional seconds different from millisecond precision
>>>     CID 1451346:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "java.time.ZonedDateTime.parse(str)".
142                 return ZonedDateTime.parse(str).toInstant();
143             } catch (IllegalArgumentException | DateTimeParseException ex) {
144                 throw new UncheckedParseException("The date string (" + str + ") could not be parsed.", ex);
145             }
146         }
147

comment:4 Changed 14 months ago by simon04

This looks like a false-positive to me.

java.time.ZonedDateTime#parse(java.lang.CharSequence) clearly states Returns: the parsed zoned date-time, not null.

After stepping through the called methods, I found java.time.temporal.TemporalAccessor#query and java.time.chrono.ChronoLocalDateTime#query which return null for other parameters not used here.

comment:5 Changed 14 months ago by Don-vip

OK. No tool is perfect :)

comment:6 Changed 14 months ago by simon04

In 17712/josm:

see #14176 - Migrate Note/NoteComment to Instant

comment:7 Changed 13 months ago by simon04

In 17715/josm:

see #14176 - Migrate GPX to Instant

comment:8 Changed 13 months ago by simon04

In 17716/josm:

see #14176 - Migrate UserInfo to Instant

comment:9 Changed 13 months ago by simon04

In 17717/josm:

see #14176 - Migrate Changeset to Instant

comment:10 Changed 13 months ago by simon04

In 17718/josm:

see #14176 - Utils.getDurationString in GpxLayer.formatTimespan

comment:11 Changed 13 months ago by simon04

In 17719/josm:

see #14176 - Migrate AnimationExtensionManager to LocalDate

comment:12 Changed 13 months ago by simon04

In 17720/josm:

see #14176 - Migrate ChangesetDiscussionComment to Instant

comment:13 Changed 13 months ago by simon04

In 17721/josm:

see #14176 - Fix BookmarkList.ChangesetBookmark

https://errorprone.info/bugpattern/FormatString

comment:14 Changed 13 months ago by simon04

In 17722/josm:

see #14176 - Fix Instant writing in GpsWriter

comment:15 Changed 13 months ago by simon04

In 17723/josm:

see #14176 - Fix Instant writing in GpsWriterTest

comment:16 Changed 13 months ago by simon04

In 35732/osm:

see #14176 - reverter: adapt ChangesetReverter for r17717

comment:17 Changed 13 months ago by simon04

In 17726/josm:

see #14176 - Migrate AnimationExtensionManager to LocalDate (ZoneId)

https://errorprone.info/bugpattern/JavaTimeDefaultTimeZone

comment:18 Changed 13 months ago by Don-vip

Another issue reported by Coverity:

*** CID 1452256:    (NULL_RETURNS)
/src/org/openstreetmap/josm/data/gpx/GpxData.java: 298 in org.openstreetmap.josm.data.gpx.GpxData$GpxTrackSegmentSpan.<init>(org.openstreetmap.josm.data.gpx.WayPoint, org.openstreetmap.josm.data.gpx.WayPoint)()
292             private final WayPoint firstWp;
293             private final WayPoint lastWp;
294     
295             GpxTrackSegmentSpan(WayPoint a, WayPoint b) {
296                 Instant at = a.getInstant();
297                 Instant bt = b.getInstant();
>>>     CID 1452256:    (NULL_RETURNS)
>>>     Dereferencing a pointer that might be "null" "at" when calling "isBefore".
298                 inv = bt.isBefore(at);
299                 if (inv) {
300                     firstWp = b;
301                     firstTime = bt;
302                     lastWp = a;
303                     lastTime = at;
/src/org/openstreetmap/josm/data/gpx/GpxData.java: 298 in org.openstreetmap.josm.data.gpx.GpxData$GpxTrackSegmentSpan.<init>(org.openstreetmap.josm.data.gpx.WayPoint, org.openstreetmap.josm.data.gpx.WayPoint)()
292             private final WayPoint firstWp;
293             private final WayPoint lastWp;
294     
295             GpxTrackSegmentSpan(WayPoint a, WayPoint b) {
296                 Instant at = a.getInstant();
297                 Instant bt = b.getInstant();
>>>     CID 1452256:    (NULL_RETURNS)
>>>     Calling a method on null object "bt".
298                 inv = bt.isBefore(at);
299                 if (inv) {
300                     firstWp = b;
301                     firstTime = bt;
302                     lastWp = a;
303                     lastTime = at;