lib/ignore: Refactor out result type (#9343)

12 files changed, 155 insertions, 87 deletions

diff --git a/lib/fs/basicfs_watch.go b/lib/fs/basicfs_watch.go
index 83aea94e7..cc8349652 100644
--- a/lib/fs/basicfs_watch.go
+++ b/lib/fs/basicfs_watch.go
@@ -43,7 +43,7 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context
 			if err != nil {
 				return true
 			}
-			return ignore.ShouldIgnore(rel)
+			return ignore.Match(rel).IsIgnored()
 		}
 		err = notify.WatchWithFilter(watchPath, backendChan, absShouldIgnore, eventMask)
 	} else {
@@ -94,7 +94,7 @@ func (f *BasicFilesystem) watchLoop(ctx context.Context, name string, roots []st
 				return
 			}
 
-			if ignore.ShouldIgnore(relPath) {
+			if ignore.Match(relPath).IsIgnored() {
 				l.Debugln(f.Type(), f.URI(), "Watch: Ignoring", relPath)
 				continue
 			}
diff --git a/lib/fs/basicfs_watch_test.go b/lib/fs/basicfs_watch_test.go
index 244a4b3a9..290434c41 100644
--- a/lib/fs/basicfs_watch_test.go
+++ b/lib/fs/basicfs_watch_test.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/syncthing/notify"
 	"github.com/syncthing/syncthing/lib/build"
+	"github.com/syncthing/syncthing/lib/ignore/ignoreresult"
 )
 
 func TestMain(m *testing.M) {
@@ -99,7 +100,7 @@ func TestWatchInclude(t *testing.T) {
 
 	file := "file"
 	ignored := "ignored"
-	testFs.MkdirAll(filepath.Join(name, ignored), 0777)
+	testFs.MkdirAll(filepath.Join(name, ignored), 0o777)
 	included := filepath.Join(ignored, "included")
 
 	testCase := func() {
@@ -317,12 +318,12 @@ func TestWatchErrorLinuxInterpretation(t *testing.T) {
 		t.Skip("testing of linux specific error codes")
 	}
 
-	var errTooManyFiles = &os.PathError{
+	errTooManyFiles := &os.PathError{
 		Op:   "error while traversing",
 		Path: "foo",
 		Err:  syscall.Errno(24),
 	}
-	var errNoSpace = &os.PathError{
+	errNoSpace := &os.PathError{
 		Op:   "error while traversing",
 		Path: "bar",
 		Err:  syscall.Errno(28),
@@ -346,13 +347,13 @@ func TestWatchSymlinkedRoot(t *testing.T) {
 	}
 
 	name := "symlinkedRoot"
-	if err := testFs.MkdirAll(name, 0755); err != nil {
+	if err := testFs.MkdirAll(name, 0o755); err != nil {
 		panic(fmt.Sprintf("Failed to create directory %s: %s", name, err))
 	}
 	defer testFs.RemoveAll(name)
 
 	root := filepath.Join(name, "root")
-	if err := testFs.MkdirAll(root, 0777); err != nil {
+	if err := testFs.MkdirAll(root, 0o777); err != nil {
 		panic(err)
 	}
 	link := filepath.Join(name, "link")
@@ -369,7 +370,7 @@ func TestWatchSymlinkedRoot(t *testing.T) {
 		panic(err)
 	}
 
-	if err := linkedFs.MkdirAll("foo", 0777); err != nil {
+	if err := linkedFs.MkdirAll("foo", 0o777); err != nil {
 		panic(err)
 	}
 
@@ -507,7 +508,7 @@ func TestTruncateFileOnly(t *testing.T) {
 // path relative to folder root, also creates parent dirs if necessary
 func createTestFile(name string, file string) string {
 	joined := filepath.Join(name, file)
-	if err := testFs.MkdirAll(filepath.Dir(joined), 0755); err != nil {
+	if err := testFs.MkdirAll(filepath.Dir(joined), 0o755); err != nil {
 		panic(fmt.Sprintf("Failed to create parent directory for %s: %s", joined, err))
 	}
 	handle, err := testFs.Create(joined)
@@ -529,7 +530,7 @@ func renameTestFile(name string, old string, new string) {
 func modifyTestFile(name string, file string, content string) {
 	joined := filepath.Join(testDirAbs, name, file)
 
-	err := os.WriteFile(joined, []byte(content), 0755)
+	err := os.WriteFile(joined, []byte(content), 0o755)
 	if err != nil {
 		panic(fmt.Sprintf("Failed to modify test file %s: %s", joined, err))
 	}
@@ -540,7 +541,7 @@ func sleepMs(ms int) {
 }
 
 func testScenario(t *testing.T, name string, testCase func(), expectedEvents, allowedEvents []Event, fm fakeMatcher, ignorePerms bool) {
-	if err := testFs.MkdirAll(name, 0755); err != nil {
+	if err := testFs.MkdirAll(name, 0o755); err != nil {
 		panic(fmt.Sprintf("Failed to create directory %s: %s", name, err))
 	}
 	defer testFs.RemoveAll(name)
@@ -567,7 +568,7 @@ func testScenario(t *testing.T, name string, testCase func(), expectedEvents, al
 }
 
 func testWatchOutput(t *testing.T, name string, in <-chan Event, expectedEvents, allowedEvents []Event, ctx context.Context, cancel context.CancelFunc) {
-	var expected = make(map[Event]struct{})
+	expected := make(map[Event]struct{})
 	for _, ev := range expectedEvents {
 		ev.Name = filepath.Join(name, ev.Name)
 		expected[ev] = struct{}{}
@@ -613,8 +614,11 @@ type fakeMatcher struct {
 	skipIgnoredDirs bool
 }
 
-func (fm fakeMatcher) ShouldIgnore(name string) bool {
-	return name != fm.include && name == fm.ignore
+func (fm fakeMatcher) Match(name string) ignoreresult.R {
+	if name != fm.include && name == fm.ignore {
+		return ignoreresult.Ignored
+	}
+	return ignoreresult.NotIgnored
 }
 
 func (fm fakeMatcher) SkipIgnoredDirs() bool {
diff --git a/lib/fs/filesystem.go b/lib/fs/filesystem.go
index cf0b50cdb..f7232c9f7 100644
--- a/lib/fs/filesystem.go
+++ b/lib/fs/filesystem.go
@@ -16,6 +16,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/syncthing/syncthing/lib/ignore/ignoreresult"
 	"github.com/syncthing/syncthing/lib/protocol"
 )
 
@@ -127,14 +128,10 @@ type Usage struct {
 }
 
 type Matcher interface {
-	ShouldIgnore(name string) bool
+	Match(name string) ignoreresult.R
 	SkipIgnoredDirs() bool
 }
 
-type MatchResult interface {
-	IsIgnored() bool
-}
-
 type Event struct {
 	Name string
 	Type EventType
diff --git a/lib/ignore/cache.go b/lib/ignore/cache.go
index 5028f6575..e169c1b47 100644
--- a/lib/ignore/cache.go
+++ b/lib/ignore/cache.go
@@ -6,7 +6,11 @@
 
 package ignore
 
-import "time"
+import (
+	"time"
+
+	"github.com/syncthing/syncthing/lib/ignore/ignoreresult"
+)
 
 type nower interface {
 	Now() time.Time
@@ -20,7 +24,7 @@ type cache struct {
 }
 
 type cacheEntry struct {
-	result Result
+	result ignoreresult.R
 	access int64 // Unix nanosecond count. Sufficient until the year 2262.
 }
 
@@ -39,7 +43,7 @@ func (c *cache) clean(d time.Duration) {
 	}
 }
 
-func (c *cache) get(key string) (Result, bool) {
+func (c *cache) get(key string) (ignoreresult.R, bool) {
 	entry, ok := c.entries[key]
 	if ok {
 		entry.access = clock.Now().UnixNano()
@@ -48,7 +52,7 @@ func (c *cache) get(key string) (Result, bool) {
 	return entry.result, ok
 }
 
-func (c *cache) set(key string, result Result) {
+func (c *cache) set(key string, result ignoreresult.R) {
 	c.entries[key] = cacheEntry{result, time.Now().UnixNano()}
 }
 
diff --git a/lib/ignore/cache_test.go b/lib/ignore/cache_test.go
index 68513203d..6aae99464 100644
--- a/lib/ignore/cache_test.go
+++ b/lib/ignore/cache_test.go
@@ -9,6 +9,8 @@ package ignore
 import (
 	"testing"
 	"time"
+
+	"github.com/syncthing/syncthing/lib/ignore/ignoreresult"
 )
 
 func TestCache(t *testing.T) {
@@ -28,7 +30,7 @@ func TestCache(t *testing.T) {
 
 	// Set and check some items
 
-	c.set("true", resultInclude|resultDeletable)
+	c.set("true", ignoreresult.IgnoredDeletable)
 	c.set("false", 0)
 
 	res, ok = c.get("true")
diff --git a/lib/ignore/ignore.go b/lib/ignore/ignore.go
index 01960f3dd..db1392f1b 100644
--- a/lib/ignore/ignore.go
+++ b/lib/ignore/ignore.go
@@ -18,28 +18,13 @@ import (
 
 	"github.com/gobwas/glob"
 
-	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/fs"
+	"github.com/syncthing/syncthing/lib/ignore/ignoreresult"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/sha256"
 	"github.com/syncthing/syncthing/lib/sync"
 )
 
-const (
-	resultNotMatched Result = 0
-	resultInclude    Result = 1 << iota
-	resultDeletable         = 1 << iota
-	resultFoldCase          = 1 << iota
-)
-
-var defaultResult Result = resultInclude
-
-func init() {
-	if build.IsDarwin || build.IsWindows {
-		defaultResult |= resultFoldCase
-	}
-}
-
 // A ParseError signifies an error with contents of an ignore file,
 // including I/O errors on included files. An I/O error on the root level
 // ignore file is not a ParseError.
@@ -70,18 +55,18 @@ func parseError(err error) error {
 type Pattern struct {
 	pattern string
 	match   glob.Glob
-	result  Result
+	result  ignoreresult.R
 }
 
 func (p Pattern) String() string {
 	ret := p.pattern
-	if p.result&resultInclude != resultInclude {
+	if !p.result.IsIgnored() {
 		ret = "!" + ret
 	}
-	if p.result&resultFoldCase == resultFoldCase {
+	if p.result.IsCaseFolded() {
 		ret = "(?i)" + ret
 	}
-	if p.result&resultDeletable == resultDeletable {
+	if p.result.IsDeletable() {
 		ret = "(?d)" + ret
 	}
 	return ret
@@ -101,20 +86,6 @@ func (p Pattern) allowsSkippingIgnoredDirs() bool {
 	return !strings.Contains(strings.TrimSuffix(p.pattern, "**"), "**")
 }
 
-type Result uint8
-
-func (r Result) IsIgnored() bool {
-	return r&resultInclude == resultInclude
-}
-
-func (r Result) IsDeletable() bool {
-	return r.IsIgnored() && r&resultDeletable == resultDeletable
-}
-
-func (r Result) IsCaseFolded() bool {
-	return r&resultFoldCase == resultFoldCase
-}
-
 // The ChangeDetector is responsible for determining if files have changed
 // on disk. It gets told to Remember() files (name and modtime) and will
 // then get asked if a file has been Seen() (i.e., Remember() has been
@@ -253,16 +224,24 @@ func (m *Matcher) parseLocked(r io.Reader, file string) error {
 	return err
 }
 
-func (m *Matcher) Match(file string) (result Result) {
-	if file == "." {
-		return resultNotMatched
+// Match matches the patterns plus temporary and internal files.
+func (m *Matcher) Match(file string) (result ignoreresult.R) {
+	switch {
+	case fs.IsTemporary(file):
+		return ignoreresult.Ignored
+
+	case fs.IsInternal(file):
+		return ignoreresult.Ignored
+
+	case file == ".":
+		return ignoreresult.NotIgnored
 	}
 
 	m.mut.Lock()
 	defer m.mut.Unlock()
 
 	if len(m.patterns) == 0 {
-		return resultNotMatched
+		return ignoreresult.NotIgnored
 	}
 
 	if m.matches != nil {
@@ -295,7 +274,7 @@ func (m *Matcher) Match(file string) (result Result) {
 	}
 
 	// Default to not matching.
-	return resultNotMatched
+	return ignoreresult.NotIgnored
 }
 
 // Lines return a list of the unprocessed lines in .stignore at last load
@@ -348,22 +327,6 @@ func (m *Matcher) clean(d time.Duration) {
 	}
 }
 
-// ShouldIgnore returns true when a file is temporary, internal or ignored
-func (m *Matcher) ShouldIgnore(filename string) bool {
-	switch {
-	case fs.IsTemporary(filename):
-		return true
-
-	case fs.IsInternal(filename):
-		return true
-
-	case m.Match(filename).IsIgnored():
-		return true
-	}
-
-	return false
-}
-
 func (m *Matcher) SkipIgnoredDirs() bool {
 	m.mut.Lock()
 	defer m.mut.Unlock()
@@ -414,7 +377,7 @@ func loadParseIncludeFile(filesystem fs.Filesystem, file string, cd ChangeDetect
 		// isNotExist is considered "ok" in a sense of that a folder doesn't have to act
 		// upon it. This is because it is allowed for .stignore to not exist. However,
 		// included ignore files are not allowed to be missing and these errors should be
-		// acted upon on. So we don't perserve the error chain here and manually set an
+		// acted upon on. So we don't preserve the error chain here and manually set an
 		// error instead, if the file is missing.
 		if fs.IsNotExist(err) {
 			err = errors.New("file not found")
@@ -431,7 +394,7 @@ func loadParseIncludeFile(filesystem fs.Filesystem, file string, cd ChangeDetect
 
 func parseLine(line string) ([]Pattern, error) {
 	pattern := Pattern{
-		result: defaultResult,
+		result: ignoreresult.Ignored,
 	}
 
 	// Allow prefixes to be specified in any order, but only once.
@@ -441,14 +404,14 @@ func parseLine(line string) ([]Pattern, error) {
 		if strings.HasPrefix(line, "!") && !seenPrefix[0] {
 			seenPrefix[0] = true
 			line = line[1:]
-			pattern.result ^= resultInclude
+			pattern.result = pattern.result.ToggleIgnored()
 		} else if strings.HasPrefix(line, "(?i)") && !seenPrefix[1] {
 			seenPrefix[1] = true
-			pattern.result |= resultFoldCase
+			pattern.result = pattern.result.WithFoldCase()
 			line = line[4:]
 		} else if strings.HasPrefix(line, "(?d)") && !seenPrefix[2] {
 			seenPrefix[2] = true
-			pattern.result |= resultDeletable
+			pattern.result = pattern.result.WithDeletable()
 			line = line[4:]
 		} else {
 			break
diff --git a/lib/ignore/ignore_test.go b/lib/ignore/ignore_test.go
index dbcdc32e7..ccdcd122e 100644
--- a/lib/ignore/ignore_test.go
+++ b/lib/ignore/ignore_test.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/fs"
+	"github.com/syncthing/syncthing/lib/ignore/ignoreresult"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/rand"
 )
@@ -415,7 +416,7 @@ func TestCommentsAndBlankLines(t *testing.T) {
 	}
 }
 
-var result Result
+var result ignoreresult.R
 
 func BenchmarkMatch(b *testing.B) {
 	testFs := newTestFS()
diff --git a/lib/ignore/ignoreresult/ignoreresult.go b/lib/ignore/ignoreresult/ignoreresult.go
new file mode 100644
index 000000000..81a96b1da
--- /dev/null
+++ b/lib/ignore/ignoreresult/ignoreresult.go
@@ -0,0 +1,75 @@
+// Copyright (C) 2024 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+// Package result provides the result type for ignore matching. This is a
+// separate package in order to break import cycles.
+package ignoreresult
+
+const (
+	NotIgnored R = 0
+	// `Ignored` is defined in platform specific files
+	IgnoredDeletable = Ignored | deletableBit
+)
+
+const (
+	// Private definitions of the bits that make up the result value
+	ignoreBit R = 1 << iota
+	deletableBit
+	foldCaseBit
+)
+
+type R uint8
+
+// IsIgnored returns true if the result is ignored.
+func (r R) IsIgnored() bool {
+	return r&ignoreBit != 0
+}
+
+// IsDeletable returns true if the result is ignored and deletable.
+func (r R) IsDeletable() bool {
+	return r.IsIgnored() && r&deletableBit != 0
+}
+
+// IsCaseFolded returns true if the result was a case-insensitive match.
+func (r R) IsCaseFolded() bool {
+	return r&foldCaseBit != 0
+}
+
+// ToggleIgnored returns a copy of the result with the ignored bit toggled.
+func (r R) ToggleIgnored() R {
+	return r ^ ignoreBit
+}
+
+// WithDeletable returns a copy of the result with the deletable bit set.
+func (r R) WithDeletable() R {
+	return r | deletableBit
+}
+
+// WithFoldCase returns a copy of the result with the fold case bit set.
+func (r R) WithFoldCase() R {
+	return r | foldCaseBit
+}
+
+// String returns a human readable representation of the result flags.
+func (r R) String() string {
+	var s string
+	if r&ignoreBit != 0 {
+		s += "i"
+	} else {
+		s += "-"
+	}
+	if r&deletableBit != 0 {
+		s += "d"
+	} else {
+		s += "-"
+	}
+	if r&foldCaseBit != 0 {
+		s += "f"
+	} else {
+		s += "-"
+	}
+	return s
+}
diff --git a/lib/ignore/ignoreresult/ignoreresult_foldcase.go b/lib/ignore/ignoreresult/ignoreresult_foldcase.go
new file mode 100644
index 000000000..a77f13329
--- /dev/null
+++ b/lib/ignore/ignoreresult/ignoreresult_foldcase.go
@@ -0,0 +1,11 @@
+// Copyright (C) 2024 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//go:build windows || darwin
+
+package ignoreresult
+
+const Ignored = ignoreBit | foldCaseBit
diff --git a/lib/ignore/ignoreresult/ignoreresult_nofoldcase.go b/lib/ignore/ignoreresult/ignoreresult_nofoldcase.go
new file mode 100644
index 000000000..55c7d23d5
--- /dev/null
+++ b/lib/ignore/ignoreresult/ignoreresult_nofoldcase.go
@@ -0,0 +1,11 @@
+// Copyright (C) 2024 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//go:build !windows && !darwin
+
+package ignoreresult
+
+const Ignored = ignoreBit
diff --git a/lib/model/folder_sendonly.go b/lib/model/folder_sendonly.go
index f1e59c467..bf7dfc984 100644
--- a/lib/model/folder_sendonly.go
+++ b/lib/model/folder_sendonly.go
@@ -53,7 +53,7 @@ func (f *sendOnlyFolder) pull() (bool, error) {
 
 		file := intf.(protocol.FileInfo)
 
-		if f.ignores.ShouldIgnore(intf.FileName()) {
+		if f.ignores.Match(intf.FileName()).IsIgnored() {
 			file.SetIgnored()
 			batch.Append(file)
 			l.Debugln(f, "Handling ignored file", file)
diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go
index 0e54cbfee..3356fc30c 100644
--- a/lib/model/folder_sendrecv.go
+++ b/lib/model/folder_sendrecv.go
@@ -343,7 +343,7 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<-
 		file := intf.(protocol.FileInfo)
 
 		switch {
-		case f.ignores.ShouldIgnore(file.Name):
+		case f.ignores.Match(file.Name).IsIgnored():
 			file.SetIgnored()
 			l.Debugln(f, "Handling ignored file", file)
 			dbUpdateChan <- dbUpdateJob{file, dbUpdateInvalidate}