feat(workflow): compile filter patterns at parse time and reject negation-only lists

commit: d7622c64d387aa5f88065dbcc56db5d5bcc055bf
author: brice · 10d ago · 2026-10-12 16:14 UTC
committer: brice · 10d ago · 2026-10-12 16:17 UTC
parent: a07f9c8
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +170 -61

M workflow/events.go +89 -14


@@ -4,6 +4,7 @@ package workflow
4	4
5	5		import (
6	6		"bnema.dev/ermite-actions/template"
	7	+	"strings"
7	8		)
8	9
9	10		// On is the "on" key: the events that trigger a workflow.

@@ -47,6 +48,8 @@ type Event struct {
47	48		Names []string
48	49		Workflows []string
49	50
	51	+	cf *compiledFilters
	52	+
50	53		// Schedule holds the entries of the "schedule" event.
51	54		Schedule []Schedule
52	55		// Inputs holds the inputs of workflow_dispatch and workflow_call.

@@ -270,29 +273,101 @@ func (c converter) convertInput(name string, m template.Mapping, call bool) In
270	273		return in
271	274		}
272	275
273		-	// filterPairs are the filters GitHub does not allow together on one event.
274		-	var filterPairs = [][2]string{{"branches", "branches-ignore"}, {"tags", "tags-ignore"}, {"paths", "paths-ignore"}}
	276	+	// filterNames are the pattern filters of an event, in the order they are
	277	+	// checked and reported.
	278	+	var filterNames = []string{"branches", "branches-ignore", "tags", "tags-ignore", "paths", "paths-ignore"}
	279	+
	280	+	// filterLists returns the lists of the six pattern filters by name.
	281	+	func (e *Event) filterLists() map[string][]string {
	282	+	return map[string][]string{
	283	+	"branches": e.Branches, "branches-ignore": e.BranchesIgnore,
	284	+	"tags": e.Tags, "tags-ignore": e.TagsIgnore,
	285	+	"paths": e.Paths, "paths-ignore": e.PathsIgnore,
	286	+	}
	287	+	}
275	288
276		-	// validateFilters reports an event that uses a filter and its "-ignore"
277		-	// counterpart. @actions/workflow-parser accepts this; GitHub rejects the
278		-	// file ("you can't use both"), so this port does too. The message is ours.
	289	+	// compiledFilters holds the compiled pattern lists of an event.
	290	+	type compiledFilters struct {
	291	+	branches, branchesIgnore, tags, tagsIgnore, paths, pathsIgnore []*Pattern
	292	+	}
	293	+
	294	+	func (e Event) compileFilters() (compiledFilters, map[string]error) {
	295	+	errs := map[string]error{}
	296	+	var cf compiledFilters
	297	+	dst := map[string][]Pattern{
	298	+	"branches": &cf.branches, "branches-ignore": &cf.branchesIgnore,
	299	+	"tags": &cf.tags, "tags-ignore": &cf.tagsIgnore,
	300	+	"paths": &cf.paths, "paths-ignore": &cf.pathsIgnore,
	301	+	}
	302	+	for name, list := range e.filterLists() {
	303	+	l, err := compileList(list)
	304	+	if err != nil {
	305	+	errs[name] = err
	306	+	}
	307	+	*dst[name] = l
	308	+	}
	309	+	return &cf, errs
	310	+	}
	311	+
	312	+	// filters returns the compiled filters: those made by Parse, or, for an
	313	+	// Event built by hand, a fresh compilation (a pattern that does not compile
	314	+	// never matches).
	315	+	func (e Event) filters() compiledFilters {
	316	+	if e.cf != nil {
	317	+	return e.cf
	318	+	}
	319	+	cf, _ := e.compileFilters()
	320	+	return cf
	321	+	}
	322	+
	323	+	// validateFilters checks the pattern filters of every event, as GitHub does
	324	+	// and @actions/workflow-parser does not:
	325	+	//
	326	+	// - a filter and its "-ignore" counterpart (branches and branches-ignore,
	327	+	// tags and tags-ignore, paths and paths-ignore) cannot be used together;
	328	+	// - a list made only of "!" patterns is an error: such a list can match
	329	+	// nothing, and GitHub requires at least one pattern without "!";
	330	+	// - every pattern must compile.
	331	+	//
	332	+	// The messages are ours. The compiled patterns are kept in the events.
