Skip to content

Commit a6a8b25

Browse files
knutwannhedenknutwannheden
authored
JS RPC: fix non-FullyQualified type on enum declarations and harden parseProject (#7425)
A string or numeric enum was parsed with `mapType(enumDecl)`, which calls `getTypeAtLocation`. TypeScript resolves that to the union of enum literals — for a string enum this ends up as `Type.Primitive.String`. The Java-side RPC receiver then failed with "A class can only be type attributed with a fully qualified type name" in `J.ClassDeclaration.withType`, aborting the entire stream of parsed source files. Resolve class/interface/enum/class-expression declaration types via the declaration's symbol (`getDeclaredTypeOfSymbol` for classes/interfaces; a minimal class shell with the declaration's FQN and `classKind` for enums, since TypeScript has no class-shaped enum type). The result is always `FullyQualified`. Also harden `parseProject`: wrap the per-file `getObject` in a try/catch so a single failing file becomes a `ParseError` instead of killing the stream, and add `sourcePath` to `ParseProjectResponse.Item` so those errors can point at the offending file. Fixes moderneinc/customer-requests#2234
1 parent cde569b commit a6a8b25

6 files changed

Lines changed: 198 additions & 39 deletions

File tree

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ export class JavaScriptParserVisitor {
733733
)),
734734
end: this.prefix(node.getLastToken()!)
735735
},
736-
type: this.mapType(node)
736+
type: this.mapDeclarationType(node)
737737
};
738738
}
739739

@@ -2723,7 +2723,7 @@ export class JavaScriptParserVisitor {
27232723
})),
27242724
end: this.prefix(node.getLastToken()!)
27252725
},
2726-
type: this.mapType(node)
2726+
type: this.mapDeclarationType(node)
27272727
} satisfies J.ClassDeclaration as J.ClassDeclaration,
27282728
}
27292729
}
@@ -3414,7 +3414,7 @@ export class JavaScriptParserVisitor {
34143414
})),
34153415
end: this.prefix(node.getLastToken()!)
34163416
},
3417-
type: this.mapType(node)
3417+
type: this.mapDeclarationType(node)
34183418
};
34193419
}
34203420

@@ -3475,7 +3475,7 @@ export class JavaScriptParserVisitor {
34753475
emptySpace)],
34763476
end: this.prefix(node.getLastToken()!)
34773477
},
3478-
type: this.mapType(node) as Type.Class
3478+
type: this.mapDeclarationType(node) as Type.Class
34793479
};
34803480
}
34813481

@@ -4427,6 +4427,10 @@ export class JavaScriptParserVisitor {
44274427
return this.typeMapping?.type(node);
44284428
}
44294429

4430+
private mapDeclarationType(node: ts.ClassDeclaration | ts.ClassExpression | ts.InterfaceDeclaration | ts.EnumDeclaration): Type.FullyQualified | undefined {
4431+
return this.typeMapping?.declarationType(node);
4432+
}
4433+
44304434
private mapPrimitiveType(node: ts.Node): Type.Primitive {
44314435
return this.typeMapping?.primitiveType(node) ?? Type.Primitive.None;
44324436
}

‎rewrite-javascript/rewrite/src/javascript/type-mapping.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,86 @@ export class JavaScriptTypeMapping {
8383
return type && this.getType(type);
8484
}
8585

