From e60f1e124d03e4ce5f68d7808a2b4d1c5cf26192 Mon Sep 17 00:00:00 2001 From: Totto16 Date: Mon, 9 Dec 2024 20:26:42 +0100 Subject: [PATCH 1/2] fix: fix meson version getter fix regex and fix slicing see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match#return_value ony why not to use the g flag --- src/introspection.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/introspection.ts b/src/introspection.ts index eff9996..c549ade 100644 --- a/src/introspection.ts +++ b/src/introspection.ts @@ -57,11 +57,11 @@ export async function getMesonBenchmarks(buildDir: string) { } export async function getMesonVersion(): Promise<[number, number, number]> { - const MESON_VERSION_REGEX = /^(\d+)\.(\d+)\.(\d+)/g; + const MESON_VERSION_REGEX = /^(\d+)\.(\d+)\.(\d+)/; const { stdout } = await exec(extensionConfiguration("mesonPath"), ["--version"]); const match = stdout.trim().match(MESON_VERSION_REGEX); - if (match) { - return match.slice(1, 3).map((s) => Number.parseInt(s)) as [number, number, number]; + if (match && match.length >= 4) { + return match.slice(1, 4).map((s) => Number.parseInt(s)) as [number, number, number]; } else throw new Error("Meson version doesn't match expected output: " + stdout.trim()); } From c0106386d0780cd0f96455950d93eef3c597c753 Mon Sep 17 00:00:00 2001 From: Totto16 Date: Mon, 9 Dec 2024 20:43:34 +0100 Subject: [PATCH 2/2] feat: add proper version type this type allows pretty printing, version comparison, and runtime checks, to detect such errors as the one fixed in the previous commit also use the proper compare methods, where we previously did it by hand add class for ToolCheckResult: it is now easier to check if it's an error or not --- src/formatters.ts | 8 ++--- src/introspection.ts | 8 +++-- src/linters.ts | 8 ++--- src/tests.ts | 2 +- src/tools/muon.ts | 21 +++++++------ src/types.ts | 56 ++++++++++++++++++++++++++++++++-- src/utils.ts | 9 +++++- src/version.ts | 72 ++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 160 insertions(+), 24 deletions(-) create mode 100644 src/version.ts diff --git a/src/formatters.ts b/src/formatters.ts index 97b8f04..1087de8 100644 --- a/src/formatters.ts +++ b/src/formatters.ts @@ -27,16 +27,16 @@ async function reloadFormatters(sourceRoot: string, context: vscode.ExtensionCon const name = extensionConfiguration("formatting").provider; const props = formatters[name]; - const { tool, error } = await props.check(); - if (error) { - getOutputChannel().appendLine(`Failed to enable formatter ${name}: ${error}`); + const checkResult = await props.check(); + if (checkResult.isError()) { + getOutputChannel().appendLine(`Failed to enable formatter ${name}: ${checkResult.error}`); getOutputChannel().show(true); return disposables; } const sub = vscode.languages.registerDocumentFormattingEditProvider("meson", { async provideDocumentFormattingEdits(document: vscode.TextDocument): Promise { - return await props.format(tool!, sourceRoot, document); + return await props.format(checkResult.tool, sourceRoot, document); }, }); diff --git a/src/introspection.ts b/src/introspection.ts index c549ade..734ff5c 100644 --- a/src/introspection.ts +++ b/src/introspection.ts @@ -1,6 +1,7 @@ import * as path from "path"; import { exec, extensionConfiguration, parseJSONFileIfExists, getOutputChannel } from "./utils"; import { Targets, Dependencies, BuildOptions, Tests, ProjectInfo, Compilers } from "./types"; +import { type VersionArray, Version } from "./version"; export function getIntrospectionFile(buildDir: string, filename: string) { return path.join(buildDir, path.join("meson-info", filename)); @@ -23,7 +24,8 @@ async function introspectMeson(buildDir: string, filename: string, introspect export async function getMesonTargets(buildDir: string) { const parsed = await introspectMeson(buildDir, "intro-targets.json", "--targets"); - if ((await getMesonVersion())[1] < 50) { + // if (mesonVersion < 0.50.0) + if ((await getMesonVersion()).compare([0, 50, 0]) < 0) { return parsed.map((t) => { if (typeof t.filename === "string") t.filename = [t.filename]; // Old versions would directly pass a string with only 1 filename on the target return t; @@ -56,12 +58,12 @@ export async function getMesonBenchmarks(buildDir: string) { return introspectMeson(buildDir, "intro-benchmarks.json", "--benchmarks"); } -export async function getMesonVersion(): Promise<[number, number, number]> { +export async function getMesonVersion(): Promise { const MESON_VERSION_REGEX = /^(\d+)\.(\d+)\.(\d+)/; const { stdout } = await exec(extensionConfiguration("mesonPath"), ["--version"]); const match = stdout.trim().match(MESON_VERSION_REGEX); if (match && match.length >= 4) { - return match.slice(1, 4).map((s) => Number.parseInt(s)) as [number, number, number]; + return new Version(match.slice(1, 4).map((s) => Number.parseInt(s)) as VersionArray); } else throw new Error("Meson version doesn't match expected output: " + stdout.trim()); } diff --git a/src/linters.ts b/src/linters.ts index 536b94d..5df7a29 100644 --- a/src/linters.ts +++ b/src/linters.ts @@ -38,14 +38,14 @@ async function reloadLinters( } const props = linters[name]; - const { tool, error } = await props.check(); - if (error) { - getOutputChannel().appendLine(`Failed to enable linter ${name}: ${error}`); + const checkResult = await props.check(); + if (checkResult.isError()) { + getOutputChannel().appendLine(`Failed to enable linter ${name}: ${checkResult.error}`); getOutputChannel().show(true); continue; } - const linter = async (document: vscode.TextDocument) => await props.lint(tool!, sourceRoot, document); + const linter = async (document: vscode.TextDocument) => await props.lint(checkResult.tool, sourceRoot, document); enabledLinters.push(linter); } diff --git a/src/tests.ts b/src/tests.ts index b60eb66..c0df752 100644 --- a/src/tests.ts +++ b/src/tests.ts @@ -88,7 +88,7 @@ export async function testDebugHandler( relevantTests.some((test) => test.depends.some((dep) => dep == target.id)), ); - var args = ["compile", "-C", buildDir]; + let args = ["compile", "-C", buildDir]; requiredTargets.forEach((target) => { args.push(target.name); }); diff --git a/src/tools/muon.ts b/src/tools/muon.ts index 59363e7..619401b 100644 --- a/src/tools/muon.ts +++ b/src/tools/muon.ts @@ -1,6 +1,7 @@ import * as vscode from "vscode"; import { ExecResult, exec, execFeed, extensionConfiguration, getOutputChannel } from "../utils"; -import { Tool } from "../types"; +import { Tool, ToolCheckResult } from "../types"; +import { Version, type VersionArray } from "../version"; export async function lint(muon: Tool, root: string, document: vscode.TextDocument): Promise { const { stdout, stderr } = await execFeed( @@ -10,8 +11,9 @@ export async function lint(muon: Tool, root: string, document: vscode.TextDocume document.getText(), ); - var out; - if (muon.version[0] == 0 && muon.version[1] <= 3) { + let out: string; + // if (muonVersion < 0.4.0) + if (muon.version.compare([0, 4, 0]) < 0) { out = stderr; } else { out = stdout; @@ -54,7 +56,8 @@ export async function format(muon: Tool, root: string, document: vscode.TextDocu let args = ["fmt"]; - if (muon.version[0] == 0 && muon.version[1] == 0) { + // if (muonVersion < 0.1.0) + if (muon.version.compare([0, 1, 0]) < 0) { args = ["fmt_unstable"]; } @@ -79,7 +82,7 @@ export async function format(muon: Tool, root: string, document: vscode.TextDocu return [new vscode.TextEdit(documentRange, stdout)]; } -export async function check(): Promise<{ tool?: Tool; error?: string }> { +export async function check(): Promise { const muon_path = extensionConfiguration("muonPath"); let stdout: string, stderr: string; @@ -88,12 +91,12 @@ export async function check(): Promise<{ tool?: Tool; error?: string }> { } catch (e) { const { error, stdout, stderr } = e as ExecResult; console.log(error); - return { error: error!.message }; + return ToolCheckResult.newError(error!.message); } const line1 = stdout.split("\n")[0].split(" "); if (line1.length !== 2) { - return { error: `Invalid version string: ${line1}` }; + return ToolCheckResult.newError(`Invalid version string: ${line1}`); } const ver = line1[1] @@ -105,7 +108,7 @@ export async function check(): Promise<{ tool?: Tool; error?: string }> { } return Number.parseInt(s); - }) as [number, number, number]; + }) as VersionArray; - return { tool: { path: muon_path, version: ver } }; + return ToolCheckResult.newTool({ path: muon_path, version: new Version(ver) }); } diff --git a/src/types.ts b/src/types.ts index ad7f559..d13dcb0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,8 +1,60 @@ import * as vscode from "vscode"; +import type { Version } from "./version"; type Dict = { [x: string]: T }; -export type Tool = { path: string; version: [number, number, number] }; -export type ToolCheckFunc = () => Promise<{ tool?: Tool; error?: string }>; + +export type Tool = { path: string; version: Version }; + +export type ToolCheckSuccessResult = { + tool: Tool; + error?: undefined; +}; +export type ToolCheckErrorResult = { + tool?: undefined; + error: string; +}; + +type ResultImpl = ToolCheckSuccessResult | ToolCheckErrorResult; + +export class ToolCheckResult { + private readonly result: ResultImpl; + + private constructor(result: ResultImpl) { + this.result = result; + } + + static newError(error: string) { + return new ToolCheckResult({ error }); + } + + static newTool(tool: Tool) { + return new ToolCheckResult({ tool }); + } + + private hasErrorImpl(result: ResultImpl): result is ToolCheckErrorResult { + return !!result.error; + } + + isError(): boolean { + return this.hasErrorImpl(this.result); + } + + get error(): string { + if (!this.hasErrorImpl(this.result)) { + throw new Error("Wrong invocation of getter for 'error', check the state first"); + } + return this.result.error; + } + + get tool(): Tool { + if (this.hasErrorImpl(this.result)) { + throw new Error("Wrong invocation of getter for 'tool', check the state first"); + } + return this.result.tool; + } +} + +export type ToolCheckFunc = () => Promise; export type LinterConfiguration = { enabled: boolean; diff --git a/src/utils.ts b/src/utils.ts index 722e8e5..18aa4ef 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -5,7 +5,14 @@ import * as vscode from "vscode"; import * as which from "which"; import { createHash, BinaryLike } from "crypto"; -import { ExtensionConfiguration, Target, SettingsKey, ModifiableExtension } from "./types"; +import { + ExtensionConfiguration, + Target, + SettingsKey, + ModifiableExtension, + type ToolCheckResult, + type ToolCheckErrorResult, +} from "./types"; import { getMesonBuildOptions } from "./introspection"; import { extensionPath, workspaceState } from "./extension"; diff --git a/src/version.ts b/src/version.ts new file mode 100644 index 0000000..901b919 --- /dev/null +++ b/src/version.ts @@ -0,0 +1,72 @@ +export type VersionArray = [number, number, number]; + +const versionNames = ["major", "minor", "patch"] as const; + +export class Version { + constructor(private readonly version: VersionArray) { + const isValid = Version.isValidVersion(this.version); + + if (isValid !== true) { + throw isValid; + } + } + + /** This checks if any type is a valid version array at runtime + * + * @param version the version to check + */ + private static isValidVersion(version: Version | any): true | Error { + if (!Array.isArray(version)) { + return new Error("Version object is not an Array"); + } + + if (version.length !== 3) { + return new Error(`Version array has ${version.length} entries, but expected 3`); + } + + for (const index in version as VersionArray) { + const num = version[index]; + if (!Number.isInteger(num)) { + const name = versionNames[index]; + return new Error(`${name} version component is not a number: '${num}'`); + } + } + + return true; + } + + /** This compares two versions + * - if the first one is bigger, a value > 0 is returned + * - if they are the same, 0 is returned + * - if the first one is smaller, a value < 0 is returned + * @param version1 + * @param version2 + */ + private static compareImpl([major1, minor1, patch1]: VersionArray, [major2, minor2, patch2]: VersionArray): number { + if (major1 !== major2) { + return major1 - major2; + } + + if (minor1 !== minor2) { + return minor1 - minor2; + } + + return patch1 - patch2; + } + + compareWithOther(otherVersion: Version): number { + return Version.compareImpl(this.version, otherVersion.version); + } + + compare(otherVersion: VersionArray): number { + return Version.compareImpl(this.version, otherVersion); + } + + private static versionToString([major, minor, patch]: VersionArray): string { + return `${major}.${minor}.${patch}`; + } + + toString(): string { + return Version.versionToString(this.version); + } +}