chore(lint): reduced disabled linter, addressed a few code quality issues

* extended the set of enabled linters

* improved readability of the set() method, which was too complex
* added systematic checks on type assertion
* removed impossible cases (type assertion _then_ same using reflect)

* test: removed the use of global variables
* test: improved the readability of tests
  * using t.Run() to label and hierarchize tests
  * using JSON test document as embedded asset

Signed-off-by: Frederic BIDON <[email protected]>

Author: fredbi

.golangci.yml

@@ -2,33 +2,22 @@ version: "2"
 linters:
   default: all
   disable:
-    - cyclop
     - depguard
-    - errchkjson
     - errorlint
     - exhaustruct
-    - forcetypeassert
     - funlen
-    - gochecknoglobals
-    - gochecknoinits
-    - gocognit
     - godot
     - godox
     - gosmopolitan
-    - inamedparam
     - ireturn
     - lll
-    - musttag
-    - nestif
     - nlreturn
     - nonamedreturns
     - noinlineerr
     - paralleltest
     - recvcheck
     - testpackage
-    - thelper
     - tparallel
-    - unparam
     - varnamelen
     - whitespace
     - wrapcheck
@@ -40,8 +29,10 @@ linters:
     goconst:
       min-len: 2
       min-occurrences: 3
+    cyclop:
+      max-complexity: 20
     gocyclo:
-      min-complexity: 45
+      min-complexity: 20
   exclusions:
     generated: lax
     presets:

README.md

@@ -1,4 +1,4 @@
-# gojsonpointer
+# jsonpointer
 
 <!-- Badges: status  -->
 [![Tests][test-badge]][test-url] [![Coverage][cov-badge]][cov-url] [![CI vuln scan][vuln-scan-badge]][vuln-scan-url] [![CodeQL][codeql-badge]][codeql-url]
@@ -14,7 +14,7 @@
 
 ---
 
-An implementation of JSON Pointer - Go language
+An implementation of JSON Pointer for golang, which supports go `struct`.
 
 ## Status
 

pointer.go

@@ -20,21 +20,16 @@ const (
 	pointerSeparator = `/`
 )
 
-var (
-	jsonPointableType = reflect.TypeOf(new(JSONPointable)).Elem()
-	jsonSetableType   = reflect.TypeOf(new(JSONSetable)).Elem()
-)
-
 // JSONPointable is an interface for structs to implement when they need to customize the
 // json pointer process
 type JSONPointable interface {
-	JSONLookup(string) (any, error)
+	JSONLookup(key string) (any, error)
 }
 
 // JSONSetable is an interface for structs to implement when they need to customize the
 // json pointer process
 type JSONSetable interface {
-	JSONSet(string, any) error
+	JSONSet(key string, value any) error
 }
 
 // Pointer is a representation of a json pointer
@@ -174,7 +169,7 @@ func (p *Pointer) set(node, data any, nameProvider *jsonname.NameProvider) error
 		nameProvider = jsonname.DefaultJSONNameProvider
 	}
 
-	// Full document when empty
+	// full document when empty
 	if len(p.referenceTokens) == 0 {
 		return nil
 	}
@@ -185,81 +180,79 @@ func (p *Pointer) set(node, data any, nameProvider *jsonname.NameProvider) error
 		decodedToken := Unescape(token)
 
 		if isLastToken {
-
 			return setSingleImpl(node, data, decodedToken, nameProvider)
 		}
 
-		// Check for nil during traversal
-		if isNil(node) {
-			return fmt.Errorf("cannot traverse through nil value at %q: %w", decodedToken, ErrPointer)
+		next, err := p.resolveNodeForToken(node, decodedToken, nameProvider)
+		if err != nil {
+			return err
 		}
 
-		rValue := reflect.Indirect(reflect.ValueOf(node))
-		kind := rValue.Kind()
+		node = next
+	}
 
