Skip to content

Fix formatter blank line and over-indent for union template argument#11010

Open
oha-4 wants to merge 4 commits into
microsoft:mainfrom
oha-4:fix/formatter-union-template-arg
Open

Fix formatter blank line and over-indent for union template argument#11010
oha-4 wants to merge 4 commits into
microsoft:mainfrom
oha-4:fix/formatter-union-template-arg

Conversation

@oha-4

@oha-4 oha-4 commented Jun 17, 2026

Copy link
Copy Markdown

Fixes #11009

Problem

When a union expression is used directly as one of multiple template arguments and the argument list is long enough to wrap, tsp format inserts a blank line before the union and indents its leading-| variants one level deeper than the sibling arguments:

model Picked
  is PickProperties<
    Sample,

      | "alpha"
      | "bravo"
      ...
  >;

Fix

printTemplateParameters already emits a softline + an indent level before each argument in a multi-argument list. printUnion was also emitting its own leading line + indent (+ align(2)), so the two stacked, producing the blank line and the extra indentation level.

printUnion now 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:

model Picked
  is PickProperties<
    Sample,
    | "alpha"
    | "bravo"
    ...
  >;

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

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 microsoft-github-policy-service Bot added compiler:core Issues for @typespec/compiler emitter:client:java Issue for the Java client emitter: @typespec/http-client-java labels Jun 17, 2026
@oha-4

oha-4 commented Jun 17, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@pkg-pr-new

pkg-pr-new Bot commented Jun 17, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/compiler@11010

commit: 4daeb1b

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

❌ There is undocummented changes. Run chronus add to add a changeset or click here.

The following packages have changes but are not documented.

  • @typespec/http-client-java

The following packages have already been documented:

  • @typespec/compiler
Show changes

@typespec/compiler - fix ✏️

Fix formatter inserting a blank line and over-indenting a union expression used directly as one of multiple template arguments (e.g. PickProperties<Source, "a" | "b">)

@timotheeguerin timotheeguerin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, I think there is also a regression this introduce in the formatting, i'll also need to see the effect this has on all of azure specs though it does seem like a good thing to fix

Comment thread packages/http-client-java/generator/http-client-generator-test/tsp/arm.tsp Outdated
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>
@oha-4

oha-4 commented Jun 17, 2026

Copy link
Copy Markdown
Author

Thanks for the careful review — you're right, fixed in 4daeb1b6b. The previous change dropped align(2) from the variants, which de-indented the nested template.

And to your question: this does follow prettier's union printer — indent-or-not depends on the parent (shouldIndent, union-type.js#L23-L38), but the per-variant align(2) is kept regardless (#L49-L56). I'd wrongly tied them together; now align(2) is unconditional, so a breaking variant stays aligned under its | , matching your screenshot. Added a regression test for it too.

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++))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:core Issues for @typespec/compiler emitter:client:java Issue for the Java client emitter: @typespec/http-client-java

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatter adds a blank line and over-indents a union used as a template argument

2 participants