Skip to content
This repository was archived by the owner on May 12, 2025. It is now read-only.

Throwing SyntaxError if a parsed source file is syntactically incorrect #202

Merged
merged 2 commits into from
Jan 27, 2025
Merged

Throwing SyntaxError if a parsed source file is syntactically incorrect #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e741de7
Throwing SyntaxError if a parsed source file is syntactically incorrect
Jan 27, 2025
bc68b5a
added exclusion code
Jan 27, 2025
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
56 changes: 12 additions & 44 deletions openrewrite/src/javascript/parser.ts
View file
Open in desktop
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} from "./parserUtils";
import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan, hasFlowAnnotation, checkSyntaxErrors} from "./parserUtils";
import {JavaScriptTypeMapping} from "./typeMapping";
import path from "node:path";
import {ExpressionStatement, TypeTreeExpression} from ".";
Expand All @@ -41,6 +41,8 @@ export class JavaScriptParser extends Parser {
module: ts.ModuleKind.CommonJS,
allowJs: true,
esModuleInterop: true,
experimentalDecorators: true,
emitDecoratorMetadata: true
};
}

Expand Down Expand Up @@ -147,19 +149,18 @@ export class JavaScriptParser extends Parser {
continue;
}

if (this.hasFlowAnnotation(sourceFile)) {
if (hasFlowAnnotation(sourceFile)) {
result.push(ParseError.build(this, input, relativeTo, ctx, new FlowSyntaxNotSupportedError(`Flow syntax not supported: ${input.path}`), null));
continue;
}

// ToDo: uncomment code after tests fixing
// const syntaxErrors = this.checkSyntaxErrors(program, sourceFile);
// if (syntaxErrors.length > 0) {
// syntaxErrors.forEach(
// e => result.push(ParseError.build(this, input, relativeTo, ctx, new SyntaxError(`Compiler error: ${e[0]} [${e[1]}]`), null))
// );
// continue;
// }
const syntaxErrors = checkSyntaxErrors(program, sourceFile);
if (syntaxErrors.length > 0) {
syntaxErrors.forEach(
e => result.push(ParseError.build(this, input, relativeTo, ctx, new SyntaxError(`Compiler error: ${e[0]} [${e[1]}]`), null))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this end up creating multiple ParseErrors for one source file?

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, there can be many ParseErrors for one source file

);
continue;
}

try {
const parsed = new JavaScriptParserVisitor(this, sourceFile, typeChecker).visit(sourceFile) as SourceFile;
Expand All @@ -171,39 +172,6 @@ export class JavaScriptParser extends Parser {
return result;
}

private checkSyntaxErrors(program: ts.Program, sourceFile: ts.SourceFile) {
const diagnostics = ts.getPreEmitDiagnostics(program, sourceFile);
// checking Parsing and Syntax Errors
let syntaxErrors : [errorMsg: string, errorCode: number][] = [];
if (diagnostics.length > 0) {
const errors = diagnostics.filter(d => d.code >= 1000 && d.code < 2000);
if (errors.length > 0) {
syntaxErrors = errors.map(e => {
let errorMsg;
if (e.file) {
let {line, character} = ts.getLineAndCharacterOfPosition(e.file, e.start!);
let message = ts.flattenDiagnosticMessageText(e.messageText, "\n");
errorMsg = `${e.file.fileName} (${line + 1},${character + 1}): ${message}`;
} else {
errorMsg = ts.flattenDiagnosticMessageText(e.messageText, "\n");
}
return [errorMsg, e.code];
});
}
}
return syntaxErrors;
}

private hasFlowAnnotation(sourceFile: ts.SourceFile) {
if (sourceFile.fileName.endsWith('.js') || sourceFile.fileName.endsWith('.jsx')) {
const comments = sourceFile.getFullText().match(/\/\*[\s\S]*?\*\/|\/\/.*(?=[\r\n])/g);
if (comments) {
return comments.some(comment => comment.includes("@flow"));
}
}
return false;
}

accept(path: string): boolean {
return path.endsWith('.ts') || path.endsWith('.tsx') || path.endsWith('.js') || path.endsWith('.jsx');
}
Expand Down Expand Up @@ -1866,7 +1834,7 @@ export class JavaScriptParserVisitor {
Markers.EMPTY
),
node.qualifier ? this.leftPadded(this.prefix(this.findChildNode(node, ts.SyntaxKind.DotToken)!), this.visit(node.qualifier)): null,
node.typeArguments ? this.mapTypeArguments(this.suffix(node.qualifier!), node.typeArguments) : null,
node.typeArguments ? this.mapTypeArguments(this.prefix(this.findChildNode(node, ts.SyntaxKind.LessThanToken)!), node.typeArguments) : null,
this.mapType(node)
);
}
Expand Down
48 changes: 48 additions & 0 deletions openrewrite/src/javascript/parserUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,51 @@ export function binarySearch<T>(arr: T[], target: T, compare: (a: T, b: T) => nu
}
return ~low; // Element not found, return bitwise complement of the insertion point
}

