Skip to content

Commit 482035c

Browse files
oha-4claudetimotheeguerin
authored
Fix formatter blank line and over-indent for union template argument (#11010)
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: ```tsp 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: ```tsp 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 - Added a regression test for the multi-argument case (#11009) and a test pinning the single-argument behavior in `formatter.test.ts`. - Reformatted two committed `http-client-java` test fixtures that contained the buggy output (whitespace only; no semantic change). - `pnpm prettier --check "**/*.tsp"`, formatter test suite, tsc, and eslint all pass. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
1 parent 4f8e319 commit 482035c

8 files changed

Lines changed: 108 additions & 18 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/compiler"
5+
---
6+
7+
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">`)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: internal
3+
packages:
4+
- "@typespec/http-client-java"
5+
---
6+
7+
Reformat union template arguments in test files to match updated formatter style.

packages/compiler/src/formatter/print/printer.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,14 +1427,30 @@ export function printUnion(
14271427
options: TypeSpecPrettierOptions,
14281428
print: PrettierChildPrint,
14291429
) {
1430-
const types = path.map((typePath) => {
1431-
const printedType: Doc = align(2, print(typePath));
1432-
return printedType;
1433-
}, "options");
1434-
1435-
const shouldAddStartLine = true;
1430+
// A union that is one of several template arguments must not add its own
1431+
// leading line + indent: the argument list already provides them, so stacking
1432+
// both yields a blank line and an extra indent level for the variants.
1433+
// The per-variant align(2) is always kept though (matching prettier's union
1434+
// printer): it accounts for the "| " prefix so a variant that breaks (e.g. a
1435+
// nested template) stays aligned under its content.
1436+
// https://github.com/microsoft/typespec/issues/11009
1437+
const inMultiTemplateArgumentList = isInMultiTemplateArgumentList(path);
1438+
const types = path.map((typePath) => align(2, print(typePath)), "options");
1439+
1440+
const shouldAddStartLine = !inMultiTemplateArgumentList;
14361441
const code = [ifBreak([shouldAddStartLine ? line : "", "| "], ""), join([line, "| "], types)];
1437-
return group(indent(code));
1442+
return inMultiTemplateArgumentList ? group(code) : group(indent(code));
1443+
}
1444+
1445+
/** Whether the node is a direct argument of a template reference with more than one argument. */
1446+
function isInMultiTemplateArgumentList(path: AstPath<Node>): boolean {
1447+
// A `TemplateArgument` only ever lives in `TypeReference.arguments`, so the
1448+
// owning `TypeReference` is always the next node ancestor.
1449+
if (path.getParentNode()?.kind !== SyntaxKind.TemplateArgument) {
1450+
return false;
1451+
}
1452+
const reference = path.getParentNode(1);
1453+
return reference?.kind === SyntaxKind.TypeReference && reference.arguments.length > 1;
14381454
}
14391455

14401456
export function printTypeReference(

packages/compiler/test/formatter/formatter.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1972,6 +1972,68 @@ union Foo {
19721972
@doc("third")
19731973
c: C,
19741974
}
1975+
`,
1976+
});
1977+
});
1978+
1979+
// Regression test for https://github.com/microsoft/typespec/issues/11009
1980+
it("does not add a blank line or extra indent for a union used as a template argument", async () => {
1981+
await assertFormat({
1982+
code: `
1983+
model Picked is PickProperties<Sample, "alpha" | "bravo" | "charlie" | "delta" | "echo" | "foxtrot" | "golf" | "hotel">;
1984+
`,
1985+
expected: `
1986+
model Picked is PickProperties<
1987+
Sample,
1988+
| "alpha"
1989+
| "bravo"
1990+
| "charlie"
1991+
| "delta"
1992+
| "echo"
1993+
| "foxtrot"
1994+
| "golf"
1995+
| "hotel"
1996+
>;
1997+
`,
1998+
});
1999+
});
2000+
2001+
it("keeps leading | and indent for a union used as the only template argument", async () => {
2002+
await assertFormat({
2003+
code: `
2004+
model Picked is PickProperties<"alpha" | "bravo" | "charlie" | "delta" | "echo" | "foxtrot" | "golf" | "hotel">;
2005+
`,
2006+
expected: `
2007+
model Picked
2008+
is PickProperties<
2009+
| "alpha"
2010+
| "bravo"
2011+
| "charlie"
2012+
| "delta"
2013+
| "echo"
2014+
| "foxtrot"
2015+
| "golf"
2016+
| "hotel">;
2017+
`,
2018+
});
2019+
});
2020+
2021+
// Regression test for https://github.com/microsoft/typespec/issues/11009
2022+
it("keeps the variant alignment so a nested template argument stays indented", async () => {
2023+
await assertFormat({
2024+
code: `
2025+
model Created is Operation<Request, Response | CreatedResponse<ResponseBodyModel, LroHeaders = AsyncOperationHeader<FinalResult = ResponseBodyModel>>, Error>;
2026+
`,
2027+
expected: `
2028+
model Created is Operation<
2029+
Request,
2030+
| Response
2031+
| CreatedResponse<
2032+
ResponseBodyModel,
2033+
LroHeaders = AsyncOperationHeader<FinalResult = ResponseBodyModel>
2034+
>,
2035+
Error
2036+
>;
19752037
`,
19762038
});
19772039
});

packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/azure-resourcemanager-armresourceprovider-generated_metadata.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/tsptest-response_metadata.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

packages/http-client-java/generator/http-client-generator-test/tsp/arm.tsp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -389,12 +389,11 @@ interface ImmutableResourceModel {
389389
Azure.ResourceManager.Foundations.DefaultBaseParameters<NginxConfigurationResponse>
390390
>,
391391
NginxConfigurationRequest,
392-
393-
| NginxConfigurationResponse
394-
| ArmResourceCreatedResponse<
395-
NginxConfigurationResponse,
396-
LroHeaders = ArmAsyncOperationHeader<FinalResult = NginxConfigurationResponse>
397-
>,
392+
| NginxConfigurationResponse
393+
| ArmResourceCreatedResponse<
394+
NginxConfigurationResponse,
395+
LroHeaders = ArmAsyncOperationHeader<FinalResult = NginxConfigurationResponse>
396+
>,
398397
ErrorResponse,
399398
OptionalRequestBody = true
400399
>;

packages/http-client-java/generator/http-client-generator-test/tsp/response.tsp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@ op RpcOperationWithAdditionalResponse<
2626
TErrorResponse = Azure.Core.Foundations.ErrorResponse
2727
> is Foundations.Operation<
2828
TParams & Azure.Core.Traits.Private.TraitProperties<Traits, TraitLocation.Parameters>,
29-
30-
| (TResponse & Azure.Core.Traits.Private.TraitProperties<Traits, TraitLocation.Response>)
31-
| TAdditionalResponse,
29+
| (TResponse & Azure.Core.Traits.Private.TraitProperties<Traits, TraitLocation.Response>)
30+
| TAdditionalResponse,
3231
Traits,
3332
TErrorResponse
3433
>;

0 commit comments

Comments
 (0)