Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gocritic to linters #285

Merged
merged 3 commits into from Jul 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions cli/options.go
Expand Up @@ -18,7 +18,7 @@ package cli

import (
"fmt"
"io/ioutil"
"io"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions cli/options_test.go
Expand Up @@ -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"),
})
}

Expand Down
2 changes: 1 addition & 1 deletion compatibility/services.go
Expand Up @@ -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")
}
Expand Down
24 changes: 11 additions & 13 deletions dotenv/godotenv.go
Expand Up @@ -17,7 +17,6 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
"regexp"
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -273,14 +271,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 (
Expand Down Expand Up @@ -353,7 +351,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
}
13 changes: 13 additions & 0 deletions golangci.yml
Expand Up @@ -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
10 changes: 5 additions & 5 deletions loader/full-struct_test.go
Expand Up @@ -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"},
Expand All @@ -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",
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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"),
Expand Down
31 changes: 19 additions & 12 deletions loader/loader.go
Expand Up @@ -17,10 +17,11 @@
package loader

import (
"bytes"
"fmt"
"io/ioutil"
"io"
"os"
"path"
paths "path"
"path/filepath"
"reflect"
"regexp"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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 != "":
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 6 additions & 7 deletions loader/loader_test.go
Expand Up @@ -19,7 +19,6 @@ package loader
import (
"bytes"
"fmt"
"io/ioutil"
"os"
"sort"
"strings"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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},
},
}

Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions loader/merge.go
Expand Up @@ -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"))
Expand Down
9 changes: 3 additions & 6 deletions loader/validate.go
Expand Up @@ -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))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion loader/windows_path.go
Expand Up @@ -44,7 +44,7 @@ func isAbs(path string) (b bool) {

// volumeNameLen returns length of the leading volume name on Windows.
// It returns 0 elsewhere.
// nolint: gocyclo
//nolint: gocyclo
func volumeNameLen(path string) int {
if len(path) < 2 {
return 0
Expand Down
4 changes: 2 additions & 2 deletions types/project.go
Expand Up @@ -23,7 +23,7 @@ import (
"sort"

"github.com/distribution/distribution/v3/reference"
"github.com/opencontainers/go-digest"
godigest "github.com/opencontainers/go-digest"
"golang.org/x/sync/errgroup"
)

Expand Down Expand Up @@ -311,7 +311,7 @@ func (p *Project) ForServices(names []string) error {
}

// ResolveImages updates services images to include digest computed by a resolver function
func (p *Project) ResolveImages(resolver func(named reference.Named) (digest.Digest, error)) error {
func (p *Project) ResolveImages(resolver func(named reference.Named) (godigest.Digest, error)) error {
eg := errgroup.Group{}
for i, s := range p.Services {
idx := i
Expand Down