feat: add file/should/have_permission expression, replace some panics…

… with errors

README.md

@@ -4,7 +4,9 @@ This project gives developers the ability to describe and check the architecture
 
 ## TODO
 
-- add docs to tell what options each expression support (even better: enforce that using type system)
+- [ ] add docs to tell what options each expression support (even better: enforce that using type system)
+- [ ] replace panics with better error handling
+- [ ] review rule builder locking
 
 ## Desired usecases
 
@@ -13,8 +15,12 @@ if a folder:
 - [ ] does not exist
 - [ ] contains a specific file
 - [ ] contains a specific set of files
-- [ ] contains a files matching a regex
-- [ ] contains a files matching a glob pattern
+- [ ] contains files matching a regex
+- [ ] contains files matching a glob pattern
+- [ ] contains only a specific file
+- [ ] contains only a specific set of files
+- [ ] contains only files matching a regex
+- [ ] contains only files matching a glob pattern
 - [ ] is gitignored
 - [ ] is gitcrypted
 - [ ] has specific permissions
@@ -30,7 +36,7 @@ if a file:
 - [x] content contains a value
 - [x] is gitignored
 - [x] is gitcrypted
-- [ ] has specific permissions
+- [x] has specific permissions
 
 if a set of files:
 - [x] exists

internal/arch/file/except/except.go

