Skip to content

Commit dbaa166

Browse files
JavaScript: Add JS.Spread for spread and rest syntax (#6436)
* JavaScript: Add `JS.Spread` for spread syntax This PR introduces a dedicated JS.Spread AST element to properly represent spread syntax in JavaScript/TypeScript, distinguishing it from rest syntax which continues to use the Spread marker. ### What's Covered: Spread Syntax The JS.Spread AST element now represents spread expressions that expand iterables or objects: Array spread: ```js const arr = [1, 2, 3]; const expanded = [...arr, 4, 5]; // Spread: expands arr into individual elements ``` Object spread: ```js const obj = { a: 1 }; const merged = { ...obj, b: 2 }; // Spread: expands obj properties ``` Function call spread: ```js const args = [1, 2, 3]; fn(...args); // Spread: expands args as individual arguments ``` ### What's NOT Covered: Rest Syntax Rest syntax (collecting elements into a single binding) continues to use the Spread marker since it's fundamentally different - it's a destructuring/binding pattern, not an expression operation: Function parameter rest: ```js function fn(first, ...rest) {} // Rest: collects remaining args into array ``` Array destructuring rest: ```js const [first, ...rest] = arr; // Rest: collects remaining elements ``` Object destructuring rest: ```js const { a, ...rest } = obj; // Rest: collects remaining properties ``` ### Why the Distinction Matters - Type semantics: Spread expressions have different types than the original (...arr produces individual elements, not an array) - AST ownership: The ... prefix now lives on the JS.Spread element itself, following the rule that whitespace/prefix belongs on the outermost element - Visitor pattern: Enables targeted transformations of spread expressions without affecting rest patterns * Polish * Also use `JS.Spread` for rest collection syntax * Revert accidental change
1 parent 3ff7488 commit dbaa166

18 files changed

Lines changed: 260 additions & 162 deletions

File tree

rewrite-javascript/rewrite/src/javascript/comparator.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,9 +852,20 @@ export class JavaScriptComparatorVisitor extends JavaScriptVisitor<J> {
852852
return this.visitElement(shebang, other as JS.Shebang);
853853
}
854854

855+
/**
856+
* Overrides the visitSpread method to compare spread expressions.
857+
*
858+
* @param spread The spread expression to visit
859+
* @param other The other spread expression to compare with
860+
* @returns The visited spread expression, or undefined if the visit was aborted
861+
*/
862+
override async visitSpread(spread: JS.Spread, other: J): Promise<J | undefined> {
863+
return this.visitElement(spread, other as JS.Spread);
864+
}
865+
855866
/**
856867
* Overrides the visitStatementExpression method to compare statement expressions.
857-
*
868+
*
858869
* @param statementExpression The statement expression to visit
859870
* @param other The other statement expression to compare with
860871
* @returns The visited statement expression, or undefined if the visit was aborted

rewrite-javascript/rewrite/src/javascript/format/format.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,8 @@ export class SpacesVisitor<P> extends JavaScriptVisitor<P> {
238238
// Apply afterComma rule to elements after the first
239239
for (let i = 1; i < draft.elements.length; i++) {
240240
const currentWs = draft.elements[i].element.prefix.whitespace;
241-
// Also check for Spread marker with newline (for spread elements the newline is in the marker)
242-
const element = draft.elements[i].element as J;
243-
const spreadMarker = element.markers?.markers?.find(m => m.kind === JS.Markers.Spread) as { prefix: J.Space } | undefined;
244-
const hasNewlineInSpread = spreadMarker?.prefix?.whitespace?.includes("\n");
245241
// Preserve original newlines - only adjust spacing when elements are on same line
246-
if (!currentWs.includes("\n") && !hasNewlineInSpread) {
242+
if (!currentWs.includes("\n")) {
247243
draft.elements[i].element.prefix.whitespace = this.style.other.afterComma ? " " : "";
248244
}
249245
}

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

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,6 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
214214
if (tree.prefix && lastWhitespace(tree.prefix).includes("\n")) {
215215
return true;
216216
}
217-
// For elements with Spread marker, check the Spread marker's prefix
218-
const spreadMarker = tree.markers?.markers?.find(m => m.kind === JS.Markers.Spread) as { prefix: J.Space } | undefined;
219-
if (spreadMarker && spaceContainsNewline(spreadMarker.prefix)) {
220-
return true;
221-
}
222217
return false;
223218
}
224219

@@ -305,20 +300,8 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
305300
let result = tree;
306301
const indentStr = this.indentString(myIndent);
307302

308-
// Check if the element has a Spread marker - if so, normalize its prefix instead
309-
const spreadMarker = result.markers?.markers?.find(m => m.kind === JS.Markers.Spread) as { prefix: J.Space } | undefined;
310-
if (spreadMarker && spaceContainsNewline(spreadMarker.prefix)) {
311-
const normalizedPrefix = normalizeSpaceIndent(spreadMarker.prefix, indentStr);
312-
if (normalizedPrefix !== spreadMarker.prefix) {
313-
result = produce(result, draft => {
314-
const spreadIdx = draft.markers.markers.findIndex(m => m.kind === JS.Markers.Spread);
315-
if (spreadIdx !== -1) {
316-
(draft.markers.markers[spreadIdx] as any).prefix = normalizedPrefix;
317-
}
318-
});
319-
}
320-
} else if (result.prefix && spaceContainsNewline(result.prefix)) {
321-
// Normalize the entire prefix space including comment suffixes
303+
// Normalize the prefix space including comment suffixes
304+
if (result.prefix && spaceContainsNewline(result.prefix)) {
322305
const normalizedPrefix = normalizeSpaceIndent(result.prefix, indentStr);
323306
if (normalizedPrefix !== result.prefix) {
324307
result = produce(result, draft => {
@@ -617,19 +600,19 @@ export class TabsAndIndentsVisitor<P> extends JavaScriptVisitor<P> {
617600
}
618601

619602
/**
620-
* Check if an element is a spread - either directly marked with Spread marker,
621-
* or a PropertyAssignment whose name has a Spread marker (for object spreads).
603+
* Check if an element is a spread - either a JS.Spread element
604+
* or a PropertyAssignment wrapping a spread.
622605
*/
623606
private isSpreadElement(element: J): boolean {
624-
// Direct spread marker on element
625-
if (findMarker(element, JS.Markers.Spread)) {
607+
// JS.Spread AST element (for spread/rest expressions like `...arr`, `...obj`, `...args`)
608+
if (element.kind === JS.Kind.Spread) {
626609
return true;
627610
}
628611
// PropertyAssignment wrapping a spread (for object spread like `...obj`)
629612
if (element.kind === JS.Kind.PropertyAssignment) {
630613
const propAssign = element as JS.PropertyAssignment;
631614
const nameElement = propAssign.name?.element;
632-
if (findMarker(nameElement, JS.Markers.Spread)) {
615+
if (nameElement?.kind === JS.Kind.Spread) {
633616
return true;
634617
}
635618
}

rewrite-javascript/rewrite/src/javascript/markers.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ declare module "./tree" {
3535
readonly Generator: "org.openrewrite.javascript.marker.Generator";
3636
readonly Optional: "org.openrewrite.javascript.marker.Optional";
3737
readonly NonNullAssertion: "org.openrewrite.javascript.marker.NonNullAssertion";
38-
readonly Spread: "org.openrewrite.javascript.marker.Spread";
3938
readonly DelegatedYield: "org.openrewrite.javascript.marker.DelegatedYield";
4039
readonly FunctionDeclaration: "org.openrewrite.javascript.marker.FunctionDeclaration";
4140
};
@@ -48,7 +47,6 @@ declare module "./tree" {
4847
DelegatedYield: "org.openrewrite.javascript.marker.DelegatedYield",
4948
Optional: "org.openrewrite.javascript.marker.Optional",
5049
NonNullAssertion: "org.openrewrite.javascript.marker.NonNullAssertion",
51-
Spread: "org.openrewrite.javascript.marker.Spread",
5250
FunctionDeclaration: "org.openrewrite.javascript.marker.FunctionDeclaration",
5351
} as const;
5452

@@ -77,11 +75,6 @@ export interface NonNullAssertion extends Marker {
7775
readonly prefix: J.Space;
7876
}
7977

80-
export interface Spread extends Marker {
81-
readonly kind: typeof JS.Markers.Spread;
82-
readonly prefix: J.Space;
83-
}
84-
8578
export interface FunctionDeclaration extends Marker {
8679
readonly kind: typeof JS.Markers.FunctionDeclaration;
8780
readonly prefix: J.Space;
@@ -112,7 +105,6 @@ registerPrefixedMarkerCodec<DelegatedYield>(JS.Markers.DelegatedYield);
112105
registerPrefixedMarkerCodec<Optional>(JS.Markers.Optional);
113106
registerPrefixedMarkerCodec<Generator>(JS.Markers.Generator);
114107
registerPrefixedMarkerCodec<NonNullAssertion>(JS.Markers.NonNullAssertion);
115-
registerPrefixedMarkerCodec<Spread>(JS.Markers.Spread);
116108
registerPrefixedMarkerCodec<FunctionDeclaration>(JS.Markers.FunctionDeclaration);
117109

118110
// Register codec for PrettierStyle (a NamedStyles that contains Prettier configuration)

rewrite-javascript/rewrite/src/javascript/parser.ts

Lines changed: 84 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
TypeTree,
3030
VariableDeclarator,
3131
} from '../java';
32-
import {DelegatedYield, FunctionDeclaration, Generator, JS, JSX, NonNullAssertion, Optional, Spread} from '.';
32+
import {DelegatedYield, FunctionDeclaration, Generator, JS, JSX, NonNullAssertion, Optional} from '.';
3333
import {emptyMarkers, markers, Markers, MarkersKind, ParseExceptionResult, replaceMarkerByKind} from "../markers";
3434
import {NamedStyles} from "../style";
3535
import {Parser, ParserInput, parserInputFile, parserInputRead, ParserOptions, Parsers, SourcePath} from "../parser";
@@ -1028,6 +1028,21 @@ export class JavaScriptParserVisitor {
10281028
}
10291029

10301030
visitParameter(node: ts.ParameterDeclaration): J.VariableDeclarations {
1031+
let name: VariableDeclarator = produce(this.convert<VariableDeclarator>(node.name), draft => {
1032+
draft.markers = this.maybeAddOptionalMarker(draft, node);
1033+
});
1034+
if (node.dotDotDotToken) {
1035+
// Wrap in JS.Spread; prefix between ... and identifier stays on the inner expression
1036+
const spread: JS.Spread = {
1037+
kind: JS.Kind.Spread,
1038+
id: randomId(),
1039+
prefix: emptySpace,
1040+
markers: emptyMarkers,
1041+
expression: name as Expression,
1042+
type: this.mapType(node)
1043+
};
1044+
name = spread;
1045+
}
10311046
return {
10321047
kind: J.Kind.VariableDeclarations,
10331048
id: randomId(),
@@ -1042,16 +1057,7 @@ export class JavaScriptParserVisitor {
10421057
id: randomId(),
10431058
prefix: node.dotDotDotToken ? this.prefix(node.dotDotDotToken) : this.prefix(node.name),
10441059
markers: emptyMarkers,
1045-
name: produce(this.convert<VariableDeclarator>(node.name), draft => {
1046-
draft.markers = this.maybeAddOptionalMarker(draft, node);
1047-
if (node.dotDotDotToken) {
1048-
draft.markers.markers.push({
1049-
kind: JS.Markers.Spread,
1050-
id: randomId(),
1051-
prefix: this.prefix(node.name)
1052-
} satisfies Spread as Spread);
1053-
}
1054-
}),
1060+
name: name,
10551061
dimensionsAfterName: [],
10561062
initializer: node.initializer && this.leftPadded(this.prefix(node.getChildAt(node.getChildren().indexOf(node.initializer) - 1)), this.visit(node.initializer)),
10571063
variableType: this.mapVariableType(node)
@@ -1579,14 +1585,16 @@ export class JavaScriptParserVisitor {
15791585
});
15801586
}
15811587

1582-
visitRestType(node: ts.RestTypeNode): Expression {
1583-
return produce(this.convert<Expression>(node.type), draft => {
1584-
draft.markers.markers.push({
1585-
kind: JS.Markers.Spread,
1586-
id: randomId(),
1587-
prefix: this.prefix(node)
1588-
} satisfies Spread as Spread);
1589-
});
1588+
visitRestType(node: ts.RestTypeNode): JS.Spread {
1589+
const innerExpr = this.convert<Expression>(node.type);
1590+
return {
1591+
kind: JS.Kind.Spread,
1592+
id: randomId(),
1593+
prefix: this.prefix(node),
1594+
markers: emptyMarkers,
1595+
expression: {...innerExpr, prefix: emptySpace},
1596+
type: this.mapType(node)
1597+
};
15901598
}
15911599

15921600
visitUnionType(node: ts.UnionTypeNode): JS.Union {
@@ -1818,6 +1826,22 @@ export class JavaScriptParserVisitor {
18181826
// and this would potentially just trip up flow analyses. The names exist purely for documentation purposes,
18191827
// they has no semantics. See https://stackoverflow.com/questions/63629315/what-are-named-or-labeled-tuples-in-typescript.
18201828
visitNamedTupleMember(node: ts.NamedTupleMember): J.VariableDeclarations {
1829+
let name: VariableDeclarator = produce(this.convert<J.Identifier>(node.name), draft => {
1830+
draft.markers = this.maybeAddOptionalMarker(draft, node);
1831+
});
1832+
if (node.dotDotDotToken) {
1833+
// Wrap in JS.Spread; use emptySpace since the whitespace belongs
1834+
// to VariableDeclarations.prefix, not to the spread itself
1835+
const spread: JS.Spread = {
1836+
kind: JS.Kind.Spread,
1837+
id: randomId(),
1838+
prefix: emptySpace,
1839+
markers: emptyMarkers,
1840+
expression: name as Expression,
1841+
type: this.mapType(node)
1842+
};
1843+
name = spread;
1844+
}
18211845
return {
18221846
kind: J.Kind.VariableDeclarations,
18231847
id: randomId(),
@@ -1832,16 +1856,7 @@ export class JavaScriptParserVisitor {
18321856
id: randomId(),
18331857
prefix: emptySpace,
18341858
markers: emptyMarkers,
1835-
name: produce(this.convert<J.Identifier>(node.name), draft => {
1836-
draft.markers = this.maybeAddOptionalMarker(draft, node);
1837-
if (node.dotDotDotToken) {
1838-
draft.markers.markers.push({
1839-
kind: JS.Markers.Spread,
1840-
id: randomId(),
1841-
prefix: this.prefix(node.dotDotDotToken)
1842-
} satisfies Spread as Spread);
1843-
}
1844-
}),
1859+
name: name,
18451860
dimensionsAfterName: [],
18461861
variableType: this.mapVariableType(node),
18471862
},
@@ -1937,21 +1952,28 @@ export class JavaScriptParserVisitor {
19371952
}
19381953

19391954
visitBindingElement(node: ts.BindingElement): JS.BindingElement {
1955+
// Capture prefix before converting name, as convert may consume whitespace
1956+
const elementPrefix = this.prefix(node);
1957+
let name: Expression = this.convert<Expression>(node.name);
1958+
if (node.dotDotDotToken) {
1959+
// Wrap in JS.Spread; use emptySpace for prefix since the whitespace
1960+
// belongs to BindingElement.prefix, not to the spread itself
1961+
name = {
1962+
kind: JS.Kind.Spread,
1963+
id: randomId(),
1964+
prefix: emptySpace,
1965+
markers: emptyMarkers,
1966+
expression: name,
1967+
type: this.mapType(node)
1968+
} satisfies JS.Spread as Expression;
1969+
}
19401970
return {
19411971
kind: JS.Kind.BindingElement,
19421972
id: randomId(),
1943-
prefix: this.prefix(node),
1973+
prefix: elementPrefix,
19441974
markers: emptyMarkers,
19451975
propertyName: node.propertyName && this.rightPadded(this.convert<J.Identifier>(node.propertyName), this.suffix(node.propertyName)),
1946-
name: produce(this.convert<Expression>(node.name), draft => {
1947-
if (node.dotDotDotToken) {
1948-
draft.markers.markers.push({
1949-
kind: JS.Markers.Spread,
1950-
id: randomId(),
1951-
prefix: this.prefix(node.dotDotDotToken)
1952-
} satisfies Spread as Spread);
1953-
}
1954-
}),
1976+
name: name,
19551977
initializer: node.initializer && this.leftPadded(this.prefix(this.findChildNode(node, ts.SyntaxKind.EqualsToken)!), this.convert<Expression>(node.initializer)),
19561978
variableType: this.mapVariableType(node)
19571979
};
@@ -2622,14 +2644,15 @@ export class JavaScriptParserVisitor {
26222644
};
26232645
}
26242646

2625-
visitSpreadElement(node: ts.SpreadElement): Expression {
2626-
return produce(this.convert<Expression>(node.expression), draft => {
2627-
draft.markers.markers.push({
2628-
kind: JS.Markers.Spread,
2629-
id: randomId(),
2630-
prefix: this.prefix(node)
2631-
} satisfies Spread as Spread);
2632-
});
2647+
visitSpreadElement(node: ts.SpreadElement): JS.Spread {
2648+
return {
2649+
kind: JS.Kind.Spread,
2650+
id: randomId(),
2651+
prefix: this.prefix(node),
2652+
markers: emptyMarkers,
2653+
expression: this.convert(node.expression),
2654+
type: this.mapType(node)
2655+
};
26332656
}
26342657

26352658
visitClassExpression(node: ts.ClassExpression): JS.StatementExpression {
@@ -3903,13 +3926,16 @@ export class JavaScriptParserVisitor {
39033926
let expr: Expression;
39043927
if (node.expression) {
39053928
if (node.dotDotDotToken) {
3906-
expr = produce(this.convert<Expression>(node.expression), draft => {
3907-
draft.markers.markers.push({
3908-
kind: JS.Markers.Spread,
3909-
id: randomId(),
3910-
prefix: this.prefix(node.dotDotDotToken!)
3911-
} satisfies Spread as Spread);
3912-
});
3929+
// Wrap in JS.Spread
3930+
const innerExpr = this.convert<Expression>(node.expression);
3931+
expr = {
3932+
kind: JS.Kind.Spread,
3933+
id: randomId(),
3934+
prefix: this.prefix(node.dotDotDotToken!),
3935+
markers: emptyMarkers,
3936+
expression: innerExpr,
3937+
type: this.mapType(node.expression)
3938+
} satisfies JS.Spread as Expression;
39133939
} else {
39143940
expr = this.convert<Expression>(node.expression);
39153941
}
@@ -4086,23 +4112,14 @@ export class JavaScriptParserVisitor {
40864112
};
40874113
}
40884114

4089-
visitSpreadAssignment(node: ts.SpreadAssignment): JS.PropertyAssignment {
4115+
visitSpreadAssignment(node: ts.SpreadAssignment): JS.Spread {
40904116
return {
4091-
kind: JS.Kind.PropertyAssignment,
4117+
kind: JS.Kind.Spread,
40924118
id: randomId(),
40934119
prefix: this.prefix(node),
40944120
markers: emptyMarkers,
4095-
name: this.rightPadded(
4096-
produce(this.convert<Expression>(node.expression), draft => {
4097-
draft.markers.markers.push({
4098-
kind: JS.Markers.Spread,
4099-
id: randomId(),
4100-
prefix: this.prefix(node)
4101-
} satisfies Spread as Spread);
4102-
}),
4103-
this.suffix(node.expression)
4104-
),
4105-
assigmentToken: JS.PropertyAssignment.Token.Empty,
4121+
expression: this.convert(node.expression),
4122+
type: this.mapType(node)
41064123
};
41074124
}
41084125

rewrite-javascript/rewrite/src/javascript/print.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {PrintOutputCapture, TreePrinters} from "../print";
2121
import {Cursor, isTree, Tree} from "../tree";
2222
import {Comment, emptySpace, J, Statement, TextComment, TrailingComma, TypedTree} from "../java";
2323
import {findMarker, Marker, Markers} from "../markers";
24-
import {DelegatedYield, FunctionDeclaration, Generator, NonNullAssertion, Optional, Spread} from "./markers";
24+
import {DelegatedYield, FunctionDeclaration, Generator, NonNullAssertion, Optional} from "./markers";
2525

2626
export class JavaScriptPrinter extends JavaScriptVisitor<PrintOutputCapture> {
2727

@@ -88,6 +88,14 @@ export class JavaScriptPrinter extends JavaScriptVisitor<PrintOutputCapture> {
8888
return statementExpression;
8989
}
9090

91+
override async visitSpread(spread: JS.Spread, p: PrintOutputCapture): Promise<J | undefined> {
92+
await this.beforeSyntax(spread, p);
93+
p.append("...");
94+
await this.visit(spread.expression, p);
95+
await this.afterSyntax(spread, p);
96+
return spread;
97+
}
98+
9199
override async visitInferType(inferType: JS.InferType, p: PrintOutputCapture): Promise<J | undefined> {
92100
await this.beforeSyntax(inferType, p);
93101
await this.visitLeftPaddedLocal("infer", inferType.typeParameter, p);
@@ -1901,12 +1909,7 @@ export class JavaScriptPrinter extends JavaScriptVisitor<PrintOutputCapture> {
19011909
}
19021910

19031911
protected async preVisit(tree: J, p: PrintOutputCapture): Promise<J | undefined> {
1904-
for (const marker of tree.markers.markers) {
1905-
if (marker.kind === JS.Markers.Spread) {
1906-
await this.visitSpace((marker as Spread).prefix, p);
1907-
p.append("...");
1908-
}
1909-
}
1912+
// Note: Spread is now handled as JS.Spread AST element via visitSpread
19101913
return tree;
19111914
}
19121915

0 commit comments

Comments
 (0)