Skip to content

Commit 848eba1

Browse files
cristianoccristianoc
committed
fix: use correct binary paths for ReScript v12+
ReScript v12 changed its binary distribution to use @rescript/{platform}/bin.js. This PR updates the extension to correctly locate binaries for v12+ projects. Changes: - Add dedicated v12+ binary finding via @rescript/{target}/bin.js - Move legacy (<v12) binary finding to separate utils-legacy.ts files - Clean top-level version check routes to v12+ or legacy code path - Code Analyzer uses rescript-tools.exe for v12+, rescript-editor-analysis.exe for legacy - Add findRescriptToolsBinary export for v12+ projects No functional changes for pre-v12 projects - legacy behavior is preserved exactly.
1 parent 07d912f commit 848eba1

File tree

8 files changed

+775
-114
lines changed

8 files changed

+775
-114
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#### :bug: Bug fix
1616

17+
- Fix binary path resolution for ReScript v12+ projects. The extension now correctly locates binaries via `@rescript/{platform}/bin.js`.
1718
- Take namespace into account for incremental cleanup. https://github.com/rescript-lang/rescript-vscode/pull/1164
1819
- Potential race condition in incremental compilation. https://github.com/rescript-lang/rescript-vscode/pull/1167
1920
- Fix extension crash triggered by incremental compilation. https://github.com/rescript-lang/rescript-vscode/pull/1169

‎client/src/commands/code_analysis.ts‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ import {
1414
OutputChannel,
1515
StatusBarItem,
1616
} from "vscode";
17+
import * as semver from "semver";
1718
import {
1819
findProjectRootOfFileInDir,
1920
getBinaryPath,
2021
NormalizedPath,
22+
findReScriptVersion,
2123
} from "../utils";
2224

