From c929fd5c8ae14c5e9b598bf97ade68279ae7da42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=90?= Date: Mon, 18 Mar 2024 22:25:25 +0800 Subject: [PATCH] feat: enable more lint rules (#1376) * feat: enable `errorlint` lint rule * feat: enable `stylecheck` lint rule without `ST1003` * feat: enable `revive` lint rule without `unused-parameter` * feat: enable `goconst` lint rule * feat: enable `cyclop` `funlen` `gocyclo` `nestif` lint rule * Revert "feat: enable `errorlint` lint rule" This reverts commit 3b56fa99ef418b938f7519fd3d9237d8697c76e1. * Revert "feat: enable `goconst` lint rule" This reverts commit 8cec40827e79af9e3c92267634b42be6d2972eac. --- .golangci.yml | 22 ++++++++++++++++------ effector/effector.go | 2 +- enforcer.go | 13 +++++++------ enforcer_distributed.go | 4 ++-- frontend_old_test.go | 26 +++++++++++++------------- model/policy.go | 10 +++------- rbac_api.go | 27 ++++++++++++++------------- 7 files changed, 56 insertions(+), 48 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index aec667949..b8d362019 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -191,6 +191,16 @@ linters-settings: # Default: false all: true + stylecheck: + # STxxxx checks in https://staticcheck.io/docs/configuration/options/#checks + # Default: ["*"] + checks: ["all", "-ST1003"] + + revive: + rules: + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter + - name: unused-parameter + disabled: true linters: disable-all: true @@ -208,7 +218,7 @@ linters: - asciicheck # checks that your code does not contain non-ASCII identifiers - bidichk # checks for dangerous unicode character sequences - bodyclose # checks whether HTTP response body is closed successfully - #- cyclop # checks function and package cyclomatic complexity + - cyclop # checks function and package cyclomatic complexity - dupl # tool for code clone detection - durationcheck # checks for two durations multiplied together - errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error @@ -217,7 +227,7 @@ linters: - exhaustive # checks exhaustiveness of enum switch statements - exportloopref # checks for pointers to enclosing loop variables #- forbidigo # forbids identifiers - #- funlen # tool for detection of long functions + - funlen # tool for detection of long functions - gocheckcompilerdirectives # validates go compiler directive comments (//go:) #- gochecknoglobals # checks that no global variables exist - gochecknoinits # checks that no init functions are present in Go code @@ -225,7 +235,7 @@ linters: #- gocognit # computes and checks the cognitive complexity of functions #- goconst # finds repeated strings that could be replaced by a constant #- gocritic # provides diagnostics that check for bugs, performance and style issues - #- gocyclo # computes and checks the cyclomatic complexity of functions + - gocyclo # computes and checks the cyclomatic complexity of functions - godot # checks if comments end in a period - goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt #- gomnd # detects magic numbers @@ -239,7 +249,7 @@ linters: - mirror # reports wrong mirror patterns of bytes/strings usage - musttag # enforces field tags in (un)marshaled structs - nakedret # finds naked returns in functions greater than a specified function length - #- nestif # reports deeply nested if statements + - nestif # reports deeply nested if statements - nilerr # finds the code that returns nil even if it checks that the error is not nil #- nilnil # checks that there is no simultaneous return of nil error and an invalid value - noctx # finds sending http request without context.Context @@ -251,12 +261,12 @@ linters: - promlinter # checks Prometheus metrics naming via promlint - protogetter # reports direct reads from proto message fields when getters should be used - reassign # checks that package variables are not reassigned - #- revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint + - revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint - rowserrcheck # checks whether Err of rows is checked successfully - sloglint # ensure consistent code style when using log/slog - spancheck # checks for mistakes with OpenTelemetry/Census spans - sqlclosecheck # checks that sql.Rows and sql.Stmt are closed - #- stylecheck # is a replacement for golint + - stylecheck # is a replacement for golint - tenv # detects using os.Setenv instead of t.Setenv since Go1.17 - testableexamples # checks if examples are testable (have an expected output) - testifylint # checks usage of github.com/stretchr/testify diff --git a/effector/effector.go b/effector/effector.go index 665848b5c..49b84c3e1 100644 --- a/effector/effector.go +++ b/effector/effector.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package effector +package effector //nolint:cyclop // TODO // Effect is the result for a policy rule. type Effect int diff --git a/enforcer.go b/enforcer.go index d5c3f5571..d8f63f9e0 100644 --- a/enforcer.go +++ b/enforcer.go @@ -100,7 +100,8 @@ func NewEnforcer(params ...interface{}) (*Enforcer, error) { } } - if paramLen-parsedParamLen == 2 { + switch paramLen - parsedParamLen { + case 2: switch p0 := params[0].(type) { case string: switch p1 := params[1].(type) { @@ -126,7 +127,7 @@ func NewEnforcer(params ...interface{}) (*Enforcer, error) { } } } - } else if paramLen-parsedParamLen == 1 { + case 1: switch p0 := params[0].(type) { case string: err := e.InitWithFile(p0, "") @@ -139,9 +140,9 @@ func NewEnforcer(params ...interface{}) (*Enforcer, error) { return nil, err } } - } else if paramLen-parsedParamLen == 0 { + case 0: return e, nil - } else { + default: return nil, errors.New("invalid parameters for enforcer") } @@ -586,7 +587,7 @@ func (e *Enforcer) invalidateMatcherMap() { } // enforce use a custom matcher to decides whether a "subject" can access a "object" with the operation "action", input parameters are usually: (matcher, sub, obj, act), use model matcher by default when matcher is "". -func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interface{}) (ok bool, err error) { +func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interface{}) (ok bool, err error) { //nolint:funlen,cyclop,gocyclo // TODO: reduce function complexity defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic: %v\n%s", r, debug.Stack()) @@ -694,7 +695,7 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac var effect effector.Effect var explainIndex int - if policyLen := len(e.model["p"][pType].Policy); policyLen != 0 && strings.Contains(expString, pType+"_") { + if policyLen := len(e.model["p"][pType].Policy); policyLen != 0 && strings.Contains(expString, pType+"_") { //nolint:nestif // TODO: reduce function complexity policyEffects = make([]effector.Effect, policyLen) matcherResults = make([]float64, policyLen) diff --git a/enforcer_distributed.go b/enforcer_distributed.go index a6e15d6e6..2ff85e80e 100644 --- a/enforcer_distributed.go +++ b/enforcer_distributed.go @@ -22,8 +22,8 @@ func NewDistributedEnforcer(params ...interface{}) (*DistributedEnforcer, error) } // SetDispatcher sets the current dispatcher. -func (e *DistributedEnforcer) SetDispatcher(dispatcher persist.Dispatcher) { - e.dispatcher = dispatcher +func (d *DistributedEnforcer) SetDispatcher(dispatcher persist.Dispatcher) { + d.dispatcher = dispatcher } // AddPoliciesSelf provides a method for dispatcher to add authorization rules to the current policy. diff --git a/frontend_old_test.go b/frontend_old_test.go index 4041862a2..c55338924 100644 --- a/frontend_old_test.go +++ b/frontend_old_test.go @@ -33,14 +33,14 @@ func TestCasbinJsGetPermissionForUserOld(t *testing.T) { if err != nil { panic(err) } - target_str, _ := CasbinJsGetPermissionForUserOld(e, "alice") - t.Log("GetPermissionForUser Alice", string(target_str)) - alice_target := make(map[string][]string) - err = json.Unmarshal(target_str, &alice_target) + targetStr, _ := CasbinJsGetPermissionForUserOld(e, "alice") + t.Log("GetPermissionForUser Alice", string(targetStr)) + aliceTarget := make(map[string][]string) + err = json.Unmarshal(targetStr, &aliceTarget) if err != nil { t.Errorf("Test error: %s", err) } - perm, ok := alice_target["read"] + perm, ok := aliceTarget["read"] if !ok { t.Errorf("Test error: Alice doesn't have read permission") } @@ -50,7 +50,7 @@ func TestCasbinJsGetPermissionForUserOld(t *testing.T) { if !contains(perm, "data2") { t.Errorf("Test error: Alice cannot read data2") } - perm, ok = alice_target["write"] + perm, ok = aliceTarget["write"] if !ok { t.Errorf("Test error: Alice doesn't have write permission") } @@ -61,18 +61,18 @@ func TestCasbinJsGetPermissionForUserOld(t *testing.T) { t.Errorf("Test error: Alice cannot write data2") } - target_str, _ = CasbinJsGetPermissionForUserOld(e, "bob") - t.Log("GetPermissionForUser Bob", string(target_str)) - bob_target := make(map[string][]string) - err = json.Unmarshal(target_str, &bob_target) + targetStr, _ = CasbinJsGetPermissionForUserOld(e, "bob") + t.Log("GetPermissionForUser Bob", string(targetStr)) + bobTarget := make(map[string][]string) + err = json.Unmarshal(targetStr, &bobTarget) if err != nil { t.Errorf("Test error: %s", err) } - _, ok = bob_target["read"] + _, ok = bobTarget["read"] if ok { t.Errorf("Test error: Bob has read permission") } - perm, ok = bob_target["write"] + perm, ok = bobTarget["write"] if !ok { t.Errorf("Test error: Bob doesn't have permission") } @@ -86,7 +86,7 @@ func TestCasbinJsGetPermissionForUserOld(t *testing.T) { t.Errorf("Test error: Bob can access a non-existing data") } - _, ok = bob_target["rm_rf"] + _, ok = bobTarget["rm_rf"] if ok { t.Errorf("Someone can have a non-existing action (rm -rf)") } diff --git a/model/policy.go b/model/policy.go index 48dcb13d2..d2cadbf61 100644 --- a/model/policy.go +++ b/model/policy.go @@ -207,15 +207,11 @@ func (model Model) AddPolicy(sec string, ptype string, rule []string) { i := len(assertion.Policy) - 1 for ; i > 0; i-- { idx, err := strconv.Atoi(assertion.Policy[i-1][assertion.FieldIndexMap[constant.PriorityIndex]]) - if err != nil { - break - } - if idx > idxInsert { - assertion.Policy[i] = assertion.Policy[i-1] - assertion.PolicyMap[strings.Join(assertion.Policy[i-1], DefaultSep)]++ - } else { + if err != nil || idx <= idxInsert { break } + assertion.Policy[i] = assertion.Policy[i-1] + assertion.PolicyMap[strings.Join(assertion.Policy[i-1], DefaultSep)]++ } assertion.Policy[i] = rule assertion.PolicyMap[strings.Join(rule, DefaultSep)] = i diff --git a/rbac_api.go b/rbac_api.go index 49946b80b..b3321bf8e 100644 --- a/rbac_api.go +++ b/rbac_api.go @@ -315,20 +315,21 @@ func (e *Enforcer) GetNamedImplicitPermissionsForUser(ptype string, user string, if matched { permission = append(permission, deepCopyPolicy(rule)) } - } else if len(domain) > 1 { + continue + } + if len(domain) > 1 { return nil, errors.ErrDomainParameter - } else { - d := domain[0] - matched := rm.Match(d, rule[domainIndex]) - if !matched { - continue - } - matched, _ = rm.HasLink(user, rule[0], d) - if matched { - newRule := deepCopyPolicy(rule) - newRule[domainIndex] = d - permission = append(permission, newRule) - } + } + d := domain[0] + matched := rm.Match(d, rule[domainIndex]) + if !matched { + continue + } + matched, _ = rm.HasLink(user, rule[0], d) + if matched { + newRule := deepCopyPolicy(rule) + newRule[domainIndex] = d + permission = append(permission, newRule) } } return permission, nil