Skip to content
This repository was archived by the owner on May 12, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions openrewrite/src/javascript/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, isStatement} from "./parserUtils";
import {JavaScriptTypeMapping} from "./typeMapping";
import path from "node:path";
import {ExpressionStatement, TypeTreeExpression} from ".";
Expand Down Expand Up @@ -2626,10 +2626,7 @@ 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) {
// 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(
Expand Down Expand Up @@ -2719,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 ( ;/*_*/; );
Expand Down
154 changes: 154 additions & 0 deletions openrewrite/src/javascript/parserUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,158 @@
import * as ts from "typescript";
import * as J from '../java'
import * as JS from "./tree";

const is_statements = [
J.Assert,
J.Assignment,
J.AssignmentOperation,
J.Block,
J.Break,
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,
J.Lambda,
J.MethodDeclaration,
J.MethodInvocation,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need something like an is_expression or how do we make sure that the types which are both expression and statement don't get wrapped? Also note the inverse wrapper (StatementExpression) which might show the same behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have removed unwrapping for the StatementExpression in a visitStatementExpression.
There is only one place in a parser where the StatementExpression is directly used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think it would be best if we could have this generated instead of maintaining it by hand, as now every time we add a new type to either J or JS, we need to make sure to check if we need to update these arrays.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am missing the following four types from J: DoWhileLoop, ForEachLoop, ForLoop, and WhileLoop. AFAIK we use all of these except for ForEachLoop in our parser.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also missing the JS types ForOfLoop and ForInLoop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks!
I've used a small util to generate the list of classes, but missed the Loop inheritors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think it would be best if we could have this generated instead of maintaining it by hand, as now every time we add a new type to either J or JS, we need to make sure to check if we need to update these arrays.

Yes, it will be a more consistent way to keep lists up-to-date.

J.NewClass,
J.Package,
J.Return,
J.Switch,
J.Synchronized,
J.Ternary,
J.Throw,
J.Try,
J.Unary,
J.Unknown,
J.VariableDeclarations,
J.WhileLoop,
J.Yield,
JS.ArrowFunction,
JS.BindingElement,
JS.Delete,
JS.Export,
JS.ExportAssignment,
JS.ExportDeclaration,
JS.FunctionDeclaration,
JS.JSForInLoop,
JS.JSForOfLoop,
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
]

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,
JS.ExpressionStatement,
JS.StatementExpression
]

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;
Expand Down
11 changes: 11 additions & 0 deletions openrewrite/test/javascript/parser/function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
`)
);
});
});
4 changes: 2 additions & 2 deletions openrewrite/test/javascript/parser/void.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -528,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;
}
Expand Down