From 48b83fdfdc9832c3fd52580d1e6695934bc908e4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 9 Apr 2026 07:38:34 +0000 Subject: [PATCH] improve: extract resolveWorkspacePath utility; fix relative standard paths - Add resolveWorkspacePath() to lib/utils.js that handles ~ expansion, ${workspaceFolder}/${workspaceRoot} substitution, and relative path resolution against the workspace root. - Refactor loadSettings() to use resolveWorkspacePath() for the standard setting, fixing #38 (relative paths like ./ruleset.xml in phpcbf.standard were silently passed through instead of being resolved). - Add 10 unit tests for resolveWorkspacePath (17 total, all pass). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension.js | 14 ++++------- lib/utils.js | 40 +++++++++++++++++++++++++++++- test/unit.test.js | 62 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 11 deletions(-) diff --git a/extension.js b/extension.js index 6457e71..f28cdc8 100644 --- a/extension.js +++ b/extension.js @@ -12,7 +12,7 @@ const path = require("path"); const fs = require("fs"); const os = require("os"); const cp = require("child_process"); -const { findFiles } = require("./lib/utils"); +const { findFiles, resolveWorkspacePath } = require("./lib/utils"); const TmpDir = os.tmpdir(); class PHPCBF { @@ -62,15 +62,11 @@ class PHPCBF { this.standard = config.get("standard", null); - // Resolve ${workspaceFolder} / ${workspaceRoot} in the standard path. - if (this.standard && configUri) { - const folder = workspace.getWorkspaceFolder(configUri); + // Resolve variables and relative paths in the standard path. + if (this.standard) { + const folder = configUri ? workspace.getWorkspaceFolder(configUri) : null; const rootPath = folder ? folder.uri.fsPath : null; - if (rootPath) { - this.standard = this.standard - .replace("${workspaceFolder}", rootPath) - .replace("${workspaceRoot}", rootPath); - } + this.standard = resolveWorkspacePath(this.standard, rootPath); } this.documentFormattingProvider = config.get( diff --git a/lib/utils.js b/lib/utils.js index 5627bec..0b7e9bb 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,6 +1,7 @@ "use strict"; const path = require("path"); const fs = require("fs"); +const os = require("os"); /** * Walk up the directory tree from `parent/directory` looking for any file @@ -32,4 +33,41 @@ function findFiles(parent, directory, name) { return null; } -module.exports = { findFiles }; +/** + * Resolve variable substitutions and relative paths in a path string. + * + * Handles (in order): + * - `~` expansion to the home directory + * - `${workspaceFolder}` and `${workspaceRoot}` substitution + * - Relative paths resolved against `rootPath` (when supplied) + * + * @param {string} inputPath - Path string to resolve. + * @param {string|null} [rootPath] - Workspace root directory; required for + * variable substitution and relative path resolution. + * @returns {string} The resolved path. + */ +function resolveWorkspacePath(inputPath, rootPath) { + if (!inputPath) return inputPath; + let resolved = inputPath; + + // Expand ~ to home directory. + if (resolved.startsWith("~/") || resolved === "~") { + resolved = resolved.replace(/^~(?=\/|$)/, os.homedir()); + } + + if (rootPath) { + // Substitute ${workspaceFolder} and ${workspaceRoot}. + resolved = resolved + .replace(/\$\{workspaceFolder\}/g, rootPath) + .replace(/\$\{workspaceRoot\}/g, rootPath); + + // Resolve relative paths against the workspace root. + if (!path.isAbsolute(resolved)) { + resolved = path.resolve(rootPath, resolved); + } + } + + return resolved; +} + +module.exports = { findFiles, resolveWorkspacePath }; diff --git a/test/unit.test.js b/test/unit.test.js index 0fb759e..5a7d590 100644 --- a/test/unit.test.js +++ b/test/unit.test.js @@ -10,7 +10,7 @@ const path = require("path"); const fs = require("fs"); const os = require("os"); -const { findFiles } = require("../lib/utils"); +const { findFiles, resolveWorkspacePath } = require("../lib/utils"); // --------------------------------------------------------------------------- // Helpers @@ -104,3 +104,63 @@ describe("findFiles", () => { assert.equal(result, expected); }); }); + +// --------------------------------------------------------------------------- +// resolveWorkspacePath +// --------------------------------------------------------------------------- + +describe("resolveWorkspacePath", () => { + const root = path.join(os.tmpdir(), "phpcbf-resolve-test"); + + test("returns null/undefined as-is", () => { + assert.equal(resolveWorkspacePath(null, root), null); + assert.equal(resolveWorkspacePath(undefined, root), undefined); + assert.equal(resolveWorkspacePath("", root), ""); + }); + + test("substitutes ${workspaceFolder}", () => { + const result = resolveWorkspacePath("${workspaceFolder}/phpcs.xml", root); + assert.equal(result, path.join(root, "phpcs.xml")); + }); + + test("substitutes ${workspaceRoot}", () => { + const result = resolveWorkspacePath("${workspaceRoot}/phpcs.xml", root); + assert.equal(result, path.join(root, "phpcs.xml")); + }); + + test("resolves relative path starting with ./", () => { + const result = resolveWorkspacePath("./ruleset.xml", root); + assert.equal(result, path.resolve(root, "ruleset.xml")); + }); + + test("resolves relative path without leading ./", () => { + const result = resolveWorkspacePath("vendor/standard/ruleset.xml", root); + assert.equal(result, path.resolve(root, "vendor/standard/ruleset.xml")); + }); + + test("leaves absolute path unchanged when rootPath supplied", () => { + const abs = path.join(os.tmpdir(), "absolute.xml"); + const result = resolveWorkspacePath(abs, root); + assert.equal(result, abs); + }); + + test("expands ~ to home directory", () => { + const result = resolveWorkspacePath("~/.phpcs.xml", root); + assert.equal(result, path.join(os.homedir(), ".phpcs.xml")); + }); + + test("expands ~ alone to home directory", () => { + const result = resolveWorkspacePath("~", root); + assert.equal(result, os.homedir()); + }); + + test("returns original path unchanged when rootPath is null", () => { + const result = resolveWorkspacePath("./relative.xml", null); + assert.equal(result, "./relative.xml"); + }); + + test("substitutes ${workspaceFolder} inside a path segment", () => { + const result = resolveWorkspacePath("${workspaceFolder}/sub/ruleset.xml", root); + assert.equal(result, path.join(root, "sub", "ruleset.xml")); + }); +});