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 3b56fa9.

* Revert "feat: enable `goconst` lint rule"

This reverts commit 8cec408.

.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,15 +227,15 @@ 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
     - gochecksumtype # checks exhaustiveness on Go "sum types"
     #- 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

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

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)
 

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.

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)")
 	}

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

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