RFR: 8358604: Trees for `var` do not have end positions

[Jan Lahoda]

On Thu, 5 Jun 2025 21:14:03 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

> The compiler replaces `var` nodes with synthetic tree nodes representing the inferred type. Previously, these new synthetic nodes were not being assigned either starting or ending source code positions.
> 
> [JDK-8329951](https://bugs.openjdk.org/browse/JDK-8329951) added starting positions, but not ending positions. This PR adds ending positions, and fixes some related bugs discovered in the research process:
> 
> * For a declaration like `final var x`, the fix for [JDK-8329951](https://bugs.openjdk.org/browse/JDK-8329951) was returning a starting position incorrectly pointing to `final` instead of `var`.
> * A declaration like `final var x` was correctly starting at `final` for normal declarations, but incorrectly starting at `var` for a lambda parameter declarations.
> * `JCVariableDecl.declaredUsingVar()` was incorrectly returning `false` for `var` variable declarations in record deconstruction patterns (thanks to @cushon for reporting this)
> 
> Background: When a `var`is parsed or a variable is implicitly typed such as a lambda parameter, `JCVariableDecl.vartype` is set to null until the type can be inferred. This causes various problems when trying to determine the starting position of (a) the declaration's type, when `var` is used, because the source position of the `var` has been lost forever (especially if there are also modifiers); and (b) the starting position of the declaration itself, when there are no modifiers, because normally that's computed from the start position of `JCVariableDecl.vartype`, which is (sometimes) null. Previously there was a field `JCVariableDecl.startPos` which was put there to workaround problem (b), but that workaround was being applied even when there _were_ modifiers, which is wrong. Etc.
> 
> This patch attempts to clarify things and includes the following changes:
> 
> * Make a `var`'s starting source position properly recoverable from a `JCVariableDecl` even while `vartype` is null.
> * Fix the three bugs mentioned above.
> * Make `TreeMaker.at()` capable of configuring both the starting and ending position; do some minor cleanup/refactoring.
> * Set ending positions for the synthetic tree nodes that replace `var`
> * Pretty print `var` as `var` instead of `/*missing*/` (drive-by improvement)
> * Add more regression test coverage

Yes, that is related to JDK-8024098, I've fixed a few of the modelling issues in the past, but many of them are quite difficult.

While I don't doubt the end position is sometimes useful,  the ability to detect the tree is synthetic is also sometimes useful (and it may be that one client would desire to know both these properties depending on the context - like imagine you would want to add something that detects `var` and allows to replace it with the inferred type - you need to be able to detect the variable's type is inferred. Or, imagine a rename refactoring for a class - the tool surely does not want to re-write an inferred type.)

Overall, I think just putting an end position to one of the synthetic trees we create is not very helpful, and is going to cause trouble. The options I see are roughly:
1. (the usual:) keep things as they are. The clients that need an end position can check for `-1` end pos, and compute the real end pos from the original text. Not very helpful, but not breaking anything either.
2. avoid exposing the synthetic tree through the API (i.e. solving JDK8268850), see below. It is actually probably not that difficult, although the clients will need to adjust.
3. accepting that we generate some synthetic trees, exposing a method that allows to detect them, and add sensible end positions to other synthetic trees. This is a bit tricky for some cases, like `@SuppressWarnings("...")`, which generates a highly-problematic synthetic assignment.

For 2. - the question is, what is the right model that closely models the original code. One possibly truthful model would be:
- for `var v`, the `VariableTree.getType()` would return an `IdentifierTree` holding `var` with both the start and end positions
- for `v` in `(v) -> {}`, `VariableTree.getType()` would return `null`
- possibly, there would be a method on `VariableTree`, which would permit the check if the type is inferred or not. (So that the clients don't need to second-guess what `var` means.)

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

PR Comment: https://git.openjdk.org/jdk/pull/25664#issuecomment-2960127102