ReferenceFilter: simplify how filters are combined

Instead of the special `IncludeExcludeFilter`, treat filters just as
sets, with the usual set operations (union, intersection, and
inversion) and build them up that way. Add a `Combiner` that knows how
to combine two `ReferenceFilter`s, and use it to implement "include"
vs. "exclude". Build special behavior into the combiners if the left
argument is `nil`, to get the old semantics that the "default" for an
unmentioned reference depends on whether the first directive is
"include" vs. "exclude".

Author: mhagger

git-sizer.go

@@ -80,25 +80,28 @@ var ReleaseVersion string
 var BuildVersion string
 
 type filterValue struct {
-	// The filter to which values will be appended:
-	filter *git.IncludeExcludeFilter
-
-	// The polarity of this option (i.e., does it cause the things
-	// that it references to be included or excluded?):
-	polarity git.Polarity
-
-	// If this is set, then it is used as the pattern. If not, then
-	// the user should supply the pattern.
+	// filter is the filter that will be modified if this option
+	// is used.
+	filter *git.ReferenceFilter
+
+	// combiner specifies how the filter generated by this option
+	// is combined with the existing filter; i.e., does it cause
+	// the matching references to be included or excluded?
+	combiner git.Combiner
+
+	// pattern, if it is set, is the pattern (prefix or regexp) to
+	// be matched. If it is not set, then the user must supply the
+	// pattern.
 	pattern string
 
-	// Should `pattern` be interpreted as a regexp (as opposed to a
-	// prefix)?
+	// regexp specifies whether `pattern` should be interpreted as
+	// a regexp (as opposed to a prefix).
 	regexp bool
 }
 
 func (v *filterValue) Set(s string) error {
 	var filter git.ReferenceFilter
-	polarity := v.polarity
+	combiner := v.combiner
 
 	var pattern string
 	if v.pattern != "" {
@@ -112,7 +115,7 @@ func (v *filterValue) Set(s string) error {
 			return err
 		}
 		if !b {
-			polarity = polarity.Inverted()
+			combiner = combiner.Inverted()
 		}
 	} else {
 		// The user must supply the pattern.
@@ -129,12 +132,7 @@ func (v *filterValue) Set(s string) error {
 		filter = git.PrefixFilter(pattern)
 	}
 
-	switch polarity {
-	case git.Include:
-		v.filter.Include(filter)
-	case git.Exclude:
-		v.filter.Exclude(filter)
-	}
+	*v.filter = combiner.Combine(*v.filter, filter)
 
 	return nil
 }
@@ -158,7 +156,7 @@ func (v *filterValue) Type() string {
 }
 
 type filterGroupValue struct {
-	filter *git.IncludeExcludeFilter
+	filter *git.ReferenceFilter
 	repo   *git.Repository
 }
 
@@ -181,7 +179,9 @@ func (v *filterGroupValue) Set(name string) error {
 	for _, entry := range config.Entries {
 		switch entry.Key {
 		case "include":
-			v.filter.Include(git.PrefixFilter(entry.Value))
+			*v.filter = git.Include.Combine(
+				*v.filter, git.PrefixFilter(entry.Value),
+			)
 		case "includeregexp":
 			filter, err := git.RegexpFilter(entry.Value)
 			if err != nil {
@@ -190,9 +190,11 @@ func (v *filterGroupValue) Set(name string) error {
 					name, entry.Key, err,
 				)
 			}
-			v.filter.Include(filter)
+			*v.filter = git.Include.Combine(*v.filter, filter)
 		case "exclude":
-			v.filter.Exclude(git.PrefixFilter(entry.Value))
+			*v.filter = git.Exclude.Combine(
+				*v.filter, git.PrefixFilter(entry.Value),
+			)
 		case "excluderegexp":
 			filter, err := git.RegexpFilter(entry.Value)
 			if err != nil {
@@ -201,7 +203,7 @@ func (v *filterGroupValue) Set(name string) error {
 					name, entry.Key, err,
 				)
 			}
-			v.filter.Exclude(filter)
+			*v.filter = git.Exclude.Combine(*v.filter, filter)
 		default:
 			// Ignore unrecognized keys.
 		}
@@ -237,7 +239,7 @@ func mainImplementation(args []string) error {
 	var threshold sizes.Threshold = 1
 	var progress bool
 	var version bool
-	var filter git.IncludeExcludeFilter
+	var filter git.ReferenceFilter
 	var showRefs bool
 
 	// Try to open the repository, but it's not an error yet if this
@@ -467,23 +469,16 @@ func mainImplementation(args []string) error {
 		progress = v
 	}
 
-	var refFilter git.ReferenceFilter = filter.Filter
+	if filter == nil {
+		filter = git.AllReferencesFilter
+	}
 
 	if showRefs {
-		oldRefFilter := refFilter
 		fmt.Fprintf(os.Stderr, "References (included references marked with '+'):\n")
-		refFilter = func(refname string) bool {
-			b := oldRefFilter(refname)
-			if b {
-				fmt.Fprintf(os.Stderr, "+ %s\n", refname)
-			} else {
-				fmt.Fprintf(os.Stderr, "  %s\n", refname)
-			}
-			return b
-		}
+		filter = showRefFilter{filter}
 	}
 
-	historySize, err := sizes.ScanRepositoryUsingGraph(repo, refFilter, nameStyle, progress)
+	historySize, err := sizes.ScanRepositoryUsingGraph(repo, filter, nameStyle, progress)
 	if err != nil {
 		return fmt.Errorf("error scanning repository: %s", err)
 	}
@@ -509,3 +504,17 @@ func mainImplementation(args []string) error {
 
 	return nil
 }
+
+type showRefFilter struct {
+	f git.ReferenceFilter
+}
+
+func (f showRefFilter) Filter(refname string) bool {
+	b := f.f.Filter(refname)
+	if b {
+		fmt.Fprintf(os.Stderr, "+ %s\n", refname)
+	} else {
+		fmt.Fprintf(os.Stderr, "  %s\n", refname)
+	}
+	return b
+}

git/ref_filter.go

@@ -5,68 +5,84 @@ import (
 	"strings"
 )
 
-type ReferenceFilter func(refname string) bool
+type ReferenceFilter interface {
+	Filter(refname string) bool
+}
 
-func AllReferencesFilter(_ string) bool {
-	return true
+// Combiner combines two `ReferenceFilter`s into one compound one.
+// `f1` is allowed to be `nil`.
+type Combiner interface {
+	Combine(f1, f2 ReferenceFilter) ReferenceFilter
+	Inverted() Combiner
 }
 
-type Polarity uint8
+type inverse struct {
+	f ReferenceFilter
+}
 
-const (
-	Include Polarity = iota
-	Exclude
-)
+func (f inverse) Filter(refname string) bool {
+	return !f.f.Filter(refname)
+}
 
-func (p Polarity) Inverted() Polarity {
-	switch p {
-	case Include:
-		return Exclude
-	case Exclude:
-		return Include
-	default:
-		// This shouldn't happen:
-		return Exclude
-	}
+type intersection struct {
+	f1, f2 ReferenceFilter
+}
+
+func (f intersection) Filter(refname string) bool {
+	return f.f1.Filter(refname) && f.f2.Filter(refname)
 }
 
-// polarizedFilter is a filter that might match, in which case it
-// includes or excludes the reference (according to its polarity). If
-// it doesn't match, then it doesn't say anything about the reference.
-type polarizedFilter struct {
-	polarity Polarity
-	filter   ReferenceFilter
+// Include is a Combiner that includes the references matched by `f2`.
+// If `f1` is `nil`, it is treated as including nothing.
+type include struct{}
+
+func (_ include) Combine(f1, f2 ReferenceFilter) ReferenceFilter {
+	if f1 == nil {
+		return f2
+	}
+	return union{f1, f2}
 }
 
-// IncludeExcludeFilter is a filter based on a bunch of
-// `polarizedFilter`s. The last one that matches a reference wins. If
-// none match, then the result is based on the polarity of the first
-// polarizedFilter: if it is `Include`, then return `false`; if it is
-// `Exclude`, then return `true`.
-type IncludeExcludeFilter struct {
-	filters []polarizedFilter
+func (_ include) Inverted() Combiner {
+	return Exclude
 }
 
-func (ief *IncludeExcludeFilter) Include(f ReferenceFilter) {
-	ief.filters = append(ief.filters, polarizedFilter{Include, f})
+var Include include
+
+type union struct {
+	f1, f2 ReferenceFilter
 }
 
-func (ief *IncludeExcludeFilter) Exclude(f ReferenceFilter) {
-	ief.filters = append(ief.filters, polarizedFilter{Exclude, f})
+func (f union) Filter(refname string) bool {
+	return f.f1.Filter(refname) || f.f2.Filter(refname)
 }
 
-func (ief *IncludeExcludeFilter) Filter(refname string) bool {
-	for i := len(ief.filters); i > 0; i-- {
-		f := ief.filters[i-1]
-		if !f.filter(refname) {
-			continue
-		}
-		return f.polarity == Include
+// Exclude is a Combiner that excludes the references matched by `f2`.
+// If `f1` is `nil`, it is treated as including everything.
+type exclude struct{}
+
+func (_ exclude) Combine(f1, f2 ReferenceFilter) ReferenceFilter {
+	if f1 == nil {
+		return inverse{f2}
 	}
+	return intersection{f1, inverse{f2}}
+
+}
 
-	return len(ief.filters) == 0 || ief.filters[0].polarity == Exclude
+func (_ exclude) Inverted() Combiner {
+	return include{}
 }
 
+var Exclude exclude
+
+type allReferencesFilter struct{}
+
+func (_ allReferencesFilter) Filter(_ string) bool {
+	return true
+}
+
+var AllReferencesFilter allReferencesFilter
+
 // PrefixFilter returns a `ReferenceFilter` that matches references
 // whose names start with the specified `prefix`, which must match at
 // a component boundary. For example,
@@ -77,16 +93,23 @@ func (ief *IncludeExcludeFilter) Filter(refname string) bool {
 // * Prefix "refs/foo/" matches "refs/foo/bar" but not "refs/foo" or
 //   "refs/foobar".
 func PrefixFilter(prefix string) ReferenceFilter {
-	if strings.HasSuffix(prefix, "/") {
-		return func(refname string) bool {
-			return strings.HasPrefix(refname, prefix)
-		}
+	if prefix == "" {
+		return AllReferencesFilter
 	}
+	return prefixFilter{prefix}
+}
 
-	return func(refname string) bool {
-		return strings.HasPrefix(refname, prefix) &&
-			(len(refname) == len(prefix) || refname[len(prefix)] == '/')
+type prefixFilter struct {
+	prefix string
+}
+
+func (f prefixFilter) Filter(refname string) bool {
+	if strings.HasSuffix(f.prefix, "/") {
+		return strings.HasPrefix(refname, f.prefix)
 	}
+
+	return strings.HasPrefix(refname, f.prefix) &&
+		(len(refname) == len(f.prefix) || refname[len(f.prefix)] == '/')
 }
 
 // RegexpFilter returns a `ReferenceFilter` that matches references
@@ -99,7 +122,13 @@ func RegexpFilter(pattern string) (ReferenceFilter, error) {
 		return nil, err
 	}
 
-	return func(refname string) bool {
-		return re.MatchString(refname)
-	}, nil
+	return regexpFilter{re}, nil
+}
+
+type regexpFilter struct {
+	re *regexp.Regexp
+}
+
+func (f regexpFilter) Filter(refname string) bool {
+	return f.re.MatchString(refname)
 }

git/ref_filter_test.go

@@ -38,7 +38,11 @@ func TestPrefixFilter(t *testing.T) {
 		t.Run(
 			fmt.Sprintf("prefix '%s', refname '%s'", p.prefix, p.refname),
 			func(t *testing.T) {
-				assert.Equal(t, p.expected, git.PrefixFilter(p.prefix)(p.refname))
+				assert.Equal(
+					t,
+					p.expected,
+					git.PrefixFilter(p.prefix).Filter(p.refname),
+				)
 			},
 		)
 	}
@@ -73,7 +77,11 @@ func TestRegexpFilter(t *testing.T) {
 		t.Run(
 			fmt.Sprintf("pattern '%s', refname '%s'", p.pattern, p.refname),
 			func(t *testing.T) {
-				assert.Equal(t, p.expected, regexpFilter(t, p.pattern)(p.refname))
+				assert.Equal(
+					t,
+					p.expected,
+					regexpFilter(t, p.pattern).Filter(p.refname),
+				)
 			},
 		)
 	}
@@ -82,11 +90,11 @@ func TestRegexpFilter(t *testing.T) {
 func TestIncludeExcludeFilter(t *testing.T) {
 	t.Parallel()
 
-	var filter git.IncludeExcludeFilter
-	filter.Include(git.PrefixFilter("refs/heads"))
-	filter.Exclude(regexpFilter(t, "refs/heads/.*foo.*"))
-	filter.Include(git.PrefixFilter("refs/remotes"))
-	filter.Exclude(git.PrefixFilter("refs/remotes/foo"))
+	var filter git.ReferenceFilter
+	filter = git.Include.Combine(filter, git.PrefixFilter("refs/heads"))
+	filter = git.Exclude.Combine(filter, regexpFilter(t, "refs/heads/.*foo.*"))
+	filter = git.Include.Combine(filter, git.PrefixFilter("refs/remotes"))
+	filter = git.Exclude.Combine(filter, git.PrefixFilter("refs/remotes/foo"))
 
 	for _, p := range []struct {
 		refname  string

sizes/graph.go

@@ -62,7 +62,7 @@ func ScanRepositoryUsingGraph(
 			if !ok {
 				break
 			}
-			if !filter(ref.Refname) {
+			if !filter.Filter(ref.Refname) {
 				continue
 			}
 			refs = append(refs, ref)