export function hasFlowAnnotation(sourceFile: ts.SourceFile) {
if (sourceFile.fileName.endsWith('.js') || sourceFile.fileName.endsWith('.jsx')) {
const comments = sourceFile.getFullText().match(/\/\*[\s\S]*?\*\/|\/\/.*(?=[\r\n])/g);
if (comments) {
return comments.some(comment => comment.includes("@flow"));
}
}
return false;
}

export function checkSyntaxErrors(program: ts.Program, sourceFile: ts.SourceFile) {
const diagnostics = ts.getPreEmitDiagnostics(program, sourceFile);
// checking Parsing and Syntax Errors
let syntaxErrors : [errorMsg: string, errorCode: number][] = [];
if (diagnostics.length > 0) {
const errors = diagnostics.filter(d => (d.category === ts.DiagnosticCategory.Error) && isCriticalDiagnostic(d.code));
if (errors.length > 0) {
syntaxErrors = errors.map(e => {
let errorMsg;
if (e.file) {
let {line, character} = ts.getLineAndCharacterOfPosition(e.file, e.start!);
let message = ts.flattenDiagnosticMessageText(e.messageText, "\n");
errorMsg = `${e.file.fileName} (${line + 1},${character + 1}): ${message}`;
} else {
errorMsg = ts.flattenDiagnosticMessageText(e.messageText, "\n");
}
return [errorMsg, e.code];
});
}
}
return syntaxErrors;
}

const additionalCriticalCodes = new Set([
// Syntax errors
17019, // "'{0}' at the end of a type is not valid TypeScript syntax. Did you mean to write '{1}'?"
17020, // "'{0}' at the start of a type is not valid TypeScript syntax. Did you mean to write '{1}'?"

// Other critical errors
]);

const excludedCodes = new Set([1039, 1064, 1101, 1107, 1111, 1155, 1166, 1170, 1183, 1203, 1207, 1215, 1238, 1239, 1240, 1241, 1244, 1250,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there constants with readable names for all these codes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can put here the code description.
The full list is available here https://github.com/microsoft/TypeScript/blob/main/src/compiler/diagnosticMessages.json

1251, 1252, 1253, 1254, 1308, 1314, 1315, 1324, 1329, 1335, 1338, 1340, 1343, 1344, 1345, 1355, 1360, 1378, 1432]);

