Fix formatter blank line and over-indent for union template argument#11010
Fix formatter blank line and over-indent for union template argument#11010oha-4 wants to merge 4 commits into
Conversation
When a `union` expression was used directly as one of multiple template arguments and the argument list wrapped, the formatter inserted a blank line before the union and indented its `|`-prefixed variants one level deeper than the sibling arguments. The argument list already supplies a line break and an indentation level before each argument, so `printUnion` no longer adds its own leading line and indent when the union is a direct argument of a multi-argument template reference. Fixes microsoft#11009 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@microsoft-github-policy-service agree |
commit: |
|
❌ There is undocummented changes. Run The following packages have changes but are not documented.
The following packages have already been documented:
Show changes
|
Address review feedback: the previous fix dropped align(2) on union variants when the union is a multi-argument template argument, which de-indented a nested template inside a variant. Keep align(2) unconditionally (matching prettier's union printer) so a breaking variant stays aligned under its "| " prefix, and add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the careful review — you're right, fixed in And to your question: this does follow prettier's union printer — indent-or-not depends on the parent ( |
Reformatting arm.tsp/response.tsp (union template arguments) changed the tcgc crossLanguageVersion hash of the generated metadata. Update the two *_metadata.json files to the regenerated hashes so the Java RegenCheck passes, and add an internal changeset for @typespec/http-client-java so the changelog check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| } | ||
| let count = 0; | ||
| let node: Node | null; | ||
| while ((node = path.getParentNode(count++))) { |
There was a problem hiding this comment.
to dig a little more on this loop, as you pointed out the indent in typescript printer depends on the parent, which we have above, but I don't fully see how the type reference is involved here. Do we really need that loop, is thre maybe then some test missing to showcase those scenarios?
There was a problem hiding this comment.
Thanks, that's a great catch — you're right, the loop was unnecessary. A TemplateArgument only ever exists inside TypeReference.arguments (the sole TemplateArgumentNode[] in the AST), so the owning TypeReference is always the next ancestor. I've replaced the loop with a direct getParentNode(1) check in 02007f6.
We still need the TypeReference itself (not just the parent) to read the argument count: a single argument hugs (no list indent, so the union keeps its own), while multiple arguments indent the list — that's the case the fix targets. Both are covered by the existing single-/multi-arg tests, and all 205 formatter tests still pass.
A TemplateArgument node only ever exists as an element of TypeReference.arguments (the sole TemplateArgumentNode[] in the AST), so when the parent is a TemplateArgument the owning TypeReference is always the next node ancestor. Replace the variable-depth loop with a direct getParentNode(1) check; behavior is unchanged and all formatter tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes #11009
Problem
When a
unionexpression is used directly as one of multiple template arguments and the argument list is long enough to wrap,tsp formatinserts a blank line before the union and indents its leading-|variants one level deeper than the sibling arguments:Fix
printTemplateParametersalready emits asoftline+ anindentlevel before each argument in a multi-argument list.printUnionwas also emitting its own leadingline+indent(+align(2)), so the two stacked, producing the blank line and the extra indentation level.printUnionnow detects when the union is a direct argument of a multi-argument template reference (isInMultiTemplateArgumentList) and, in that case, lets the surrounding argument list control the line break and indentation. Result:The standalone/value-position cases (e.g.
alias X = "a" | "b" | ...) and the single-template-argument (shouldHug) case are unchanged, since there the surrounding context does not supply the line break/indent.Tests
unionused as a template argument #11009) and a test pinning the single-argument behavior informatter.test.ts.http-client-javatest fixtures that contained the buggy output (whitespace only; no semantic change).pnpm prettier --check "**/*.tsp", formatter test suite, tsc, and eslint all pass.