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

Commit 473fa06

Browse files
arodionovAndrii Rodionov
arodionov
and
Andrii Rodionov
authored
Throwing SyntaxError if a parsed source file is syntactically incorrect (#202)
* Throwing SyntaxError if a parsed source file is syntactically incorrect If a source file has syntax errors, the ParseError is generated, and the file is skipped from parsing. Syntactically check is based on TS compiler function `ts.getPreEmitDiagnostics`. There are a list of error codes, that can be ignored or added. --------- Co-authored-by: Andrii Rodionov <[email protected]>
1 parent ae74ed9 commit 473fa06

19 files changed

+135
-92
lines changed

‎openrewrite/src/javascript/parser.ts‎

Lines changed: 12 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
randomId,
2424
SourceFile
2525
} from "../core";
26-
import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan} from "./parserUtils";
26+
import {binarySearch, compareTextSpans, getNextSibling, getPreviousSibling, TextSpan, hasFlowAnnotation, checkSyntaxErrors} from "./parserUtils";
2727
import {JavaScriptTypeMapping} from "./typeMapping";
2828
import path from "node:path";
2929
import {ExpressionStatement, TypeTreeExpression} from ".";
@@ -41,6 +41,8 @@ export class JavaScriptParser extends Parser {
4141
module: ts.ModuleKind.CommonJS,
4242
allowJs: true,
4343
esModuleInterop: true,
44+
experimentalDecorators: true,
45+
emitDecoratorMetadata: true
4446
};
4547
}
4648

@@ -147,19 +149,18 @@ export class JavaScriptParser extends Parser {
147149
continue;
148150
}
149151

150-
if (this.hasFlowAnnotation(sourceFile)) {
152+
if (hasFlowAnnotation(sourceFile)) {
151153
result.push(ParseError.build(this, input, relativeTo, ctx, new FlowSyntaxNotSupportedError(`Flow syntax not supported: ${input.path}`), null));
152154
continue;
153155
}
154156

155-
// ToDo: uncomment code after tests fixing
156-
// const syntaxErrors = this.checkSyntaxErrors(program, sourceFile);
157-
// if (syntaxErrors.length > 0) {
158-
// syntaxErrors.forEach(
159-
// e => result.push(ParseError.build(this, input, relativeTo, ctx, new SyntaxError(`Compiler error: ${e[0]} [${e[1]}]`), null))
160-
// );
161-
// continue;
162-
// }
157+
const syntaxErrors = checkSyntaxErrors(program, sourceFile);
158+
if (syntaxErrors.length > 0) {
159+
syntaxErrors.forEach(
160+
e => result.push(ParseError.build(this, input, relativeTo, ctx, new SyntaxError(`Compiler error: ${e[0]} [${e[1]}]`), null))
161+
);
162+
continue;
163+
}
163164

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

174-
private checkSyntaxErrors(program: ts.Program, sourceFile: ts.SourceFile) {
175-
const diagnostics = ts.getPreEmitDiagnostics(program, sourceFile);
176-
// checking Parsing and Syntax Errors
177-
let syntaxErrors : [errorMsg: string, errorCode: number][] = [];
178-
if (diagnostics.length > 0) {
179-
const errors = diagnostics.filter(d => d.code >= 1000 && d.code < 2000);
180-
if (errors.length > 0) {
181-
syntaxErrors = errors.map(e => {
182-
let errorMsg;
183-
if (e.file) {
184-
let {line, character} = ts.getLineAndCharacterOfPosition(e.file, e.start!);
185-
let message = ts.flattenDiagnosticMessageText(e.messageText, "\n");
186-
errorMsg = `${e.file.fileName} (${line + 1},${character + 1}): ${message}`;
187-
} else {
188-
errorMsg = ts.flattenDiagnosticMessageText(e.messageText, "\n");
189-
}
190-
return [errorMsg, e.code];
191-
});
192-
}
193-
}
194-
return syntaxErrors;
195-
}
196-
197-
private hasFlowAnnotation(sourceFile: ts.SourceFile) {
198-
if (sourceFile.fileName.endsWith('.js') || sourceFile.fileName.endsWith('.jsx')) {
199-
const comments = sourceFile.getFullText().match(/\/\*[\s\S]*?\*\/|\/\/.*(?=[\r\n])/g);
200-
if (comments) {
201-
return comments.some(comment => comment.includes("@flow"));
202-
}
203-
}
204-
return false;
205-
}
206-
207175
accept(path: string): boolean {
208176
return path.endsWith('.ts') || path.endsWith('.tsx') || path.endsWith('.js') || path.endsWith('.jsx');
209177
}
@@ -1866,7 +1834,7 @@ export class JavaScriptParserVisitor {
18661834
Markers.EMPTY
18671835
),
18681836
node.qualifier ? this.leftPadded(this.prefix(this.findChildNode(node, ts.SyntaxKind.DotToken)!), this.visit(node.qualifier)): null,
1869-
node.typeArguments ? this.mapTypeArguments(this.suffix(node.qualifier!), node.typeArguments) : null,
1837+
node.typeArguments ? this.mapTypeArguments(this.prefix(this.findChildNode(node, ts.SyntaxKind.LessThanToken)!), node.typeArguments) : null,
18701838
this.mapType(node)
18711839
);
18721840
}

