fix: make sure glob patterns match on windows (#1039)

Fix an issue if the drive letter case does not match on windows.
streetsidesoftware · Mar 5, 2021 · 1e58e4c · 1e58e4c
1 parent 693552b
commit 1e58e4c
Show file tree

Hide file tree

Showing 10 changed files with 429 additions and 173 deletions.
diff --git a/cspell-dict.txt b/cspell-dict.txt
@@ -15,6 +15,7 @@ deserialize
 deserializer
 deserializers
 exonum
+globstar
 gzipped
 issuehunt
 lcov
@@ -35,6 +36,7 @@ patreon
 popd
 pushd
 pycontribs
+rebased
 repo
 repos
 retryable

diff --git a/packages/cspell-glob/.vscode/launch.json b/packages/cspell-glob/.vscode/launch.json
@@ -4,12 +4,15 @@
     // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
     "version": "0.2.0",
     "configurations": [
-		{
+        {
             "type": "node",
             "request": "launch",
             "name": "cspell-glob: Jest current-file",
             "program": "${workspaceFolder}/../../node_modules/.bin/jest",
-            "args": ["--runInBand", "${file}"],
+            "args": [
+                "--runInBand",
+                "${fileBasename}"
+            ],
             "console": "integratedTerminal",
             "internalConsoleOptions": "neverOpen",
             "disableOptimisticBPs": true,
@@ -22,7 +25,9 @@
             "request": "launch",
             "name": "cspell-glob: Jest All",
             "program": "${workspaceFolder}/../../node_modules/.bin/jest",
-            "args": ["--runInBand"],
+            "args": [
+                "--runInBand"
+            ],
             "console": "integratedTerminal",
             "internalConsoleOptions": "neverOpen",
             "disableOptimisticBPs": true,
@@ -31,4 +36,4 @@
             }
         }
     ]
-}
+}
diff --git a/packages/cspell-glob/src/GlobMatcher.test.ts b/packages/cspell-glob/src/GlobMatcher.test.ts
@@ -1,5 +1,11 @@
 import { GlobMatcher, GlobMatchOptions } from './GlobMatcher';
-import { GlobMatch, GlobPatternWithOptionalRoot, PathInterface } from './GlobMatcherTypes';
+import {
+    GlobMatch,
+    GlobPattern,
+    GlobPatternNormalized,
+    GlobPatternWithOptionalRoot,
+    PathInterface,
+} from './GlobMatcherTypes';
 
 import * as path from 'path';
 import mm = require('micromatch');
