Skip to content

Commit a957713

Browse files
JavaScript: Allow incomplete \x hex escape sequences (#6174)
Legacy JS source files can contain incomplete `\x` hex code escape sequences, as in this regex example: `/\x-(.*)/`. The parser should not fail when parsing JS source files like this. Additionally, the parser will now allow object literals with multiple properties of the same name. There is a corresponding `org.openrewrite.javascript.migrate.es6.remove-duplicate-object-keys` ES6 upgrade recipe to deduplicate these, as ES6 does not allow this anymore.
1 parent 9a62bbf commit a957713

14 files changed

Lines changed: 538 additions & 117 deletions

File tree

rewrite-java/src/main/java/org/openrewrite/java/RemoveImport.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,10 @@ public RemoveImport(String type, boolean force) {
112112
JavaSourceFile c = cu;
113113

114114
boolean keepImport = !force && (typeUsed || !otherTypesInPackageUsed.isEmpty() && type.endsWith(".*"));
115-
AtomicReference<Space> spaceForNextImport = new AtomicReference<>();
115+
AtomicReference<@Nullable Space> spaceForNextImport = new AtomicReference<>();
116116
c = c.withImports(ListUtils.flatMap(c.getImports(), import_ -> {
117-
if (spaceForNextImport.get() != null) {
118-
Space removedPrefix = spaceForNextImport.get();
117+
Space removedPrefix = spaceForNextImport.get();
118+
if (removedPrefix != null) {
119119
Space currentPrefix = import_.getPrefix();
120120
if (removedPrefix.getLastWhitespace().isEmpty() ||
121121
(countTrailingLinebreaks(removedPrefix) > countTrailingLinebreaks(currentPrefix))) {

rewrite-javascript/rewrite/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ export * from "./run";
3636
// register all recipes in this package
3737
export async function activate(registry: RecipeRegistry): Promise<void> {
3838
const {OrderImports} = await import("./recipe/index.js");
39-
const {ModernizeOctalLiterals} = await import("./javascript/migrate/es6/index.js");
39+
const {ModernizeOctalLiterals, RemoveDuplicateObjectKeys} = await import("./javascript/migrate/es6/index.js");
4040
registry.register(OrderImports);
4141
registry.register(ModernizeOctalLiterals);
42+
registry.register(RemoveDuplicateObjectKeys);
4243
}
4344

4445
RpcCodecs.registerCodec(MarkersKind.ParseExceptionResult, {
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import {J} from "../java";
18+
import {produce} from "immer";
19+
20+
/**
21+
* Applies the prefix from a removed element to the next element.
22+
*
23+
* This is used when removing elements from a list to preserve formatting:
24+
* - If the next element has no comments (or only an inline trailing comment to remove),
25+
* uses the removed element's prefix whitespace
26+
* - If the next element has comments to preserve, keeps the next element's whitespace
27+
* to maintain proper spacing (e.g., blank lines before comments)
28+
* - Removes inline line comments (//...) that were on the removed element's line
29+
*
30+
* @param removedElement The element that was removed
31+
* @param nextElement The element that follows the removed one
32+
* @returns The next element with adjusted prefix, or the original if no changes needed
33+
*/
34+
export function applyRemovedElementPrefix<T extends J>(removedElement: J, nextElement: T): T {
35+
if (!removedElement.prefix || !nextElement.prefix) {
36+
return nextElement;
37+
}
38+
39+
const removedPrefix = removedElement.prefix;
40+
const currentPrefix = nextElement.prefix;
41+
const currentComments = currentPrefix.comments || [];
42+
43+
// If the next element has no comments, just use the removed element's prefix entirely
44+
if (currentComments.length === 0) {
45+
if (currentPrefix === removedPrefix) {
46+
return nextElement;
47+
}
48+
return produce(nextElement, draft => {
49+
draft.prefix = removedPrefix;
50+
});
51+
}
52+
53+
// The next element has comments - check if we need to remove inline line comments
54+
const currentWhitespace = currentPrefix.whitespace || '';
55+
const hasLeadingNewline = /[\r\n]/.test(currentWhitespace);
56+
57+
let commentsToKeep = currentComments;
58+
if (!hasLeadingNewline && currentComments.length > 0) {
59+
// First comment has no leading newline - check if it's an inline line comment
60+
const firstComment: any = currentComments[0];
61+
const commentText = firstComment.text || firstComment.message || '';
62+
const isLineComment = commentText.includes('//') || firstComment.multiline === false;
63+
64+
if (isLineComment) {
65+
// Remove the inline line comment
66+
commentsToKeep = currentComments.slice(1);
67+
}
68+
}
69+
70+
// If we still have comments after filtering, use removed element's whitespace with filtered comments
71+
if (commentsToKeep.length > 0) {
72+
return produce(nextElement, draft => {
73+
draft.prefix = {
74+
...removedPrefix,
75+
comments: commentsToKeep
76+
};
77+
});
78+
}
79+
80+
// No comments left after filtering, use removed element's prefix entirely
81+
return produce(nextElement, draft => {
82+
draft.prefix = removedPrefix;
83+
});
84+
}

rewrite-javascript/rewrite/src/javascript/migrate/es6/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
*/
1616

1717
export {ModernizeOctalLiterals} from "./modernize-octal-literals";
18+
export {RemoveDuplicateObjectKeys} from "./remove-duplicate-object-keys";

rewrite-javascript/rewrite/src/javascript/migrate/es6/modernize-octal-literals.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export class ModernizeOctalLiterals extends Recipe {
2929
async editor(): Promise<TreeVisitor<any, ExecutionContext>> {
3030
return new class extends JavaScriptVisitor<ExecutionContext> {
3131

32-
protected async visitLiteral(literal: J.Literal, p: ExecutionContext): Promise<J | undefined> {
32+
protected async visitLiteral(literal: J.Literal, _ctx: ExecutionContext): Promise<J | undefined> {
3333
// Only process numeric literals
3434
if (typeof literal.value !== 'number') {
3535
return literal;
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import {Recipe} from "../../../recipe";
18+
import {TreeVisitor} from "../../../visitor";
19+
import {ExecutionContext} from "../../../execution";
20+
import {JavaScriptVisitor} from "../../visitor";
21+
import {J} from "../../../java";
22+
import {JS} from "../../tree";
23+
import {produce} from "immer";
24+
import {applyRemovedElementPrefix} from "../../../java/formatting-utils";
25+
26+
export class RemoveDuplicateObjectKeys extends Recipe {
27+
name = "org.openrewrite.javascript.migrate.es6.remove-duplicate-object-keys";
28+
displayName = "Remove duplicate object keys";
29+
description = "Remove duplicate keys in object literals, keeping only the last occurrence (last-wins semantics).";
30+
31+
async editor(): Promise<TreeVisitor<any, ExecutionContext>> {
32+
return new class extends JavaScriptVisitor<ExecutionContext> {
33+
34+
protected async visitNewClass(newClass: J.NewClass, ctx: ExecutionContext): Promise<J | undefined> {
35+
newClass = await super.visitNewClass(newClass, ctx) as J.NewClass;
36+
37+
// Only process object literals (NewClass with body but no class or arguments)
38+
if (!newClass.body || newClass.class || (newClass.arguments?.elements && newClass.arguments.elements.length > 0)) {
39+
return newClass;
40+
}
41+
42+
const statements = newClass.body.statements;
43+
if (!statements || statements.length === 0) {
44+
return newClass;
45+
}
46+
47+
// Build a map of property names to their last occurrence index
48+
const propertyNameToLastIndex = new Map<string, number>();
49+
const propertyNames: (string | null)[] = [];
50+
51+
for (let i = 0; i < statements.length; i++) {
52+
const stmt = statements[i];
53+
if (stmt.element.kind === JS.Kind.PropertyAssignment) {
54+
const prop = stmt.element as JS.PropertyAssignment;
55+
const propName = this.getPropertyName(prop);
56+
propertyNames.push(propName);
57+
58+
if (propName !== null) {
59+
propertyNameToLastIndex.set(propName, i);
60+
}
61+
} else {
62+
propertyNames.push(null);
63+
}
64+
}
65+
66+
// Check if there are any duplicates to remove
67+
const indicesToRemove = new Set<number>();
68+
for (let i = 0; i < propertyNames.length; i++) {
69+
const propName = propertyNames[i];
70+
if (propName !== null) {
71+
const lastIndex = propertyNameToLastIndex.get(propName)!;
72+
if (i < lastIndex) {
73+
indicesToRemove.add(i);
74+
}
75+
}
76+
}
77+
78+
if (indicesToRemove.size === 0) {
79+
return newClass;
80+
}
81+
82+
// Remove duplicate properties and adjust prefixes
83+
return produce(newClass, draft => {
84+
const filteredStatements: typeof statements = [];
85+
let removedElement: J | undefined = undefined;
86+
87+
for (let i = 0; i < statements.length; i++) {
88+
if (indicesToRemove.has(i)) {
89+
// Track the first removed element
90+
if (!removedElement && statements[i].element) {
91+
removedElement = statements[i].element;
92+
}
93+
continue;
94+
}
95+
96+
const stmt = statements[i];
97+
98+
// If we removed previous elements, apply the removed element's prefix to this one
99+
if (removedElement) {
100+
const adjustedElement = applyRemovedElementPrefix(removedElement, stmt.element);
101+
filteredStatements.push({
102+
...stmt,
103+
element: adjustedElement
104+
});
105+
removedElement = undefined;
106+
} else {
107+
filteredStatements.push(stmt);
108+
}
109+
}
110+
111+
draft.body!.statements = filteredStatements;
112+
});
113+
}
114+
115+
private getPropertyName(prop: JS.PropertyAssignment): string | null {
116+
const name = prop.name.element;
117+
118+
// Handle identifier: { foo: 1 }
119+
if (name.kind === J.Kind.Identifier) {
120+
return (name as J.Identifier).simpleName;
121+
}
122+
123+
// Handle string literal: { "foo": 1 }
124+
if (name.kind === J.Kind.Literal) {
125+
const literal = name as J.Literal;
126+
if (typeof literal.value === 'string') {
127+
return literal.value;
128+
}
129+
}
130+
131+
// For computed properties { [expr]: 1 }, we can't statically determine the name
132+
// So we return null and don't consider them for deduplication
133+
return null;
134+
}
135+
}
136+
}
137+
}

rewrite-javascript/rewrite/src/javascript/parser-utils.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ export function checkSyntaxErrors(program: ts.Program, sourceFile: ts.SourceFile
355355
// checking Parsing and Syntax Errors
356356
let syntaxErrors : [errorMsg: string, errorCode: number][] = [];
357357
if (diagnostics.length > 0) {
358-
const errors = diagnostics.filter(d => (d.category === ts.DiagnosticCategory.Error) && isCriticalDiagnostic(d.code));
358+
const errors = diagnostics.filter(d => (d.category === ts.DiagnosticCategory.Error) && isCriticalDiagnostic(d.code, sourceFile));
359359
if (errors.length > 0) {
360360
syntaxErrors = errors.map(e => {
361361
let errorMsg;
@@ -385,10 +385,9 @@ const additionalCriticalCodes = new Set([
385385
const excludedCodes = new Set([
386386
1039, // Initializers are not allowed in ambient contexts.
387387
1064, // The return type of an async function or method must be the global Promise<T> type. Did you mean to write 'Promise<{0}>'?
388-
1101, // 'with' statements are not allowed in strict mode.
389388
1107, // Jump target cannot cross function boundary.
390389
1111, // Private field '{0}' must be declared in an enclosing class.
391-
1121, // Octal literals are not allowed. Use the syntax '{0}'.
390+
1117, // An object literal cannot have multiple properties with the same name.
392391
1155, // '{0}' declarations must be initialized.
393392
1166, // A computed property name in a class property declaration must have a simple literal type or a 'unique symbol' type.
394393
1170, // A computed property name in a type literal must refer to an expression whose type is a literal type or a 'unique symbol' type.
@@ -424,7 +423,25 @@ const excludedCodes = new Set([
424423
1432, // Top-level 'for await' loops are only allowed when the 'module' option is set to 'es2022', 'esnext', 'system', 'node16', 'node18', 'node20', 'nodenext', or 'preserve', and the 'target' option is set to 'es2017' or higher.
425424
]);
426425

427-
function isCriticalDiagnostic(code: number): boolean {
426+
// Errors to exclude only for JavaScript files (.js, .jsx, .mjs, .cjs)
427+
// TypeScript files (.ts, .tsx, .mts, .cts) should still report these as errors
428+
const jsOnlyExcludedCodes = new Set([
429+
1101, // 'with' statements are not allowed in strict mode.
430+
1121, // Octal literals are not allowed. Use the syntax '{0}'.
431+
1125, // Hexadecimal digit expected.
432+
]);
433+
434+
function isCriticalDiagnostic(code: number, sourceFile: ts.SourceFile): boolean {
435+
// Check if this error should be excluded for JavaScript files
436+
if (jsOnlyExcludedCodes.has(code)) {
437+
const fileName = sourceFile.fileName.toLowerCase();
438+
const isJavaScript = fileName.endsWith('.js') || fileName.endsWith('.jsx') ||
439+
fileName.endsWith('.mjs') || fileName.endsWith('.cjs');
440+
if (isJavaScript) {
441+
return false; // Not critical for JS files
442+
}
443+
}
444+
428445
return (code > 1000 && code < 2000 && !excludedCodes.has(code)) || additionalCriticalCodes.has(code);
429446
}
430447

0 commit comments

Comments
 (0)