From efc1b29f09efe879241e3b11e710d114dc56ef1f Mon Sep 17 00:00:00 2001 From: Jameel Al-Aziz <247849+jalaziz@users.noreply.github.com> Date: Fri, 19 Jun 2020 07:55:47 -0700 Subject: [PATCH] Fix "any" matching when not all globs match (#75) The previous logic had a bug where the "any" pattern list could match against changed files even when not all globs matched a single changed file. The bug arose individual globs in the "any" list were tested against all changed files individually. Therefore, as long as at least one changed file matched an individual glob in the list, a successful match would be found. The correct behavior is to match all the globs in the list against each individual file. This ensures that is possible to define exlcusions correctly and matched the documented behavior. --- dist/index.js | 51 +++++++++++++++++++++++++++++------------------- src/main.ts | 54 ++++++++++++++++++++++++++++++++------------------- 2 files changed, 65 insertions(+), 40 deletions(-) diff --git a/dist/index.js b/dist/index.js index 5edc4203b..3840d586e 100644 --- a/dist/index.js +++ b/dist/index.js @@ -4207,6 +4207,9 @@ function toMatchConfig(config) { } return config; } +function printPattern(matcher) { + return (matcher.negate ? "!" : "") + matcher.pattern; +} function checkGlobs(changedFiles, globs) { for (const glob of globs) { core.debug(` checking pattern ${JSON.stringify(glob)}`); @@ -4217,45 +4220,53 @@ function checkGlobs(changedFiles, globs) { } return false; } +function isMatch(changedFile, matchers) { + core.debug(` matching patterns against file ${changedFile}`); + for (const matcher of matchers) { + core.debug(` - ${printPattern(matcher)}`); + if (!matcher.match(changedFile)) { + core.debug(` ${printPattern(matcher)} did not match`); + return false; + } + } + core.debug(` all patterns matched`); + return true; +} // equivalent to "Array.some()" but expanded for debugging and clarity -function checkAny(changedFiles, glob) { - core.debug(` checking "any" pattern ${glob}`); - const matcher = new minimatch_1.Minimatch(glob); +function checkAny(changedFiles, globs) { + const matchers = globs.map(g => new minimatch_1.Minimatch(g)); + core.debug(` checking "any" patterns`); for (const changedFile of changedFiles) { - core.debug(` - ${changedFile}`); - if (matcher.match(changedFile)) { - core.debug(` ${changedFile} matches`); + if (isMatch(changedFile, matchers)) { + core.debug(` "any" patterns matched against ${changedFile}`); return true; } } + core.debug(` "any" patterns did not match any files`); return false; } // equivalent to "Array.every()" but expanded for debugging and clarity -function checkAll(changedFiles, glob) { - core.debug(` checking "all" pattern ${glob}`); - const matcher = new minimatch_1.Minimatch(glob); +function checkAll(changedFiles, globs) { + const matchers = globs.map(g => new minimatch_1.Minimatch(g)); + core.debug(` checking "all" patterns`); for (const changedFile of changedFiles) { - core.debug(` - ${changedFile}`); - if (!matcher.match(changedFile)) { - core.debug(` ${changedFile} did not match`); + if (!isMatch(changedFile, matchers)) { + core.debug(` "all" patterns did not match against ${changedFile}`); return false; } } + core.debug(` "all" patterns matched all files`); return true; } function checkMatch(changedFiles, matchConfig) { if (matchConfig.all !== undefined) { - for (const glob of matchConfig.all) { - if (!checkAll(changedFiles, glob)) { - return false; - } + if (!checkAll(changedFiles, matchConfig.all)) { + return false; } } if (matchConfig.any !== undefined) { - for (const glob of matchConfig.any) { - if (!checkAny(changedFiles, glob)) { - return false; - } + if (!checkAny(changedFiles, matchConfig.any)) { + return false; } } return true; diff --git a/src/main.ts b/src/main.ts index dab60ecfd..3fffa6393 100644 --- a/src/main.ts +++ b/src/main.ts @@ -135,6 +135,10 @@ function toMatchConfig(config: StringOrMatchConfig): MatchConfig { return config; } +function printPattern(matcher: IMinimatch): string { + return (matcher.negate ? "!" : "") + matcher.pattern; +} + function checkGlobs( changedFiles: string[], globs: StringOrMatchConfig[] @@ -149,50 +153,60 @@ function checkGlobs( return false; } +function isMatch(changedFile: string, matchers: IMinimatch[]): boolean { + core.debug(` matching patterns against file ${changedFile}`); + for (const matcher of matchers) { + core.debug(` - ${printPattern(matcher)}`); + if (!matcher.match(changedFile)) { + core.debug(` ${printPattern(matcher)} did not match`); + return false; + } + } + + core.debug(` all patterns matched`); + return true; +} + // equivalent to "Array.some()" but expanded for debugging and clarity -function checkAny(changedFiles: string[], glob: string): boolean { - core.debug(` checking "any" pattern ${glob}`); - const matcher = new Minimatch(glob); +function checkAny(changedFiles: string[], globs: string[]): boolean { + const matchers = globs.map(g => new Minimatch(g)); + core.debug(` checking "any" patterns`); for (const changedFile of changedFiles) { - core.debug(` - ${changedFile}`); - if (matcher.match(changedFile)) { - core.debug(` ${changedFile} matches`); + if (isMatch(changedFile, matchers)) { + core.debug(` "any" patterns matched against ${changedFile}`); return true; } } + core.debug(` "any" patterns did not match any files`); return false; } // equivalent to "Array.every()" but expanded for debugging and clarity -function checkAll(changedFiles: string[], glob: string): boolean { - core.debug(` checking "all" pattern ${glob}`); - const matcher = new Minimatch(glob); +function checkAll(changedFiles: string[], globs: string[]): boolean { + const matchers = globs.map(g => new Minimatch(g)); + core.debug(` checking "all" patterns`); for (const changedFile of changedFiles) { - core.debug(` - ${changedFile}`); - if (!matcher.match(changedFile)) { - core.debug(` ${changedFile} did not match`); + if (!isMatch(changedFile, matchers)) { + core.debug(` "all" patterns did not match against ${changedFile}`); return false; } } + core.debug(` "all" patterns matched all files`); return true; } function checkMatch(changedFiles: string[], matchConfig: MatchConfig): boolean { if (matchConfig.all !== undefined) { - for (const glob of matchConfig.all) { - if (!checkAll(changedFiles, glob)) { - return false; - } + if (!checkAll(changedFiles, matchConfig.all)) { + return false; } } if (matchConfig.any !== undefined) { - for (const glob of matchConfig.any) { - if (!checkAny(changedFiles, glob)) { - return false; - } + if (!checkAny(changedFiles, matchConfig.any)) { + return false; } }