Skip to content

Commit

Permalink
Fix "any" matching when not all globs match (#75)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jalaziz
jalaziz committed Jun 19, 2020
1 parent 4b52aec commit efc1b29
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 40 deletions.
51 changes: 31 additions & 20 deletions dist/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`);
Expand All @@ -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;
Expand Down
54 changes: 34 additions & 20 deletions src/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand All @@ -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;
}
}

Expand Down

0 comments on commit efc1b29

Please sign in to comment.