86+
/**
87+
* Resolve the declared type of a class / interface / enum / class-expression.
88+
*
89+
* Using {@code getTypeAtLocation} on these declaration nodes is unsafe: TypeScript returns
90+
* the type of the declared *value*, not the type itself. For string or numeric enums that
91+
* ends up being the union of enum literals, which on the JS side resolves to
92+
* {@link Type.Primitive} (e.g. String for string enums). The Java-side RPC receiver then
93+
* rejects it with "A class can only be type attributed with a fully qualified type name",
94+
* because {@code J.ClassDeclaration.type} must be {@link Type.FullyQualified}.
95+
*
96+
* Instead, we resolve through the declaration's own symbol. For classes and interfaces we
97+
* reuse the full type mapping pipeline (so members, methods, supertypes and type parameters
98+
* are populated). For enums — which have no class-shaped representation in TypeScript — and
99+
* any residual non-FQ result, we build a minimal class shell whose FQN and {@code classKind}
100+
* are derived from the declaration itself.
101+
*/
102+
declarationType(node: ts.ClassDeclaration | ts.ClassExpression | ts.InterfaceDeclaration | ts.EnumDeclaration): Type.FullyQualified | undefined {
103+
const symbol = (node as { symbol?: ts.Symbol }).symbol
104+
?? (node.name ? this.checker.getSymbolAtLocation(node.name) : undefined);
105+
if (!symbol) {
106+
return undefined;
107+
}
108+
109+
// For classes and interfaces `getDeclaredTypeOfSymbol` yields a class-shaped ts.Type whose
110+
// symbol we can feed through the normal pipeline. For enums, however, TypeScript returns the
111+
// union of enum literals — which on the JS side resolves to `Type.Primitive` (e.g. String for
112+
// string enums) and therefore is NOT `FullyQualified`. The Java side then rejects it with
113+
// "A class can only be type attributed with a fully qualified type name".
114+
//
115+
// Since JavaScript/TypeScript has no separate enum runtime representation, we build the
116+
// class-shaped type directly from the declaration's own symbol. The `classKind` is derived
117+
// from the declaration node kind, not the (lossy) resolved type.
118+
let classKind: Type.Class.Kind;
119+
if (ts.isEnumDeclaration(node)) {
120+
classKind = Type.Class.Kind.Enum;
121+
} else if (ts.isInterfaceDeclaration(node)) {
122+
classKind = Type.Class.Kind.Interface;
123+
} else {
124+
classKind = Type.Class.Kind.Class;
125+
}
126+
127+
const fullyQualifiedName = this.getFullyQualifiedNameFromSymbol(symbol);
128+
const cacheKey = `decl:${classKind}:${fullyQualifiedName}`;
129+
const cached = this.typeCache.get(cacheKey);
130+
if (cached && Type.isFullyQualified(cached)) {
131+
return cached as Type.FullyQualified;
132+
}
133+
134+
// For classes and interfaces reuse the existing type-based path so that members, methods,
135+
// supertypes and type parameters are populated. Enums (and any fallback that produced a
136+
// non-FQ result) get a minimal class shell with the correct FQN + kind.
137+
if (!ts.isEnumDeclaration(node)) {
138+
const declaredType = this.checker.getDeclaredTypeOfSymbol(symbol);
139+
if (declaredType) {
140+
const mapped = this.getType(declaredType);
141+
if (Type.isFullyQualified(mapped)) {
142+
this.typeCache.set(cacheKey, mapped);
143+
return mapped;
144+
}
145+
}
146+
}
147+
148+
const classType: Type.Class = {
149+
kind: Type.Kind.Class,
150+
flags: 0,
151+
classKind,
152+
fullyQualifiedName,
153+
typeParameters: [],
154+
annotations: [],
155+
interfaces: [],
156+
members: [],
157+
methods: [],
158+
toJSON: function () {
159+
return Type.signature(this);
160+
}
161+
} as Type.Class;
162+
this.typeCache.set(cacheKey, classType);
163+
return classType;
164+
}
165+
86166
private getType(type: ts.Type): Type {
87167
// Check for error types first - these indicate type-checking failures
88168
// and should not be processed further
@@ -990,7 +1070,10 @@ export class JavaScriptTypeMapping {
9901070
if (!symbol) {
9911071
return "unknown";
9921072
}
1073+
return this.getFullyQualifiedNameFromSymbol(symbol);
1074+
}
9931075

1076+
private getFullyQualifiedNameFromSymbol(symbol: ts.Symbol): string {
9941077
// First, check if this symbol is an import/alias
9951078
// For imported types, we want to use the module specifier instead of the file path
9961079
if (symbol.flags & ts.SymbolFlags.Alias) {

‎rewrite-javascript/rewrite/src/rpc/request/parse-project.ts‎

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ import {replaceMarkerByKind} from "../../markers";
2828
export interface ParseProjectResponseItem {
2929
id: UUID;
3030
sourceFileType: string;
31+
/**
32+
* Relative source path of the discovered file. Used by the Java side to report
33+
* per-file failures (e.g., a failed `GetObject` deserialization) without aborting
34+
* the rest of the stream. Optional for older callers.
35+
*/
36+
sourcePath?: string;
3137
}
3238

3339
/**
@@ -88,15 +94,16 @@ export class ParseProject {
8894
});
8995
const generator = parser.parse(...discovered.packageJsonFiles);
9096

91-
for (const _ of discovered.packageJsonFiles) {
97+
for (const filePath of discovered.packageJsonFiles) {
9298
const id = randomId();
9399
localObjects.set(id, async (id: string) => {
94100
const sourceFile: SourceFile = (await generator.next()).value;
95101
return { ...sourceFile, id };
96102
});
97103
resultItems.push({
98104
id,
99-
sourceFileType: "org.openrewrite.json.tree.Json$Document" // break cycle
105+
sourceFileType: "org.openrewrite.json.tree.Json$Document", // break cycle
106+
sourcePath: path.relative(relativeTo, filePath)
100107
});
101108
}
102109
}
@@ -106,15 +113,16 @@ export class ParseProject {
106113
const parser = Parsers.createParser("json", {ctx, relativeTo});
107114
const generator = parser.parse(...discovered.lockFiles.json);
108115

109-
for (const _ of discovered.lockFiles.json) {
116+
for (const filePath of discovered.lockFiles.json) {
110117
const id = randomId();
111118
localObjects.set(id, async (id: string) => {
112119
const sourceFile: SourceFile = (await generator.next()).value;
113120
return { ...sourceFile, id };
114121
});
115122
resultItems.push({
116123
id,
117-
sourceFileType: "org.openrewrite.json.tree.Json$Document" // break cycle
124+
sourceFileType: "org.openrewrite.json.tree.Json$Document", // break cycle
125+
sourcePath: path.relative(relativeTo, filePath)
118126
});
119127
}
120128
}
@@ -124,15 +132,16 @@ export class ParseProject {
124132
const parser = Parsers.createParser("yaml", {ctx, relativeTo});
125133
const generator = parser.parse(...discovered.lockFiles.yaml);
126134

127-
for (const _ of discovered.lockFiles.yaml) {
135+
for (const filePath of discovered.lockFiles.yaml) {
128136
const id = randomId();
129137
localObjects.set(id, async (id: string) => {
130138
const sourceFile: SourceFile = (await generator.next()).value;
131139
return { ...sourceFile, id };
132140
});
133141
resultItems.push({
134142
id,
135-
sourceFileType: "org.openrewrite.yaml.tree.Yaml$Documents" // break cycle
143+
sourceFileType: "org.openrewrite.yaml.tree.Yaml$Documents", // break cycle
144+
sourcePath: path.relative(relativeTo, filePath)
136145
});
137146
}
138147
}
@@ -142,15 +151,16 @@ export class ParseProject {
142151
const parser = Parsers.createParser("plainText", {ctx, relativeTo});
143152
const generator = parser.parse(...discovered.lockFiles.text);
144153

145-
for (const _ of discovered.lockFiles.text) {
154+
for (const filePath of discovered.lockFiles.text) {
146155
const id = randomId();
147156
localObjects.set(id, async (id: string) => {
148157
const sourceFile: SourceFile = (await generator.next()).value;
149158
return { ...sourceFile, id };
150159
});
151160
resultItems.push({
152161
id,
153-
sourceFileType: "org.openrewrite.text.PlainText" // break cycle
162+
sourceFileType: "org.openrewrite.text.PlainText", // break cycle
163+
sourcePath: path.relative(relativeTo, filePath)
154164
});
155165
}
156166
}
@@ -185,16 +195,18 @@ export class ParseProject {
185195
});
186196
resultItems.push({
187197
id,
188-
sourceFileType: "org.openrewrite.javascript.tree.JS$CompilationUnit" // break cycle
198+
sourceFileType: "org.openrewrite.javascript.tree.JS$CompilationUnit", // break cycle
199+
sourcePath: path.relative(relativeTo, filePath)
189200
});
190201
}
191202
} else {
192203
// Prettier is NOT available: auto-detect styles from parsed files
193204
// Parse all files first to sample them
194-
const parsedFiles: {id: string, sourceFile: SourceFile}[] = [];
205+
const parsedFiles: {id: string, sourceFile: SourceFile, filePath: string}[] = [];
206+
let fileIndex = 0;
195207
for await (const sourceFile of parser.parse(...discovered.jsFiles)) {
196208
const id = randomId();
197-
parsedFiles.push({id, sourceFile});
209+
parsedFiles.push({id, sourceFile, filePath: discovered.jsFiles[fileIndex++]});
198210
}
199211

200212
// Sample all parsed files and build Autodetect marker using ProjectParser helper
@@ -203,7 +215,7 @@ export class ParseProject {
203215
);
204216

205217
// Store thunks that add the Autodetect marker
206-
for (const {id, sourceFile} of parsedFiles) {
218+
for (const {id, sourceFile, filePath} of parsedFiles) {
207219
localObjects.set(id, async (newId: string) => {
208220
return {
209221
...sourceFile,
@@ -213,7 +225,8 @@ export class ParseProject {
213225
});
214226
resultItems.push({
215227
id,
216-
sourceFileType: "org.openrewrite.javascript.tree.JS$CompilationUnit" // break cycle
228+
sourceFileType: "org.openrewrite.javascript.tree.JS$CompilationUnit", // break cycle
229+
sourcePath: path.relative(relativeTo, filePath)
217230
});
218231
}
219232
}
@@ -224,15 +237,16 @@ export class ParseProject {
224237
const parser = Parsers.createParser("yaml", {ctx, relativeTo});
225238
const generator = parser.parse(...discovered.yamlFiles);
226239

227-
for (const _ of discovered.yamlFiles) {
240+
for (const filePath of discovered.yamlFiles) {
228241
const id = randomId();
229242
localObjects.set(id, async (id: string) => {
230243
const sourceFile: SourceFile = (await generator.next()).value;
231244
return { ...sourceFile, id };
232245
});
233246
resultItems.push({
234247
id,
235-
sourceFileType: "org.openrewrite.yaml.tree.Yaml$Documents" // break cycle
248+
sourceFileType: "org.openrewrite.yaml.tree.Yaml$Documents", // break cycle
249+
sourcePath: path.relative(relativeTo, filePath)
236250
});
237251
}
238252
}
@@ -242,15 +256,16 @@ export class ParseProject {
242256
const parser = Parsers.createParser("json", {ctx, relativeTo});
243257
const generator = parser.parse(...discovered.jsonFiles);
244258

245-
for (const _ of discovered.jsonFiles) {
259+
for (const filePath of discovered.jsonFiles) {
246260
const id = randomId();
247261
localObjects.set(id, async (id: string) => {
248262
const sourceFile: SourceFile = (await generator.next()).value;
249263
return { ...sourceFile, id };
250264
});
251265
resultItems.push({
252266
id,
253-
sourceFileType: "org.openrewrite.json.tree.Json$Document" // break cycle
267+
sourceFileType: "org.openrewrite.json.tree.Json$Document", // break cycle
268+
sourcePath: path.relative(relativeTo, filePath)
254269
});
255270
}
256271
}
@@ -260,15 +275,16 @@ export class ParseProject {
260275
const parser = Parsers.createParser("plainText", {ctx, relativeTo});
261276
const generator = parser.parse(...discovered.textFiles);
262277

263-
for (const _ of discovered.textFiles) {
278+
for (const filePath of discovered.textFiles) {
264279
const id = randomId();
265280
localObjects.set(id, async (id: string) => {
266281
const sourceFile: SourceFile = (await generator.next()).value;
267282
return { ...sourceFile, id };
268283
});
269284
resultItems.push({
270285
id,
271-
sourceFileType: "org.openrewrite.text.PlainText" // break cycle
286+
sourceFileType: "org.openrewrite.text.PlainText", // break cycle
287+
sourcePath: path.relative(relativeTo, filePath)
272288
});
273289
}
274290
}

‎rewrite-javascript/src/integTest/java/org/openrewrite/javascript/rpc/JavaScriptRewriteRpcTest.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.openrewrite.java.JavaIsoVisitor;
2727
import org.openrewrite.java.JavaVisitor;
2828
import org.openrewrite.java.tree.J;
29+
import org.openrewrite.java.tree.JavaType;
2930
import org.openrewrite.javascript.JavaScriptIsoVisitor;
3031
import org.openrewrite.javascript.JavaScriptParser;
3132
import org.openrewrite.javascript.style.Autodetect;
@@ -619,6 +620,37 @@ void jsRecipeDelegatingToJavaRecipe(@TempDir Path projectDir) {
619620
);
620621
}
621622

623+
/**
624+
* Regression test for <a href="https://github.com/moderneinc/customer-requests/issues/2234">#2234</a>:
625+
* a string enum declaration would send a {@link JavaType.Primitive} (resolved from the union of
626+
* enum literals) for {@code J.ClassDeclaration.type}, which the Java-side RPC receiver rejects
627+
* with "A class can only be type attributed with a fully qualified type name".
628+
*/
629+
@Test
630+
void parseStringEnumDeclaration() {
631+
rewriteRun(
632+
typescript(
633+
"""
634+
export enum ContentType {
635+
APPLICATION_JSON = 'application/json',
636+
}
637+
""",
638+
spec -> spec.afterRecipe(cu -> new JavaScriptIsoVisitor<Integer>() {
639+
@Override
640+
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Integer p) {
641+
assertThat(classDecl.getType())
642+
.as("enum declaration must have a FullyQualified type")
643+
.isInstanceOf(JavaType.Class.class);
644+
JavaType.Class type = (JavaType.Class) classDecl.getType();
645+
assertThat(type.getKind()).isEqualTo(JavaType.FullyQualified.Kind.Enum);
646+
assertThat(type.getFullyQualifiedName()).contains("ContentType");
647+
return classDecl;
648+
}
649+
}.visit(cu, 0))
650+
)
651+
);
652+
}
653+
622654
private void installRecipes() {
623655
var exampleRecipes = new File("rewrite/dist-fixtures/example-recipe.js");
624656
assertThat(exampleRecipes).exists();

0 commit comments

Comments
 (0)
⚡