From 8b1e49c6c0bc04e41cd4ba2489ef1de5eacfb4d2 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 12:05:52 +0100 Subject: [PATCH 1/7] Use more descriptive names --- extensions/ql-vscode/src/pure/files.ts | 10 ++-- .../test/unit-tests/pure/files.test.ts | 46 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/extensions/ql-vscode/src/pure/files.ts b/extensions/ql-vscode/src/pure/files.ts index f987bed0a34..d1b6a603c99 100644 --- a/extensions/ql-vscode/src/pure/files.ts +++ b/extensions/ql-vscode/src/pure/files.ts @@ -72,15 +72,15 @@ export function pathsEqual( } /** - * Returns true if path1 contains path2. + * Returns true if `parent` contains `child`. */ export function containsPath( - path1: string, - path2: string, + parent: string, + child: string, platform: NodeJS.Platform, ): boolean { - return normalizePath(path2, platform).startsWith( - normalizePath(path1, platform), + return normalizePath(child, platform).startsWith( + normalizePath(parent, platform), ); } diff --git a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts index 35adba6605d..29472b38055 100644 --- a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts +++ b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts @@ -207,79 +207,79 @@ describe("pathsEqual", () => { describe("containsPath", () => { const testCases: Array<{ - path1: string; - path2: string; + parent: string; + child: string; platform: NodeJS.Platform; expected: boolean; }> = [ { - path1: + parent: "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", platform: "linux", expected: true, }, { - path1: + parent: "/HOME/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", platform: "linux", expected: false, }, { - path1: + parent: "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", - path2: + child: "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", platform: "linux", expected: false, }, { - path1: + parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", platform: "win32", expected: true, }, { - path1: + parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "c:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", platform: "win32", expected: true, }, { - path1: + parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "D:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", platform: "win32", expected: false, }, { - path1: + parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", - path2: + child: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", platform: "win32", expected: false, }, { - path1: + parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "C:\\Users\\github\\projects\\vscode-codeql-starter\\codeql-custom-queries-javascript\\example.ql", platform: "win32", expected: true, }, { - path1: + parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", - path2: + child: "D:\\Users\\github\\projects\\vscode-codeql-starter\\codeql-custom-queries-javascript\\example.ql", platform: "win32", expected: false, @@ -287,15 +287,15 @@ describe("containsPath", () => { ]; test.each(testCases)( - "$path1 contains $path2 on $platform = $expected", - ({ path1, path2, platform, expected }) => { + "$parent contains $child on $platform = $expected", + ({ parent, child, platform, expected }) => { if (platform !== process.platform) { // We're using the platform-specific path.resolve, so we can't really run // these tests on all platforms. return; } - expect(containsPath(path1, path2, platform)).toEqual(expected); + expect(containsPath(parent, child, platform)).toEqual(expected); }, ); }); From bdd2319297ba5781b2aa8d63f84c170ba3fbce04 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 14:41:03 +0100 Subject: [PATCH 2/7] Update docs to mention about paths being equal --- extensions/ql-vscode/src/pure/files.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/pure/files.ts b/extensions/ql-vscode/src/pure/files.ts index d1b6a603c99..27e9df31946 100644 --- a/extensions/ql-vscode/src/pure/files.ts +++ b/extensions/ql-vscode/src/pure/files.ts @@ -72,7 +72,7 @@ export function pathsEqual( } /** - * Returns true if `parent` contains `child`. + * Returns true if `parent` contains `child`, or if they are equal. */ export function containsPath( parent: string, From 300503e1c9a06bf5cae9508108c50735f7cfba07 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 12:08:00 +0100 Subject: [PATCH 3/7] Remove platform arg as it's never not the process.platform --- .../local-databases/database-item-impl.ts | 1 - .../local-databases/database-manager.ts | 2 +- extensions/ql-vscode/src/pure/files.ts | 22 +++++-------------- .../ql-vscode/test/matchers/toEqualPath.ts | 2 +- .../test/unit-tests/pure/files.test.ts | 4 ++-- 5 files changed, 10 insertions(+), 21 deletions(-) diff --git a/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts b/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts index 4b206d4991a..6b73ecd49be 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-item-impl.ts @@ -207,7 +207,6 @@ export class DatabaseItemImpl implements DatabaseItem { return pathsEqual( databasePath, join(testdir, `${testdirbase}.testproj`), - process.platform, ); } } catch { diff --git a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts index 587c4b7bb7e..8a6f65a175c 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts @@ -652,7 +652,7 @@ export class DatabaseManager extends DisposableObject { private isExtensionControlledLocation(uri: vscode.Uri) { const storageUri = this.ctx.storageUri || this.ctx.globalStorageUri; if (storageUri) { - return containsPath(storageUri.fsPath, uri.fsPath, process.platform); + return containsPath(storageUri.fsPath, uri.fsPath); } return false; } diff --git a/extensions/ql-vscode/src/pure/files.ts b/extensions/ql-vscode/src/pure/files.ts index 27e9df31946..284863f6fbc 100644 --- a/extensions/ql-vscode/src/pure/files.ts +++ b/extensions/ql-vscode/src/pure/files.ts @@ -51,37 +51,27 @@ export async function getDirectoryNamesInsidePath( return dirNames; } -function normalizePath(path: string, platform: NodeJS.Platform): string { +function normalizePath(path: string): string { // On Windows, "C:/", "C:\", and "c:/" are all equivalent. We need // to normalize the paths to ensure they all get resolved to the // same format. On Windows, we also need to do the comparison // case-insensitively. path = resolve(path); - if (platform === "win32") { + if (process.platform === "win32") { path = path.toLowerCase(); } return path; } -export function pathsEqual( - path1: string, - path2: string, - platform: NodeJS.Platform, -): boolean { - return normalizePath(path1, platform) === normalizePath(path2, platform); +export function pathsEqual(path1: string, path2: string): boolean { + return normalizePath(path1) === normalizePath(path2); } /** * Returns true if `parent` contains `child`, or if they are equal. */ -export function containsPath( - parent: string, - child: string, - platform: NodeJS.Platform, -): boolean { - return normalizePath(child, platform).startsWith( - normalizePath(parent, platform), - ); +export function containsPath(parent: string, child: string): boolean { + return normalizePath(child).startsWith(normalizePath(parent)); } export async function readDirFullPaths(path: string): Promise { diff --git a/extensions/ql-vscode/test/matchers/toEqualPath.ts b/extensions/ql-vscode/test/matchers/toEqualPath.ts index d598fcf981a..2e828cb7b03 100644 --- a/extensions/ql-vscode/test/matchers/toEqualPath.ts +++ b/extensions/ql-vscode/test/matchers/toEqualPath.ts @@ -10,7 +10,7 @@ const toEqualPath: MatcherFunction<[expectedPath: unknown]> = function ( throw new Error("These must be of type string!"); } - const pass = pathsEqual(actual, expectedPath, process.platform); + const pass = pathsEqual(actual, expectedPath); if (pass) { return { message: () => diff --git a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts index 29472b38055..6d3445aeea6 100644 --- a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts +++ b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts @@ -200,7 +200,7 @@ describe("pathsEqual", () => { return; } - expect(pathsEqual(path1, path2, platform)).toEqual(expected); + expect(pathsEqual(path1, path2)).toEqual(expected); }, ); }); @@ -295,7 +295,7 @@ describe("containsPath", () => { return; } - expect(containsPath(parent, child, platform)).toEqual(expected); + expect(containsPath(parent, child)).toEqual(expected); }, ); }); From 6e3d0147c9ef096ff720e167572b57893a87fcc8 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 12:19:22 +0100 Subject: [PATCH 4/7] Add test cases for paths with the same prefix --- .../test/unit-tests/pure/files.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts index 6d3445aeea6..df2c59610b7 100644 --- a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts +++ b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts @@ -236,6 +236,22 @@ describe("containsPath", () => { platform: "linux", expected: false, }, + { + parent: + "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-java", + child: + "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", + platform: "linux", + expected: false, + }, + { + parent: + "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-java", + child: + "/home/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", + platform: "linux", + expected: false, + }, { parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", @@ -284,6 +300,22 @@ describe("containsPath", () => { platform: "win32", expected: false, }, + { + parent: + "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-java", + child: + "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", + platform: "win32", + expected: false, + }, + { + parent: + "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-java", + child: + "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript/example.ql", + platform: "win32", + expected: false, + }, ]; test.each(testCases)( From f7c1f06354340a33d2be5b5cb3cc7c3205454ab5 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 12:31:45 +0100 Subject: [PATCH 5/7] Add another test case for case insensitivity on windows --- extensions/ql-vscode/test/unit-tests/pure/files.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts index df2c59610b7..859da16d138 100644 --- a/extensions/ql-vscode/test/unit-tests/pure/files.test.ts +++ b/extensions/ql-vscode/test/unit-tests/pure/files.test.ts @@ -268,6 +268,14 @@ describe("containsPath", () => { platform: "win32", expected: true, }, + { + parent: + "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", + child: + "C:/USERS/GITHUB/PROJECTS/VSCODE-CODEQL-STARTER/CODEQL-CUSTOM-QUERIES-JAVASCRIPT/EXAMPLE.QL", + platform: "win32", + expected: true, + }, { parent: "C:/Users/github/projects/vscode-codeql-starter/codeql-custom-queries-javascript", From aa4c459cddf57a8ae48aed0930cef5bd769a1685 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 12:19:50 +0100 Subject: [PATCH 6/7] Use relative instead of startsWith to handle paths with the same prefix --- extensions/ql-vscode/src/pure/files.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/pure/files.ts b/extensions/ql-vscode/src/pure/files.ts index 284863f6fbc..69ecb731af6 100644 --- a/extensions/ql-vscode/src/pure/files.ts +++ b/extensions/ql-vscode/src/pure/files.ts @@ -1,5 +1,5 @@ import { pathExists, stat, readdir } from "fs-extra"; -import { join, resolve } from "path"; +import { isAbsolute, join, relative, resolve } from "path"; /** * Recursively finds all .ql files in this set of Uris. @@ -71,7 +71,13 @@ export function pathsEqual(path1: string, path2: string): boolean { * Returns true if `parent` contains `child`, or if they are equal. */ export function containsPath(parent: string, child: string): boolean { - return normalizePath(child).startsWith(normalizePath(parent)); + const relativePath = relative(parent, child); + return ( + !relativePath.startsWith("..") && + // On windows, if the two paths are in different drives, then the + // relative path will be an absolute path to the other drive. + !(process.platform === "win32" && isAbsolute(relativePath)) + ); } export async function readDirFullPaths(path: string): Promise { From c920b7e49e77a88525e1893c7276abf9e29e8d40 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 12 Jun 2023 15:16:43 +0100 Subject: [PATCH 7/7] Remove explicit check for windows --- extensions/ql-vscode/src/pure/files.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/pure/files.ts b/extensions/ql-vscode/src/pure/files.ts index 69ecb731af6..d413e3446cc 100644 --- a/extensions/ql-vscode/src/pure/files.ts +++ b/extensions/ql-vscode/src/pure/files.ts @@ -76,7 +76,7 @@ export function containsPath(parent: string, child: string): boolean { !relativePath.startsWith("..") && // On windows, if the two paths are in different drives, then the // relative path will be an absolute path to the other drive. - !(process.platform === "win32" && isAbsolute(relativePath)) + !isAbsolute(relativePath) ); }