Skip to content

Commit

Permalink
feat: Support global globs (#3157)
Browse files Browse the repository at this point in the history 
**Minor Breakage**

Globs starting with `**` are considered global and will not be evaluated relative to the containing configuration file's path.

To make a relative glob, start it with `/`, like `/**`.

* feat: Support global globs
* Do not adjust the path if it is a global pattern
* Adjust the unit tests
* lint: fix warning
  • Loading branch information
@Jason3S
Jason3S committed Jun 29, 2022
1 parent 198948a commit 7c0ee59
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 90 deletions.
2 changes: 1 addition & 1 deletion integration-tests/src/repositoryHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as fs from 'fs';
import Chalk from 'chalk';
import { ShouldCheckOptions, shouldCheckRepo } from './shouldCheckRepo';
import { Logger } from './types';
import simpleGit from 'simple-git';
import { simpleGit } from 'simple-git';
import mkdirp from 'mkdirp';
import { Octokit } from '@octokit/rest';
import { Repository } from './configDef';
Expand Down
59 changes: 38 additions & 21 deletions packages/cspell-glob/src/GlobMatcher.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ describe('Validate Micromatch assumptions', () => {
${'**/*.json'} | ${'src/settings.json'} | ${true}
${'**/*.json'} | ${'/src/settings.json'} | ${true}
${'**/temp'} | ${'/src/temp/data.json'} | ${false}
${'**/temp/'} | ${'/src/temp/data.json'} | ${false}
${'**/temp/**'} | ${'/src/temp/data.json'} | ${true}
${'src/*.json'} | ${'src/settings.json'} | ${true}
${'**/{*.json,*.json/**}'} | ${'settings.json'} | ${true}
${'**/{*.json,*.json/**}'} | ${'/settings.json'} | ${true}
Expand Down Expand Up @@ -130,8 +132,11 @@ describe('Tests .gitignore file contents', () => {
!**/settings.js
!!*.txt
# *.py,cover
**/temp/**
/**/temp-local/**
`;
const root = '/Users/code/project/cspell/';
const nonRoot = '/Users/guest/code/';
// cspell:ignore nobrace
// const matcher = new GlobMatcher(pattern, root);
const matcher = new GlobMatcher(pattern, { root, nobrace: false });
Expand All @@ -143,26 +148,32 @@ describe('Tests .gitignore file contents', () => {
}

test.each`
filename | expected | comment
${root + 'src/code.py'} | ${false} | ${'Ensure that .py files are allowed'}
${root + 'src/code.ts'} | ${false} | ${'Ensure that .ts files are allowed'}
${root + 'dist/code.ts'} | ${true} | ${'Ensure that `dest` .ts files are not allowed'}
${root + 'src/code.js'} | ${true} | ${'Ensure that no .js files are allowed'}
${root + 'src/code.test.ts'} | ${true} | ${'Ensure that test.ts files are not allowed'}
${root + 'src/code.spec.ts'} | ${true} | ${'Ensure that spec.ts files are not allowed'}
${'/Users/guest/code/' + 'src/code.test.ts'} | ${false} | ${'Ensure that test files in a different root are allowed'}
${'/Users/guest/code/' + 'src/code.js'} | ${false} | ${'Ensure *.js files are allowed under a different root.'}
${root + 'node_modules/cspell/code.ts'} | ${true} | ${'Ensure that node modules are not allowed in the current root.'}
${root + 'nested/node_modules/cspell/code.ts'} | ${false} | ${'Ensure that nested node modules are allowed in the current root.'}
${'/Users/guest/code/' + 'node_modules/cspell/code.ts'} | ${false} | ${'Ensure that node modules in a different root are allowed'}
${root + 'settings.js'} | ${false} | ${'Ensure that settings.js is kept'}
${root + 'dist/settings.js'} | ${false} | ${'Ensure that settings.js is kept'}
${root + 'node_modules/settings.js'} | ${false} | ${'Ensure that settings.js is kept'}
${root + 'src.txt'} | ${true} | ${'Ensure that double negative means block'}
${root + 'src/code.ts'} | ${{ matched: false }} | ${'Ensure that .ts files are allowed'}
${root + 'dist/code.ts'} | ${{ matched: true, pattern: p('**/dist/**'), isNeg: false }} | ${'Ensure that `dest` .ts files are not allowed'}
${root + 'src/code.js'} | ${{ matched: true, pattern: p('**/*.js'), isNeg: false }} | ${'Ensure that no .js files are allowed'}
${root + 'dist/settings.js'} | ${{ matched: false, pattern: p('!**/settings.js'), isNeg: true }} | ${'Ensure that settings.js is kept'}
filename | expected | comment
${root + 'src/code.py'} | ${false} | ${'Ensure that .py files are allowed'}
${root + 'src/code.ts'} | ${false} | ${'Ensure that .ts files are allowed'}
${root + 'dist/code.ts'} | ${true} | ${'Ensure that `dest` .ts files are not allowed'}
${root + 'src/code.js'} | ${true} | ${'Ensure that no .js files are allowed'}
${root + 'src/code.test.ts'} | ${true} | ${'Ensure that test.ts files are not allowed'}
${root + 'src/code.spec.ts'} | ${true} | ${'Ensure that spec.ts files are not allowed'}
${nonRoot + 'src/code.test.ts'} | ${false} | ${'Ensure that test files in a different root are allowed'}
${nonRoot + 'src/code.js'} | ${false} | ${'Ensure *.js files are allowed under a different root.'}
${root + 'node_modules/cspell/code.ts'} | ${true} | ${'Ensure that node modules are not allowed in the current root.'}
${root + 'nested/node_modules/cspell/code.ts'} | ${false} | ${'Ensure that nested node modules are allowed in the current root.'}
${nonRoot + 'node_modules/cspell/code.ts'} | ${false} | ${'Ensure that node modules in a different root are allowed'}
${root + 'settings.js'} | ${false} | ${'Ensure that settings.js is kept'}
${root + 'dist/settings.js'} | ${false} | ${'Ensure that settings.js is kept'}
${root + 'node_modules/settings.js'} | ${false} | ${'Ensure that settings.js is kept'}
${root + 'src/src.cpp'} | ${false} | ${'Ensure code is kept'}
${root + 'temp/src/src.cpp'} | ${true} | ${'Ensure temp is rejected'}
${root + 'nested/temp/src/src.cpp'} | ${true} | ${'Ensure nested temp is rejected'}
${root + 'nested/temp-local/src/src.cpp'} | ${true} | ${'Ensure nested temp-local is rejected'}
${nonRoot + 'nested/temp/src/src.cpp'} | ${true} | ${'Ensure non-root nested temp is rejected'}
${root + 'src.txt'} | ${true} | ${'Ensure that double negative means block'}
${root + 'src.txt'} | ${true} | ${'Ensure that double negative means block'}
${root + 'src/code.ts'} | ${{ matched: false }} | ${'Ensure that .ts files are allowed'}
${root + 'dist/code.ts'} | ${{ matched: true, pattern: p('**/dist/**'), isNeg: false }} | ${'Ensure that `dest` .ts files are not allowed'}
${root + 'src/code.js'} | ${{ matched: true, pattern: p('**/*.js'), isNeg: false }} | ${'Ensure that no .js files are allowed'}
${root + 'dist/settings.js'} | ${{ matched: false, pattern: p('!**/settings.js'), isNeg: true }} | ${'Ensure that settings.js is kept'}
`('match && matchEx "$comment" File: "$filename" $expected', ({ filename, expected }: TestCase) => {
expected = typeof expected === 'boolean' ? { matched: expected } : expected;
expect(matcher.match(filename)).toBe(expected.matched);
Expand Down Expand Up @@ -447,7 +458,13 @@ describe('Validate GlobMatcher excludeMode patternsNormalizedToRoot', () => {
);
});

type TestCase = [string[] | string, string | undefined, string, boolean, string];
type TestCase = [
patterns: string[] | string,
root: string | undefined,
filename: string,
expected: boolean,
description: string
];

function tests(): TestCase[] {
const from = 0;
Expand Down
32 changes: 16 additions & 16 deletions packages/cspell-glob/src/GlobMatcher.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import mm = require('micromatch');
import * as Path from 'path';
import { normalizeGlobPatterns, doesRootContainPath, normalizeGlobToRoot } from './globHelper';
import { PathInterface, GlobMatch, GlobPattern, GlobPatternWithRoot } from './GlobMatcherTypes';
import { PathInterface, GlobMatch, GlobPattern, GlobPatternWithRoot, GlobPatternNormalized } from './GlobMatcherTypes';

// cspell:ignore fname

Expand Down Expand Up @@ -80,7 +80,7 @@ export class GlobMatcher {
readonly matchEx: (filename: string) => GlobMatch;
readonly path: PathInterface;
readonly patterns: GlobPatternWithRoot[];
readonly patternsNormalizedToRoot: GlobPatternWithRoot[];
readonly patternsNormalizedToRoot: GlobPatternNormalized[];
readonly root: string;
readonly dot: boolean;
readonly options: NormalizedGlobMatchOptions;
Expand Down Expand Up @@ -119,7 +119,7 @@ export class GlobMatcher {
nested = isExcludeMode,
cwd = process.cwd(),
nobrace,
} = clean(options);
} = options;

const normalizedRoot = nodePath.resolve(nodePath.normalize(root));
this.options = { root: normalizedRoot, dot, nodePath, nested, mode, nobrace, cwd };
Expand Down Expand Up @@ -157,9 +157,19 @@ export class GlobMatcher {
type GlobMatchFn = (filename: string) => GlobMatch;

interface GlobRule {
/** The pattern */
pattern: GlobPatternWithRoot;
/**
* Index of the glob in the list.
*/
index: number;
/**
* Is a negated pattern
*/
isNeg: boolean;
/**
* the glob converted to a regexp.
*/
reg: RegExp;
fn: (filename: string) => boolean;
}
Expand Down Expand Up @@ -204,10 +214,11 @@ function buildMatcherFn(patterns: GlobPatternWithRoot[], options: NormalizedGlob
for (const rule of rules) {
const pattern = rule.pattern;
const root = pattern.root;
if (!doesRootContainPath(root, filename, path)) {
const isRelPat = !pattern.isGlobalPattern;
if (isRelPat && !doesRootContainPath(root, filename, path)) {
continue;
}
const relName = path.relative(root, filename);
const relName = isRelPat ? path.relative(root, filename) : filename;
const fname = path.sep === '\\' ? relName.replace(/\\/g, '/') : relName;
if (rule.fn(fname)) {
return {
Expand All @@ -226,14 +237,3 @@ function buildMatcherFn(patterns: GlobPatternWithRoot[], options: NormalizedGlob
};
return fn;
}

function clean<T>(obj: T): T {
if (typeof obj !== 'object') return obj;
const r = <Record<string, unknown>>obj;
for (const key of Object.keys(r)) {
if (r[key] === undefined) {
delete r[key];
}
}
return obj;
}
4 changes: 4 additions & 0 deletions packages/cspell-glob/src/GlobMatcherTypes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ export interface GlobPatternWithOptionalRoot {

export interface GlobPatternWithRoot extends GlobPatternWithOptionalRoot {
root: string;
/**
* Global patterns do not need to be relative to the root.
*/
isGlobalPattern: boolean;
}

export interface GlobPatternNormalized extends GlobPatternWithRoot {
Expand Down
Loading

0 comments on commit 7c0ee59

Please sign in to comment.