Skip to content

Commit

Permalink
fix(ConfigManager): Resolve ESM import exception on Windows
Browse files Browse the repository at this point in the history
import(path) does not accept absolute Windows-paths since those seem to
get confused with URLs.

Before, this was mitigated by always calculating a relative path
(relative to the UI5 linter install directory). This however breaks on
Windows if the target project is located on another drive as reported in
#458

Instead convert all paths to "file://"-URLs before passing them to
import().

Also, in linter.ts, enforce the project root path to always be absolute
to simplify handling the path later on.

Resolves #458
  • Loading branch information
RandomByte committed Jan 7, 2025
1 parent 2bb7e9d commit 87c21e6
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 13 deletions.
3 changes: 3 additions & 0 deletions src/linter/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import {Minimatch} from "minimatch";
export async function lintProject({
rootDir, filePatterns, ignorePatterns, coverage, details, configPath, ui5Config, noConfig,
}: LinterOptions): Promise<LintResult[]> {
if (!path.isAbsolute(rootDir)) {
throw new Error(`rootDir must be an absolute path. Received: ${rootDir}`);
}
let config: UI5LintConfigType = {};
if (noConfig !== true) {
const configMngr = new ConfigManager(rootDir, configPath);
Expand Down
14 changes: 4 additions & 10 deletions src/utils/ConfigManager.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import path, {dirname} from "node:path";
import {fileURLToPath} from "node:url";
import {fileURLToPath, pathToFileURL} from "node:url";
import {FilePattern} from "../linter/LinterContext.js";
const __dirname = dirname(fileURLToPath(import.meta.url));

Expand Down Expand Up @@ -28,22 +28,16 @@ export default class ConfigManager {
}

#resolveModulePaths(fileName: string): string {
// Node on Windows behaves strange, does not work with absolute paths
// and modifies files extensions in tests i.e. js -> ts, mjs -> tjs
// Keeping the relative path in POSIX format resolves those issues.
return path.posix.join(
path.relative(__dirname, this.#projectRootDir).replaceAll(path.win32.sep, path.posix.sep),
fileName);
const resolvedPath = path.join(this.#projectRootDir, fileName);
return pathToFileURL(resolvedPath).toString();
}

async getConfiguration(): Promise<UI5LintConfigType> {
let config = {} as UI5LintConfigType;

if (this.#configFile) {
// If it's an relative path, transform to POSIX format
const configFilePath = path.isAbsolute(this.#configFile) ?
this.#configFile :
this.#resolveModulePaths(this.#configFile);
const configFilePath = this.#resolveModulePaths(this.#configFile);

({default: config} = await import(configFilePath) as {default: UI5LintConfigType});
} else {
Expand Down
15 changes: 15 additions & 0 deletions test/lib/linter/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,3 +424,18 @@ test.serial("lint: getProjectGraph with different directory structures", async (
},
});
});

// Test project fixtures individually
test.serial("lint: Relative rootDir path throws an exception", async (t) => {
const projectPath = path.join(fixturesProjectsPath, "com.ui5.troublesome.app");
const {lintProject} = t.context;

await t.throwsAsync(() => {
return lintProject({
rootDir: path.relative(__dirname, projectPath),
filePatterns: [],
coverage: true,
details: true,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const fixturesBasePath = path.join(__dirname, "..", "..", "fixtures", "linter");
const fixturesProjectsPath = path.join(fixturesBasePath, "projects");

test("Check config file", async (t) => {
const confManager = new ConfigManager("./test/fixtures/linter/projects/com.ui5.troublesome.app/",
const confManager = new ConfigManager(
path.join(fixturesProjectsPath, "com.ui5.troublesome.app"),
"ui5lint-custom.config.cjs");

const config = await confManager.getConfiguration();
Expand All @@ -24,7 +25,8 @@ test("Check config file", async (t) => {
});

test("Check config file auto discovery", async (t) => {
const confManager = new ConfigManager("./test/fixtures/linter/projects/com.ui5.troublesome.app/");
const confManager = new ConfigManager(
path.join(fixturesProjectsPath, "com.ui5.troublesome.app"));

const config = await confManager.getConfiguration();

Expand All @@ -48,7 +50,7 @@ test("Throws an error if config file has Syntax errors", async (t) => {

test("Resolves to an empty config if default module is not found", async (t) => {
const confManager = new ConfigManager(
"./test/fixtures/linter/projects/library.with.custom.paths/");
path.join(fixturesProjectsPath, "library.with.custom.paths/"));

const config = await confManager.getConfiguration();

Expand Down

0 comments on commit 87c21e6

Please sign in to comment.