-		if rValue.Type().Implements(jsonPointableType) {
-			r, err := node.(JSONPointable).JSONLookup(decodedToken)
-			if err != nil {
-				return err
-			}
-			fld := reflect.ValueOf(r)
-			if fld.CanAddr() && fld.Kind() != reflect.Interface && fld.Kind() != reflect.Map && fld.Kind() != reflect.Slice && fld.Kind() != reflect.Pointer {
-				node = fld.Addr().Interface()
-				continue
-			}
-			node = r
-			continue
+	return nil
+}
+
+func (p *Pointer) resolveNodeForToken(node any, decodedToken string, nameProvider *jsonname.NameProvider) (next any, err error) {
+	// check for nil during traversal
+	if isNil(node) {
+		return nil, fmt.Errorf("cannot traverse through nil value at %q: %w", decodedToken, ErrPointer)
+	}
+
+	pointable, ok := node.(JSONPointable)
+	if ok {
+		r, err := pointable.JSONLookup(decodedToken)
+		if err != nil {
+			return nil, err
 		}
 
-		switch kind { //nolint:exhaustive
-		case reflect.Struct:
-			nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken)
-			if !ok {
-				return fmt.Errorf("object has no field %q: %w", decodedToken, ErrPointer)
-			}
-			fld := rValue.FieldByName(nm)
-			if fld.CanAddr() && fld.Kind() != reflect.Interface && fld.Kind() != reflect.Map && fld.Kind() != reflect.Slice && fld.Kind() != reflect.Pointer {
-				node = fld.Addr().Interface()
-				continue
-			}
-			node = fld.Interface()
+		fld := reflect.ValueOf(r)
+		if fld.CanAddr() && fld.Kind() != reflect.Interface && fld.Kind() != reflect.Map && fld.Kind() != reflect.Slice && fld.Kind() != reflect.Pointer {
+			return fld.Addr().Interface(), nil
+		}
 
-		case reflect.Map:
-			kv := reflect.ValueOf(decodedToken)
-			mv := rValue.MapIndex(kv)
+		return r, nil
+	}
 
-			if !mv.IsValid() {
-				return fmt.Errorf("object has no key %q: %w", decodedToken, ErrPointer)
-			}
-			if mv.CanAddr() && mv.Kind() != reflect.Interface && mv.Kind() != reflect.Map && mv.Kind() != reflect.Slice && mv.Kind() != reflect.Pointer {
-				node = mv.Addr().Interface()
-				continue
-			}
-			node = mv.Interface()
+	rValue := reflect.Indirect(reflect.ValueOf(node))
+	kind := rValue.Kind()
 
-		case reflect.Slice:
-			tokenIndex, err := strconv.Atoi(decodedToken)
-			if err != nil {
-				return err
-			}
-			sLength := rValue.Len()
-			if tokenIndex < 0 || tokenIndex >= sLength {
-				return fmt.Errorf("index out of bounds array[0,%d] index '%d': %w", sLength, tokenIndex, ErrPointer)
-			}
+	switch kind { //nolint:exhaustive
+	case reflect.Struct:
+		nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken)
+		if !ok {
+			return nil, fmt.Errorf("object has no field %q: %w", decodedToken, ErrPointer)
+		}
 
-			elem := rValue.Index(tokenIndex)
-			if elem.CanAddr() && elem.Kind() != reflect.Interface && elem.Kind() != reflect.Map && elem.Kind() != reflect.Slice && elem.Kind() != reflect.Pointer {
-				node = elem.Addr().Interface()
-				continue
-			}
-			node = elem.Interface()
+		return typeFromValue(rValue.FieldByName(nm)), nil
 
-		default:
-			return fmt.Errorf("invalid token reference %q: %w", decodedToken, ErrPointer)
+	case reflect.Map:
+		kv := reflect.ValueOf(decodedToken)
+		mv := rValue.MapIndex(kv)
+
+		if !mv.IsValid() {
+			return nil, fmt.Errorf("object has no key %q: %w", decodedToken, ErrPointer)
 		}
-	}
 
-	return nil
+		return typeFromValue(mv), nil
+
+	case reflect.Slice:
+		tokenIndex, err := strconv.Atoi(decodedToken)
+		if err != nil {
+			return nil, errors.Join(err, ErrPointer)
+		}
+
+		sLength := rValue.Len()
+		if tokenIndex < 0 || tokenIndex >= sLength {
+			return nil, fmt.Errorf("index out of bounds array[0,%d] index '%d': %w", sLength, tokenIndex, ErrPointer)
+		}
+
+		return typeFromValue(rValue.Index(tokenIndex)), nil
+
+	default:
+		return nil, fmt.Errorf("invalid token reference %q: %w", decodedToken, ErrPointer)
+	}
 }
 
 func isNil(input any) bool {
@@ -276,6 +269,14 @@ func isNil(input any) bool {
 	}
 }
 
+func typeFromValue(v reflect.Value) any {
+	if v.CanAddr() && v.Kind() != reflect.Interface && v.Kind() != reflect.Map && v.Kind() != reflect.Slice && v.Kind() != reflect.Pointer {
+		return v.Addr().Interface()
+	}
+
+	return v.Interface()
+}
+
 // GetForToken gets a value for a json pointer token 1 level deep
 func GetForToken(document any, decodedToken string) (any, reflect.Kind, error) {
 	return getSingleImpl(document, decodedToken, jsonname.DefaultJSONNameProvider)
@@ -348,14 +349,10 @@ func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.N
 		return fmt.Errorf("cannot set field %q on nil value: %w", decodedToken, ErrPointer)
 	}
 
-	if ns, ok := node.(JSONSetable); ok { // pointer impl
+	if ns, ok := node.(JSONSetable); ok {
 		return ns.JSONSet(decodedToken, data)
 	}
 
-	if rValue.Type().Implements(jsonSetableType) {
-		return node.(JSONSetable).JSONSet(decodedToken, data)
-	}
-
 	switch rValue.Kind() { //nolint:exhaustive
 	case reflect.Struct:
 		nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken)
@@ -499,8 +496,8 @@ const (
 )
 
 var (
-	encRefTokReplacer = strings.NewReplacer(encRefTok1, decRefTok1, encRefTok0, decRefTok0)
-	decRefTokReplacer = strings.NewReplacer(decRefTok1, encRefTok1, decRefTok0, encRefTok0)
+	encRefTokReplacer = strings.NewReplacer(encRefTok1, decRefTok1, encRefTok0, decRefTok0) //nolint:gochecknoglobals // it's okay to declare a replacer as a private global
+	decRefTokReplacer = strings.NewReplacer(decRefTok1, encRefTok1, decRefTok0, encRefTok0) //nolint:gochecknoglobals // it's okay to declare a replacer as a private global
 )
 
 // Unescape unescapes a json pointer reference token string to the original representation