279	333		//
280		-	// TODO(verify with recording): the text and position of GitHub's error, and
281		-	// whether it applies to every event that has these filters.
	334	+	// TODO(verify with recording): the text and position of GitHub's errors,
	335	+	// whether they apply to every event that has these filters, and whether a
	336	+	// "!" pattern is refused in the "-ignore" lists too.
282	337		func (c *converter) validateFilters() {
283	338		if c.w.On == nil {
284	339		return
285	340		}
286	341		for _, ev := range c.w.On.Events {
287		-	have := map[string][]string{
288		-	"branches": ev.Branches, "branches-ignore": ev.BranchesIgnore,
289		-	"tags": ev.Tags, "tags-ignore": ev.TagsIgnore,
290		-	"paths": ev.Paths, "paths-ignore": ev.PathsIgnore,
291		-	}
292		-	for _, pr := range filterPairs {
293		-	if have[pr[0]] != nil && have[pr[1]] != nil {
	342	+	lists := ev.filterLists()
	343	+	for _, pr := range [][2]string{{"branches", "branches-ignore"}, {"tags", "tags-ignore"}, {"paths", "paths-ignore"}} {
	344	+	if lists[pr[0]] != nil && lists[pr[1]] != nil {
294	345		c.ctx.Errorf(ev.Token, "Event '%s' cannot use both '%s' and '%s'. Use only one of them.", ev.Name, pr[0], pr[1])
295	346		}
296	347		}
	348	+	cf, errs := ev.compileFilters()
	349	+	for _, name := range filterNames {
	350	+	list := lists[name]
	351	+	if err := errs[name]; err != nil {
	352	+	c.ctx.Errorf(ev.Token, "Event '%s' filter '%s': %v", ev.Name, name, err)
	353	+	}
	354	+	if onlyNegated(list) {
	355	+	c.ctx.Errorf(ev.Token, "Event '%s' filter '%s' has only patterns that start with '!'. Add at least one pattern without '!'.", ev.Name, name)
	356	+	}
	357	+	}
	358	+	ev.cf = cf
	359	+	}
	360	+	}
	361	+
	362	+	// onlyNegated reports whether a non-empty list has no positive pattern.
	363	+	func onlyNegated(patterns []string) bool {
	364	+	if len(patterns) == 0 {
	365	+	return false
	366	+	}
	367	+	for _, p := range patterns {
	368	+	if !strings.HasPrefix(p, "!") {
	369	+	return false
	370	+	}
297	371		}
	372	+	return true
298	373		}


@@ -4,6 +4,7 @@ package workflow
4			4
5		import (	5		import (
6		"bnema.dev/ermite-actions/template"	6		"bnema.dev/ermite-actions/template"
			7	+	"strings"
7		)	8		)
8			9
9		// On is the "on" key: the events that trigger a workflow.	10		// On is the "on" key: the events that trigger a workflow.

@@ -47,6 +48,8 @@ type Event struct {
47		Names []string	48		Names []string
48		Workflows []string	49		Workflows []string
49			50
			51	+	cf *compiledFilters
			52	+
50		// Schedule holds the entries of the "schedule" event.	53		// Schedule holds the entries of the "schedule" event.
51		Schedule []Schedule	54		Schedule []Schedule
52		// Inputs holds the inputs of workflow_dispatch and workflow_call.	55		// Inputs holds the inputs of workflow_dispatch and workflow_call.

@@ -270,29 +273,101 @@ func (c converter) convertInput(name string, m template.Mapping, call bool) In
270		return in	273		return in
271		}	274		}
272			275
273	-	// filterPairs are the filters GitHub does not allow together on one event.	276	+	// filterNames are the pattern filters of an event, in the order they are
274	-	var filterPairs = [][2]string{{"branches", "branches-ignore"}, {"tags", "tags-ignore"}, {"paths", "paths-ignore"}}	277	+	// checked and reported.
			278	+	var filterNames = []string{"branches", "branches-ignore", "tags", "tags-ignore", "paths", "paths-ignore"}
			279	+
			280	+	// filterLists returns the lists of the six pattern filters by name.
			281	+	func (e *Event) filterLists() map[string][]string {
			282	+	return map[string][]string{
			283	+	"branches": e.Branches, "branches-ignore": e.BranchesIgnore,
			284	+	"tags": e.Tags, "tags-ignore": e.TagsIgnore,
			285	+	"paths": e.Paths, "paths-ignore": e.PathsIgnore,
			286	+	}
			287	+	}
275			288
276	-	// validateFilters reports an event that uses a filter and its "-ignore"	289	+	// compiledFilters holds the compiled pattern lists of an event.
277	-	// counterpart. @actions/workflow-parser accepts this; GitHub rejects the	290	+	type compiledFilters struct {
278	-	// file ("you can't use both"), so this port does too. The message is ours.	291	+	branches, branchesIgnore, tags, tagsIgnore, paths, pathsIgnore []*Pattern
			292	+	}
			293	+
			294	+	func (e Event) compileFilters() (compiledFilters, map[string]error) {
			295	+	errs := map[string]error{}
			296	+	var cf compiledFilters
			297	+	dst := map[string][]Pattern{
			298	+	"branches": &cf.branches, "branches-ignore": &cf.branchesIgnore,
			299	+	"tags": &cf.tags, "tags-ignore": &cf.tagsIgnore,
			300	+	"paths": &cf.paths, "paths-ignore": &cf.pathsIgnore,
			301	+	}
			302	+	for name, list := range e.filterLists() {
			303	+	l, err := compileList(list)
			304	+	if err != nil {
			305	+	errs[name] = err
			306	+	}
			307	+	*dst[name] = l
			308	+	}
			309	+	return &cf, errs
			310	+	}
			311	+
			312	+	// filters returns the compiled filters: those made by Parse, or, for an
			313	+	// Event built by hand, a fresh compilation (a pattern that does not compile
			314	+	// never matches).
			315	+	func (e Event) filters() compiledFilters {
			316	+	if e.cf != nil {
			317	+	return e.cf
			318	+	}
			319	+	cf, _ := e.compileFilters()
			320	+	return cf
			321	+	}
			322	+
			323	+	// validateFilters checks the pattern filters of every event, as GitHub does
			324	+	// and @actions/workflow-parser does not:
			325	+	//
			326	+	// - a filter and its "-ignore" counterpart (branches and branches-ignore,
			327	+	// tags and tags-ignore, paths and paths-ignore) cannot be used together;
			328	+	// - a list made only of "!" patterns is an error: such a list can match
			329	+	// nothing, and GitHub requires at least one pattern without "!";
			330	+	// - every pattern must compile.
			331	+	//
			332	+	// The messages are ours. The compiled patterns are kept in the events.
