Skip to content

Commit df575d1

Browse files
committed
JavaScript: More formatter fixes
1 parent ffa10ae commit df575d1

2 files changed

Lines changed: 89 additions & 14 deletions

File tree

rewrite-javascript/rewrite/src/javascript/tabs-and-indents-visitor.ts

Lines changed: 68 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,31 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
6363
const mi = tree as J.MethodInvocation;
6464
if (mi.select && mi.select.after.whitespace.includes("\n")) {
6565
// This MethodInvocation has a chained method call after it
66-
// Store the ORIGINAL parent indent context in "chainedIndentContext"
66+
// Store the BASE indent context in "chainedIndentContext"
6767
// This will be propagated down and used when we reach the chain's innermost element
68-
const parentContext = this.getParentIndentContext(cursor);
69-
cursor.messages.set("chainedIndentContext", parentContext);
70-
// For children (arguments), use continuation indent
71-
// But the prefix will be normalized in postVisit using chainedIndentContext
72-
const [parentIndent, parentIndentKind] = parentContext;
73-
cursor.messages.set("indentContext", [parentIndent + this.indentSize, parentIndentKind] as IndentContext);
68+
const [parentIndent, parentIndentKind] = this.getParentIndentContext(cursor);
69+
70+
// If this chain-start itself is on a new line and we're in continuation context,
71+
// the base indent includes continuation.
72+
// BUT if parentIndentKind is 'align', we're likely in a Block child context where
73+
// the parent already set the correct indent - don't add extra continuation.
74+
const prefixHasNewline = this.prefixContainsNewline(tree);
75+
let baseIndent = parentIndent;
76+
if (prefixHasNewline && parentIndentKind !== 'align') {
77+
baseIndent += this.indentSize;
78+
}
79+
80+
// If we're inside a Lambda expression body that's inside a Container (like method arguments),
81+
// add another continuation for the Container context
82+
const isLambdaBodyContainer = this.isLambdaBodyInsideContainer(cursor);
83+
if (prefixHasNewline && isLambdaBodyContainer) {
84+
baseIndent += this.indentSize;
85+
}
86+
87+
cursor.messages.set("chainedIndentContext", [baseIndent, parentIndentKind] as IndentContext);
88+
// For children (arguments), use continuation indent from base
89+
cursor.messages.set("indentContext", [baseIndent + this.indentSize, parentIndentKind] as IndentContext);
90+
7491
return;
7592
}
7693
// Check if we're at the base of a chain (no select) and parent has chainedIndentContext
@@ -122,11 +139,23 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
122139
private getParentIndentContext(cursor: Cursor): IndentContext {
123140
// Walk up the cursor chain to find the nearest indent context
124141
// We need to walk because intermediate nodes like RightPadded may not have context set
142+
let passedScopeBoundary = false;
143+
125144
for (let c = cursor.parent; c != null; c = c.parent) {
145+
// Check if we're passing a scope boundary (Lambda creates a new scope)
146+
// After crossing a Lambda, we should ignore chainedIndentContext from outer scopes
147+
const nodeKind = (c.value as any)?.kind;
148+
if (nodeKind === J.Kind.Lambda) {
149+
passedScopeBoundary = true;
150+
}
151+
126152
// chainedIndentContext stores the original context - prefer it
127-
const chainedContext = c.messages.get("chainedIndentContext") as IndentContext | undefined;
128-
if (chainedContext !== undefined) {
129-
return chainedContext;
153+
// BUT only if we haven't crossed a scope boundary (like Lambda)
154+
if (!passedScopeBoundary) {
155+
const chainedContext = c.messages.get("chainedIndentContext") as IndentContext | undefined;
156+
if (chainedContext !== undefined) {
157+
return chainedContext;
158+
}
130159
}
131160

132161
const context = c.messages.get("indentContext") as IndentContext | undefined;
@@ -219,6 +248,28 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
219248
}
220249
}
221250

251+
private isLambdaBodyInsideContainer(cursor: Cursor): boolean {
252+
// Check if we're the DIRECT Lambda body (not inside a Block that's the Lambda body)
253+
// AND that Lambda is inside a Container (method arguments)
254+
// Walk up to find Lambda, but stop if we hit a Block (meaning we're inside a block body, not expression body)
255+
let foundLambda = false;
256+
for (let c = cursor.parent; c != null; c = c.parent) {
257+
const kind = (c.value as any)?.kind;
258+
// If we hit a Block before Lambda, we're inside the block body, not expression body
259+
if (!foundLambda && kind === J.Kind.Block) {
260+
return false;
261+
}
262+
if (kind === J.Kind.Lambda) {
263+
foundLambda = true;
264+
}
265+
// After finding Lambda, check for Container
266+
if (foundLambda && kind && kind === J.Kind.Container) {
267+
return true;
268+
}
269+
}
270+
return false;
271+
}
272+
222273
override async postVisit(tree: J, _p: P): Promise<J | undefined> {
223274
if (this.stopAfter != null && isScope(this.stopAfter, tree)) {
224275
this.cursor?.root.messages.set("stop", true);
@@ -241,12 +292,12 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
241292
let [myIndent] = indentContext;
242293

243294
// For chain-start MethodInvocations, the prefix contains whitespace before the chain BASE
244-
// Use chainedIndentContext (the original indent) for the prefix, not the continuation indent
295+
// Use chainedIndentContext for the prefix - it already accounts for newlines
245296
const chainedContext = this.cursor.messages.get("chainedIndentContext") as IndentContext | undefined;
246297
if (chainedContext !== undefined && tree.kind === J.Kind.MethodInvocation) {
247298
const mi = tree as J.MethodInvocation;
248299
if (mi.select && mi.select.after.whitespace.includes("\n")) {
249-
// This is a chain-start - use original indent for prefix normalization
300+
// This is a chain-start - use the base indent from chainedIndentContext
250301
myIndent = chainedContext[0];
251302
}
252303
}
@@ -521,16 +572,19 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
521572

522573
// Check if parent has chainedIndentContext - if so, this is the select of a method chain
523574
// Propagate chainedIndentContext but do NOT set indentContext
575+
// EXCEPTION: Do NOT propagate into Lambda bodies - arrow functions create a new scope
524576
const parentChainedContext = this.cursor.parent?.messages.get("chainedIndentContext") as IndentContext | undefined;
525-
if (parentChainedContext !== undefined) {
577+
const elementKind = (right.element as any)?.kind;
578+
const isLambdaBody = elementKind === J.Kind.Lambda;
579+
580+
if (parentChainedContext !== undefined && !isLambdaBody) {
526581
this.cursor.messages.set("chainedIndentContext", parentChainedContext);
527582
// Do NOT set indentContext - child elements will use chainedIndentContext
528583
return;
529584
}
530585

531586
// Check if Parentheses wraps a Binary expression - if so, let Binary handle its own indent
532587
const rightPaddedParentKind = this.cursor.parent?.value?.kind;
533-
const elementKind = (right.element as any)?.kind;
534588
const isParenthesesWrappingBinary = rightPaddedParentKind === J.Kind.Parentheses &&
535589
elementKind === J.Kind.Binary;
536590

rewrite-javascript/rewrite/test/javascript/format/tabs-and-indents-visitor.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,4 +1313,25 @@ const { data, error } = await (
13131313
// @formatter:on
13141314
)
13151315
})
1316+
1317+
test('ternary with arrow function and method chain should preserve indentation', () => {
1318+
const spec = new RecipeSpec();
1319+
spec.recipe = fromVisitor(new TabsAndIndentsVisitor(tabsAndIndents(draft => {
1320+
draft.indentSize = 2;
1321+
draft.continuationIndent = 2;
1322+
})));
1323+
return spec.rewriteRun(
1324+
// @formatter:off
1325+
//language=typescript
1326+
typescript(
1327+
`const filteredDependencies = dependencies
1328+
? dependencies.map((dep: string) =>
1329+
dep
1330+
.split("\\n"),
1331+
)
1332+
: [];`
1333+
)
1334+
// @formatter:on
1335+
)
1336+
})
13161337
});

0 commit comments

Comments
 (0)