‎openrewrite/src/javascript/parserUtils.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,51 @@ export function binarySearch<T>(arr: T[], target: T, compare: (a: T, b: T) => nu
180180
}
181181
return ~low; // Element not found, return bitwise complement of the insertion point
182182
}
183+
184+
export function hasFlowAnnotation(sourceFile: ts.SourceFile) {
185+
if (sourceFile.fileName.endsWith('.js') || sourceFile.fileName.endsWith('.jsx')) {
186+
const comments = sourceFile.getFullText().match(/\/\*[\s\S]*?\*\/|\/\/.*(?=[\r\n])/g);
187+
if (comments) {
188+
return comments.some(comment => comment.includes("@flow"));
189+
}
190+
}
191+
return false;
192+
}
193+
194+
export function checkSyntaxErrors(program: ts.Program, sourceFile: ts.SourceFile) {
195+
const diagnostics = ts.getPreEmitDiagnostics(program, sourceFile);
196+
// checking Parsing and Syntax Errors
197+
let syntaxErrors : [errorMsg: string, errorCode: number][] = [];
198+
if (diagnostics.length > 0) {
199+
const errors = diagnostics.filter(d => (d.category === ts.DiagnosticCategory.Error) && isCriticalDiagnostic(d.code));
200+
if (errors.length > 0) {
201+
syntaxErrors = errors.map(e => {
202+
let errorMsg;
203+
if (e.file) {
204+
let {line, character} = ts.getLineAndCharacterOfPosition(e.file, e.start!);
205+
let message = ts.flattenDiagnosticMessageText(e.messageText, "\n");
206+
errorMsg = `${e.file.fileName} (${line + 1},${character + 1}): ${message}`;
207+
} else {
208+
errorMsg = ts.flattenDiagnosticMessageText(e.messageText, "\n");
209+
}
210+
return [errorMsg, e.code];
211+
});
212+
}
213+
}
214+
return syntaxErrors;
215+
}
216+
217+
const additionalCriticalCodes = new Set([
218+
// Syntax errors
219+
17019, // "'{0}' at the end of a type is not valid TypeScript syntax. Did you mean to write '{1}'?"
220+
17020, // "'{0}' at the start of a type is not valid TypeScript syntax. Did you mean to write '{1}'?"
221+
222+
// Other critical errors
223+
]);
224+
225+
const excludedCodes = new Set([1039, 1064, 1101, 1107, 1111, 1155, 1166, 1170, 1183, 1203, 1207, 1215, 1238, 1239, 1240, 1241, 1244, 1250,
226+
1251, 1252, 1253, 1254, 1308, 1314, 1315, 1324, 1329, 1335, 1338, 1340, 1343, 1344, 1345, 1355, 1360, 1378, 1432]);
227+
228+
function isCriticalDiagnostic(code: number): boolean {
229+
return (code > 1000 && code < 2000 && !excludedCodes.has(code)) || additionalCriticalCodes.has(code);
230+
}

