From 60329463c31687c66683ac908aab7924ff2aa22a Mon Sep 17 00:00:00 2001 From: Andrii Rodionov Date: Mon, 21 Oct 2024 17:34:45 +0200 Subject: [PATCH] Reworked classDeclaration visitor (#127) The `visitClassDeclaration` method has been rewritten to be able to parse classes with several declared properties correctly --- openrewrite/src/javascript/parser.ts | 54 ++++----- .../test/javascript/parser/class.test.ts | 114 ++++++++++++++++++ .../parser/variableDeclarations.test.ts | 2 +- .../internal/JavaScriptPrinter.java | 1 - 4 files changed, 139 insertions(+), 32 deletions(-) diff --git a/openrewrite/src/javascript/parser.ts b/openrewrite/src/javascript/parser.ts index 167ba70d..b8435d64 100644 --- a/openrewrite/src/javascript/parser.ts +++ b/openrewrite/src/javascript/parser.ts @@ -259,7 +259,7 @@ export class JavaScriptParserVisitor { } else if (ts.isClassDeclaration(node)) { return node.modifiers ? node.modifiers?.filter(ts.isModifier).map(this.mapModifier) : []; } else if (ts.isPropertyDeclaration(node)) { - return []; // FIXME + return node.modifiers ? node.modifiers?.filter(ts.isModifier).map(this.mapModifier) : []; } else if (ts.isFunctionDeclaration(node) || ts.isParameter(node) || ts.isMethodDeclaration(node)) { return node.modifiers ? node.modifiers?.filter(ts.isModifier).map(this.mapModifier) : []; } else if (ts.isVariableDeclarationList(node)) { @@ -389,7 +389,18 @@ export class JavaScriptParserVisitor { this.mapExtends(node), this.mapImplements(node), null, - this.convertBlock(node.getChildren(this.sourceFile).slice(-3)), + new J.Block( + randomId(), + this.prefix(node.getChildren().find(v => v.kind === ts.SyntaxKind.OpenBraceToken)!), + Markers.EMPTY, + new JRightPadded(false, Space.EMPTY, Markers.EMPTY), + node.members.map((ce : ts.ClassElement) => new JRightPadded( + this.convert(ce), + ce.getLastToken()?.kind === ts.SyntaxKind.SemicolonToken ? this.prefix(ce.getLastToken()!) : Space.EMPTY, + ce.getLastToken()?.kind === ts.SyntaxKind.SemicolonToken ? Markers.build([new Semicolon(randomId())]) : Markers.EMPTY + )), + this.prefix(node.getLastToken()!) + ), this.mapType(node) ); } @@ -592,7 +603,7 @@ export class JavaScriptParserVisitor { Markers.EMPTY, [], this.mapModifiers(node), - node.type ? this.visit(node.type) : null, + this.mapTypeInfo(node), null, [], [this.rightPadded( @@ -654,17 +665,17 @@ export class JavaScriptParserVisitor { Markers.EMPTY, [], this.mapModifiers(node), - node.type ? this.visit(node.type!) : null, + this.mapTypeInfo(node), null, [], [this.rightPadded( new J.VariableDeclarations.NamedVariable( randomId(), - this.prefix(node), + this.prefix(node.name), Markers.EMPTY, this.visit(node.name), [], - node.initializer ? this.leftPadded(this.prefix(node.getChildAt(node.getChildCount(this.sourceFile) - 2)), this.visit(node.initializer)) : null, + node.initializer ? this.leftPadded(this.prefix(node.getChildAt(node.getChildren().indexOf(node.initializer) - 1)), this.visit(node.initializer)) : null, this.mapVariableType(node) ), Space.EMPTY // FIXME check for semicolon @@ -686,7 +697,7 @@ export class JavaScriptParserVisitor { node.typeParameters ? new J.TypeParameters(randomId(), this.suffix(node.name), Markers.EMPTY, [], node.typeParameters.map(tp => this.rightPadded(this.visit(tp), this.suffix(tp)))) : null, - node.type ? new JS.TypeInfo(randomId(), this.prefix(node.getChildAt(node.getChildren().indexOf(node.type) - 1)), Markers.EMPTY, this.visit(node.type)): null, + this.mapTypeInfo(node), new J.MethodDeclaration.IdentifierWithAnnotations( node.name ? this.visit(node.name) : this.mapIdentifier(node, ""), [] @@ -699,6 +710,10 @@ export class JavaScriptParserVisitor { ); } + private mapTypeInfo(node: ts.MethodDeclaration | ts.PropertyDeclaration | ts.VariableDeclaration | ts.ParameterDeclaration) { + return node.type ? new JS.TypeInfo(randomId(), this.prefix(node.getChildAt(node.getChildren().indexOf(node.type) - 1)), Markers.EMPTY, this.visit(node.type)) : null; + } + visitClassStaticBlockDeclaration(node: ts.ClassStaticBlockDeclaration) { return this.visitUnknown(node); } @@ -1454,8 +1469,7 @@ export class JavaScriptParserVisitor { randomId(), this.prefix(node), Markers.EMPTY, - // NOTE: the name prefix will be used as a suffix by the printer - this.visit(node.name).withPrefix(node.type ? this.suffix(node.name) : Space.EMPTY), + this.visit(node.name), [], node.initializer ? this.leftPadded(this.prefix(node.getChildAt(node.getChildCount(this.sourceFile) - 2)), this.visit(node.initializer)) : null, this.mapVariableType(node) @@ -1477,7 +1491,7 @@ export class JavaScriptParserVisitor { Markers.EMPTY, [], // FIXME decorators? [], // FIXME modifiers? - declaration.type ? this.visit(declaration.type) : null, + this.mapTypeInfo(declaration), null, // FIXME varargs [], [this.rightPadded(this.visit(declaration), Space.EMPTY)] @@ -2050,26 +2064,6 @@ export class JavaScriptParserVisitor { return null; // FIXME } - private convertBlock(nodes: ts.Node[]): J.Block { - const prefix = this.prefix(nodes[0]); - let statementList = nodes[1] as ts.SyntaxList; - - const statements: JRightPadded[] = this.rightPaddedSeparatedList( - [...statementList.getChildren(this.sourceFile)], - ts.SyntaxKind.SemicolonToken, - (nodes, i) => nodes[i].getLastToken()?.kind == ts.SyntaxKind.SemicolonToken ? Markers.build([new Semicolon(randomId())]) : Markers.EMPTY - ); - - return new J.Block( - randomId(), - prefix, - Markers.EMPTY, - this.rightPadded(false, Space.EMPTY), - statements, - this.prefix(nodes[nodes.length - 1]) - ); - } - private findChildNode(node: ts.Node, kind: ts.SyntaxKind): ts.Node | undefined { for (let i = 0; i < node.getChildCount(this.sourceFile); i++) { if (node.getChildAt(i, this.sourceFile).kind == kind) { diff --git a/openrewrite/test/javascript/parser/class.test.ts b/openrewrite/test/javascript/parser/class.test.ts index ad356564..e1ee2770 100644 --- a/openrewrite/test/javascript/parser/class.test.ts +++ b/openrewrite/test/javascript/parser/class.test.ts @@ -65,4 +65,118 @@ describe('class mapping', () => { typeScript('export default class A {}') ); }); + + test('class with properties', () => { + rewriteRun( + //language=typescript + typeScript(` + class X { + a = 5 + b = 6 + } + `) + ); + }); + + test('class with properties and semicolon', () => { + rewriteRun( + //language=typescript + typeScript(` + class X { + a = 5; + b = 6; + } /*asdasdas*/ + //asdasf + `) + ); + }); + + test('class with mixed properties with semicolons', () => { + rewriteRun( + //language=typescript + typeScript(` + class X { + + b = 6 + c = 10; + a /*asdasd*/ = /*abc*/ 5 + d = "d"; + + } /*asdasdas*/ + //asdasf + `) + ); + }); + + test('class with properties, semicolon, methods, comments', () => { + rewriteRun( + //language=typescript + typeScript(` + class X { + a /*asdasd*/ = /*abc*/ 5; + b = 6; + + // method 1 + abs(): string{ + return "1"; + } + + //method 2 + max(): number { + return 2; + } + } /*asdasdas*/ + //asdasf + `) + ); + }); + + test('class with several typed properties', () => { + rewriteRun( + //language=typescript + typeScript(` + class X { + + b: number = 6 + c: string = "abc"; + a /*asdasd*/ = /*abc*/ 5 + + } /*asdasdas*/ + //asdasf + `) + ); + }); + + test('class with reference-typed property', () => { + rewriteRun( + //language=typescript + typeScript(` + class X { + + a: globalThis.Promise = null; + b: number = 6; + c /*asdasd*/ = /*abc*/ 5 + + } /*asdasdas*/ + //asdasf + `) + ); + }); + + test('class with typed properties, modifiers, comments', () => { + rewriteRun( + //language=typescript + typeScript(` + class X /*abc*/ { + public name /*asdasda*/ : /*dasdasda*/ string; + private surname /*asdasda*/ : /*dasdasda*/ string = "abc"; + b: number /* abc */ = 6; + c = 10; + a /*asdasd*/ = /*abc*/ 5 + + } + //asdasf + `) + ); + }); }); diff --git a/openrewrite/test/javascript/parser/variableDeclarations.test.ts b/openrewrite/test/javascript/parser/variableDeclarations.test.ts index 6f9fec5e..4e9dcb92 100644 --- a/openrewrite/test/javascript/parser/variableDeclarations.test.ts +++ b/openrewrite/test/javascript/parser/variableDeclarations.test.ts @@ -63,7 +63,7 @@ describe('variable declaration mapping', () => { test('multi typed', () => { rewriteRun( //language=typescript - typeScript('let a: number =2, b: string = "2" ') + typeScript(' /*0.1*/ let /*0.2*/ a /*1*/ : /*2*/ number =2 /*3*/ , /*4*/ b /*5*/:/*6*/ /*7*/string /*8*/ =/*9*/ "2" /*10*/ ; //11') ); }); }); diff --git a/rewrite-javascript/src/main/java/org/openrewrite/javascript/internal/JavaScriptPrinter.java b/rewrite-javascript/src/main/java/org/openrewrite/javascript/internal/JavaScriptPrinter.java index 206cb619..481cb231 100644 --- a/rewrite-javascript/src/main/java/org/openrewrite/javascript/internal/JavaScriptPrinter.java +++ b/rewrite-javascript/src/main/java/org/openrewrite/javascript/internal/JavaScriptPrinter.java @@ -769,7 +769,6 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, PrintOu visitSpace(variable.getAfter(), Space.Location.NAMED_VARIABLE_SUFFIX, p); if (multiVariable.getTypeExpression() != null) { - p.append(":"); visit(multiVariable.getTypeExpression(), p); }