Merge pull request #98 from kenjones-cisco/task/lint-fixes

Task: Resolve lint errors

Author: hsluoyz

config/config.go

@@ -27,11 +27,15 @@ import (
 )
 
 var (
-	DEFAULT_SECTION     = "default"
-	DEFAULT_COMMENT     = []byte{'#'}
+	// DEFAULT_SECTION specifies the name of a section if no name provided
+	DEFAULT_SECTION = "default"
+	// DEFAULT_COMMENT defines what character(s) indicate a comment `#`
+	DEFAULT_COMMENT = []byte{'#'}
+	// DEFAULT_COMMENT_SEM defines what alternate character(s) indicate a comment `;`
 	DEFAULT_COMMENT_SEM = []byte{';'}
 )
 
+// ConfigInterface defines the behavior of a Config implemenation
 type ConfigInterface interface {
 	String(key string) string
 	Strings(key string) []string
@@ -42,6 +46,7 @@ type ConfigInterface interface {
 	Set(key string, value string) error
 }
 
+// Config represents an implementation of the ConfigInterface
 type Config struct {
 	// map is not safe.
 	sync.RWMutex
@@ -133,26 +138,32 @@ func (c *Config) parseBuffer(buf *bufio.Reader) (err error) {
 	return nil
 }
 
+// Bool lookups up the value using the provided key and converts the value to a bool
 func (c *Config) Bool(key string) (bool, error) {
 	return strconv.ParseBool(c.get(key))
 }
 
+// Int lookups up the value using the provided key and converts the value to a int
 func (c *Config) Int(key string) (int, error) {
 	return strconv.Atoi(c.get(key))
 }
 
+// Int64 lookups up the value using the provided key and converts the value to a int64
 func (c *Config) Int64(key string) (int64, error) {
 	return strconv.ParseInt(c.get(key), 10, 64)
 }
 
+// Float64 lookups up the value using the provided key and converts the value to a float64
 func (c *Config) Float64(key string) (float64, error) {
 	return strconv.ParseFloat(c.get(key), 64)
 }
 
+// String lookups up the value using the provided key and converts the value to a string
 func (c *Config) String(key string) string {
 	return c.get(key)
 }
 
+// Strings lookups up the value using the provided key and converts the value to an array of string by splitting the string by comma
 func (c *Config) Strings(key string) []string {
 	v := c.get(key)
 	if v == "" {
@@ -161,6 +172,7 @@ func (c *Config) Strings(key string) []string {
 	return strings.Split(v, ",")
 }
 
+// Set sets the value for the specific key in the Config
 func (c *Config) Set(key string, value string) error {
 	c.Lock()
 	defer c.Unlock()

config/config_test.go

@@ -19,14 +19,14 @@ import (
 )
 
 func TestGet(t *testing.T) {
-	config, err := NewConfig("testdata/testini.ini")
-	if err != nil {
-		t.Errorf("Configuration file loading failed, err:%v", err.Error())
-		t.Fatalf("err: %v", err)
+	config, cerr := NewConfig("testdata/testini.ini")
+	if cerr != nil {
+		t.Errorf("Configuration file loading failed, err:%v", cerr.Error())
+		t.Fatalf("err: %v", cerr)
 	}
 
 	// default::key test
-	if v, err := config.Bool("debug"); err != nil || v != true {
+	if v, err := config.Bool("debug"); err != nil || !v {
 		t.Errorf("Get failure: expected different value for debug (expected: [%#v] got: [%#v])", true, v)
 		t.Fatalf("err: %v", err)
 	}
@@ -58,19 +58,16 @@ func TestGet(t *testing.T) {
 	// other::key test
 	if v := config.String("other::name"); v != "ATC自动化测试^-^&($#……#" {
 		t.Errorf("Get failure: expected different value for other::name (expected: [%#v] got: [%#v])", "ATC自动化测试^-^&($#……#", v)
-		t.Fatalf("err: %v", err)
 	}
 
 	if v := config.String("other::key1"); v != "test key" {
 		t.Errorf("Get failure: expected different value for other::key1 (expected: [%#v] got: [%#v])", "test key", v)
-		t.Fatalf("err: %v", err)
 	}
 
 	config.Set("other::key1", "new test key")
 
 	if v := config.String("other::key1"); v != "new test key" {
 		t.Errorf("Get failure: expected different value for other::key1 (expected: [%#v] got: [%#v])", "new test key", v)
-		t.Fatalf("err: %v", err)
 	}
 
 	config.Set("other::key1", "test key")

enforcer.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/Knetic/govaluate"
+
 	"github.com/casbin/casbin/effect"
 	"github.com/casbin/casbin/model"
 	"github.com/casbin/casbin/persist"
@@ -124,6 +125,7 @@ func (e *Enforcer) InitWithModelAndAdapter(m model.Model, adapter persist.Adapte
 	e.initialize()
 
 	if e.adapter != nil {
+		// error intentionally ignored
 		e.LoadPolicy()
 	}
 }
@@ -184,6 +186,7 @@ func (e *Enforcer) SetAdapter(adapter persist.Adapter) {
 // SetWatcher sets the current watcher.
 func (e *Enforcer) SetWatcher(watcher persist.Watcher) {
 	e.watcher = watcher
+	// error intentionally ignored
 	watcher.SetUpdateCallback(func(string) { e.LoadPolicy() })
 }
 
@@ -205,8 +208,7 @@ func (e *Enforcer) ClearPolicy() {
 // LoadPolicy reloads the policy from file/database.
 func (e *Enforcer) LoadPolicy() error {
 	e.model.ClearPolicy()
-	err := e.adapter.LoadPolicy(e.model)
-	if err != nil {
+	if err := e.adapter.LoadPolicy(e.model); err != nil {
 		return err
 	}
 
@@ -230,8 +232,7 @@ func (e *Enforcer) LoadFilteredPolicy(filter interface{}) error {
 	default:
 		return errors.New("filtered policies are not supported by this adapter")
 	}
-	err := filteredAdapter.LoadFilteredPolicy(e.model, filter)
-	if err != nil {
+	if err := filteredAdapter.LoadFilteredPolicy(e.model, filter); err != nil {
 		return err
 	}
 
@@ -256,13 +257,13 @@ func (e *Enforcer) SavePolicy() error {
 	if e.IsFiltered() {
 		return errors.New("cannot save a filtered policy")
 	}
-	err := e.adapter.SavePolicy(e.model)
-	if err == nil {
-		if e.watcher != nil {
-			e.watcher.Update()
-		}
+	if err := e.adapter.SavePolicy(e.model); err != nil {
+		return err
+	}
+	if e.watcher != nil {
+		return e.watcher.Update()
 	}
-	return err
+	return nil
 }
 
 // EnableEnforce changes the enforcing state of Casbin, when Casbin is disabled, all access will be allowed by the Enforce() function.
@@ -287,6 +288,7 @@ func (e *Enforcer) EnableAutoBuildRoleLinks(autoBuildRoleLinks bool) {
 
 // BuildRoleLinks manually rebuild the role inheritance relations.
 func (e *Enforcer) BuildRoleLinks() {
+	// error intentionally ignored
 	e.rm.Clear()
 	e.model.BuildRoleLinks(e.rm)
 }
@@ -334,39 +336,41 @@ func (e *Enforcer) Enforce(rvals ...interface{}) bool {
 			if err != nil {
 				policyEffects[i] = effect.Indeterminate
 				panic(err)
-			} else {
-				switch result.(type) {
-				case bool:
-					if !result.(bool) {
-						policyEffects[i] = effect.Indeterminate
-						continue
-					}
-				case float64:
-					if result.(float64) == 0 {
-						policyEffects[i] = effect.Indeterminate
-						continue
-					} else {
-						matcherResults[i] = result.(float64)
-					}
-				default:
-					panic(errors.New("matcher result should be bool, int or float"))
+			}
+
+			switch result.(type) {
+			case bool:
+				if !result.(bool) {
+					policyEffects[i] = effect.Indeterminate
+					continue
 				}
-				if eft, ok := parameters["p_eft"]; ok {
-					if eft == "allow" {
-						policyEffects[i] = effect.Allow
-					} else if eft == "deny" {
-						policyEffects[i] = effect.Deny
-					} else {
-						policyEffects[i] = effect.Indeterminate
-					}
+			case float64:
+				if result.(float64) == 0 {
+					policyEffects[i] = effect.Indeterminate
+					continue
 				} else {
-					policyEffects[i] = effect.Allow
+					matcherResults[i] = result.(float64)
 				}
+			default:
+				panic(errors.New("matcher result should be bool, int or float"))
+			}
 
-				if e.model["e"]["e"].Value == "priority(p_eft) || deny" {
-					break
+			if eft, ok := parameters["p_eft"]; ok {
+				if eft == "allow" {
+					policyEffects[i] = effect.Allow
+				} else if eft == "deny" {
+					policyEffects[i] = effect.Deny
+				} else {
+					policyEffects[i] = effect.Indeterminate
 				}
+			} else {
+				policyEffects[i] = effect.Allow
+			}
+
+			if e.model["e"]["e"].Value == "priority(p_eft) || deny" {
+				break
 			}
+
 		}
 	} else {
 		policyEffects = make([]effect.Effect, 1)
@@ -386,12 +390,12 @@ func (e *Enforcer) Enforce(rvals ...interface{}) bool {
 		if err != nil {
 			policyEffects[0] = effect.Indeterminate
 			panic(err)
+		}
+
+		if result.(bool) {
+			policyEffects[0] = effect.Allow
 		} else {
-			if result.(bool) {
-				policyEffects[0] = effect.Allow
-			} else {
-				policyEffects[0] = effect.Indeterminate
-			}
+			policyEffects[0] = effect.Indeterminate
 		}
 	}
 
@@ -402,16 +406,20 @@ func (e *Enforcer) Enforce(rvals ...interface{}) bool {
 		panic(err)
 	}
 
-	reqStr := "Request: "
-	for i, rval := range rvals {
-		if i != len(rvals)-1 {
-			reqStr += fmt.Sprintf("%v, ", rval)
-		} else {
-			reqStr += fmt.Sprintf("%v", rval)
+	// only generate the request --> result string if the message
+	// is going to be logged.
+	if util.EnableLog {
+		reqStr := "Request: "
+		for i, rval := range rvals {
+			if i != len(rvals)-1 {
+				reqStr += fmt.Sprintf("%v, ", rval)
+			} else {
+				reqStr += fmt.Sprintf("%v", rval)
+			}
 		}
+		reqStr += fmt.Sprintf(" ---> %t", result)
+		util.LogPrint(reqStr)
 	}
-	reqStr += fmt.Sprintf(" ---> %t", result)
-	util.LogPrint(reqStr)
 
 	return result
 }

enforcer_safe.go

@@ -15,15 +15,14 @@
 package casbin
 
 import (
-	"errors"
 	"fmt"
 )
 
 // NewEnforcerSafe calls NewEnforcer in a safe way, returns error instead of causing panic.
 func NewEnforcerSafe(params ...interface{}) (e *Enforcer, err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("%v", r))
+			err = fmt.Errorf("%v", r)
 			e = nil
 		}
 	}()
@@ -37,7 +36,7 @@ func NewEnforcerSafe(params ...interface{}) (e *Enforcer, err error) {
 func (e *Enforcer) LoadModelSafe() (err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("%v", r))
+			err = fmt.Errorf("%v", r)
 		}
 	}()
 
@@ -50,7 +49,7 @@ func (e *Enforcer) LoadModelSafe() (err error) {
 func (e *Enforcer) EnforceSafe(rvals ...interface{}) (result bool, err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("%v", r))
+			err = fmt.Errorf("%v", r)
 			result = false
 		}
 	}()
@@ -60,10 +59,11 @@ func (e *Enforcer) EnforceSafe(rvals ...interface{}) (result bool, err error) {
 	return
 }
 
+// AddPolicySafe calls AddPolicy in a safe way, returns error instead of causing panic.
 func (e *Enforcer) AddPolicySafe(params ...interface{}) (result bool, err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("%v", r))
+			err = fmt.Errorf("%v", r)
 			result = false
 		}
 	}()
@@ -73,10 +73,11 @@ func (e *Enforcer) AddPolicySafe(params ...interface{}) (result bool, err error)
 	return
 }
 
+// RemovePolicySafe calls RemovePolicy in a safe way, returns error instead of causing panic.
 func (e *Enforcer) RemovePolicySafe(params ...interface{}) (result bool, err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("%v", r))
+			err = fmt.Errorf("%v", r)
 			result = false
 		}
 	}()
@@ -86,15 +87,16 @@ func (e *Enforcer) RemovePolicySafe(params ...interface{}) (result bool, err err
 	return
 }
 
+// RemoveFilteredPolicySafe calls RemoveFilteredPolicy in a safe way, returns error instead of causing panic.
 func (e *Enforcer) RemoveFilteredPolicySafe(fieldIndex int, fieldValues ...string) (result bool, err error) {
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("%v", r))
+			err = fmt.Errorf("%v", r)
 			result = false
 		}
 	}()
 
 	result = e.RemoveFilteredNamedPolicy("p", fieldIndex, fieldValues...)
 	err = nil
 	return
-}
+}