@@ -216,46 +222,91 @@ describe('Validate Options', () => {
 });
 
 describe('Validate GlobMatcher', () => {
-    interface TestCase {
-        patterns: string | string[];
+    function resolvePattern(p: GlobPattern, path: PathInterface): GlobPattern;
+    function resolvePattern(p: GlobPattern[], path: PathInterface): GlobPattern[];
+    function resolvePattern(p: GlobPattern | GlobPattern[], path: PathInterface): GlobPattern | GlobPattern[];
+    function resolvePattern(p: GlobPattern | GlobPattern[], path: PathInterface): GlobPattern | GlobPattern[] {
+        if (Array.isArray(p)) {
+            return p.map((g) => resolvePattern(g, path));
+        }
+        if (typeof p === 'string' || p.root === undefined) {
+            return p;
+        }
+        return {
+            ...p,
+            root: path.resolve(p.root),
+        };
+    }
+
+    function g(glob: string, root?: string): GlobPatternWithOptionalRoot {
+        return {
+            glob,
+            root,
+        };
+    }
+
+    function ocg(g: Partial<GlobPatternNormalized>, path: PathInterface): GlobPatternNormalized {
+        const { root, rawRoot, ...rest } = g;
+        const gg: Partial<GlobPatternNormalized> = {};
+        if (root !== undefined) {
+            gg.root = path.resolve(root);
+        }
+        if (rawRoot !== undefined) {
+            gg.rawRoot = path.resolve(rawRoot);
+        }
+        return expect.objectContaining({ ...rest, ...gg });
+    }
+
+    interface TestCaseMatcher {
+        patterns: GlobPattern | GlobPattern[];
         root: string | undefined;
         filename: string;
         expected: boolean;
         description: string;
     }
 
     const tests = test.each`
-        patterns                     | root         | filename                                  | expected | description
-        ${['*.json']}                | ${undefined} | ${'./settings.json'}                      | ${true}  | ${'*.json'}
-        ${['*.json']}                | ${undefined} | ${'settings.json'}                        | ${true}  | ${'*.json'}
-        ${['*.json']}                | ${undefined} | ${'${cwd}/settings.json'}                 | ${true}  | ${'*.json'}
-        ${'*.json'}                  | ${undefined} | ${'${cwd}/settings.json'}                 | ${true}  | ${'*.json'}
-        ${'#.gitignore\n *.json'}    | ${undefined} | ${'${cwd}/settings.json'}                 | ${true}  | ${'*.json'}
-        ${['middle']}                | ${''}        | ${'${cwd}/packages/middle/settings.json'} | ${true}  | ${'match middle of path'}
-        ${['.vscode']}               | ${undefined} | ${'.vscode/settings.json'}                | ${true}  | ${'.vscode'}
-        ${['/*.json']}               | ${'/'}       | ${'/settings.json'}                       | ${true}  | ${'Matches root level files, /*.json'}
-        ${['/*.json']}               | ${undefined} | ${'/src/settings.json'}                   | ${false} | ${'Matches pattern but not cwd /*.json'}
-        ${['*.js']}                  | ${undefined} | ${'${cwd}/src/settings.js'}               | ${true}  | ${'// Matches nested files, *.js'}
-        ${['.vscode/']}              | ${undefined} | ${'${cwd}/.vscode/settings.json'}         | ${true}  | ${'.vscode/'}
-        ${['.vscode/']}              | ${undefined} | ${'${cwd}/.vscode'}                       | ${false} | ${'.vscode/'}
-        ${['/.vscode/']}             | ${undefined} | ${'${cwd}/.vscode'}                       | ${false} | ${'should match root'}
-        ${['/.vscode/']}             | ${undefined} | ${'${cwd}/.vscode/settings.json'}         | ${true}  | ${'should match root'}
-        ${['/.vscode/']}             | ${undefined} | ${'${cwd}/package/.vscode'}               | ${false} | ${'should only match root'}
-        ${['.vscode/']}              | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}     | ${true}  | ${'should match nested .vscode/'}
-        ${['**/.vscode/']}           | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}     | ${true}  | ${'should match nested .vscode/'}
-        ${['**/.vscode/']}           | ${undefined} | ${'${cwd}/src/.vscode'}                   | ${false} | ${'should match nested .vscode'}
-        ${['**/.vscode']}            | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}     | ${false} | ${'should not match nested **/.vscode'}
-        ${['**/.vscode/**']}         | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}     | ${true}  | ${'should match nested **/.vscode'}
-        ${['/User/user/Library/**']} | ${undefined} | ${'/src/User/user/Library/settings.json'} | ${false} | ${'No match'}
-        ${['/User/user/Library/**']} | ${'/'}       | ${'/User/user/Library/settings.json'}     | ${true}  | ${'Match system root'}
+        patterns                     | root         | filename                                    | expected | description
+        ${['*.json']}                | ${undefined} | ${'./settings.json'}                        | ${true}  | ${'*.json'}
+        ${['*.json']}                | ${undefined} | ${'settings.json'}                          | ${true}  | ${'*.json'}
+        ${['*.json']}                | ${undefined} | ${'${cwd}/settings.json'}                   | ${true}  | ${'*.json'}
+        ${'*.json'}                  | ${undefined} | ${'${cwd}/settings.json'}                   | ${true}  | ${'*.json'}
+        ${'#.gitignore\n *.json'}    | ${undefined} | ${'${cwd}/settings.json'}                   | ${true}  | ${'*.json'}
+        ${['middle']}                | ${''}        | ${'${cwd}/packages/middle/settings.json'}   | ${true}  | ${'match middle of path'}
+        ${['.vscode']}               | ${undefined} | ${'.vscode/settings.json'}                  | ${true}  | ${'.vscode'}
+        ${['/*.json']}               | ${'/'}       | ${'/settings.json'}                         | ${true}  | ${'Matches root level files, /*.json'}
+        ${['/*.json']}               | ${undefined} | ${'/src/settings.json'}                     | ${false} | ${'Matches pattern but not cwd /*.json'}
+        ${['*.js']}                  | ${undefined} | ${'${cwd}/src/settings.js'}                 | ${true}  | ${'// Matches nested files, *.js'}
+        ${'*.js'}                    | ${undefined} | ${'${cwd}/src/settings.js'}                 | ${true}  | ${'// Matches nested files, *.js'}
+        ${g('*.js')}                 | ${'.'}       | ${'${cwd}/src/settings.js'}                 | ${true}  | ${'// Matches nested files, *.js'}
+        ${g('*.js', 'src')}          | ${'.'}       | ${'${cwd}/src/settings.js'}                 | ${true}  | ${'// Matches nested files, *.js'}
+        ${g('*.js', 'a')}            | ${'a/b'}     | ${'a/b/src/settings.js'}                    | ${true}  | ${'// Matches nested files, *.js'}
+        ${g('*.js', 'a')}            | ${'a/b'}     | ${'a/c/src/settings.js'}                    | ${true}  | ${'// Matches even if NOT under root.'}
+        ${g('*.js', 'a')}            | ${'a/b'}     | ${'c/src/settings.js'}                      | ${false} | ${'// Must match glob root'}
+        ${['.vscode/']}              | ${undefined} | ${'${cwd}/.vscode/settings.json'}           | ${true}  | ${'.vscode/'}
+        ${['.vscode/']}              | ${undefined} | ${'${cwd}/.vscode'}                         | ${false} | ${'.vscode/'}
+        ${['/.vscode/']}             | ${undefined} | ${'${cwd}/.vscode'}                         | ${false} | ${'should not match file'}
+        ${['/.vscode/']}             | ${undefined} | ${'${cwd}/.vscode/settings.json'}           | ${true}  | ${'should match root'}
+        ${['/.vscode/']}             | ${undefined} | ${'${cwd}/package/.vscode'}                 | ${false} | ${'should only match root'}
+        ${['.vscode/**']}            | ${undefined} | ${'${cwd}/.vscode/settings.json'}           | ${true}  | ${'should match root .vscode/**'}
+        ${['.vscode/']}              | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}       | ${true}  | ${'should match nested .vscode/'}
+        ${['**/.vscode/']}           | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}       | ${true}  | ${'should match nested .vscode/'}
+        ${['**/.vscode/']}           | ${undefined} | ${'${cwd}/src/.vscode'}                     | ${false} | ${'should match nested .vscode'}
+        ${['**/.vscode']}            | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}       | ${false} | ${'should not match nested **/.vscode'}
+        ${['**/.vscode/**']}         | ${undefined} | ${'${cwd}/src/.vscode/settings.json'}       | ${true}  | ${'should match nested **/.vscode'}
+        ${['/User/user/Library/**']} | ${undefined} | ${'/src/User/user/Library/settings.json'}   | ${false} | ${'No match'}
+        ${['/User/user/Library/**']} | ${undefined} | ${'${cwd}/User/user/Library/settings.json'} | ${true}  | ${'Match cwd root'}
+        ${['/User/user/Library/**']} | ${'/'}       | ${'/User/user/Library/settings.json'}       | ${true}  | ${'Match system root'}
+        ${g('*.js', 'a')}            | ${'a/b'}     | ${'a/b/src/settings.js'}                    | ${true}  | ${'// Matches nested files, *.js'}
     `;
-
     tests(
         'win32 $description, patterns: $patterns, filename: $filename, root: $root, $expected',
-        ({ filename, root, patterns, expected }: TestCase) => {
+        ({ filename, root, patterns, expected }: TestCaseMatcher) => {
             const pathInstance = pathWin32;
             root = resolveFilename(pathInstance, root);
             filename = resolveFilename(pathInstance, filename);
+            patterns = resolvePattern(patterns, pathInstance);
+            // console.log(`root: ${root}, filename: ${filename}, pattern: ${JSON.stringify(patterns)}`);
             const matcher = new GlobMatcher(patterns, root, pathInstance);
             // eslint-disable-next-line jest/no-standalone-expect
             expect(matcher.match(filename)).toEqual(expected);
@@ -264,15 +315,46 @@ describe('Validate GlobMatcher', () => {
 
     tests(
         'posix $description, patterns: $patterns, filename: $filename, root: $root, $expected',
-        ({ filename, root, patterns, expected }: TestCase) => {
+        ({ filename, root, patterns, expected }: TestCaseMatcher) => {
             const pathInstance = pathPosix;
             root = resolveFilename(pathInstance, root);
             filename = resolveFilename(pathInstance, filename);
+            patterns = resolvePattern(patterns, pathInstance);
             const matcher = new GlobMatcher(patterns, root, pathInstance);
             // eslint-disable-next-line jest/no-standalone-expect
             expect(matcher.match(filename)).toEqual(expected);
         }
     );
+
+    interface TestCaseNormalizedToRoot {
+        patterns: GlobPattern | GlobPattern[];
+        root: string | undefined;
+        pathInstance: PathInterface;
+        mode: GlobMatchOptions['mode'];
+        expected: Partial<GlobPatternNormalized>[];
+        description: string;
+    }
+
+    test.each`
+        patterns            | root     | mode         | pathInstance | expected                                                              | description
+        ${'*.json'}         | ${''}    | ${'exclude'} | ${pathPosix} | ${[{ glob: '**/{*.json,*.json/**}' }]}                                | ${''}
+        ${'*.json'}         | ${''}    | ${'exclude'} | ${pathWin32} | ${[{ glob: '**/{*.json,*.json/**}' }]}                                | ${''}
+        ${'*.json\n *.js'}  | ${''}    | ${'exclude'} | ${pathWin32} | ${[{ glob: '**/{*.json,*.json/**}' }, { glob: '**/{*.js,*.js/**}' }]} | ${''}
+        ${g('*.js', 'a')}   | ${''}    | ${'exclude'} | ${pathWin32} | ${[{ glob: 'a/**/{*.js,*.js/**}', root: '' }]}                        | ${''}
+        ${g('*.js', 'a')}   | ${'a'}   | ${'exclude'} | ${pathWin32} | ${[{ glob: '**/{*.js,*.js/**}', root: 'a' }]}                         | ${''}
+        ${g('*.js', 'a')}   | ${'a/b'} | ${'exclude'} | ${pathWin32} | ${[{ glob: '**/{*.js,*.js/**}', root: 'a/b' }]}                       | ${''}
+        ${g('*.js', 'a/c')} | ${'a/b'} | ${'exclude'} | ${pathWin32} | ${[]}                                                                 | ${''}
+        ${g('*.js', 'a/c')} | ${'a/b'} | ${'include'} | ${pathWin32} | ${[]}                                                                 | ${''}
+    `(
+        'excludeMode patternsNormalizedToRoot $description $patterns $root',
+        ({ patterns, root, pathInstance, expected, mode }: TestCaseNormalizedToRoot) => {
+            root = resolveFilename(pathInstance, root);
+            patterns = resolvePattern(patterns, pathInstance);
+            const matcher = new GlobMatcher(patterns, { mode, root, nodePath: pathInstance });
+            expected = expected.map((e) => ocg(e, pathInstance));
+            expect(matcher.patternsNormalizedToRoot).toEqual(expected);
+        }
+    );
 });
 
 type TestCase = [string[] | string, string | undefined, string, boolean, string];

diff --git a/packages/cspell-glob/src/GlobMatcher.ts b/packages/cspell-glob/src/GlobMatcher.ts
@@ -1,6 +1,6 @@
 import mm = require('micromatch');
 import * as Path from 'path';
-import { normalizeGlobPatterns } from './globHelper';
+import { normalizeGlobPatterns, doesRootContainPath, normalizeGlobToRoot } from './globHelper';
 import { PathInterface, GlobMatch, GlobPattern, GlobPatternWithRoot } from './GlobMatcherTypes';
 
 // cspell:ignore fname
@@ -63,6 +63,7 @@ export class GlobMatcher {
     readonly matchEx: (filename: string) => GlobMatch;
     readonly path: PathInterface;
     readonly patterns: GlobPatternWithRoot[];
+    readonly patternsNormalizedToRoot: GlobPatternWithRoot[];
     readonly root: string;
     readonly dot: boolean;
     readonly options: NormalizedGlobMatchOptions;
@@ -109,9 +110,11 @@ export class GlobMatcher {
             : typeof patterns === 'string'
             ? patterns.split(/\r?\n/g)
             : [patterns];
-        const globPatterns = normalizeGlobPatterns(patterns, this.options)
+        const globPatterns = normalizeGlobPatterns(patterns, this.options);
+        this.patternsNormalizedToRoot = globPatterns
+            .map((g) => normalizeGlobToRoot(g, normalizedRoot, nodePath))
             // Only keep globs that do not match the root when using exclude mode.
-            .filter((g) => isExcludeMode || g.root === normalizedRoot);
+            .filter((g) => nodePath.relative(g.root, normalizedRoot) === '');
 
         this.patterns = globPatterns;
         this.root = normalizedRoot;
@@ -203,14 +206,3 @@ function buildMatcherFn(patterns: GlobPatternWithRoot[], options: NormalizedGlob
     };
     return fn;
 }
-
-/**
- * Decide if a childPath is contained within a root or at the same level.
- * @param root - absolute path
- * @param childPath - absolute path
- */
-function doesRootContainPath(root: string, child: string, path: PathInterface): boolean {
-    if (child.startsWith(root)) return true;
-    const rel = path.relative(root, child);
-    return rel !== child && !rel.startsWith('..') && !path.isAbsolute(rel);
-}