8250802: Refactor StringConverter and its subclasses

[nlisker]

Refactoring of all StringConverters and their tests. General notes:

• The documentation language has been unified and null parameter rules have been documented.
• Tests have been cleaned up in the vein of https://github.com/openjdk/jfx/pull/1759 and unneeded @BeforeAlls were removed.
• Internal fields were made private final to guarantee immutability.

Incremental commits are provided for easier reviewing:

Parent classes

• StringConverter: updated documentation
• BaseStringConverter: a new internal class that implements repeated code from converter implementations and serves as an intermediate superclass. It does empty and null string checks that are handled uniformly, except for DefaultStringConverter, which has a different formatting mechanism.

Primitive-related converters

• All primitive (wrapper) converters also document their formatting and parsing mechanism since these are "well-established".

Format converter

• Checked for null during constriction time to avoid runtime NPEs.
• There is no test class for this converter. A followup might be desirable.
• A followup should deprecate for removal protected Format getFormat() (as in JDK-8314597 and JDK-8260475).

Number and subclasses converters

• The intermediate locale and pattern fields were removed (along with their tests). The class generated a new formatter from these on each call. This only makes sense for mutable fields where the resulting formatter can change, but here the formatter can be computed once on construction and stored.
• The only difference between these classes is a single method for creating a format from a null pattern, which was encapsulated in the getSpecializedNumberFormat method.
• The terminally deprecated protected NumberFormat getNumberFormat() was removed. Can be split to its own issue if preferred. In my opinion, it shouldn't exist even internally since testing the internal formatter doesn't help. The only tests here should be for to/from strings, and these are lacking. A followup can be filed for adding more conversion tests.

Date/Time converters

• Added a documentation note advising users to use the java.time classes instead of the old Date class.
• As with Number converters, only the dateFormat field was kept, which is created once on construction instead of on each call.
• As with Number converters, the getSpecialziedDateFormat method is the only difference between the classes.
• As with Number converters, protected DateFormat getDateFormat() has been removed from the subclasses and shouldn't exist internally either in my opinion.

LocalDate/Time converters

• The structure of these classes is different from the above 2 groups since there is no subclassing. Instead, the Date and Time converters used the DateTime converter's LdtConverter implementation by passing their class type, which is not a good design. Instead, an internal superclass BaseTemporalStringConverter<T extends Temporal> was introduced along with its shim.
• The code that fixes the 4 year pattern, fixFourDigitYear, is now checked before creating a formatter, avoiding unneeded formatter creations.
• As above, only the parser and formatter fields were kept.
• As above, the getLocalizedFormatter and getTemporalQuery() methods are the only differences between the subclasses.
• As above, testing the formatter and parser isn't useful in my opinion.

/reviewers 2 /csr

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change requires a CSR request matching fixVersion jfx26 to be approved (needs to be created)
• [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8250802: Refactor StringConverter and its subclasses (Enhancement - P4)

Reviewers

• John Hendrikx (@hjohn - Reviewer)
• Andy Goryachev (@andy-goryachev-oracle - Reviewer) 🔄 Re-review required (review applies to a030ce58)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1880/head:pull/1880
$ git checkout pull/1880

Update a local copy of the PR:
$ git checkout pull/1880
$ git pull https://git.openjdk.org/jfx.git pull/1880/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1880

View PR using the GUI difftool:
$ git pr show -t 1880

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1880.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back nlisker! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

[openjdk[bot]]

@nlisker The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[openjdk[bot]]

@nlisker has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@nlisker please create a CSR request for issue JDK-8250802 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

[mlbridge[bot]]

Webrevs

• 04: Full - Incremental (1b4248c5)
• 03: Full - Incremental (a030ce58)
• 02: Full - Incremental (ffad73e5)
• 01: Full - Incremental (3448438b)
• 00: Full (2b610c6a)

[bridgekeeper[bot]]

@nlisker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[nlisker]

/keepalive

[openjdk[bot]]

@nlisker The pull request is being re-evaluated and the inactivity timeout has been reset.

[bridgekeeper[bot]]

@nlisker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[nlisker]

/keepalive

[openjdk[bot]]

@nlisker The pull request is being re-evaluated and the inactivity timeout has been reset.

[bridgekeeper[bot]]

@nlisker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!