Skip to content

Commit 3f51882

Browse files
committed
Fix RPC lifecycle and default export method name resolution
- Remove premature client.shutdown() in JavaToJavaScriptRpcTest that killed the RPC process between tests without clearing the ThreadLocal reference, causing 324/377 test failures. The @AfterSuite already handles cleanup. - Fix default import method name detection in type-mapping.ts to check syntactic import form (isImportClause/isNamespaceImport) rather than relying on aliasedSymbol.name, which can be an internal name (e.g. "e" for express) instead of "default". - Add null-safety fallbacks (?? emptySpace) from origin/main merge.
1 parent 0e76221 commit 3f51882

4 files changed

Lines changed: 60 additions & 12 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ export class JavaScriptParserVisitor {
644644
private rightPadded<T extends J | boolean>(t: T, trailing: J.Space, markers?: Markers): J.RightPadded<T> {
645645
// For tree types (J), use intersection type: spread element and add padding
646646
// For primitives (boolean), use wrapper type with element property
647-
const padding: J.Suffix = { after: trailing, markers: markers ?? emptyMarkers };
647+
const padding: J.Suffix = { after: trailing ?? emptySpace, markers: markers ?? emptyMarkers };
648648
if (typeof t === 'boolean') {
649649
return {
650650
element: t,
@@ -685,7 +685,7 @@ export class JavaScriptParserVisitor {
685685

686686
private leftPadded<T extends J | J.Space | number | string | boolean>(before: J.Space, t: T, markers?: Markers): J.LeftPadded<T> {
687687
// For primitives (boolean, number, string), use wrapper type with element property
688-
const padding: J.Prefix = { before: before, markers: markers ?? emptyMarkers };
688+
const padding: J.Prefix = { before: before ?? emptySpace, markers: markers ?? emptyMarkers };
689689
if (typeof t === 'boolean' || typeof t === 'number' || typeof t === 'string') {
690690
return {
691691
element: t,

rewrite-javascript/rewrite/src/javascript/type-mapping.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -830,8 +830,13 @@ export class JavaScriptTypeMapping {
830830
flags: 0, // TODO - determine flags
831831
fullyQualifiedName: moduleSpecifier
832832
} as Type.FullyQualified;
833-
// For aliased imports, use the original function name from the aliased symbol
834-
if (aliasedSymbol && aliasedSymbol.name) {
833+
// Determine the method name based on import type
834+
const isDefaultImport = exprSymbol!.declarations?.some(d =>
835+
ts.isImportClause(d) || ts.isNamespaceImport(d)
836+
);
837+
if (isDefaultImport) {
838+
methodName = '<default>';
839+
} else if (aliasedSymbol && aliasedSymbol.name) {
835840
methodName = aliasedSymbol.name;
836841
} else {
837842
methodName = '<default>';

rewrite-javascript/rewrite/test/javascript/type-mapping.test.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* limitations under the License.
1717
*/
1818
import {RecipeSpec} from "../../src/test";
19-
import {javascript, JavaScriptVisitor, npm, packageJson, tsx, typescript} from "../../src/javascript";
19+
import {javascript, JavaScriptVisitor, JS, npm, packageJson, tsx, typescript} from "../../src/javascript";
2020
import {J, Type} from "../../src/java";
2121
import {ExecutionContext, foundSearchResult, Recipe} from "../../src";
2222
import {withDir} from "tmp-promise";
@@ -456,6 +456,53 @@ describe('JavaScript type mapping', () => {
456456
}, {unsafeCleanup: true});
457457
})
458458

459+
test('default export function call has <default> method name', async () => {
460+
const spec = new RecipeSpec();
461+
spec.recipe = markTypes((node, type) => {
462+
if (Type.isMethod(type)) {
463+
const method = type as Type.Method;
464+
if (FullyQualified.getFullyQualifiedName(method.declaringType) === 'express') {
465+
return `${FullyQualified.getFullyQualifiedName(method.declaringType)} ${method.name}`;
466+
}
467+
}
468+
return null;
469+
});
470+
471+
await withDir(async (repo) => {
472+
await spec.rewriteRun(
473+
npm(
474+
repo.path,
475+
typescript(
476+
`
477+
import express from 'express';
478+
const app = express();
479+
`,
480+
//@formatter:off
481+
`
482+
import express from 'express';
483+
const app = /*~~(express <default>)~~>*/express();
484+
`
485+
//@formatter:on
486+
),
487+
packageJson(
488+
`
489+
{
490+
"name": "test-project",
491+
"version": "1.0.0",
492+
"dependencies": {
493+
"express": "^4.18.2"
494+
},
495+
"devDependencies": {
496+
"@types/express": "^4.17.21"
497+
}
498+
}
499+
`
500+
)
501+
)
502+
);
503+
}, {unsafeCleanup: true});
504+
});
505+
459506
test('deprecated node methods with ES6 imports', async () => {
460507
const spec = new RecipeSpec();
461508
spec.recipe = markTypes((_, type) => {

rewrite-javascript/src/integTest/java/org/openrewrite/javascript/rpc/JavaToJavaScriptRpcTest.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,9 @@ static void beforeSuite() {
5555
@Override
5656
public J preVisit(J tree, ExecutionContext ctx) {
5757
SourceFile t = (SourceFile) modifyAll.getVisitor().visitNonNull(tree, ctx);
58-
try {
59-
assertThat(t.printAll()).isEqualTo(((SourceFile) tree).printAll());
60-
stopAfterPreVisit();
61-
return tree;
62-
} finally {
63-
client.shutdown();
64-
}
58+
assertThat(t.printAll()).isEqualTo(((SourceFile) tree).printAll());
59+
stopAfterPreVisit();
60+
return tree;
6561
}
6662
};
6763
}));

0 commit comments

Comments
 (0)