From 6513eb41355be8fcbfb620caeb9e57ba44c03178 Mon Sep 17 00:00:00 2001 From: Andrii Rodionov Date: Mon, 24 Feb 2025 12:16:51 +0100 Subject: [PATCH 1/3] Prevent wrapping statements into JS.ExpressionStatement In a visitExpressionStatement parser method, if the expression is a Statement, it shouldn't be wrap into ExpressionStatement. Also in a JavaScriptVisitor class remove unwrapping ExpressionStatement into Statement, if it is a Statement. --- openrewrite/src/javascript/parser.ts | 7 +-- openrewrite/src/javascript/parserUtils.ts | 63 +++++++++++++++++++ .../test/javascript/parser/function.test.ts | 11 ++++ .../test/javascript/parser/void.test.ts | 4 +- .../javascript/JavaScriptVisitor.java | 3 - 5 files changed, 79 insertions(+), 9 deletions(-) diff --git a/openrewrite/src/javascript/parser.ts b/openrewrite/src/javascript/parser.ts index 60800798..00135bd4 100644 --- a/openrewrite/src/javascript/parser.ts +++ b/openrewrite/src/javascript/parser.ts @@ -23,7 +23,7 @@ import { randomId, SourceFile } from "../core"; -import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan, hasFlowAnnotation, checkSyntaxErrors, isValidSurrogateRange} from "./parserUtils"; +import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan, hasFlowAnnotation, checkSyntaxErrors, isValidSurrogateRange, statement_implementations_list} from "./parserUtils"; import {JavaScriptTypeMapping} from "./typeMapping"; import path from "node:path"; import {ExpressionStatement, TypeTreeExpression} from "."; @@ -2626,9 +2626,8 @@ export class JavaScriptParserVisitor { visitExpressionStatement(node: ts.ExpressionStatement): J.Statement { const expression = this.visit(node.expression) as J.Expression; - if (expression instanceof J.MethodInvocation || expression instanceof J.NewClass || expression instanceof J.Unknown || - expression instanceof J.AssignmentOperation || expression instanceof J.Ternary || expression instanceof J.Empty || - expression instanceof JS.ExpressionStatement || expression instanceof J.Assignment || expression instanceof J.FieldAccess) { + const isStatement = statement_implementations_list.some(cls => expression instanceof cls); + if (isStatement) { // FIXME this is a hack we currently require because our `Expression` and `Statement` interfaces don't have any type guards return expression as J.Statement; } diff --git a/openrewrite/src/javascript/parserUtils.ts b/openrewrite/src/javascript/parserUtils.ts index 74ef20fb..a1c7ae8a 100644 --- a/openrewrite/src/javascript/parserUtils.ts +++ b/openrewrite/src/javascript/parserUtils.ts @@ -1,4 +1,67 @@ import * as ts from "typescript"; +import * as J from '../java' +import * as JS from "./tree"; + +export const statement_implementations_list: Array<{ new (...args: any[]): J.Statement }> = [ + J.Assert, + J.Assignment, + J.AssignmentOperation, + J.Block, + J.Break, + J.Case, + J.ClassDeclaration, + J.Continue, + J.Empty, + J.EnumValueSet, + J.Erroneous, + J.FieldAccess, + J.If, + J.Import, + J.Label, + J.Lambda, + J.MethodDeclaration, + J.MethodInvocation, + J.NewClass, + J.Package, + J.Return, + J.Switch, + J.Synchronized, + J.Ternary, + J.Throw, + J.Try, + J.Unary, + J.Unknown, + J.VariableDeclarations, + J.Yield, + JS.ArrowFunction, + JS.BindingElement, + JS.Delete, + JS.Export, + JS.ExportAssignment, + JS.ExportDeclaration, + JS.FunctionDeclaration, + JS.ImportAttribute, + JS.IndexSignatureDeclaration, + JS.JsAssignmentOperation, + JS.JsImport, + JS.JSMethodDeclaration, + JS.JSTry, + JS.JSVariableDeclarations, + JS.MappedType.KeysRemapping, + JS.MappedType.MappedTypeParameter, + JS.NamespaceDeclaration, + JS.PropertyAssignment, + JS.ScopedVariableDeclarations, + JS.TaggedTemplateExpression, + JS.TemplateExpression, + JS.TrailingTokenStatement, + JS.TypeDeclaration, + JS.Unary, + JS.Void, + JS.WithStatement, + JS.ExpressionStatement, + JS.StatementExpression +] export function getNextSibling(node: ts.Node): ts.Node | null { const parent = node.parent; diff --git a/openrewrite/test/javascript/parser/function.test.ts b/openrewrite/test/javascript/parser/function.test.ts index c7c29032..766731b0 100644 --- a/openrewrite/test/javascript/parser/function.test.ts +++ b/openrewrite/test/javascript/parser/function.test.ts @@ -442,4 +442,15 @@ describe('function mapping', () => { `) ); }); + + test('function invocation', () => { + rewriteRun( + //language=typescript + typeScript(` + !function(e, t) { + console.log("This is an IIFE", e, t); + }("Hello", "World"); + `) + ); + }); }); diff --git a/openrewrite/test/javascript/parser/void.test.ts b/openrewrite/test/javascript/parser/void.test.ts index 1c29697c..023be5d0 100644 --- a/openrewrite/test/javascript/parser/void.test.ts +++ b/openrewrite/test/javascript/parser/void.test.ts @@ -11,8 +11,8 @@ describe('void operator mapping', () => { //language=typescript typeScript('void 1', cu => { const statement = cu.statements[0] as JS.ExpressionStatement; - expect(statement.expression).toBeInstanceOf(JS.Void); - const type = (statement.expression as JS.Void).type as JavaType.Primitive; + expect(statement).toBeInstanceOf(JS.Void); + const type = statement.type as JavaType.Primitive; expect(type.kind).toBe(JavaType.PrimitiveKind.Void); }) ); diff --git a/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java b/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java index 0df79b64..d6d0cb3e 100644 --- a/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java +++ b/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java @@ -214,9 +214,6 @@ public J visitExpressionStatement(JS.ExpressionStatement statement, P p) { es = (JS.ExpressionStatement) temp; } J expression = visit(es.getExpression(), p); - if (expression instanceof Statement) { - return expression; - } es = es.withExpression((Expression) Objects.requireNonNull(expression)); return es; } From 278b6522031747c7027f1cd9b26121d75ed817b8 Mon Sep 17 00:00:00 2001 From: Andrii Rodionov Date: Mon, 24 Feb 2025 15:03:52 +0100 Subject: [PATCH 2/3] - added type guards methods isStatement and isExpression - remove unwrapping for visitStatementExpression --- openrewrite/src/javascript/parser.ts | 8 +- openrewrite/src/javascript/parserUtils.ts | 85 ++++++++++++++++++- .../javascript/JavaScriptVisitor.java | 3 - 3 files changed, 87 insertions(+), 9 deletions(-) diff --git a/openrewrite/src/javascript/parser.ts b/openrewrite/src/javascript/parser.ts index 00135bd4..f13575d7 100644 --- a/openrewrite/src/javascript/parser.ts +++ b/openrewrite/src/javascript/parser.ts @@ -23,7 +23,7 @@ import { randomId, SourceFile } from "../core"; -import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan, hasFlowAnnotation, checkSyntaxErrors, isValidSurrogateRange, statement_implementations_list} from "./parserUtils"; +import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan, hasFlowAnnotation, checkSyntaxErrors, isValidSurrogateRange, isStatement} from "./parserUtils"; import {JavaScriptTypeMapping} from "./typeMapping"; import path from "node:path"; import {ExpressionStatement, TypeTreeExpression} from "."; @@ -2626,9 +2626,7 @@ export class JavaScriptParserVisitor { visitExpressionStatement(node: ts.ExpressionStatement): J.Statement { const expression = this.visit(node.expression) as J.Expression; - const isStatement = statement_implementations_list.some(cls => expression instanceof cls); - if (isStatement) { - // FIXME this is a hack we currently require because our `Expression` and `Statement` interfaces don't have any type guards + if (isStatement(expression)) { return expression as J.Statement; } return new JS.ExpressionStatement( @@ -2718,7 +2716,7 @@ export class JavaScriptParserVisitor { Markers.EMPTY, [node.initializer ? (ts.isVariableDeclarationList(node.initializer) ? this.rightPadded(this.visit(node.initializer), Space.EMPTY) : - this.rightPadded(new ExpressionStatement(randomId(), this.visit(node.initializer)), this.suffix(node.initializer))) : + this.rightPadded(ts.isStatement(node.initializer) ? this.visit(node.initializer) : new ExpressionStatement(randomId(), this.visit(node.initializer)), this.suffix(node.initializer))) : this.rightPadded(this.newJEmpty(), this.suffix(this.findChildNode(node, ts.SyntaxKind.OpenParenToken)!))], // to handle for (/*_*/; ; ); node.condition ? this.rightPadded(this.visit(node.condition), this.suffix(node.condition)) : this.rightPadded(this.newJEmpty(), this.suffix(this.findChildNode(node, ts.SyntaxKind.SemicolonToken)!)), // to handle for ( ;/*_*/; ); diff --git a/openrewrite/src/javascript/parserUtils.ts b/openrewrite/src/javascript/parserUtils.ts index a1c7ae8a..b434ed02 100644 --- a/openrewrite/src/javascript/parserUtils.ts +++ b/openrewrite/src/javascript/parserUtils.ts @@ -2,7 +2,7 @@ import * as ts from "typescript"; import * as J from '../java' import * as JS from "./tree"; -export const statement_implementations_list: Array<{ new (...args: any[]): J.Statement }> = [ +const is_statements = [ J.Assert, J.Assignment, J.AssignmentOperation, @@ -63,6 +63,89 @@ export const statement_implementations_list: Array<{ new (...args: any[]): J.Sta JS.StatementExpression ] +const is_expressions = [ + J.AnnotatedType, + J.Annotation, + J.ArrayAccess, + J.ArrayType, + J.Assignment, + J.AssignmentOperation, + J.Binary, + J.ControlParentheses, + J.Empty, + J.Erroneous, + J.FieldAccess, + J.Identifier, + J.InstanceOf, + J.IntersectionType, + J.Lambda, + J.Literal, + J.MethodInvocation, + J.MemberReference, + J.NewArray, + J.NewClass, + J.NullableType, + J.ParameterizedType, + J.Parentheses, + J.ParenthesizedTypeTree, + J.Primitive, + J.SwitchExpression, + J.Ternary, + J.TypeCast, + J.Unary, + J.Unknown, + J.Wildcard, + JS.Alias, + JS.ArrayBindingPattern, + JS.ArrowFunction, + JS.Await, + JS.BindingElement, + JS.ConditionalType, + JS.DefaultType, + JS.Delete, + JS.ExportSpecifier, + JS.ExpressionWithTypeArguments, + JS.FunctionDeclaration, + JS.FunctionType, + JS.ImportType, + JS.IndexedAccessType, + JS.IndexedAccessType.IndexType, + JS.InferType, + JS.Intersection, + JS.JsAssignmentOperation, + JS.JsBinary, + JS.JsImportSpecifier, + JS.LiteralType, + JS.MappedType, + JS.NamedExports, + JS.NamedImports, + JS.ObjectBindingDeclarations, + JS.SatisfiesExpression, + JS.TaggedTemplateExpression, + JS.TemplateExpression, + JS.TrailingTokenStatement, + JS.Tuple, + JS.TypeInfo, + JS.TypeLiteral, + JS.TypeOf, + JS.TypeOperator, + JS.TypePredicate, + JS.TypeQuery, + JS.TypeTreeExpression, + JS.Unary, + JS.Union, + JS.Void, + JS.Yield +] + +export function isStatement(statement: J.J): statement is J.Statement { + return is_statements.some((cls: any) => statement instanceof cls); +} + +export function isExpression(expression: J.J): expression is J.Expression { + return is_expressions.some((cls: any) => expression instanceof cls); +} + export function getNextSibling(node: ts.Node): ts.Node | null { const parent = node.parent; if (!parent) { diff --git a/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java b/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java index d6d0cb3e..96c67fef 100644 --- a/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java +++ b/rewrite-javascript/src/main/java/org/openrewrite/javascript/JavaScriptVisitor.java @@ -525,9 +525,6 @@ public J visitStatementExpression(JS.StatementExpression expression, P p) { se = (JS.StatementExpression) temp; } J statement = visit(se.getStatement(), p); - if (statement instanceof Expression) { - return statement; - } se = se.withStatement((Statement) Objects.requireNonNull(statement)); return se; } From bf2a55539f1bb854328da5017e81c67802ad1182 Mon Sep 17 00:00:00 2001 From: Andrii Rodionov Date: Tue, 25 Feb 2025 12:57:58 +0100 Subject: [PATCH 3/3] - added missed classes to the list --- openrewrite/src/javascript/parserUtils.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/openrewrite/src/javascript/parserUtils.ts b/openrewrite/src/javascript/parserUtils.ts index b434ed02..9e41edb6 100644 --- a/openrewrite/src/javascript/parserUtils.ts +++ b/openrewrite/src/javascript/parserUtils.ts @@ -11,10 +11,13 @@ const is_statements = [ J.Case, J.ClassDeclaration, J.Continue, + J.DoWhileLoop, J.Empty, J.EnumValueSet, J.Erroneous, J.FieldAccess, + J.ForEachLoop, + J.ForLoop, J.If, J.Import, J.Label, @@ -32,6 +35,7 @@ const is_statements = [ J.Unary, J.Unknown, J.VariableDeclarations, + J.WhileLoop, J.Yield, JS.ArrowFunction, JS.BindingElement, @@ -40,6 +44,8 @@ const is_statements = [ JS.ExportAssignment, JS.ExportDeclaration, JS.FunctionDeclaration, + JS.JSForInLoop, + JS.JSForOfLoop, JS.ImportAttribute, JS.IndexSignatureDeclaration, JS.JsAssignmentOperation, @@ -135,7 +141,9 @@ const is_expressions = [ JS.Unary, JS.Union, JS.Void, - JS.Yield + JS.Yield, + JS.ExpressionStatement, + JS.StatementExpression ] export function isStatement(statement: J.J): statement is J.Statement {