RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v21]

[Roger Riggs]

On Mon, 14 Nov 2022 17:51:24 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> Enhance the Java programming language with string templates, which are similar to string literals but contain embedded expressions. A string template is interpreted at run time by replacing each expression with the result of evaluating that expression, possibly after further validation and transformation. This is a [preview language feature and API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Wrong line separator

Mostly comments to make the code easier to read and understand.

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1826:

> 1824: 
> 1825:     // Used by StringConcatHelper via JLA.
> 1826:     long mix(long lengthCoder) {

`mix` is not a very descriptive name (in all of the places it is used).
Perhaps a name that captures the combination of coder and length.

`coderAndLength()` or similar

The implementation here can do the wrong thing if the `lengthCoder` already includes the bit for UTF16
and the coder for the AbstractStringBuilder is LATIN1. Or is intentional that it will upgrade to UTF16?
That could definately use a comment.

src/java.base/share/classes/java/lang/runtime/TemplateRuntime.java line 58:

> 56:  * optimized {@link StringTemplate StringTemplates} based on StringTemplateImpl.
> 57:  * <p>
> 58:  * Bootstraps in the for of (Lookup, String, MethodType, MethodHandle, String...)

"in the for" -> "in the form"

src/java.base/share/classes/java/lang/runtime/TemplateRuntime.java line 116:

> 114:      */
> 115:     public static CallSite stringTemplateBSM(
> 116:             MethodHandles.Lookup lookup,

The predominate style for method declarations puts the first argument on the same line as the method name
and the closing ")" on the same line with the last argument and exceptions.

src/java.base/share/classes/java/lang/template/Carriers.java line 628:

> 626:      * types.
> 627:      */
> 628:     private static class CarrierShape {

Is this a candidate to be a Record?

src/java.base/share/classes/java/lang/template/StringProcessor.java line 31:

> 29: 
> 30: /**
> 31:  * This interface simplifies declaration of

The first sentence should describe the purpose or function of the interface.
The "simplify declaration" language is a design rationale, not a functional description of the interface.

src/java.base/share/classes/java/lang/template/StringTemplate.java line 38:

> 36: 
> 37: /**
> 38:  * The Java compiler produces implementations of {@link StringTemplate} to

I'd rather see a first sentence that says what a StringTemplate is, not who produces it.
For example, a StringTemplate holds and controls the processing of the fragments and expressions of a string template.

src/java.base/share/classes/java/lang/template/StringTemplate.java line 189:

> 187: 
> 188:     /**
> 189:      * Produces a diagnostic string representing the supplied

The description should include that it describe the fragments and values.

src/java.base/share/classes/java/lang/template/StringTemplate.java line 208:

> 206: 
> 207:     /**
> 208:      * Returns a StringTemplate composed from a string.

and no values.

src/java.base/share/classes/java/lang/template/StringTemplate.java line 249:

> 247: 
> 248:     /**
> 249:      * Creates a string that interleaves the elements of values between the

It might be worth noting that this is the same as STR::interpolate.

src/java.base/share/classes/java/lang/template/StringTemplate.java line 273:

> 271: 
> 272:     /**
> 273:       * Combine one or more {@link StringTemplate StringTemplates} to produce a combined {@link StringTemplate}.

The description should be more specific about how the lists of fragments and values are combined.
For example, is each StringTemplate well formed and have the required string and value counts.
(Or when combined, is it only the case that the total number of strings and values meet the n+1/n requirement.


I'm not sure this API point pulls it weight in the API.

src/java.base/share/classes/java/lang/template/StringTemplate.java line 291:

> 289: 
> 290:     /**
> 291:      * Interpolation template processor instance.

Flesh out this description since it is automatically imported and will appear in user code, it should have a clear description. (Same for RAW, expand the description).

src/java.base/share/classes/java/lang/template/StringTemplateImpl.java line 35:

> 33: 
> 34: /**
> 35:  * This class implements specialized {@link StringTemplate StringTemplates} produced by

A cross reference to the public api the compiler uses to create these would be useful.
So the reader can find the place to start reading.

src/java.base/share/classes/java/lang/template/StringTemplateImplFactory.java line 187:

> 185:     @Override
> 186:     public StringTemplate newStringTemplate(List<String> fragments, List<?> values) {
> 187:         return new SimpleStringTemplate(List.copyOf(fragments), toList(values.stream().toArray()));

`values.toArray()` returns a new array; no need to stream the list content.
Alt: `Arrays.asList(values.toArray())`

src/java.base/share/classes/java/lang/template/StringTemplateImplFactory.java line 202:

> 200:     @SuppressWarnings({"unchecked", "varargs"})
> 201:     private static <E> List<E> toList(E... elements) {
> 202:         return Collections.unmodifiableList(Arrays.asList(elements));

Is a defensive copy needed here?
The caller of `newStringTemplate` could retain a reference to the Object[] array and modify it later.

src/java.base/share/classes/java/util/FormatProcessor.java line 102:

> 100:      *          specifier that is incompatible with the given arguments,
> 101:      *          insufficient arguments given the format string, or other
> 102:      *          illegal conditions.  For specification of all possible

This fragment "For specification of all possible formatting errors."
seems to be incomplete or unnecessary.

src/java.base/share/classes/java/util/FormatProcessor.java line 154:

> 152: 
> 153:     /**
> 154:      * Convert a {@link StringTemplate} fragments, containing format specifications,

Extraneous "a".

src/java.base/share/classes/jdk/internal/util/FormatConcatItem.java line 51:

> 49:      *
> 50:      * @param lengthCoder current value of the length + coder
> 51:      * @param buffer      buffer to right into

right -> write

-------------

PR: https://git.openjdk.org/jdk/pull/10889