2325
export let statusBarItem = {
@@ -216,12 +218,20 @@ export const runCodeAnalysisWithReanalyze = (
216218
currentDocument.uri.fsPath,
217219
);
218220

219-
// This little weird lookup is because in the legacy setup reanalyze needs to be
220-
// run from the analysis binary, whereas in the new setup it's run from the tools
221-
// binary.
222-
let binaryPath =
223-
getBinaryPath("rescript-tools.exe", projectRootPath) ??
224-
getBinaryPath("rescript-editor-analysis.exe");
221+
// v12+: reanalyze is run from rescript-tools.exe
222+
// Legacy: reanalyze is run from rescript-editor-analysis.exe
223+
const rescriptVersion = findReScriptVersion(projectRootPath);
224+
const isReScript12OrHigher =
225+
rescriptVersion != null &&
226+
semver.valid(rescriptVersion) &&
227+
semver.gte(rescriptVersion, "12.0.0");
228+
229+
let binaryPath: string | null;
230+
if (isReScript12OrHigher) {
231+
binaryPath = getBinaryPath("rescript-tools.exe", projectRootPath);
232+
} else {
233+
binaryPath = getBinaryPath("rescript-editor-analysis.exe", projectRootPath);
234+
}
225235

226236
if (binaryPath === null) {
227237
window.showErrorMessage("Binary executable not found.");

‎client/src/utils-legacy.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* Legacy binary finding for ReScript < 12.
3+
* This code is kept separate to avoid polluting the main utils with pre-v12 complexity.
4+
*/
5+
6+
import * as fs from "fs";
7+
import * as path from "path";
8+
import { NormalizedPath } from "./utils";
9+
10+
type binaryName = "rescript-editor-analysis.exe" | "rescript-tools.exe";
11+
12+
// Legacy format: no hyphen (e.g., "darwinarm64")
13+
const platformDir =
14+
process.arch === "arm64" ? process.platform + process.arch : process.platform;
15+
16+
const getLegacyBinaryDevPath = (b: binaryName) =>
17+
path.join(path.dirname(__dirname), "..", "analysis", b);
18+
19+
export const getLegacyBinaryProdPath = (b: binaryName) =>
20+
path.join(
21+
path.dirname(__dirname),
22+
"..",
23+
"server",
24+
"analysis_binaries",
25+
platformDir,
26+
b,
27+
);
28+
29+
/**
30+
* Finds binaries for ReScript < 12 using old path structure.
31+
* Tries project binary first, then falls back to builtin binaries.
32+
*/
33+
export const getBinaryPathLegacy = (
34+
projectRootPath: NormalizedPath | null,
35+
binaryName: binaryName,
36+
): string | null => {
37+
// Try project binary first
38+
if (projectRootPath != null) {
39+
const binaryFromCompilerPackage = path.join(
40+
projectRootPath,
41+
"node_modules",
42+
"rescript",
43+
platformDir,
44+
binaryName,
45+
);
46+
if (fs.existsSync(binaryFromCompilerPackage)) {
47+
return binaryFromCompilerPackage;
48+
}
49+
}
50+
51+
// Fall back to builtin binaries
52+
if (fs.existsSync(getLegacyBinaryDevPath(binaryName))) {
53+
return getLegacyBinaryDevPath(binaryName);
54+
} else if (fs.existsSync(getLegacyBinaryProdPath(binaryName))) {
55+
return getLegacyBinaryProdPath(binaryName);
56+
}
57+
58+
return null;
59+
};
60+

‎client/src/utils.ts‎

Lines changed: 96 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import * as path from "path";
22
import * as fs from "fs";
33
import * as os from "os";
44
import { DocumentUri } from "vscode-languageclient";
5+
import * as semver from "semver";
6+
import { getBinaryPathLegacy } from "./utils-legacy";
57

68
/*
79
* Much of the code in here is duplicated from the server code.
@@ -32,42 +34,110 @@ export function normalizePath(filePath: string | null): NormalizedPath | null {
3234

3335
type binaryName = "rescript-editor-analysis.exe" | "rescript-tools.exe";
3436

35-
const platformDir =
36-
process.arch === "arm64" ? process.platform + process.arch : process.platform;
37+
// v12+ format: with hyphen (e.g., "darwin-arm64")
38+
const platformTarget = `${process.platform}-${process.arch}`;
3739

38-
const getLegacyBinaryDevPath = (b: binaryName) =>
39-
path.join(path.dirname(__dirname), "..", "analysis", b);
40+
// ============================================================================
41+
// Version Detection
42+
// ============================================================================
4043

41-
export const getLegacyBinaryProdPath = (b: binaryName) =>
42-
path.join(
43-
path.dirname(__dirname),
44-
"..",
45-
"server",
46-
"analysis_binaries",
47-
platformDir,
48-
b,
44+
/**
45+
* Finds the ReScript version from package.json in the project.
46+
*/
47+
export const findReScriptVersion = (
48+
projectRootPath: NormalizedPath | null,
49+
): string | null => {
50+
if (projectRootPath == null) {
51+
return null;
52+
}
53+
try {
54+
const packageJsonPath = path.join(
55+
projectRootPath,
56+
"node_modules",
57+
"rescript",
58+
"package.json",
59+
);
60+
if (!fs.existsSync(packageJsonPath)) {
61+
return null;
62+
}
63+
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf-8"));
64+
return packageJson.version ?? null;
65+
} catch {
66+
return null;
67+
}
68+
};
69+
70+
// ============================================================================
71+
// ReScript 12+ Binary Finding (Clean, self-contained)
72+
// ============================================================================
73+
74+
/**
75+
* Finds binaries for ReScript 12+ using @rescript/${target}/bin.js structure.
76+
* This is the single source of truth for binary locations in v12+.
77+
* Returns null if binary not found, throws on critical errors.
78+
*/
79+
const getBinaryPathReScript12 = (
80+
projectRootPath: NormalizedPath,
81+
binaryName: binaryName,
82+
): string | null => {
83+
const binJsPath = path.join(
84+
projectRootPath,
85+
"node_modules",
86+
"@rescript",
87+
platformTarget,
88+
"bin.js",
4989
);
5090

91+
if (!fs.existsSync(binJsPath)) {
92+
return null;
93+
}
94+
95+
// Read bin.js and extract the binary path
96+
// bin.js exports binPaths object with paths to binaries
97+
const binDir = path.join(
98+
projectRootPath,
99+
"node_modules",
100+
"@rescript",
101+
platformTarget,
102+
"bin",
103+
);
104+
105+
let binaryPath: string | null = null;
106+
if (binaryName === "rescript-tools.exe") {
107+
binaryPath = path.join(binDir, "rescript-tools.exe");
108+
} else if (binaryName === "rescript-editor-analysis.exe") {
109+
binaryPath = path.join(binDir, "rescript-editor-analysis.exe");
110+
}
111+
112+
if (binaryPath != null && fs.existsSync(binaryPath)) {
113+
return binaryPath;
114+
}
115+
return null;
116+
};
117+
118+
// ============================================================================
119+
// Main Binary Finding Function (Routes to v12 or legacy)
120+
// ============================================================================
121+
122+
/**
123+
* Finds a ReScript binary, routing to v12+ or legacy implementation.
124+
* Top-level if separates the two code paths completely.
125+
*/
51126
export const getBinaryPath = (
52127
binaryName: "rescript-editor-analysis.exe" | "rescript-tools.exe",
53128
projectRootPath: NormalizedPath | null = null,
54129
): string | null => {
55-
const binaryFromCompilerPackage = path.join(
56-
projectRootPath ?? "",
57-
"node_modules",
58-
"rescript",
59-
platformDir,
60-
binaryName,
61-
);
130+
const rescriptVersion = findReScriptVersion(projectRootPath);
131+
const isReScript12OrHigher =
132+
rescriptVersion != null &&
133+
semver.valid(rescriptVersion) &&
134+
semver.gte(rescriptVersion, "12.0.0");
62135

63-
if (projectRootPath != null && fs.existsSync(binaryFromCompilerPackage)) {
64-
return binaryFromCompilerPackage;
65-
} else if (fs.existsSync(getLegacyBinaryDevPath(binaryName))) {
66-
return getLegacyBinaryDevPath(binaryName);
67-
} else if (fs.existsSync(getLegacyBinaryProdPath(binaryName))) {
68-
return getLegacyBinaryProdPath(binaryName);
136+
// Top-level separation: v12+ or legacy
137+
if (isReScript12OrHigher && projectRootPath != null) {
138+
return getBinaryPathReScript12(projectRootPath, binaryName);
69139
} else {
70-
return null;
140+
return getBinaryPathLegacy(projectRootPath, binaryName);
71141
}
72142
};
73143

0 commit comments

Comments
 (0)