RFR: 8354724: BufferedReader readAllLines and readString methods [v14]

[Stuart Marks]

On Wed, 23 Apr 2025 22:04:25 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Implement the requested methods and add a test thereof.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8354724: Fix readAllChars gaffe in Reader returned by Readed.of and account for it in test

Archie Cobbs wrote:
> For those who enjoy rehashing the past, here it is :) [JDK-8307863](https://bugs.openjdk.org/browse/JDK-8307863)

Thanks for the pointer. I don't want to rehash the past, but looking at the previous discussion was informative. Holy cow that was two years ago already; I thought it was just a couple months ago.

In any case, the effect I'm trying for here is to have the specification be able to use code or pseudo-code, which is sometimes the most intuitive way to specify something. Sometimes this _description_ involves another method (or methods) on the same class, but it doesn't want to _specify_ that the other method is actually called. The reason this is significant is that, if the other method is overridable by a subclass, the self-call is observable by the subclass and the subclass implementation can affect the behavior of this method.

More concretely, suppose we have a method specification for the `foo()` method that is like this. (Intentionally vague wording).

> This method behaves like `bar().length()`.

... where the `bar()` method on this class is overridable by a subclass. A reasonable person might think, ok, if I override `bar()` and change its behavior, then `foo()` will also be affected.

Two questions are relevant here:

1. Do we want self-calls, or do we want to avoid self-calls?
2. What specification wording should be used to require self-calls or lack of self-calls?

Note that I've omitted another option for 2 which is to leave it unspecified. Astute readers will note that leaving it unspecified eventually leads to the [fragile base class problem](https://en.wikipedia.org/wiki/Fragile_base_class). Java has suffered from this a fair amount over the years, and the way to avoid it is to be explicit in the specification about what self-calls are or are not made. That's a primary role of the `@implSpec` javadoc tag.

(Sorry for lecturing here, but I want to make sure all readers are on the same page.)

Here are my proposed answers to the questions.

1. In this case `Reader` is an abstract class, and therefore it relies on a subclass to provide _some_ implementation; so some self-calls are required. I observe that the only abstract methods on `Reader` are `close()` and `read(char[], int, int)`, and further that all the concrete methods are implemented in terms of this three-arg `read` method. Based on this, the new methods here should be implemented in terms of that three-arg `read` method, and also they should be specified to do so in an `@implSpec` clause.

(In principle, the other concrete methods of `Reader` should also have `@implSpec` clauses that say they use the three-arg `read` method, but that can probably be handled separately.)

2. Since there will be specific self-calls, there needs to be an `@implSpec` that specifies that. I'd recommend something like this for `readAllChars()`:


    @implSpec
    The implementation in this class reads all the characters from the input by
    repeatedly calling the read(char[], int, int) method with a positive len argument
    until a call returns -1.


There is still the matter of the wording to use for the non-self-call case, although that isn't operative here. Continuing the above example, I'd suggest something like "the result is as if `bar().length()` had been called." The use of the subjunctive mood hints reasonably strongly that `bar()` isn't _really_ called, though it is somewhat subtle. The JDK has used similar wording, sometimes "the effect is as if" but even "effect" is somewhat ambiguous, which is why I'm suggesting "result" here.

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

PR Comment: https://git.openjdk.org/jdk/pull/24728#issuecomment-2848332415