279		//	333		//
280	-	// TODO(verify with recording): the text and position of GitHub's error, and	334	+	// TODO(verify with recording): the text and position of GitHub's errors,
281	-	// whether it applies to every event that has these filters.	335	+	// whether they apply to every event that has these filters, and whether a
			336	+	// "!" pattern is refused in the "-ignore" lists too.
282		func (c *converter) validateFilters() {	337		func (c *converter) validateFilters() {
283		if c.w.On == nil {	338		if c.w.On == nil {
284		return	339		return
285		}	340		}
286		for _, ev := range c.w.On.Events {	341		for _, ev := range c.w.On.Events {
287	-	have := map[string][]string{	342	+	lists := ev.filterLists()
288	-	"branches": ev.Branches, "branches-ignore": ev.BranchesIgnore,	343	+	for _, pr := range [][2]string{{"branches", "branches-ignore"}, {"tags", "tags-ignore"}, {"paths", "paths-ignore"}} {
289	-	"tags": ev.Tags, "tags-ignore": ev.TagsIgnore,	344	+	if lists[pr[0]] != nil && lists[pr[1]] != nil {
290	-	"paths": ev.Paths, "paths-ignore": ev.PathsIgnore,
291	-	}
292	-	for _, pr := range filterPairs {
293	-	if have[pr[0]] != nil && have[pr[1]] != nil {
294		c.ctx.Errorf(ev.Token, "Event '%s' cannot use both '%s' and '%s'. Use only one of them.", ev.Name, pr[0], pr[1])	345		c.ctx.Errorf(ev.Token, "Event '%s' cannot use both '%s' and '%s'. Use only one of them.", ev.Name, pr[0], pr[1])
295		}	346		}
296		}	347		}
			348	+	cf, errs := ev.compileFilters()
			349	+	for _, name := range filterNames {
			350	+	list := lists[name]
			351	+	if err := errs[name]; err != nil {
			352	+	c.ctx.Errorf(ev.Token, "Event '%s' filter '%s': %v", ev.Name, name, err)
			353	+	}
			354	+	if onlyNegated(list) {
			355	+	c.ctx.Errorf(ev.Token, "Event '%s' filter '%s' has only patterns that start with '!'. Add at least one pattern without '!'.", ev.Name, name)
			356	+	}
			357	+	}
			358	+	ev.cf = cf
			359	+	}
			360	+	}
			361	+
			362	+	// onlyNegated reports whether a non-empty list has no positive pattern.
			363	+	func onlyNegated(patterns []string) bool {
			364	+	if len(patterns) == 0 {
			365	+	return false
			366	+	}
			367	+	for _, p := range patterns {
			368	+	if !strings.HasPrefix(p, "!") {
			369	+	return false
			370	+	}
297		}	371		}
			372	+	return true
298		}	373		}

