Merge pull request #285 from compose-spec/add-gocritic-linter

Add gocritic to linters

Author: ulyssessouza

cli/options.go

@@ -18,7 +18,7 @@ package cli
 
 import (
 	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -323,7 +323,7 @@ func ProjectFromOptions(options *ProjectOptions) (*types.Project, error) {
 	for _, f := range configPaths {
 		var b []byte
 		if f == "-" {
-			b, err = ioutil.ReadAll(os.Stdin)
+			b, err = io.ReadAll(os.Stdin)
 			if err != nil {
 				return nil, err
 			}
@@ -332,7 +332,7 @@ func ProjectFromOptions(options *ProjectOptions) (*types.Project, error) {
 			if err != nil {
 				return nil, err
 			}
-			b, err = ioutil.ReadFile(f)
+			b, err = os.ReadFile(f)
 			if err != nil {
 				return nil, err
 			}

cli/options_test.go

@@ -182,8 +182,8 @@ func TestProjectComposefilesFromWorkingDir(t *testing.T) {
 	assert.NilError(t, err)
 	currentDir, _ := os.Getwd()
 	assert.DeepEqual(t, p.ComposeFiles, []string{
-		filepath.Join(currentDir, "testdata/simple/compose.yaml"),
-		filepath.Join(currentDir, "testdata/simple/compose-with-overrides.yaml"),
+		filepath.Join(currentDir, "testdata", "simple", "compose.yaml"),
+		filepath.Join(currentDir, "testdata", "simple", "compose-with-overrides.yaml"),
 	})
 }
 

compatibility/services.go

@@ -550,7 +550,7 @@ func (c *AllowList) CheckPortsTarget(p *types.ServicePortConfig) {
 }
 
 func (c *AllowList) CheckPortsPublished(p *types.ServicePortConfig) {
-	if !c.supported("services.ports.published") && len(p.Published) != 0 {
+	if !c.supported("services.ports.published") && p.Published == "" {
 		p.Published = ""
 		c.Unsupported("services.ports.published")
 	}

dotenv/godotenv.go

@@ -17,7 +17,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"regexp"
@@ -44,7 +43,7 @@ func Parse(r io.Reader) (map[string]string, error) {
 
 // ParseWithLookup reads an env file from io.Reader, returning a map of keys and values.
 func ParseWithLookup(r io.Reader, lookupFn LookupFn) (map[string]string, error) {
-	data, err := ioutil.ReadAll(r)
+	data, err := io.ReadAll(r)
 	if err != nil {
 		return nil, err
 	}
@@ -184,7 +183,7 @@ func Marshal(envMap map[string]string) (string, error) {
 		if d, err := strconv.Atoi(v); err == nil {
 			lines = append(lines, fmt.Sprintf(`%s=%d`, k, d))
 		} else {
-			lines = append(lines, fmt.Sprintf(`%s="%s"`, k, doubleQuoteEscape(v)))
+			lines = append(lines, fmt.Sprintf(`%s="%s"`, k, doubleQuoteEscape(v))) //nolint // Cannot use %q here
 		}
 	}
 	sort.Strings(lines)
@@ -235,10 +234,9 @@ var exportRegex = regexp.MustCompile(`^\s*(?:export\s+)?(.*?)\s*$`)
 func parseLine(line string, envMap map[string]string) (key string, value string, err error) {
 	return parseLineWithLookup(line, envMap, nil)
 }
-func parseLineWithLookup(line string, envMap map[string]string, lookupFn LookupFn) (key string, value string, err error) {
-	if len(line) == 0 {
-		err = errors.New("zero length string")
-		return
+func parseLineWithLookup(line string, envMap map[string]string, lookupFn LookupFn) (string, string, error) {
+	if line == "" {
+		return "", "", errors.New("zero length string")
 	}
 
 	// ditch the comments (but keep quoted hashes)
@@ -271,14 +269,14 @@ func parseLineWithLookup(line string, envMap map[string]string, lookupFn LookupF
 	}
 
 	if len(splitString) != 2 {
-		err = errors.New("can't separate key from value")
-		return
+		return "", "", errors.New("can't separate key from value")
 	}
-	key = exportRegex.ReplaceAllString(splitString[0], "$1")
+	key := exportRegex.ReplaceAllString(splitString[0], "$1")
 
 	// Parse the value
-	value = parseValue(splitString[1], envMap, lookupFn)
-	return
+	value := parseValue(splitString[1], envMap, lookupFn)
+
+	return key, value, nil
 }
 
 var (
@@ -351,7 +349,7 @@ func doubleQuoteEscape(line string) string {
 		if c == '\r' {
 			toReplace = `\r`
 		}
-		line = strings.Replace(line, string(c), toReplace, -1)
+		line = strings.ReplaceAll(line, string(c), toReplace)
 	}
 	return line
 }

golangci.yml

@@ -4,10 +4,23 @@ run:
 linters:
   disable-all: true
   enable:
+  - gocritic
   - gofmt
   - goimports
   - revive
   - gosimple
   - ineffassign
   - misspell
   - govet
+linters-settings:
+  gocritic:
+    # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
+    # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
+    enabled-tags:
+      - diagnostic
+      - opinionated
+      - style
+    disabled-checks:
+      - paramTypeCombine
+      - unnamedResult
+      - whyNoLint

loader/full-struct_test.go

@@ -67,7 +67,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 						Target: "my_secret",
 						UID:    "103",
 						GID:    "103",
-						Mode:   uint32Ptr(0440),
+						Mode:   uint32Ptr(0o440),
 					},
 				},
 				Tags: []string{"foo:v1.0.0", "docker.io/username/foo:my-other-tag", "full_example_project_name:1.0.0"},
@@ -85,7 +85,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 					Target: "/my_config",
 					UID:    "103",
 					GID:    "103",
-					Mode:   uint32Ptr(0440),
+					Mode:   uint32Ptr(0o440),
 				},
 			},
 			ContainerName: "my-web-container",
@@ -389,7 +389,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 					Target: "my_secret",
 					UID:    "103",
 					GID:    "103",
-					Mode:   uint32Ptr(0440),
+					Mode:   uint32Ptr(0o440),
 				},
 			},
 			SecurityOpt: []string{
@@ -420,7 +420,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 				{Source: "/opt/data", Target: "/var/lib/mysql", Type: "bind", Bind: &types.ServiceVolumeBind{CreateHostPath: true}},
 				{Source: workingDir, Target: "/code", Type: "bind", Bind: &types.ServiceVolumeBind{CreateHostPath: true}},
 				{Source: filepath.Join(workingDir, "static"), Target: "/var/www/html", Type: "bind", Bind: &types.ServiceVolumeBind{CreateHostPath: true}},
-				{Source: filepath.Join(homeDir, "/configs"), Target: "/etc/configs", Type: "bind", ReadOnly: true, Bind: &types.ServiceVolumeBind{CreateHostPath: true}},
+				{Source: filepath.Join(homeDir, "configs"), Target: "/etc/configs", Type: "bind", ReadOnly: true, Bind: &types.ServiceVolumeBind{CreateHostPath: true}},
 				{Source: "datavolume", Target: "/var/lib/mysql", Type: "volume", Volume: &types.ServiceVolumeVolume{}},
 				{Source: filepath.Join(workingDir, "opt"), Target: "/opt", Consistency: "cached", Type: "bind"},
 				{Target: "/opt", Type: "tmpfs", Tmpfs: &types.ServiceVolumeTmpfs{
@@ -1003,7 +1003,7 @@ x-nested:
   bar: baz
   foo: bar
 `,
-		filepath.Join(workingDir),
+		workingDir,
 		filepath.Join(workingDir, "static"),
 		filepath.Join(homeDir, "configs"),
 		filepath.Join(workingDir, "opt"),

loader/loader.go

@@ -17,10 +17,11 @@
 package loader
 
 import (
+	"bytes"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
-	"path"
+	paths "path"
 	"path/filepath"
 	"reflect"
 	"regexp"
@@ -524,12 +525,12 @@ func loadServiceWithExtends(filename, name string, servicesDict map[string]inter
 			// Resolve the path to the imported file, and load it.
 			baseFilePath := absPath(workingDir, *file)
 
-			bytes, err := ioutil.ReadFile(baseFilePath)
+			b, err := os.ReadFile(baseFilePath)
 			if err != nil {
 				return nil, err
 			}
 
-			baseFile, err := parseConfig(bytes, opts)
+			baseFile, err := parseConfig(b, opts)
 			if err != nil {
 				return nil, err
 			}
@@ -629,8 +630,16 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, l
 			if err != nil {
 				return err
 			}
-			defer file.Close()
-			fileVars, err := dotenv.ParseWithLookup(file, dotenv.LookupFn(lookupEnv))
+
+			b, err := io.ReadAll(file)
+			if err != nil {
+				return err
+			}
+
+			// Do not defer to avoid it inside a loop
+			file.Close() //nolint:errcheck
+
+			fileVars, err := dotenv.ParseWithLookup(bytes.NewBuffer(b), dotenv.LookupFn(lookupEnv))
 			if err != nil {
 				return err
 			}
@@ -656,7 +665,7 @@ func resolveVolumePath(volume types.ServiceVolumeConfig, workingDir string, look
 	// Note that this is not required for Docker for Windows when specifying
 	// a local Windows path, because Docker for Windows translates the Windows
 	// path into a valid path within the VM.
-	if !path.IsAbs(filePath) && !isAbs(filePath) {
+	if !paths.IsAbs(filePath) && !isAbs(filePath) {
 		filePath = absPath(workingDir, filePath)
 	}
 	volume.Source = filePath
@@ -810,10 +819,8 @@ func loadFileObjectConfig(name string, objType string, obj types.FileObjectConfi
 			logrus.Warnf("%[1]s %[2]s: %[1]s.external.name is deprecated in favor of %[1]s.name", objType, name)
 			obj.Name = obj.External.Name
 			obj.External.Name = ""
-		} else {
-			if obj.Name == "" {
-				obj.Name = name
-			}
+		} else if obj.Name == "" {
+			obj.Name = name
 		}
 		// if not "external: true"
 	case obj.Driver != "":
@@ -945,7 +952,7 @@ func cleanTarget(target string) string {
 	if target == "" {
 		return ""
 	}
-	return path.Clean(target)
+	return paths.Clean(target)
 }
 
 var transformBuildConfig TransformerFunc = func(data interface{}) (interface{}, error) {

loader/loader_test.go

@@ -19,7 +19,6 @@ package loader
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"sort"
 	"strings"
@@ -926,7 +925,7 @@ func uint32Ptr(value uint32) *uint32 {
 }
 
 func TestFullExample(t *testing.T) {
-	b, err := ioutil.ReadFile("full-example.yml")
+	b, err := os.ReadFile("full-example.yml")
 	assert.NilError(t, err)
 
 	homeDir, err := os.UserHomeDir()
@@ -1728,13 +1727,13 @@ secrets:
 }
 
 func TestComposeFileWithVersion(t *testing.T) {
-	bytes, err := ioutil.ReadFile("testdata/compose-test-with-version.yaml")
+	b, err := os.ReadFile("testdata/compose-test-with-version.yaml")
 	assert.NilError(t, err)
 
 	homeDir, err := os.UserHomeDir()
 	assert.NilError(t, err)
 	env := map[string]string{"HOME": homeDir, "QUX": "qux_from_environment"}
-	config, err := loadYAMLWithEnv(string(bytes), env)
+	config, err := loadYAMLWithEnv(string(b), env)
 	assert.NilError(t, err)
 
 	workingDir, err := os.Getwd()
@@ -1751,13 +1750,13 @@ func TestComposeFileWithVersion(t *testing.T) {
 }
 
 func TestLoadWithExtends(t *testing.T) {
-	bytes, err := ioutil.ReadFile("testdata/compose-test-extends.yaml")
+	b, err := os.ReadFile("testdata/compose-test-extends.yaml")
 	assert.NilError(t, err)
 
 	configDetails := types.ConfigDetails{
 		WorkingDir: "testdata",
 		ConfigFiles: []types.ConfigFile{
-			{Filename: "testdata/compose-test-extends.yaml", Content: bytes},
+			{Filename: "testdata/compose-test-extends.yaml", Content: b},
 		},
 	}
 
@@ -1896,7 +1895,7 @@ services:
 	assert.NilError(t, err)
 	svc, err := actual.GetService("test")
 	assert.NilError(t, err)
-	assert.Check(t, nil == svc.Build.SSH)
+	assert.Check(t, svc.Build.SSH == nil)
 }
 
 func TestLoadSSHWithoutValueInBuildConfig(t *testing.T) {

loader/merge.go

@@ -299,15 +299,15 @@ func mergeLoggingConfig(dst, src reflect.Value) error {
 	return nil
 }
 
-// nolint: unparam
+//nolint: unparam
 func mergeUlimitsConfig(dst, src reflect.Value) error {
 	if src.Interface() != reflect.Zero(reflect.TypeOf(src.Interface())).Interface() {
 		dst.Elem().Set(src.Elem())
 	}
 	return nil
 }
 
-// nolint: unparam
+//nolint: unparam
 func mergeServiceNetworkConfig(dst, src reflect.Value) error {
 	if src.Interface() != reflect.Zero(reflect.TypeOf(src.Interface())).Interface() {
 		dst.Elem().FieldByName("Aliases").Set(src.Elem().FieldByName("Aliases"))

loader/validate.go

@@ -52,12 +52,9 @@ func checkConsistency(project *types.Project) error {
 		}
 
 		for _, volume := range s.Volumes {
-			switch volume.Type {
-			case types.VolumeTypeVolume:
-				if volume.Source != "" { // non anonymous volumes
-					if _, ok := project.Volumes[volume.Source]; !ok {
-						return errors.Wrap(errdefs.ErrInvalid, fmt.Sprintf("service %q refers to undefined volume %s", s.Name, volume.Source))
-					}
+			if volume.Type == types.VolumeTypeVolume && volume.Source != "" { // non anonymous volumes
+				if _, ok := project.Volumes[volume.Source]; !ok {
+					return errors.Wrap(errdefs.ErrInvalid, fmt.Sprintf("service %q refers to undefined volume %s", s.Name, volume.Source))
 				}
 			}
 		}