From fd7e2f696fb94ac65dc40f973001e147fa843c40 Mon Sep 17 00:00:00 2001 From: Lukas Jarosch Date: Wed, 6 Mar 2024 16:37:55 +0100 Subject: [PATCH] fix: satisfy golangci-lint --- class.go | 7 ++----- data/container.go | 2 +- data/container_test.go | 1 + data/data.go | 5 +---- data/data_test.go | 35 ++++++++++++++++++----------------- fs.go | 5 ++++- inventory.go | 4 ++-- reference/value.go | 6 +++--- reference_test.go | 2 +- registry.go | 15 ++++++--------- registry_test.go | 2 -- 11 files changed, 39 insertions(+), 45 deletions(-) diff --git a/class.go b/class.go index 3d3bf72..99c0f88 100644 --- a/class.go +++ b/class.go @@ -126,10 +126,7 @@ func (c Class) GetPath(path data.Path) (data.Value, error) { // HasPath returns true if the given path exists within the Class. func (c Class) HasPath(path data.Path) bool { _, err := c.container.GetPath(path) - if err != nil { - return false - } - return true + return err == nil } // GetAll returns the whole data represented by this class. @@ -227,7 +224,7 @@ func (c *Class) WalkValues(walkFunc func(data.Path, data.Value) error) error { func (c *Class) Values() map[string]data.Value { valueMap := make(map[string]data.Value) - c.WalkValues(func(p data.Path, v data.Value) error { + _ = c.WalkValues(func(p data.Path, v data.Value) error { valueMap[p.String()] = v return nil }) diff --git a/data/container.go b/data/container.go index f17660d..c4b2934 100644 --- a/data/container.go +++ b/data/container.go @@ -214,7 +214,7 @@ func (container *Container) HasPath(path Path) bool { // The function does not check whether the path is valid within the container. // This is to allow setting new paths, which do not yet exist. func (container *Container) AbsolutePath(path Path) (Path, error) { - if path == nil || len(path) == 0 { + if len(path) == 0 { return nil, ErrEmptyPath } diff --git a/data/container_test.go b/data/container_test.go index d68122d..ef4daf0 100644 --- a/data/container_test.go +++ b/data/container_test.go @@ -170,6 +170,7 @@ func TestContainer_Set(t *testing.T) { assert.NotNil(t, container) err = container.SetPath(NewPath("test.array.1.0"), []interface{}{1, 2, 3}) + assert.NoError(t, err) } func TestContainer_Merge(t *testing.T) { diff --git a/data/data.go b/data/data.go index 2c807be..40a53b4 100644 --- a/data/data.go +++ b/data/data.go @@ -382,10 +382,7 @@ var SelectAllPaths PathSelectorFunc = func(_ interface{}, path Path, isLeaf bool } var SelectLeafPaths PathSelectorFunc = func(_ interface{}, path Path, isLeaf bool) bool { - if isLeaf { - return true - } - return false + return isLeaf } func Paths(data map[string]interface{}, selectorFn PathSelectorFunc) []Path { diff --git a/data/data_test.go b/data/data_test.go index e1855ef..9aafc23 100644 --- a/data/data_test.go +++ b/data/data_test.go @@ -4,8 +4,9 @@ import ( "reflect" "testing" - "github.com/lukasjarosch/skipper/data" "github.com/stretchr/testify/assert" + + "github.com/lukasjarosch/skipper/data" ) func TestWalk(t *testing.T) { @@ -186,7 +187,6 @@ func TestWalk(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // we need to store the paths in a map to prevent the 'append' behavior // where the last element is overwritten by the next pathMap := make(map[string]bool) @@ -211,8 +211,6 @@ func TestWalk(t *testing.T) { assert.NoError(t, err) }) } - - return } func TestGet(t *testing.T) { @@ -224,7 +222,6 @@ func TestGet(t *testing.T) { errExpected bool err error }{ - { name: "nil data", data: nil, @@ -503,7 +500,6 @@ func TestSet(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - returned, err := data.Set(tt.input, tt.key, tt.value) if tt.errExpected { @@ -824,7 +820,6 @@ func TestDeepSet(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - returned, err := data.DeepSet(tt.input, tt.path, tt.value) if tt.errExpected { @@ -853,7 +848,8 @@ func TestPaths_SelectAll(t *testing.T) { }, expected: []data.Path{ data.NewPath("foo"), - data.NewPath("baz")}, + data.NewPath("baz"), + }, }, { name: "NestedMap", @@ -866,21 +862,24 @@ func TestPaths_SelectAll(t *testing.T) { expected: []data.Path{ data.NewPath("parent"), data.NewPath("parent.foo"), - data.NewPath("parent.baz")}, + data.NewPath("parent.baz"), + }, }, { name: "ArrayWithNestedMap", input: map[string]interface{}{ "items": []interface{}{ map[string]interface{}{"name": "item1"}, - map[string]interface{}{"name": "item2"}}, + map[string]interface{}{"name": "item2"}, + }, }, expected: []data.Path{ data.NewPath("items"), data.NewPath("items.0"), data.NewPath("items.0.name"), data.NewPath("items.1"), - data.NewPath("items.1.name")}, + data.NewPath("items.1.name"), + }, }, { name: "SliceOfSlices", @@ -945,7 +944,6 @@ func TestPaths_SelectAll(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - result := data.Paths(tt.input, data.SelectAllPaths) data.SortPaths(result) @@ -970,7 +968,8 @@ func TestPaths_SelectLeaves(t *testing.T) { }, expected: []data.Path{ data.NewPath("foo"), - data.NewPath("baz")}, + data.NewPath("baz"), + }, }, { name: "NestedMap", @@ -982,18 +981,21 @@ func TestPaths_SelectLeaves(t *testing.T) { }, expected: []data.Path{ data.NewPath("parent.foo"), - data.NewPath("parent.baz")}, + data.NewPath("parent.baz"), + }, }, { name: "ArrayWithNestedMap", input: map[string]interface{}{ "items": []interface{}{ map[string]interface{}{"name": "item1"}, - map[string]interface{}{"name": "item2"}}, + map[string]interface{}{"name": "item2"}, + }, }, expected: []data.Path{ data.NewPath("items.0.name"), - data.NewPath("items.1.name")}, + data.NewPath("items.1.name"), + }, }, { name: "SliceOfSlices", @@ -1051,7 +1053,6 @@ func TestPaths_SelectLeaves(t *testing.T) { // Run test cases for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - result := data.Paths(tt.input, data.SelectLeafPaths) data.SortPaths(result) diff --git a/fs.go b/fs.go index 5b40258..69371f4 100644 --- a/fs.go +++ b/fs.go @@ -36,7 +36,7 @@ func DiscoverFiles(rootPath string, pathSelector *regexp.Regexp) ([]string, erro } var files []string - filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error { + err = filepath.Walk(rootPath, func(path string, info fs.FileInfo, err error) error { if info.IsDir() { return nil } @@ -47,6 +47,9 @@ func DiscoverFiles(rootPath string, pathSelector *regexp.Regexp) ([]string, erro return nil }) + if err != nil { + return nil, err + } return files, nil } diff --git a/inventory.go b/inventory.go index 0ba0659..eb58572 100644 --- a/inventory.go +++ b/inventory.go @@ -165,10 +165,10 @@ func (inv *Inventory) WalkValues(walkFunc func(data.Path, data.Value) error) err // The context can be any path within the class to which the path is relative to. // In case the paths are empty or are not valid within the given context, an error is returned. func (inv *Inventory) AbsolutePath(path data.Path, context data.Path) (data.Path, error) { - if path == nil || len(path) == 0 { + if len(path) == 0 { return nil, data.ErrEmptyPath } - if context == nil || len(context) == 0 { + if len(context) == 0 { return nil, fmt.Errorf("context path cannot be empty: %w", data.ErrEmptyPath) } diff --git a/reference/value.go b/reference/value.go index dffd745..4ac5b30 100644 --- a/reference/value.go +++ b/reference/value.go @@ -153,7 +153,7 @@ type ValueTarget interface { // ReplaceValues will replace all given references within the given ValueReferenceTarget. func ReplaceValues(target ValueTarget, references []ValueReference) error { - if references == nil || len(references) == 0 { + if len(references) == 0 { return nil } if target == nil { @@ -366,10 +366,10 @@ func ValueReplacementOrder(dependencyGraph graph.Graph[string, ValueReference]) // If the order is empty/nil, then allReferences is just re-emitted. // If allReferences is empty/nil, then nil is returned. func ReorderValueReferences(order []ValueReference, allReferences []ValueReference) []ValueReference { - if order == nil || len(order) == 0 { + if len(order) == 0 { return allReferences } - if allReferences == nil || len(allReferences) == 0 { + if len(allReferences) == 0 { return nil } diff --git a/reference_test.go b/reference_test.go index 69f4fd6..9b116fc 100644 --- a/reference_test.go +++ b/reference_test.go @@ -126,7 +126,7 @@ func TestValueManager_SetHooks_Registry(t *testing.T) { }) t.Run("registering a new class with invalid references must fail", func(t *testing.T) { registry := NewRegistry() - NewValueReferenceManager(registry) + _, _ = NewValueReferenceManager(registry) // the common class is the only class, hence some references are invalid common, err := NewClass("testdata/references/registry/common.yaml", codec.NewYamlCodec(), data.NewPath("common")) diff --git a/registry.go b/registry.go index 8eb9c6c..c2d7641 100644 --- a/registry.go +++ b/registry.go @@ -80,7 +80,7 @@ func (reg *Registry) RegisterClass(class *Class) error { // Create a slice of all absolute paths defined by the class var classPaths []string - class.Walk(func(path data.Path, _ data.Value, _ bool) error { + _ = class.Walk(func(path data.Path, _ data.Value, _ bool) error { classPaths = append(classPaths, pathPrefix.AppendPath(path).String()) return nil }) @@ -137,7 +137,7 @@ func (reg *Registry) classPreSetHook(class *Class) SetHookFunc { // if the path does already exist and is owned by a *different* class, then // we need to prevent the Set call as it would introduce a duplicate path. if classIdentifier, exists := reg.paths[registryPath.String()]; exists { - existingPathClass, _ := reg.classes[classIdentifier] + existingPathClass := reg.classes[classIdentifier] if !existingPathClass.Identifier.Equals(class.Identifier) { return fmt.Errorf("path already owned by '%s': %w: %s", classIdentifier, ErrDuplicatePath, registryPath) } @@ -206,10 +206,7 @@ func (reg *Registry) GetPath(path data.Path) (data.Value, error) { // HasPath returns true if the path exists within the registry. func (reg *Registry) HasPath(path data.Path) bool { _, err := reg.GetPath(path) - if err != nil { - return false - } - return true + return err == nil } // GetClassRelativePath attempts to resolve the target-class using the given classPath. Then it attempts to resolve the path (which might be class-local only). @@ -265,7 +262,7 @@ func (reg *Registry) WalkValues(walkFunc func(data.Path, data.Value) error) erro func (reg *Registry) Values() map[string]data.Value { valueMap := make(map[string]data.Value) - reg.WalkValues(func(p data.Path, v data.Value) error { + _ = reg.WalkValues(func(p data.Path, v data.Value) error { valueMap[p.String()] = v return nil }) @@ -304,10 +301,10 @@ func (reg *Registry) ClassIdentifiers() []data.Path { // The context can be any path within the class to which the path is relative to, even just the classIdentifier. // In case the paths are empty or are not valid within the given context, an error is returned. func (reg *Registry) AbsolutePath(path data.Path, context data.Path) (data.Path, error) { - if path == nil || len(path) == 0 { + if len(path) == 0 { return nil, data.ErrEmptyPath } - if context == nil || len(context) == 0 { + if len(context) == 0 { return nil, fmt.Errorf("context path cannot be empty: %w", data.ErrEmptyPath) } diff --git a/registry_test.go b/registry_test.go index 59d3ab7..f4cb6f5 100644 --- a/registry_test.go +++ b/registry_test.go @@ -20,8 +20,6 @@ var ( janeClassPath = "testdata/classes/people/jane.yaml" peopleClassPath = "testdata/classes/people.yaml" - stripPrefix = data.NewPathFromOsPath("testdata/classes") - personClass, pizzaClass, foodCommonClass,