M workflow/glob.go +19 -24


@@ -3,9 +3,9 @@
3	3		package workflow
4	4
5	5		import (
	6	+	"fmt"
6	7		"regexp"
7	8		"strings"
8		-	"sync"
9	9		)
10	10
11	11		// Pattern is a compiled filter pattern in the syntax GitHub documents as the

@@ -56,25 +56,6 @@ func CompilePattern(p string) (*Pattern, error) {
56	56		// Match reports whether name matches the pattern, ignoring any "!" prefix.
57	57		func (p *Pattern) Match(name string) bool { return p.re != nil && p.re.MatchString(name) }
58	58
59		-	var patternCache sync.Map // string -> *Pattern
60		-
61		-	func cachedPattern(p string) *Pattern {
62		-	if v, ok := patternCache.Load(p); ok {
63		-	return v.(*Pattern)
64		-	}
65		-	pat, _ := CompilePattern(p)
66		-	if n := cacheSize(); n < 4096 {
67		-	patternCache.Store(p, pat)
68		-	}
69		-	return pat
70		-	}
71		-
72		-	func cacheSize() int {
73		-	n := 0
74		-	patternCache.Range(func(_, _ any) bool { n++; return n < 4096 })
75		-	return n
76		-	}
77		-
78	59		// translateGlob converts a pattern to an RE2 expression.
79	60		func translateGlob(p string) string {
80	61		var b strings.Builder

@@ -161,16 +142,30 @@ func parseClass(rs []rune) (string, int, bool) {
161	142		return "", 0, false
162	143		}
163	144
164		-	// matchList applies a list of patterns to name in order. A pattern that
	145	+	// matchList applies compiled patterns to name in order. A pattern that
165	146		// matches includes the name, a "!" pattern that matches excludes it again, and
166	147		// the last pattern that matches decides. A list with no positive pattern
167	148		// therefore matches nothing.
168		-	func matchList(patterns []string, name string) bool {
	149	+	func matchList(patterns []*Pattern, name string) bool {
169	150		matched := false
170		-	for _, s := range patterns {
171		-	if p := cachedPattern(s); p.Match(name) {
	151	+	for _, p := range patterns {
	152	+	if p.Match(name) {
172	153		matched = !p.Negated
173	154		}
174	155		}
175	156		return matched
176	157		}
	158	+
	159	+	// compileList compiles a filter list. It reports the first pattern that does
	160	+	// not compile.
	161	+	func compileList(patterns []string) ([]*Pattern, error) {
	162	+	out := make([]*Pattern, 0, len(patterns))
	163	+	for _, s := range patterns {
	164	+	p, err := CompilePattern(s)
	165	+	if err != nil {
	166	+	return nil, fmt.Errorf("invalid pattern '%s': %w", s, err)
	167	+	}
	168	+	out = append(out, p)
	169	+	}
	170	+	return out, nil
	171	+	}

 package workflow
 import (
+	"fmt"
 	"regexp"
 	"strings"
-	"sync"
 )
 // Pattern is a compiled filter pattern in the syntax GitHub documents as the
 // Match reports whether name matches the pattern, ignoring any "!" prefix.
 func (p *Pattern) Match(name string) bool { return p.re != nil && p.re.MatchString(name) }
-var patternCache sync.Map // string -> *Pattern
-func cachedPattern(p string) *Pattern {
-	if v, ok := patternCache.Load(p); ok {
-		return v.(*Pattern)
-	}
-	pat, _ := CompilePattern(p)
-	if n := cacheSize(); n < 4096 {
-		patternCache.Store(p, pat)
-	}
-	return pat
-}
-func cacheSize() int {
-	n := 0
-	patternCache.Range(func(_, _ any) bool { n++; return n < 4096 })
-	return n
-}
 // translateGlob converts a pattern to an RE2 expression.
 func translateGlob(p string) string {
 	var b strings.Builder
 	return "", 0, false
 }
-// matchList applies a list of patterns to name in order. A pattern that
+// matchList applies compiled patterns to name in order. A pattern that
 // matches includes the name, a "!" pattern that matches excludes it again, and
 // the last pattern that matches decides. A list with no positive pattern
 // therefore matches nothing.
-func matchList(patterns []string, name string) bool {
+func matchList(patterns []*Pattern, name string) bool {
 	matched := false
-	for _, s := range patterns {
+	for _, p := range patterns {
-		if p := cachedPattern(s); p.Match(name) {
+		if p.Match(name) {
 			matched = !p.Negated
 		}
 	}
 	return matched
 }
+// compileList compiles a filter list. It reports the first pattern that does
+// not compile.
+func compileList(patterns []string) ([]*Pattern, error) {
+	out := make([]*Pattern, 0, len(patterns))
+	for _, s := range patterns {
+		p, err := CompilePattern(s)
+		if err != nil {
+			return nil, fmt.Errorf("invalid pattern '%s': %w", s, err)
+		}
+		out = append(out, p)
+	}
+	return out, nil
+}

M workflow/glob_test.go +12 -11


@@ -117,23 +117,24 @@ func TestMatchListOrder(t *testing.T) {
117	117		{nil, "x", false},
118	118		}
119	119		for _, tc := range tests {
120		-	if got := matchList(tc.patterns, tc.name); got != tc.want {
	120	+	if got := matchList(mustList(t, tc.patterns), tc.name); got != tc.want {
121	121		t.Errorf("matchList(%q, %q) = %v, want %v", tc.patterns, tc.name, got, tc.want)
122	122		}
123	123		}
124	124		}
125	125
126		-	func TestBrokenPatternNeverMatches(t *testing.T) {
127		-	for _, p := range []string{"[", "[]", "[a-", "a[!b]", "\\"} {
128		-	pat, err := CompilePattern(p)
129		-	if err != nil {
130		-	continue
131		-	}
132		-	_ = pat.Match("x")
	126	+	func mustList(t testing.TB, patterns []string) []*Pattern {
	127	+	t.Helper()
	128	+	l, err := compileList(patterns)
	129	+	if err != nil {
	130	+	t.Fatal(err)
133	131		}
134		-	if matchList([]string{"[abc"}, "[abc") != true {
135		-	// An unclosed bracket is a literal.
136		-	t.Error("unclosed bracket should be literal")
	132	+	return l
	133	+	}
	134	+
	135	+	func TestUnclosedBracketIsLiteral(t *testing.T) {
	136	+	if !matchList(mustList(t, []string{"[abc"}), "[abc") {
	137	+	t.Error("an unclosed bracket should be literal")
137	138		}
138	139		}
139	140


@@ -117,23 +117,24 @@ func TestMatchListOrder(t *testing.T) {
117		{nil, "x", false},	117		{nil, "x", false},
118		}	118		}
119		for _, tc := range tests {	119		for _, tc := range tests {
120	-	if got := matchList(tc.patterns, tc.name); got != tc.want {	120	+	if got := matchList(mustList(t, tc.patterns), tc.name); got != tc.want {
121		t.Errorf("matchList(%q, %q) = %v, want %v", tc.patterns, tc.name, got, tc.want)	121		t.Errorf("matchList(%q, %q) = %v, want %v", tc.patterns, tc.name, got, tc.want)
122		}	122		}
123		}	123		}
124		}	124		}
125			125
126	-	func TestBrokenPatternNeverMatches(t *testing.T) {	126	+	func mustList(t testing.TB, patterns []string) []*Pattern {
127	-	for _, p := range []string{"[", "[]", "[a-", "a[!b]", "\\"} {	127	+	t.Helper()
128	-	pat, err := CompilePattern(p)	128	+	l, err := compileList(patterns)
129	-	if err != nil {	129	+	if err != nil {
130	-	continue	130	+	t.Fatal(err)
131	-	}
132	-	_ = pat.Match("x")
133		}	131		}
134	-	if matchList([]string{"[abc"}, "[abc") != true {	132	+	return l
135	-	// An unclosed bracket is a literal.	133	+	}
136	-	t.Error("unclosed bracket should be literal")	134	+
			135	+	func TestUnclosedBracketIsLiteral(t *testing.T) {
			136	+	if !matchList(mustList(t, []string{"[abc"}), "[abc") {
			137	+	t.Error("an unclosed bracket should be literal")
137		}	138		}
138		}	139		}
139			140

M workflow/push.go +12 -12


@@ -82,21 +82,20 @@ func (w *Workflow) MatchPush(p PushInfo) bool {
82	82		// refFilterAllows applies the branches or tags filters to the name of a
83	83		// branch (branch is true) or a tag.
84	84		func (e *Event) refFilterAllows(branch bool, name string) bool {
85		-	var inc, exc, otherInc, otherExc []string
86		-	if branch {
87		-	inc, exc, otherInc, otherExc = e.Branches, e.BranchesIgnore, e.Tags, e.TagsIgnore
88		-	} else {
89		-	inc, exc, otherInc, otherExc = e.Tags, e.TagsIgnore, e.Branches, e.BranchesIgnore
	85	+	cf := e.filters()
	86	+	inc, exc := cf.branches, cf.branchesIgnore
	87	+	own, ownIgnore, other, otherIgnore := e.Branches, e.BranchesIgnore, e.Tags, e.TagsIgnore
	88	+	if !branch {
	89	+	inc, exc = cf.tags, cf.tagsIgnore
	90	+	own, ownIgnore, other, otherIgnore = e.Tags, e.TagsIgnore, e.Branches, e.BranchesIgnore
90	91		}
91		-	hasOwn := inc != nil \|\| exc != nil
92		-	hasOther := otherInc != nil \|\| otherExc != nil
93		-	if !hasOwn && hasOther {
	92	+	if own == nil && ownIgnore == nil && (other != nil \|\| otherIgnore != nil) {
94	93		return false
95	94		}
96		-	if inc != nil && !matchList(inc, name) {
	95	+	if own != nil && !matchList(inc, name) {
97	96		return false
98	97		}
99		-	if exc != nil && matchList(exc, name) {
	98	+	if ownIgnore != nil && matchList(exc, name) {
100	99		return false
101	100		}
102	101		return true

@@ -110,6 +109,7 @@ func (e *Event) pathsAllow(p PushInfo) bool {
110	109		if p.FilesTruncated {
111	110		return true
112	111		}
	112	+	cf := e.filters()
113	113		files := p.ChangedFiles
114	114		if len(files) > MaxFilesEvaluated {
115	115		files = files[:MaxFilesEvaluated]

@@ -117,7 +117,7 @@ func (e *Event) pathsAllow(p PushInfo) bool {
117	117		if e.Paths != nil {
118	118		any := false
119	119		for _, f := range files {
120		-	if matchList(e.Paths, f) {
	120	+	if matchList(cf.paths, f) {
121	121		any = true
122	122		break
123	123		}

@@ -129,7 +129,7 @@ func (e *Event) pathsAllow(p PushInfo) bool {
129	129		if e.PathsIgnore != nil && len(files) > 0 {
130	130		any := false
131	131		for _, f := range files {
132		-	if !matchList(e.PathsIgnore, f) {
	132	+	if !matchList(cf.pathsIgnore, f) {
133	133		any = true
134	134		break
135	135		}

 // refFilterAllows applies the branches or tags filters to the name of a
 // branch (branch is true) or a tag.
 func (e *Event) refFilterAllows(branch bool, name string) bool {
-	var inc, exc, otherInc, otherExc []string
+	cf := e.filters()
-	if branch {
+	inc, exc := cf.branches, cf.branchesIgnore
-		inc, exc, otherInc, otherExc = e.Branches, e.BranchesIgnore, e.Tags, e.TagsIgnore
+	own, ownIgnore, other, otherIgnore := e.Branches, e.BranchesIgnore, e.Tags, e.TagsIgnore
-	} else {
+	if !branch {
-		inc, exc, otherInc, otherExc = e.Tags, e.TagsIgnore, e.Branches, e.BranchesIgnore
+		inc, exc = cf.tags, cf.tagsIgnore
+		own, ownIgnore, other, otherIgnore = e.Tags, e.TagsIgnore, e.Branches, e.BranchesIgnore
 	}
-	hasOwn := inc != nil || exc != nil
+	if own == nil && ownIgnore == nil && (other != nil || otherIgnore != nil) {
-	hasOther := otherInc != nil || otherExc != nil
-	if !hasOwn && hasOther {
 		return false
 	}
-	if inc != nil && !matchList(inc, name) {
+	if own != nil && !matchList(inc, name) {
 		return false
 	}
-	if exc != nil && matchList(exc, name) {
+	if ownIgnore != nil && matchList(exc, name) {
 		return false
 	}
 	return true
 	if p.FilesTruncated {
 		return true
 	}
+	cf := e.filters()
 	files := p.ChangedFiles
 	if len(files) > MaxFilesEvaluated {
 		files = files[:MaxFilesEvaluated]
 	if e.Paths != nil {
 		any := false
 		for _, f := range files {
-			if matchList(e.Paths, f) {
+			if matchList(cf.paths, f) {
 				any = true
 				break
 			}
 	if e.PathsIgnore != nil && len(files) > 0 {
 		any := false
 		for _, f := range files {
-			if !matchList(e.PathsIgnore, f) {
+			if !matchList(cf.pathsIgnore, f) {
 				any = true
 				break
 			}

M workflow/push_test.go +38 -0


@@ -4,6 +4,7 @@ package workflow
4	4
5	5		import (
6	6		"fmt"
	7	+	"strings"
7	8		"testing"
8	9		)
9	10

@@ -140,3 +141,40 @@ func TestFilterPairsAreRejected(t *testing.T) {
140	141		}
141	142		}
142	143		}
	144	+
	145	+	func TestFilterValidation(t *testing.T) {
	146	+	tail := "\njobs:\n a:\n runs-on: x\n steps: [{run: a}]\n"
	147	+	tests := []struct{ name, on, want string }{
	148	+	{"only negated branches", "on:\n push:\n branches: ['!main']", "Event 'push' filter 'branches' has only patterns that start with '!'"},
	149	+	{"only negated paths", "on:\n push:\n paths: ['!docs/**', '!x']", "filter 'paths' has only patterns"},
	150	+	{"negated with positive", "on:\n push:\n branches: ['**', '!main']", ""},
	151	+	{"empty list", "on:\n push:\n branches: []", ""},
	152	+	{"pull_request too", "on:\n pull_request:\n branches: ['!x']", "Event 'pull_request' filter 'branches'"},
	153	+	}
	154	+	for _, tc := range tests {
	155	+	_, errs := Parse("w.yml", []byte(tc.on+tail))
	156	+	switch {
	157	+	case tc.want == "" && len(errs) > 0:
	158	+	t.Errorf("%s: unexpected errors %v", tc.name, errs)
	159	+	case tc.want != "" && (len(errs) != 1 \|\| !strings.Contains(errs[0].Error(), tc.want)):
	160	+	t.Errorf("%s: errors = %v, want %q", tc.name, errs, tc.want)
	161	+	}
	162	+	}
	163	+	}
	164	+
	165	+	func TestFiltersAreCompiledOnce(t *testing.T) {
	166	+	w := pushWorkflow(t, "on:\n push:\n branches: ['release/**']")
	167	+	ev := w.On.Event("push")
	168	+	if ev.cf == nil {
	169	+	t.Fatal("Parse did not compile the filters")
	170	+	}
	171	+	first := ev.filters()
	172	+	if ev.filters() != first \|\| len(first.branches) != 1 {
	173	+	t.Error("filters are recompiled on each use")
	174	+	}
	175	+	// An Event built by hand still works.
	176	+	hand := &Workflow{On: &On{Events: []*Event{{Name: "push", Branches: []string{"main"}}}}}
	177	+	if !hand.MatchPush(PushInfo{Ref: "refs/heads/main"}) \|\| hand.MatchPush(PushInfo{Ref: "refs/heads/dev"}) {
	178	+	t.Error("hand-built workflow does not filter")
	179	+	}
	180	+	}


@@ -4,6 +4,7 @@ package workflow
4		4
5	import (	5		import (
6	"fmt"	6		"fmt"
		7	+	"strings"
7	"testing"	8		"testing"
8	)	9		)
9		10

@@ -140,3 +141,40 @@ func TestFilterPairsAreRejected(t *testing.T) {
140	}	141		}
141	}	142		}
142	}	143		}
		144	+
		145	+	func TestFilterValidation(t *testing.T) {
		146	+	tail := "\njobs:\n a:\n runs-on: x\n steps: [{run: a}]\n"
		147	+	tests := []struct{ name, on, want string }{
		148	+	{"only negated branches", "on:\n push:\n branches: ['!main']", "Event 'push' filter 'branches' has only patterns that start with '!'"},
		149	+	{"only negated paths", "on:\n push:\n paths: ['!docs/**', '!x']", "filter 'paths' has only patterns"},
		150	+	{"negated with positive", "on:\n push:\n branches: ['**', '!main']", ""},
		151	+	{"empty list", "on:\n push:\n branches: []", ""},
		152	+	{"pull_request too", "on:\n pull_request:\n branches: ['!x']", "Event 'pull_request' filter 'branches'"},
		153	+	}
		154	+	for _, tc := range tests {
		155	+	_, errs := Parse("w.yml", []byte(tc.on+tail))
		156	+	switch {
		157	+	case tc.want == "" && len(errs) > 0:
		158	+	t.Errorf("%s: unexpected errors %v", tc.name, errs)
		159	+	case tc.want != "" && (len(errs) != 1 \|\| !strings.Contains(errs[0].Error(), tc.want)):
		160	+	t.Errorf("%s: errors = %v, want %q", tc.name, errs, tc.want)
		161	+	}
		162	+	}
		163	+	}
		164	+
		165	+	func TestFiltersAreCompiledOnce(t *testing.T) {
		166	+	w := pushWorkflow(t, "on:\n push:\n branches: ['release/**']")
		167	+	ev := w.On.Event("push")
		168	+	if ev.cf == nil {
		169	+	t.Fatal("Parse did not compile the filters")
		170	+	}
		171	+	first := ev.filters()
		172	+	if ev.filters() != first \|\| len(first.branches) != 1 {
		173	+	t.Error("filters are recompiled on each use")
		174	+	}
		175	+	// An Event built by hand still works.
		176	+	hand := &Workflow{On: &On{Events: []*Event{{Name: "push", Branches: []string{"main"}}}}}
		177	+	if !hand.MatchPush(PushInfo{Ref: "refs/heads/main"}) \|\| hand.MatchPush(PushInfo{Ref: "refs/heads/dev"}) {
		178	+	t.Error("hand-built workflow does not filter")
		179	+	}
		180	+	}