simply resolve logic, add comments and review lint rules

.golangci.yml

@@ -66,20 +66,17 @@ linters:
   disable-all: true
   enable:
     - bodyclose
-    - deadcode
-#    - depguard
     - dogsled
     - dupl
     - errcheck
     - exhaustive
     - funlen
     - gochecknoinits
     - goconst
-#    - gocritic # per https://github.com/golangci/golangci-lint/issues/3933
+    - gocritic # per https://github.com/golangci/golangci-lint/issues/3933
     - gocyclo
     - gofmt
     - goimports
-    - golint
     - gomnd
     - goprintffuncname
     - gosec
@@ -91,16 +88,14 @@ linters:
     - nakedret
     - noctx
     - nolintlint
+    - revive
     - rowserrcheck
-    - scopelint
     - staticcheck
-    - structcheck
     - stylecheck
     - typecheck
     - unconvert
     - unparam
     - unused
-    - varcheck
     - whitespace
 
   # don't enable:
@@ -115,12 +110,14 @@ linters:
   # - nestif
   # - prealloc
   # - testpackage
-  # - revive
   # - wsl
 
 issues:
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
+    - path: _test\.go
+      linters:
+        - revive
     - path: _test\.go
       linters:
         - gomnd

dotpath.go

@@ -12,7 +12,7 @@ type getPathPart struct {
 
 //nolint:gocyclo
 func parseDotPath(path string) ([]*getPathPart, error) {
-	if len(path) == 0 {
+	if path == "" {
 		return nil, nil
 	}
 	var paths []*getPathPart

feature.go

@@ -8,3 +8,25 @@ type Feature interface {
 	Name() string
 	Resolve(ctx context.Context, loader *Loader, node *Node) (*Node, error)
 }
+
+type ResolveFunc func(ctx context.Context, loader *Loader, node *Node) (*Node, error)
+
+type featureFunc struct {
+	name    string
+	resolve ResolveFunc
+}
+
+func (f *featureFunc) Name() string {
+	return f.name
+}
+
+func (f *featureFunc) Resolve(ctx context.Context, loader *Loader, node *Node) (*Node, error) {
+	return f.resolve(ctx, loader, node)
+}
+
+func FeatureFunc(name string, resolve ResolveFunc) Feature {
+	return &featureFunc{
+		name:    name,
+		resolve: resolve,
+	}
+}

loader.go

@@ -13,14 +13,11 @@ import (
 type Loader struct {
 	features []Feature
 
-	root        *Node
-	loadedFiles map[string]bool
+	root *Node
 }
 
 func New() *Loader {
-	return &Loader{
-		loadedFiles: map[string]bool{},
-	}
+	return &Loader{}
 }
 
 func (l *Loader) WithFeatures(features ...Feature) *Loader {
@@ -37,9 +34,11 @@ func (l *Loader) Load(name string, contents []byte) error {
 		return fmt.Errorf("unable to unmarshal file %q: %w", name, errors.Join(err, ErrConfigParseError))
 	}
 
+	// root node is a document node, and the first child is a map holds all the values
 	fileNode := NewNode(yamlNode.Content[0], NodeFilepath(name))
 	name = strings.TrimSuffix(name, filepath.Ext(name))
 	names := strings.Split(name, string(filepath.Separator))
+	// nest the file with its path, e.g, config/app.yaml -> config.app
 	fileNode = PackNodeInNestedKeys(fileNode, names...)
 
 	if l.root == nil {
@@ -52,7 +51,6 @@ func (l *Loader) Load(name string, contents []byte) error {
 		l.root = newNode
 	}
 
-	l.loadedFiles[name] = true
 	return nil
 }
 
@@ -68,19 +66,6 @@ func (l *Loader) Get(ctx context.Context, path string, target any) error {
 }
 
 func (l *Loader) GetNode(ctx context.Context, path string) (*Node, error) {
-	checkFile := strings.ReplaceAll(path, ".", string(filepath.Separator))
-	loaded := false
-	for file := range l.loadedFiles {
-		if strings.HasPrefix(checkFile, file) {
-			loaded = true
-			break
-		}
-	}
-
-	if !loaded {
-		return nil, fmt.Errorf("no file contains path %q: %w", path, ErrPathNotFound)
-	}
-
 	current := l.root
 	if len(path) > 0 {
 		paths, err := parseDotPath(path)
@@ -105,12 +90,12 @@ func (l *Loader) GetNode(ctx context.Context, path string) (*Node, error) {
 				break
 			}
 
-			if !current.resolved {
-				current, err = l.resolve(ctx, current)
-				if err != nil {
-					return nil, err
-				}
+			// always try to resolve the node, so if it has resolvedNode, it will be used instead
+			current, err = l.resolve(ctx, current)
+			if err != nil {
+				return nil, err
 			}
+
 			if current == nil {
 				break
 			}
@@ -124,20 +109,25 @@ func (l *Loader) GetNode(ctx context.Context, path string) (*Node, error) {
 	return l.resolve(ctx, current)
 }
 
-func (l *Loader) resolve(ctx context.Context, node *Node) (result *Node, reterr error) {
+func (l *Loader) resolve(ctx context.Context, node *Node) (resultNode *Node, reterr error) {
 	if node.resolved {
+		// if the node is resolved by tagged resolver, the result is stored in resolvedNode (so the original value can be preserved)
 		if node.resolvedNode != nil {
 			return node.resolvedNode, nil
 		}
 		return node, nil
 	}
+
+	// set it to true first to avoid infinite loop
 	node.resolved = true
 	defer func() {
 		if reterr != nil {
 			node.resolved = false
 		}
 	}()
 
+	// resolve children first for mapping and sequence nodes
+
 	if node.kind == yaml.MappingNode {
 		for key := range node.mappingNodes {
 			childNode, err := l.resolve(ctx, node.mappingNodes[key])
@@ -158,10 +148,12 @@ func (l *Loader) resolve(ctx context.Context, node *Node) (result *Node, reterr
 		}
 	}
 
+	// if not tagged, no further action is needed
 	if node.style&yaml.TaggedStyle == 0 {
 		return node, nil
 	}
 
+	// resolve the node with the feature if matched
 	for _, feature := range l.features {
 		if feature.Name() == node.tag {
 			result, err := feature.Resolve(ctx, l, node)
@@ -170,7 +162,7 @@ func (l *Loader) resolve(ctx context.Context, node *Node) (result *Node, reterr
 				return nil, err
 			}
 			node.resolvedNode = result
-			return result, nil
+			return node, nil
 		}
 	}
 

loader_test.go

@@ -16,7 +16,6 @@ name: John`))).To(BeNil())
 		Expect(loader.root.kind).To(Equal(yaml.MappingNode))
 		Expect(loader.root.mappingNodes["config"].mappingNodes["app"].mappingNodes["env"].value).To(Equal("dev"))
 		Expect(loader.root.mappingNodes["config"].mappingNodes["app"].mappingNodes["name"].value).To(Equal("John"))
-		Expect(loader.loadedFiles["config/app"]).To(BeTrue())
 
 		Expect(loader.Load("config/app.yaml", []byte(`env: test`))).To(BeNil())
 		Expect(loader.root.mappingNodes["config"].mappingNodes["app"].mappingNodes["env"].value).To(Equal("test"))
@@ -25,7 +24,6 @@ name: John`))).To(BeNil())
 		Expect(loader.Load("another.yaml", []byte(`value: another`))).To(BeNil())
 		Expect(loader.root.kind).To(Equal(yaml.MappingNode))
 		Expect(loader.root.mappingNodes["another"].mappingNodes["value"].value).To(Equal("another"))
-		Expect(loader.loadedFiles["another"]).To(BeTrue())
 	})
 
 	It("should Get", func() {
@@ -43,4 +41,14 @@ name: John`))).To(BeNil())
 		Expect(value).To(Equal(789))
 	})
 
+	It("should preserve original value of tagged node", func() {
+		loader := New().WithFeatures(FeatureFunc("!test", func(ctx context.Context, loader *Loader, node *Node) (*Node, error) {
+			return NewScalarNode("hello world"), nil
+		}))
+		Expect(loader.Load("app.yaml", []byte(`value: !test origin`))).To(BeNil())
+		var value string
+		Expect(loader.Get(context.Background(), "app.value", &value)).To(BeNil())
+		Expect(value).To(Equal("hello world"))
+		Expect(loader.root.mappingNodes["app"].mappingNodes["value"].value).To(Equal("origin"))
+	})
 })

node.go

@@ -128,7 +128,7 @@ func (n *Node) GetSequenceChild(index int) (*Node, error) {
 }
 
 func (n *Node) GetDeep(path string) (*Node, error) {
-	if len(path) == 0 {
+	if path == "" {
 		return n, nil
 	}
 
@@ -260,6 +260,13 @@ func (n *Node) Style() yaml.Style {
 }
 
 func (n *Node) Value() string {
+	if n.resolved && n.resolvedNode != nil {
+		return n.resolvedNode.Value()
+	}
+	return n.value
+}
+
+func (n *Node) RawValue() string {
 	return n.value
 }
 

realworld_test/realworld_test.go

@@ -12,7 +12,7 @@ import (
 
 var _ = Describe("All Realworld Tests", func() {
 	It("should resolve after load (can reference  another file)", func() {
-		loader := gofigure.New().WithFeatures(feature.All...)
+		loader := gofigure.New().WithFeatures(feature.Reference(), feature.Template())
 		Expect(loader.Load("app.yaml", []byte(`env: dev
 port: 8080
 host: localhost