function isCriticalDiagnostic(code: number): boolean {
return (code > 1000 && code < 2000 && !excludedCodes.has(code)) || additionalCriticalCodes.has(code);
}
6 changes: 2 additions & 4 deletions openrewrite/test/javascript/e2e/electron-server.files.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,10 @@ describe('electron-release-server files tests', () => {
}

// If not the first changenote, prefix with new line
var newChangeNote = !prevNotes.length ? '' : '
';
var newChangeNote = !prevNotes.length ? '' : '';

// Add the version name and notes
newChangeNote += '## ' + newVersion.name + '
' + newVersion.notes;
newChangeNote += '## ' + newVersion.name + '' + newVersion.notes;

// Add the new changenote to the previous ones
return prevNotes + newChangeNote;
Expand Down
24 changes: 13 additions & 11 deletions openrewrite/test/javascript/parser/arrow.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,19 @@ describe('arrow mapping', () => {
rewriteRun(
//language=typescript
typeScript(`
prop: </*a*/const /*b*/ S extends SchemaObj, A, E>(
name: string,
schemas: S,
self: TestFunction<
A,
E,
R,
[{ [K in keyof S]: Schema.Schema.Type<S[K]> }, V.TaskContext<V.RunnerTestCase<{}>> & V.TestContext]
>,
timeout?: number | V.TestOptions
) => void
class A {
prop: </*a*/const /*b*/ S extends SchemaObj, A, E>(
name: string,
schemas: S,
self: TestFunction<
A,
E,
R,
[{ [K in keyof S]: Schema.Schema.Type<S[K]> }, V.TaskContext<V.RunnerTestCase<{}>> & V.TestContext]
>,
timeout?: number | V.TestOptions
) => void;
}
`)
);
});
Expand Down
2 changes: 1 addition & 1 deletion openrewrite/test/javascript/parser/await.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('await mapping', () => {
test('simple', () => {
rewriteRun(
//language=typescript
typeScript('await 1')
typeScript('export {}; await 1')
);
});
});
6 changes: 0 additions & 6 deletions openrewrite/test/javascript/parser/class.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ describe('class mapping', () => {
typeScript('export class A {}')
);
});
test('public', () => {
rewriteRun(
//language=typescript
typeScript('public class A {}')
);
});
test('export default', () => {
rewriteRun(
//language=typescript
Expand Down
2 changes: 1 addition & 1 deletion openrewrite/test/javascript/parser/decorator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('class decorator mapping', () => {
`)
);
});
test('decorator on class expression', () => {
test.skip('decorator on class expression', () => {
rewriteRun(
//language=typescript
typeScript(`
Expand Down
4 changes: 2 additions & 2 deletions openrewrite/test/javascript/parser/enum.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {connect, disconnect, rewriteRun, typeScript} from '../testHarness';

describe('empty mapping', () => {
describe('enum mapping', () => {
beforeAll(() => connect());
afterAll(() => disconnect());

Expand Down Expand Up @@ -142,7 +142,7 @@ describe('empty mapping', () => {
typeScript(`
enum Test {
A = "AA",
B,
B = undefined,
C = 10,
D = globalThis.NaN,
E = (2 + 2),
Expand Down
16 changes: 15 additions & 1 deletion openrewrite/test/javascript/parser/flowAnnotation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe('flow annotation checking test', () => {
beforeAll(() => connect());
afterAll(() => disconnect());

test('@flow in a comment in js', () => {
test('@flow in a one line comment in js', () => {
const faultyTest = () => rewriteRun(
//language=javascript
javaScript(`
Expand All @@ -18,6 +18,20 @@ describe('flow annotation checking test', () => {
expect(faultyTest).toThrow(/FlowSyntaxNotSupportedError/);
});

test('@flow in a comment in js', () => {
const faultyTest = () => rewriteRun(
//language=javascript
javaScript(`
/* @flow */

import Rocket from './rocket';
import RocketLaunch from './rocket-launch';
`)
);

expect(faultyTest).toThrow(/FlowSyntaxNotSupportedError/);
});

test('@flow in a multiline comment in js', () => {
const faultyTest = () => rewriteRun(
//language=javascript
Expand Down
16 changes: 9 additions & 7 deletions openrewrite/test/javascript/parser/for.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('for mapping', () => {
test('for-of with await and comments', () => {
rewriteRun(
//language=typescript
typeScript('/*a*/for/*b*/ await/*bb*/(/*c*/const /*d*/char /*e*/of /*f*/ "text"/*g*/)/*h*/ {/*j*/} /*k*/;/*l*/')
typeScript('export {};/*a*/for/*b*/ await/*bb*/(/*c*/const /*d*/char /*e*/of /*f*/ "text"/*g*/)/*h*/ {/*j*/} /*k*/;/*l*/')
);
});

Expand Down Expand Up @@ -181,12 +181,14 @@ describe('for mapping', () => {
rewriteRun(
//language=typescript
typeScript(`
let b;
for (b in a) return !1;
for (b of a) return !1;
let i, str;
for (i = 0; i < 9; i++) {
str = str + i;
function foo () {
let b;
for (b in a) return !1;
for (b of a) return !1;
let i, str;
for (i = 0; i < 9; i++) {
str = str + i;
}
}
`)
);
Expand Down
10 changes: 7 additions & 3 deletions openrewrite/test/javascript/parser/if.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@ describe('if mapping', () => {
rewriteRun(
//language=typescript
typeScript(`
function foo() {
for (let opt of el.options)
if (opt.selected !== opt.defaultSelected) return !0;
if (opt.selected !== opt.defaultSelected) return !0;
}
`)
);
});
Expand All @@ -104,8 +106,10 @@ describe('if mapping', () => {
rewriteRun(
//language=typescript
typeScript(`
if (prevProps)
return abs(prevProps)/*a*/;/*b*/
function foo() {
if (prevProps)
return abs(prevProps)/*a*/;/*b*/
}
`)
);
});
Expand Down
8 changes: 4 additions & 4 deletions openrewrite/test/javascript/parser/import.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('import mapping', () => {
test('dynamic import', () => {
rewriteRun(
//language=typescript
typeScript('const module = await import("module-name");')
typeScript('export {};const module = await import("module-name");')
)
});

Expand Down Expand Up @@ -105,13 +105,13 @@ describe('import mapping', () => {
);
});

test('experimental: import with import attributes', () => {
test('import with import attributes', () => {
rewriteRun(
//language=typescript
typeScript(`
import type { SpyInstance } from 'jest';
///*{1}*/import /*{2}*/type /*{3}*/{ /*{4}*/ SpyInstance /*{5}*/} /*{6}*/ from /*{7}*/ 'jest' /*{8}*/;
import SpyInstance = jest.SpyInstance;
import type SpyInstance = jest.SpyInstance;
/*{1}*/import /*{2}*/type /*{3}*/SpyInstance /*{4}*/= /*{5}*/jest.SpyInstance/*{6}*/;
`)
);
});
Expand Down
15 changes: 13 additions & 2 deletions openrewrite/test/javascript/parser/importType.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,23 @@ describe('import type mapping', () => {
//language=typescript
typeScript(`
export type LocalInterface =
& import("pkg", { with: {"resolution-mode": "foobar"} }).RequireInterface
& import("pkg", { with: {"resolution-mode": "require"} }).RequireInterface
& import("pkg", { with: {"resolution-mode": "import"} }).ImportInterface;

export const a = (null as any as import("pkg", { with: {"resolution-mode": "foobar"} }).RequireInterface);
export const a = (null as any as import("pkg", { with: {"resolution-mode": "require"} }).RequireInterface);
export const b = (null as any as import("pkg", { with: {"resolution-mode": "import"} }).ImportInterface);
`)
);
});

test('import type without qualifier an with type argument', () => {
rewriteRun(
//language=typescript
typeScript(`
declare module "ContextUtils" {
export function createContext<Config extends import("tailwindcss").Config>(config: ReturnType<typeof import("tailwindcss/resolveConfig")<Config>>,): import("./types.ts").TailwindContext;
}
`)
);
});
});
3 changes: 2 additions & 1 deletion openrewrite/test/javascript/parser/namespace.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe('namespace mapping', () => {
typeScript(`
/*pref*/ declare namespace /*middle*/ TestNamespace /*after*/ {
/*bcd*/
/*1*/ a = 10
/*1*/ const a = 10;
/*efg*/
/*2*/ function abc() {
return null
Expand Down Expand Up @@ -244,6 +244,7 @@ describe('namespace mapping', () => {
rewriteRun(
//language=typescript
typeScript(`
export {}
declare namespace MyLibrary {
function sayHello(name: string): void;
}
Expand Down
Loading
Loading