This commit is contained in:
Hesham Salman 2026-04-27 16:35:43 -04:00 committed by GitHub
commit acb8e488c5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 316 additions and 16 deletions

View File

@ -17,6 +17,24 @@ describe('yaml filter parsing tests', () => {
const t = () => new Filter(yaml)
expect(t).toThrow(/^Invalid filter.*/)
})
test('throws when a rule contains only negation patterns', () => {
const yaml = `
excludes:
- '!**/*.md'
- '!**/*.txt'
`
expect(() => new Filter(yaml)).toThrow(/at least one positive pattern/)
})
test('throws when a status-tagged rule contains only negation patterns', () => {
const yaml = `
docs:
- modified:
- '!**/*.md'
`
expect(() => new Filter(yaml)).toThrow(/at least one positive pattern/)
})
})
describe('matching tests', () => {
@ -148,6 +166,92 @@ describe('matching tests', () => {
expect(otherPkgJpegMatch.backend).toEqual([])
})
test('negation patterns under default quantifier exclude files instead of matching everything (issue #260)', () => {
const yaml = `
mobile:
- 'mobile/**'
- '!mobile/**/*.md'
- '!mobile/.config/**'
- '.github/workflows/test_mobile.yml'
`
const filter = new Filter(yaml)
// Files outside the included path must NOT match purely because they are
// not mobile markdown files. This was the original bug: a standalone
// '!mobile/**/*.md' picomatch returned true for any non-markdown path,
// and the default 'some' quantifier flipped the rule into a near-universal match.
const unrelated = modified(['web/src/foo.tsx', 'docs/README.md', 'server/main.go'])
expect(filter.match(unrelated).mobile).toEqual([])
// Mobile sources should still match.
const mobileSrc = modified(['mobile/src/app.ts', 'mobile/lib/index.ts'])
expect(filter.match(mobileSrc).mobile).toEqual(mobileSrc)
// Negated paths inside the include set must be excluded.
const mobileExcluded = modified(['mobile/README.md', 'mobile/.config/eslint.json'])
expect(filter.match(mobileExcluded).mobile).toEqual([])
// The standalone workflow path must still match.
const workflow = modified(['.github/workflows/test_mobile.yml'])
expect(filter.match(workflow).mobile).toEqual(workflow)
})
test('negation across YAML anchors is honored under default quantifier', () => {
const yaml = `
shared: &shared
- 'common/**'
- '!**/*.md'
- '!**/*.txt'
src:
- 'src/**'
- *shared
`
const filter = new Filter(yaml)
// Anchor-inherited positives still match.
expect(filter.match(modified(['common/util.ts'])).src).toEqual(modified(['common/util.ts']))
// The rule's own positive still matches.
expect(filter.match(modified(['src/app.ts'])).src).toEqual(modified(['src/app.ts']))
// Anchor-inherited negations exclude files even when a sibling positive matches.
expect(filter.match(modified(['src/README.md'])).src).toEqual([])
expect(filter.match(modified(['common/notes.txt'])).src).toEqual([])
// Files outside every positive pattern do not match.
expect(filter.match(modified(['other/file.ts'])).src).toEqual([])
})
test('status-tagged array honors negation patterns (issue #260, status form)', () => {
const yaml = `
src:
- modified:
- 'src/**'
- '!src/**/*.md'
`
const filter = new Filter(yaml)
const tsFile = modified(['src/app.ts'])
const mdFile = modified(['src/README.md'])
const unrelated = modified(['docs/intro.md'])
expect(filter.match(tsFile).src).toEqual(tsFile)
expect(filter.match(mdFile).src).toEqual([])
expect(filter.match(unrelated).src).toEqual([])
})
test('mixing string patterns and status-tagged patterns still matches both forms', () => {
const yaml = `
backend:
- 'src/**'
- '!src/**/*.md'
- added: 'migrations/**'
`
const filter = new Filter(yaml)
expect(filter.match(modified(['src/server.ts'])).backend).toEqual(modified(['src/server.ts']))
expect(filter.match(modified(['src/README.md'])).backend).toEqual([])
const addedMigration: File[] = [{status: ChangeStatus.Added, filename: 'migrations/0001.sql'}]
expect(filter.match(addedMigration).backend).toEqual(addedMigration)
const modifiedMigration = modified(['migrations/0001.sql'])
expect(filter.match(modifiedMigration).backend).toEqual([])
})
test('matches path based on rules included using YAML anchor', () => {
const yaml = `
shared: &shared

111
dist/index.js vendored
View File

@ -138,7 +138,33 @@ class Filter {
}
}
parseFilterItemYaml(item) {
var _a;
if (Array.isArray(item)) {
// Under the default 'some' quantifier, group all (recursively flattened)
// bare string patterns into a single matcher with gitignore-style
// semantics: a file matches the rule when it matches at least one
// positive pattern AND does not match any negation pattern.
//
// Without this grouping, each '!pattern' is compiled into its own
// picomatch matcher that returns true for every file *not* matching the
// pattern. The default 'some' quantifier then OR's those predicates
// together, so a standalone '!**/*.md' makes the whole rule match
// nearly any path. The 'every' quantifier already produces correct
// subtractive semantics under per-pattern matching, so it keeps the
// legacy parsing path unchanged.
if (((_a = this.filterConfig) === null || _a === void 0 ? void 0 : _a.predicateQuantifier) !== PredicateQuantifier.EVERY) {
const { stringPatterns, otherItems } = this.collectArrayItems(item);
const grouped = this.groupedStringMatcher(stringPatterns);
if (grouped === undefined && otherItems.length === 0) {
this.throwInvalidFormatError('Filter rule must contain at least one positive pattern; got only negation patterns or an empty pattern list');
}
const result = [];
if (grouped !== undefined) {
result.push({ status: undefined, isMatch: grouped });
}
result.push(...otherItems);
return result;
}
return flat(item.map(i => this.parseFilterItemYaml(i)));
}
if (typeof item === 'string') {
@ -149,18 +175,87 @@ class Filter {
if (typeof key !== 'string' || (typeof pattern !== 'string' && !Array.isArray(pattern))) {
this.throwInvalidFormatError(`Expected [key:string]= pattern:string | string[], but [${key}:${typeof key}]= ${pattern}:${typeof pattern} found`);
}
return {
status: key
.split('|')
.map(x => x.trim())
.filter(x => x.length > 0)
.map(x => x.toLowerCase()),
isMatch: (0, picomatch_1.default)(pattern, MatchOptions)
};
const status = key
.split('|')
.map(x => x.trim())
.filter(x => x.length > 0)
.map(x => x.toLowerCase());
return { status, isMatch: this.compileStatusPattern(pattern, key) };
});
}
this.throwInvalidFormatError(`Unexpected element type '${typeof item}'`);
}
// Recursively walk a YAML array (which may contain nested arrays from YAML
// anchors) and partition its leaves into raw string patterns vs. fully
// parsed FilterRuleItems for status-tagged objects.
collectArrayItems(item) {
if (Array.isArray(item)) {
const stringPatterns = [];
const otherItems = [];
for (const i of item) {
const sub = this.collectArrayItems(i);
stringPatterns.push(...sub.stringPatterns);
otherItems.push(...sub.otherItems);
}
return { stringPatterns, otherItems };
}
if (typeof item === 'string') {
return { stringPatterns: [item], otherItems: [] };
}
return { stringPatterns: [], otherItems: this.parseFilterItemYaml(item) };
}
// Compiles the right-hand side of a status-tagged YAML entry (e.g.
// `added: 'src/**'` or `added: ['src/**', '!src/**/*.md']`) into a single
// matcher. String-array forms are routed through groupedStringMatcher so
// they get the same gitignore-style negation semantics as bare-array rules
// - otherwise the same #260 bug shape would still bite under a status tag.
// A single string pattern keeps the legacy single-picomatch compilation,
// which preserves existing behavior for `!(extglob)` and plain literals.
compileStatusPattern(pattern, key) {
if (typeof pattern === 'string') {
return (0, picomatch_1.default)(pattern, MatchOptions);
}
const matcher = this.groupedStringMatcher(pattern);
if (matcher === undefined) {
this.throwInvalidFormatError(`Status-tagged filter '${key}' must contain at least one positive pattern; got only negation patterns or an empty list`);
}
return matcher;
}
// Builds a single matcher with gitignore-style semantics over a list of
// string patterns: a file matches when at least one positive pattern matches
// and no negation pattern matches.
//
// A pattern is treated as a gitignore-style negation only when it begins
// with '!' followed by anything other than '('. The '!(...)' form is an
// extglob expression that picomatch parses as a single pattern, so it must
// not be split into a positive/negative pair.
//
// Returns undefined when the list contains no positive patterns - in that
// case there is nothing to include, so the rule cannot match any file.
groupedStringMatcher(patterns) {
const positives = [];
const negatives = [];
for (const p of patterns) {
if (this.isNegationPrefix(p)) {
negatives.push(p.slice(1));
}
else {
positives.push(p);
}
}
if (positives.length === 0) {
return undefined;
}
const positiveMatcher = (0, picomatch_1.default)(positives, MatchOptions);
if (negatives.length === 0) {
return positiveMatcher;
}
const negativeMatcher = (0, picomatch_1.default)(negatives, MatchOptions);
return (str) => positiveMatcher(str) && !negativeMatcher(str);
}
isNegationPrefix(pattern) {
return pattern.length > 1 && pattern.startsWith('!') && !pattern.startsWith('!(');
}
throwInvalidFormatError(message) {
throw new Error(`Invalid filter YAML format: ${message}.`);
}

View File

@ -116,6 +116,33 @@ export class Filter {
private parseFilterItemYaml(item: FilterItemYaml): FilterRuleItem[] {
if (Array.isArray(item)) {
// Under the default 'some' quantifier, group all (recursively flattened)
// bare string patterns into a single matcher with gitignore-style
// semantics: a file matches the rule when it matches at least one
// positive pattern AND does not match any negation pattern.
//
// Without this grouping, each '!pattern' is compiled into its own
// picomatch matcher that returns true for every file *not* matching the
// pattern. The default 'some' quantifier then OR's those predicates
// together, so a standalone '!**/*.md' makes the whole rule match
// nearly any path. The 'every' quantifier already produces correct
// subtractive semantics under per-pattern matching, so it keeps the
// legacy parsing path unchanged.
if (this.filterConfig?.predicateQuantifier !== PredicateQuantifier.EVERY) {
const {stringPatterns, otherItems} = this.collectArrayItems(item)
const grouped = this.groupedStringMatcher(stringPatterns)
if (grouped === undefined && otherItems.length === 0) {
this.throwInvalidFormatError(
'Filter rule must contain at least one positive pattern; got only negation patterns or an empty pattern list'
)
}
const result: FilterRuleItem[] = []
if (grouped !== undefined) {
result.push({status: undefined, isMatch: grouped})
}
result.push(...otherItems)
return result
}
return flat(item.map(i => this.parseFilterItemYaml(i)))
}
@ -130,20 +157,94 @@ export class Filter {
`Expected [key:string]= pattern:string | string[], but [${key}:${typeof key}]= ${pattern}:${typeof pattern} found`
)
}
return {
status: key
.split('|')
.map(x => x.trim())
.filter(x => x.length > 0)
.map(x => x.toLowerCase()) as ChangeStatus[],
isMatch: picomatch(pattern, MatchOptions)
}
const status = key
.split('|')
.map(x => x.trim())
.filter(x => x.length > 0)
.map(x => x.toLowerCase()) as ChangeStatus[]
return {status, isMatch: this.compileStatusPattern(pattern, key)}
})
}
this.throwInvalidFormatError(`Unexpected element type '${typeof item}'`)
}
// Recursively walk a YAML array (which may contain nested arrays from YAML
// anchors) and partition its leaves into raw string patterns vs. fully
// parsed FilterRuleItems for status-tagged objects.
private collectArrayItems(item: FilterItemYaml): {stringPatterns: string[]; otherItems: FilterRuleItem[]} {
if (Array.isArray(item)) {
const stringPatterns: string[] = []
const otherItems: FilterRuleItem[] = []
for (const i of item) {
const sub = this.collectArrayItems(i)
stringPatterns.push(...sub.stringPatterns)
otherItems.push(...sub.otherItems)
}
return {stringPatterns, otherItems}
}
if (typeof item === 'string') {
return {stringPatterns: [item], otherItems: []}
}
return {stringPatterns: [], otherItems: this.parseFilterItemYaml(item)}
}
// Compiles the right-hand side of a status-tagged YAML entry (e.g.
// `added: 'src/**'` or `added: ['src/**', '!src/**/*.md']`) into a single
// matcher. String-array forms are routed through groupedStringMatcher so
// they get the same gitignore-style negation semantics as bare-array rules
// - otherwise the same #260 bug shape would still bite under a status tag.
// A single string pattern keeps the legacy single-picomatch compilation,
// which preserves existing behavior for `!(extglob)` and plain literals.
private compileStatusPattern(pattern: string | string[], key: string): (str: string) => boolean {
if (typeof pattern === 'string') {
return picomatch(pattern, MatchOptions)
}
const matcher = this.groupedStringMatcher(pattern)
if (matcher === undefined) {
this.throwInvalidFormatError(
`Status-tagged filter '${key}' must contain at least one positive pattern; got only negation patterns or an empty list`
)
}
return matcher
}
// Builds a single matcher with gitignore-style semantics over a list of
// string patterns: a file matches when at least one positive pattern matches
// and no negation pattern matches.
//
// A pattern is treated as a gitignore-style negation only when it begins
// with '!' followed by anything other than '('. The '!(...)' form is an
// extglob expression that picomatch parses as a single pattern, so it must
// not be split into a positive/negative pair.
//
// Returns undefined when the list contains no positive patterns - in that
// case there is nothing to include, so the rule cannot match any file.
private groupedStringMatcher(patterns: string[]): ((str: string) => boolean) | undefined {
const positives: string[] = []
const negatives: string[] = []
for (const p of patterns) {
if (this.isNegationPrefix(p)) {
negatives.push(p.slice(1))
} else {
positives.push(p)
}
}
if (positives.length === 0) {
return undefined
}
const positiveMatcher = picomatch(positives, MatchOptions)
if (negatives.length === 0) {
return positiveMatcher
}
const negativeMatcher = picomatch(negatives, MatchOptions)
return (str: string) => positiveMatcher(str) && !negativeMatcher(str)
}
private isNegationPrefix(pattern: string): boolean {
return pattern.length > 1 && pattern.startsWith('!') && !pattern.startsWith('!(')
}
private throwInvalidFormatError(message: string): never {
throw new Error(`Invalid filter YAML format: ${message}.`)
}