@@ -7,11 +7,18 @@ import (
 
 type Expression interface {
 	Evaluate(rb rule.Builder)
+	GetErrors() []error
 }
 
 type evaluateFunc func(filePath string) bool
 
-type baseExpression struct{}
+type baseExpression struct {
+	errors []error
+}
+
+func (e *baseExpression) GetErrors() []error {
+	return e.errors
+}
 
 func (e baseExpression) evaluate(rb rule.Builder, eval evaluateFunc) {
 	frb := rb.(*file.RuleBuilder)

internal/arch/file/rule.go

@@ -86,16 +86,27 @@ func (rb *RuleBuilder) Because(b rule.Because) ([]rule.Violation, []error) {
 	rb.because = b
 
 	for _, that := range rb.thats {
+		if len(that.GetErrors()) > 0 {
+			return nil, that.GetErrors()
+		}
+
 		that.Evaluate(rb)
 	}
 
 	for _, except := range rb.excepts {
+		if len(except.GetErrors()) > 0 {
+			return nil, except.GetErrors()
+		}
+
 		except.Evaluate(rb)
 	}
 
 	for _, should := range rb.shoulds {
-		vs := should.Evaluate(rb)
-		if len(vs) > 0 {
+		if len(should.GetErrors()) > 0 {
+			return nil, should.GetErrors()
+		}
+
+		if vs := should.Evaluate(rb); len(vs) > 0 {
 			rb.violations = append(rb.violations, vs...)
 		}
 	}

internal/arch/file/should/be_gitencrypted.go

@@ -30,7 +30,9 @@ func (e gitEncryptedExpression) doEvaluate(rb rule.Builder, filePath string) boo
 	cmd := exec.Command("git", "crypt", "status", filePath)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		panic(err)
+		rb.AddError(err)
+
+		return true
 	}
 
 	return bytes.Contains(out, []byte("not encrypted"))

internal/arch/file/should/be_gitencrypted_test.go

@@ -39,20 +39,16 @@ func Test_BeGitencrypted(t *testing.T) {
 		{
 			desc:        "negated: file 'encrypted.txt' should not be gitencrypted",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/encrypted.txt")),
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file 'encrypted.txt' is gitencrypted"),
 			},
 		},
 		{
 			desc:        "negated: file 'not_encrypted.txt' should not be gitencrypted",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/not_encrypted.txt")),
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 	}
 

internal/arch/file/should/be_gitignored.go

@@ -32,7 +32,9 @@ func (e gitIgnoredExpression) doEvaluate(rb rule.Builder, filePath string) bool
 		case *exec.ExitError:
 			return err.(*exec.ExitError).ExitCode() != 0
 		default:
-			panic(err)
+			rb.AddError(err)
+
+			return true
 		}
 	}
 

internal/arch/file/should/be_gitignored_test.go

@@ -39,20 +39,16 @@ func Test_BeGitignored(t *testing.T) {
 		{
 			desc:        "negated: file 'ignored.txt' should not be gitignored",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/ignored.txt")),
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file 'ignored.txt' is gitignored"),
 			},
 		},
 		{
 			desc:        "negated: file 'not_ignored.txt' should not be gitignored",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/not_ignored.txt")),
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 	}
 

internal/arch/file/should/contain_value_test.go

@@ -52,9 +52,7 @@ func Test_ContainValue(t *testing.T) {
 			desc:        "negated: file 'foobar.txt' contains the value 'bar'",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/foobar.txt")),
 			value:       "bar",
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file 'foobar.txt' does contain the value 'bar'"),
 			},
@@ -75,10 +73,8 @@ func Test_ContainValue(t *testing.T) {
 			desc:        "negated: file 'foobar.txt' does not contain the value 'something else'",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/foobar.txt")),
 			value:       "something else",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 	}
 

internal/arch/file/should/end_with_test.go

@@ -36,18 +36,14 @@ func Test_EndWith(t *testing.T) {
 			desc:        "negated: foobar does not end with baz",
 			ruleBuilder: file.One("foobar"),
 			suffix:      "baz",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 		{
 			desc:        "negated: foobar ends with bar",
 			ruleBuilder: file.One("foobar"),
 			suffix:      "bar",
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file's name 'foobar' does end with 'bar'"),
 			},

internal/arch/file/should/exist_test.go

@@ -32,19 +32,15 @@ func Test_Exist(t *testing.T) {
 		{
 			desc:        "negated: exist.go exists",
 			ruleBuilder: file.One("exist.go"),
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file 'exist.go' does exist"),
 			},
 		},
 		{
 			desc:        "negated: abc.xyz does not exist",
 			ruleBuilder: file.One("abc.xyz"),
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 		},
 	}
 	for _, tC := range testCases {

internal/arch/file/should/have_content_matching_regex_test.go

@@ -83,10 +83,8 @@ func Test_HaveContentMatchingRegex(t *testing.T) {
 			desc:        "negated: content of file 'foobar.txt' does not match regex",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/foobar.txt")),
 			regexp:      "^something\\ else.+",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 	}
 

internal/arch/file/should/have_content_matching_test.go

@@ -83,10 +83,8 @@ func Test_HaveContentMatching(t *testing.T) {
 			desc:        "negated: content of file 'foobar.txt' does not match expected content",
 			ruleBuilder: file.One(filepath.Join(basePath, "test/foobar.txt")),
 			content:     "something else\n",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 	}
 

internal/arch/file/should/have_permissions.go

@@ -0,0 +1,76 @@
+package should
+
+import (
+	"errors"
+	"fmt"
+	"goarkitect/internal/arch/rule"
+	"os"
+	"path/filepath"
+	"regexp"
+)
+
+var (
+	ErrInvalidPermissions = errors.New(
+		"permissions must only contain the following characters: 'd', 'r', 'w', 'x', '-'",
+	)
+)
+
+func HavePermissions(want string, opts ...Option) *havePermissionsExpression {
+	rx := regexp.MustCompile("[d-][rwx-]{9}")
+
+	errs := make([]error, 0)
+	if !rx.MatchString(want) {
+		errs = append(errs, ErrInvalidPermissions)
+	}
+
+	expr := &havePermissionsExpression{
+		want: want,
+		baseExpression: baseExpression{
+			errors: errs,
+		},
+	}
+
+	for _, opt := range opts {
+		opt.apply(&expr.options)
+	}
+
+	return expr
+}
+
+type havePermissionsExpression struct {
+	baseExpression
+
+	want string
+}
+
+func (e havePermissionsExpression) Evaluate(rb rule.Builder) []rule.Violation {
+	return e.evaluate(rb, e.doEvaluate, e.getViolation)
+}
+
+func (e havePermissionsExpression) doEvaluate(rb rule.Builder, filePath string) bool {
+	info, err := os.Stat(filePath)
+	if err != nil {
+		rb.AddError(err)
+
+		return true
+	}
+
+	return e.want != info.Mode().String()
+}
+
+func (e havePermissionsExpression) getViolation(filePath string) rule.Violation {
+	iNodeType := "file"
+	if info, _ := os.Stat(filePath); info.IsDir() {
+		iNodeType = "directory"
+	}
+
+	format := "%s '%s' does not have permissions matching '%s'"
+
+	if e.options.negated {
+		format = "%s '%s' does have permissions matching '%s'"
+	}
+
+	return rule.NewViolation(
+		fmt.Sprintf(format, iNodeType, filepath.Base(filePath), e.want),
+	)
+}

internal/arch/file/should/have_permissions_test.go

@@ -0,0 +1,113 @@
+package should_test
+
+import (
+	"goarkitect/internal/arch/file"
+	"goarkitect/internal/arch/file/should"
+	"goarkitect/internal/arch/rule"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+)
+
+func Test_HavePermissions(t *testing.T) {
+	basePath, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testCases := []struct {
+		desc        string
+		ruleBuilder *file.RuleBuilder
+		permissions string
+		options     []should.Option
+		want        []rule.Violation
+		wantErrs    []error
+	}{
+		{
+			desc:        "wrong permissions string",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions/0700.txt")),
+			permissions: "foobarbaz-",
+			want:        nil,
+			wantErrs:    []error{should.ErrInvalidPermissions},
+		},
+		{
+			desc:        "permissions of directory 'test/permissions' match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions")),
+			permissions: "drwxr-xr-x",
+			want:        nil,
+		},
+		{
+			desc:        "permissions of file '0700.txt' match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions/0700.txt")),
+			permissions: "-rwx------",
+			want:        nil,
+		},
+		{
+			desc:        "permissions of directory 'test/permissions' do not match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions")),
+			permissions: "dr--r--r--",
+			want: []rule.Violation{
+				rule.NewViolation("directory 'permissions' does not have permissions matching 'dr--r--r--'"),
+			},
+		},
+		{
+			desc:        "permissions of file '0700.txt' do not match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions/0700.txt")),
+			permissions: "-rwxrwxrwx",
+			want: []rule.Violation{
+				rule.NewViolation("file '0700.txt' does not have permissions matching '-rwxrwxrwx'"),
+			},
+		},
+		{
+			desc:        "negated: permissions of directory 'test/permissions' match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions")),
+			permissions: "drwxr-xr-x",
+			options:     []should.Option{should.Negated{}},
+			want: []rule.Violation{
+				rule.NewViolation("directory 'permissions' does have permissions matching 'drwxr-xr-x'"),
+			},
+		},
+		{
+			desc:        "negated: permissions of file '0700.txt' match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions/0700.txt")),
+			permissions: "-rwx------",
+			options:     []should.Option{should.Negated{}},
+			want: []rule.Violation{
+				rule.NewViolation("file '0700.txt' does have permissions matching '-rwx------'"),
+			},
+		},
+		{
+			desc:        "negated: permissions of directory 'test/permissions' do not match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions")),
+			permissions: "dr--r--r--",
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
+		},
+		{
+			desc:        "negated: permissions of file '0700.txt' do not match expected one",
+			ruleBuilder: file.One(filepath.Join(basePath, "test/permissions/0700.txt")),
+			permissions: "-rwxrwxrwx",
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
+		},
+	}
+
+	for _, tC := range testCases {
+		t.Run(tC.desc, func(t *testing.T) {
+			hcm := should.HavePermissions(tC.permissions, tC.options...)
+			got := hcm.Evaluate(tC.ruleBuilder)
+			gotErrs := hcm.GetErrors()
+
+			if !cmp.Equal(gotErrs, tC.wantErrs, cmpopts.EquateErrors(), cmpopts.EquateEmpty()) {
+				t.Errorf("wantErr = %+v, gotErr = %+v", tC.wantErrs, gotErrs)
+			}
+
+			if !cmp.Equal(got, tC.want, cmp.AllowUnexported(rule.Violation{}), cmpopts.EquateEmpty()) {
+				t.Errorf("want = %+v, got = %+v", tC.want, got)
+			}
+		})
+	}
+}