‎openrewrite/test/javascript/e2e/electron-server.files.test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,12 +281,10 @@ describe('electron-release-server files tests', () => {
281281
}
282282
283283
// If not the first changenote, prefix with new line
284-
var newChangeNote = !prevNotes.length ? '' : '
285-
';
284+
var newChangeNote = !prevNotes.length ? '' : '';
286285
287286
// Add the version name and notes
288-
newChangeNote += '## ' + newVersion.name + '
289-
' + newVersion.notes;
287+
newChangeNote += '## ' + newVersion.name + '' + newVersion.notes;
290288
291289
// Add the new changenote to the previous ones
292290
return prevNotes + newChangeNote;

‎openrewrite/test/javascript/parser/arrow.test.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,19 @@ describe('arrow mapping', () => {
195195
rewriteRun(
196196
//language=typescript
197197
typeScript(`
198-
prop: </*a*/const /*b*/ S extends SchemaObj, A, E>(
199-
name: string,
200-
schemas: S,
201-
self: TestFunction<
202-
A,
203-
E,
204-
R,
205-
[{ [K in keyof S]: Schema.Schema.Type<S[K]> }, V.TaskContext<V.RunnerTestCase<{}>> & V.TestContext]
206-
>,
207-
timeout?: number | V.TestOptions
208-
) => void
198+
class A {
199+
prop: </*a*/const /*b*/ S extends SchemaObj, A, E>(
200+
name: string,
201+
schemas: S,
202+
self: TestFunction<
203+
A,
204+
E,
205+
R,
206+
[{ [K in keyof S]: Schema.Schema.Type<S[K]> }, V.TaskContext<V.RunnerTestCase<{}>> & V.TestContext]
207+
>,
208+
timeout?: number | V.TestOptions
209+
) => void;
210+
}
209211
`)
210212
);
211213
});

‎openrewrite/test/javascript/parser/await.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ describe('await mapping', () => {
77
test('simple', () => {
88
rewriteRun(
99
//language=typescript
10-
typeScript('await 1')
10+
typeScript('export {}; await 1')
1111
);
1212
});
1313
});

‎openrewrite/test/javascript/parser/class.test.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ describe('class mapping', () => {
5252
typeScript('export class A {}')
5353
);
5454
});
55-
test('public', () => {
56-
rewriteRun(
57-
//language=typescript
58-
typeScript('public class A {}')
59-
);
60-
});
6155
test('export default', () => {
6256
rewriteRun(
6357
//language=typescript

‎openrewrite/test/javascript/parser/decorator.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('class decorator mapping', () => {
6767
`)
6868
);
6969
});
70-
test('decorator on class expression', () => {
70+
test.skip('decorator on class expression', () => {
7171
rewriteRun(
7272
//language=typescript
7373
typeScript(`

‎openrewrite/test/javascript/parser/enum.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {connect, disconnect, rewriteRun, typeScript} from '../testHarness';
22

3-
describe('empty mapping', () => {
3+
describe('enum mapping', () => {
44
beforeAll(() => connect());
55
afterAll(() => disconnect());
66

@@ -142,7 +142,7 @@ describe('empty mapping', () => {
142142
typeScript(`
143143
enum Test {
144144
A = "AA",
145-
B,
145+
B = undefined,
146146
C = 10,
147147
D = globalThis.NaN,
148148
E = (2 + 2),

‎openrewrite/test/javascript/parser/flowAnnotation.test.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ describe('flow annotation checking test', () => {
44
beforeAll(() => connect());
55
afterAll(() => disconnect());
66

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

21+
test('@flow in a comment in js', () => {
22+
const faultyTest = () => rewriteRun(
23+
//language=javascript
24+
javaScript(`
25+
/* @flow */
26+
27+
import Rocket from './rocket';
28+
import RocketLaunch from './rocket-launch';
29+
`)
30+
);
31+
32+
expect(faultyTest).toThrow(/FlowSyntaxNotSupportedError/);
33+
});
34+
2135
test('@flow in a multiline comment in js', () => {
2236
const faultyTest = () => rewriteRun(
2337
//language=javascript

‎openrewrite/test/javascript/parser/for.test.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ describe('for mapping', () => {
125125
test('for-of with await and comments', () => {
126126
rewriteRun(
127127
//language=typescript
128-
typeScript('/*a*/for/*b*/ await/*bb*/(/*c*/const /*d*/char /*e*/of /*f*/ "text"/*g*/)/*h*/ {/*j*/} /*k*/;/*l*/')
128+
typeScript('export {};/*a*/for/*b*/ await/*bb*/(/*c*/const /*d*/char /*e*/of /*f*/ "text"/*g*/)/*h*/ {/*j*/} /*k*/;/*l*/')
129129
);
130130
});
131131

@@ -181,12 +181,14 @@ describe('for mapping', () => {
181181
rewriteRun(
182182
//language=typescript
183183
typeScript(`
184-
let b;
185-
for (b in a) return !1;
186-
for (b of a) return !1;
187-
let i, str;
188-
for (i = 0; i < 9; i++) {
189-
str = str + i;
184+
function foo () {
185+
let b;
186+
for (b in a) return !1;
187+
for (b of a) return !1;
188+
let i, str;
189+
for (i = 0; i < 9; i++) {
190+
str = str + i;
191+
}
190192
}
191193
`)
192194
);

0 commit comments

Comments
 (0)