internal/arch/file/should/match_glob_test.go

@@ -59,27 +59,21 @@ func Test_MatchGlob(t *testing.T) {
 			desc:        "negated: project3 does not match '*.xls'",
 			ruleBuilder: newRuleBuilder(),
 			glob:        "*/*/*.xls",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 		{
 			desc:        "negated: project3 does not match 'test/*/*.xls'",
 			ruleBuilder: newRuleBuilder(),
 			glob:        "test/*/*.xls",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 		{
 			desc:        "negated: project3 does match 'test/*/*.txt'",
 			ruleBuilder: newRuleBuilder(),
 			glob:        "test/*/*.txt",
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file's path 'baz.txt' does match glob pattern 'test/*/*.txt'"),
 				rule.NewViolation("file's path 'quux.txt' does match glob pattern 'test/*/*.txt'"),

internal/arch/file/should/match_regex_test.go

@@ -42,9 +42,7 @@ func Test_MatchRegex(t *testing.T) {
 			desc:        "negated: foobar matches '[a-z]+'",
 			ruleBuilder: file.One("foobar"),
 			regexp:      "[a-z]+",
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file's name 'foobar' does match regex '[a-z]+'"),
 			},
@@ -53,9 +51,7 @@ func Test_MatchRegex(t *testing.T) {
 			desc:        "negated: foobar matches 'foobar'",
 			ruleBuilder: file.One("foobar"),
 			regexp:      "foobar",
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file's name 'foobar' does match regex 'foobar'"),
 			},
@@ -64,10 +60,8 @@ func Test_MatchRegex(t *testing.T) {
 			desc:        "negated: foobar does not match '[0-9]+'",
 			ruleBuilder: file.One("foobar"),
 			regexp:      "[0-9]+",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 	}
 	for _, tC := range testCases {

internal/arch/file/should/should.go

@@ -12,7 +12,8 @@ var (
 
 type Expression interface {
 	Evaluate(rb rule.Builder) []rule.Violation
-	applyOption(opt Option) error
+	GetErrors() []error
+	applyOption(opt Option)
 	doEvaluate(rb rule.Builder, filePath string) bool
 	getViolation(filePath string) rule.Violation
 }
@@ -30,13 +31,22 @@ type options struct {
 type baseExpression struct {
 	options      options
 	getViolation getViolationFunc
+	errors       []error
+}
+
+func (e *baseExpression) GetErrors() []error {
+	return e.errors
 }
 
 func (e *baseExpression) evaluate(
 	rb rule.Builder,
 	evaluate evaluateFunc,
 	getViolation getViolationFunc,
 ) []rule.Violation {
+	if len(e.errors) > 0 {
+		return nil
+	}
+
 	violations := make([]rule.Violation, 0)
 	for _, fp := range rb.(*file.RuleBuilder).GetFiles() {
 		result := evaluate(rb, fp)
@@ -48,14 +58,14 @@ func (e *baseExpression) evaluate(
 	return violations
 }
 
-func (e *baseExpression) applyOption(opt Option) error {
-	return opt.apply(&e.options)
+func (e *baseExpression) applyOption(opt Option) {
+	if err := opt.apply(&e.options); err != nil {
+		e.errors = append(e.errors, err)
+	}
 }
 
 func Not(expr Expression) Expression {
-	if err := expr.applyOption(Negated{}); err != nil {
-		panic(err)
-	}
+	expr.applyOption(Negated{})
 
 	return expr
 }

internal/arch/file/should/start_with_test.go

@@ -36,18 +36,14 @@ func Test_StartWith(t *testing.T) {
 			desc:        "negated: foobar does not start with baz",
 			ruleBuilder: file.One("foobar"),
 			prefix:      "baz",
-			options: []should.Option{
-				should.Negated{},
-			},
-			want: nil,
+			options:     []should.Option{should.Negated{}},
+			want:        nil,
 		},
 		{
 			desc:        "negated: foobar starts with foo",
 			ruleBuilder: file.One("foobar"),
 			prefix:      "foo",
-			options: []should.Option{
-				should.Negated{},
-			},
+			options:     []should.Option{should.Negated{}},
 			want: []rule.Violation{
 				rule.NewViolation("file's name 'foobar' does start with 'foo'"),
 			},

internal/arch/file/should/test/permissions/0700.txt

@@ -0,0 +1 @@
+0700

internal/arch/file/that/are_in_folder.go

@@ -18,6 +18,11 @@ func AreInFolder(folder string, recursive bool) *AreInFolderExpression {
 type AreInFolderExpression struct {
 	folder    string
 	recursive bool
+	errors    []error
+}
+
+func (e *AreInFolderExpression) GetErrors() []error {
+	return e.errors
 }
 
 func (e *AreInFolderExpression) Evaluate(rb rule.Builder) {

internal/arch/file/that/end_with.go

@@ -14,6 +14,12 @@ func EndWith(s string) *EndWithExpression {
 
 type EndWithExpression struct {
 	suffix string
+
+	errors []error
+}
+
+func (e *EndWithExpression) GetErrors() []error {
+	return e.errors
 }
 
 func (e EndWithExpression) Evaluate(rb rule.Builder) {

internal/arch/rule/builder.go

@@ -13,14 +13,17 @@ type Builder interface {
 
 type That interface {
 	Evaluate(rule Builder)
+	GetErrors() []error
 }
 
 type Except interface {
 	Evaluate(rule Builder)
+	GetErrors() []error
 }
 
 type Should interface {
 	Evaluate(rule Builder) []Violation
+	GetErrors() []error
 }
 
 type Because string