Unverified Commit 47a74256 authored by Pedro Pombeiro's avatar Pedro Pombeiro
Browse files

Enable optional checks in gocritic linter

parent b21be6bd
......@@ -19,8 +19,44 @@ linters-settings:
min-len: 3
min-occurrences: 3
gocritic:
disabled-checks:
- appendAssign
enabled-checks:
- appendCombine
- badCall
- badCond
- boolExprSimplify
- codegenComment
- commentFormatting
- commentedOutCode
- deprecatedComment
- dupImport
- emptyFallthrough
- emptyStringTest
- equalFold
- evalOrder
- exitAfterDefer
- flagName
- hexLiteral
- indexAlloc
- initClause
- methodExprCall
- nestingReduce
- newDeref
- nilValReturn
- offBy1
- ptrToRefParam
- rangeExprCopy
- regexpPattern
- sloppyReassign
- stringXbytes
- truncateCmp
- typeAssertChain
- typeUnparen
- underef
- unnecessaryBlock
- valSwap
- weakCond
- wrapperFunc
- yodaStyleExpr
gocyclo:
min-complexity: 10
govet:
......
......@@ -259,7 +259,7 @@ func TestGenerateObjectName(t *testing.T) {
objectName, err := generateObjectName(test.build, test.cache, test.key)
assert.Equal(t, test.expectedObjectName, objectName)
if len(test.expectedError) == 0 {
if test.expectedError == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.expectedError)
......
......@@ -77,7 +77,7 @@ func (c *configOptionsWithListenAddress) listenAddress() (string, error) {
return "", err
}
if len(port) == 0 {
if port == "" {
return fmt.Sprintf("%s:%d", address, common.DefaultMetricsServerPort), nil
}
return address, nil
......
......@@ -52,7 +52,7 @@ func (c *ArtifactsDownloaderCommand) download(file string, retry int) error {
func (c *ArtifactsDownloaderCommand) Execute(context *cli.Context) {
log.SetRunnerFormatter()
if len(c.URL) == 0 || len(c.Token) == 0 {
if c.URL == "" || c.Token == "" {
logrus.Fatalln("Missing runner credentials")
}
if c.ID <= 0 {
......
......@@ -149,7 +149,7 @@ func (c *ArtifactsUploaderCommand) ShouldRetry(tries int, err error) bool {
func (c *ArtifactsUploaderCommand) Execute(*cli.Context) {
log.SetRunnerFormatter()
if len(c.URL) == 0 || len(c.Token) == 0 {
if c.URL == "" || c.Token == "" {
logrus.Fatalln("Missing runner credentials")
}
if c.ID <= 0 {
......
......@@ -106,7 +106,7 @@ func (c *CacheExtractorCommand) getCache() (*http.Response, error) {
func (c *CacheExtractorCommand) Execute(context *cli.Context) {
log.SetRunnerFormatter()
if len(c.File) == 0 {
if c.File == "" {
logrus.Fatalln("Missing cache file")
}
......
......@@ -162,32 +162,37 @@ func getFlags() []cli.Flag {
func getInstallFlags() []cli.Flag {
installFlags := getFlags()
installFlags = append(installFlags, cli.StringFlag{
Name: "working-directory, d",
Value: helpers.GetCurrentWorkingDirectory(),
Usage: "Specify custom root directory where all data are stored",
})
installFlags = append(installFlags, cli.StringFlag{
Name: "config, c",
Value: getDefaultConfigFile(),
Usage: "Specify custom config file",
})
installFlags = append(installFlags, cli.BoolFlag{
Name: "syslog",
Usage: "Setup system logging integration",
})
installFlags = append(
installFlags,
cli.StringFlag{
Name: "working-directory, d",
Value: helpers.GetCurrentWorkingDirectory(),
Usage: "Specify custom root directory where all data are stored",
},
cli.StringFlag{
Name: "config, c",
Value: getDefaultConfigFile(),
Usage: "Specify custom config file",
},
cli.BoolFlag{
Name: "syslog",
Usage: "Setup system logging integration",
},
)
if runtime.GOOS == osTypeWindows {
installFlags = append(installFlags, cli.StringFlag{
Name: "user, u",
Value: "",
Usage: "Specify user-name to secure the runner",
})
installFlags = append(installFlags, cli.StringFlag{
Name: "password, p",
Value: "",
Usage: "Specify user password to install service (required)",
})
installFlags = append(
installFlags,
cli.StringFlag{
Name: "user, u",
Value: "",
Usage: "Specify user-name to secure the runner",
},
cli.StringFlag{
Name: "password, p",
Value: "",
Usage: "Specify user password to install service (required)",
})
} else if os.Getuid() == 0 {
installFlags = append(installFlags, cli.StringFlag{
Name: "user, u",
......
......@@ -127,13 +127,13 @@ func (r *RunSingleCommand) checkFinishedConditions() {
}
func (r *RunSingleCommand) Execute(c *cli.Context) {
if len(r.URL) == 0 {
if r.URL == "" {
logrus.Fatalln("Missing URL")
}
if len(r.Token) == 0 {
if r.Token == "" {
logrus.Fatalln("Missing Token")
}
if len(r.Executor) == 0 {
if r.Executor == "" {
logrus.Fatalln("Missing Executor")
}
......
......@@ -77,7 +77,7 @@ func mockingExecutionStack(
p := common.MockExecutorProvider{}
mockNetwork := common.MockNetwork{}
//Network
// Network
jobData := common.JobResponse{}
_, cancel := context.WithCancel(context.Background())
jobTrace := common.Trace{Writer: ioutil.Discard}
......@@ -88,7 +88,7 @@ func mockingExecutionStack(
processJob.Run(job)
}
//ExecutorProvider
// ExecutorProvider
p.On("CanCreate").Return(true).Once()
p.On("GetDefaultShell").Return("bash").Once()
p.On("GetFeatures", mock.Anything).Return(nil).Times(maxBuilds + 1)
......@@ -97,7 +97,7 @@ func mockingExecutionStack(
p.On("Acquire", mock.Anything).Return(&common.MockExecutorData{}, nil).Times(maxBuilds)
p.On("Release", mock.Anything, mock.Anything).Return(nil).Times(maxBuilds)
//Executor
// Executor
e.On("Prepare", mock.Anything, mock.Anything, mock.Anything).Return(nil).Times(maxBuilds)
e.On("Finish", nil).Times(maxBuilds)
e.On("Cleanup").Times(maxBuilds)
......
......@@ -22,7 +22,7 @@ func (c *UnregisterCommand) unregisterAllRunners() (runners []*common.RunnerConf
for _, r := range c.config.Runners {
if !c.network.UnregisterRunner(r.RunnerCredentials) {
logrus.Errorln("Failed to unregister runner", r.Name)
//If unregister fails, leave the runner in the config
// If unregister fails, leave the runner in the config
runners = append(runners, r)
}
}
......
......@@ -860,20 +860,22 @@ func (b *Build) GetTLSVariables(caFile, certFile, keyFile string) JobVariables {
}
if b.TLSAuthCert != "" && b.TLSAuthKey != "" {
variables = append(variables, JobVariable{
Key: certFile,
Value: b.TLSAuthCert,
Public: true,
Internal: true,
File: true,
})
variables = append(variables, JobVariable{
Key: keyFile,
Value: b.TLSAuthKey,
Internal: true,
File: true,
})
variables = append(
variables,
JobVariable{
Key: certFile,
Value: b.TLSAuthCert,
Public: true,
Internal: true,
File: true,
},
JobVariable{
Key: keyFile,
Value: b.TLSAuthKey,
Internal: true,
File: true,
},
)
}
return variables
......@@ -961,7 +963,7 @@ func (b *Build) GetGitCheckout() bool {
}
strCheckout := b.GetAllVariables().Get("GIT_CHECKOUT")
if len(strCheckout) == 0 {
if strCheckout == "" {
return true
}
......
......@@ -589,9 +589,7 @@ func TestSharedEnvVariables(t *testing.T) {
present := "CI_SHARED_ENVIRONMENT=true"
absent := "CI_DISPOSABLE_ENVIRONMENT=true"
if !shared {
tmp := present
present = absent
absent = tmp
present, absent = absent, present
}
assert.Contains(vars, present)
......
......@@ -299,8 +299,11 @@ func TestBuildWithGitStrategyCloneNoCheckoutWithoutLFS(t *testing.T) {
defer cleanup()
build.Runner.PreCloneScript = "echo pre-clone-script"
build.Variables = append(build.Variables, common.JobVariable{Key: "GIT_STRATEGY", Value: "clone"})
build.Variables = append(build.Variables, common.JobVariable{Key: "GIT_CHECKOUT", Value: "false"})
build.Variables = append(
build.Variables,
common.JobVariable{Key: "GIT_STRATEGY", Value: "clone"},
common.JobVariable{Key: "GIT_CHECKOUT", Value: "false"},
)
out, err := buildtest.RunBuildReturningOutput(t, build)
assert.NoError(t, err)
......@@ -322,8 +325,11 @@ func TestBuildWithGitSubmoduleStrategyRecursiveAndGitStrategyNone(t *testing.T)
build, cleanup := newBuild(t, successfulBuild, shell)
defer cleanup()
build.Variables = append(build.Variables, common.JobVariable{Key: "GIT_STRATEGY", Value: "none"})
build.Variables = append(build.Variables, common.JobVariable{Key: "GIT_SUBMODULE_STRATEGY", Value: "recursive"})
build.Variables = append(
build.Variables,
common.JobVariable{Key: "GIT_STRATEGY", Value: "none"},
common.JobVariable{Key: "GIT_SUBMODULE_STRATEGY", Value: "recursive"},
)
out, err := buildtest.RunBuildReturningOutput(t, build)
assert.NoError(t, err)
......@@ -468,8 +474,11 @@ func TestBuildPowerShellCatchesExceptions(t *testing.T) {
build, cleanup := newBuild(t, successfulBuild, "powershell")
defer cleanup()
build.Variables = append(build.Variables, common.JobVariable{Key: "ErrorActionPreference", Value: "Stop"})
build.Variables = append(build.Variables, common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"})
build.Variables = append(
build.Variables,
common.JobVariable{Key: "ErrorActionPreference", Value: "Stop"},
common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"},
)
out, err := buildtest.RunBuildReturningOutput(t, build)
assert.NoError(t, err)
......
......@@ -397,7 +397,7 @@ func (e *executor) createService(
serviceDefinition common.Image,
linkNames []string,
) (*types.Container, error) {
if len(service) == 0 {
if service == "" {
return nil, fmt.Errorf("invalid service name: %s", serviceDefinition.Name)
}
......@@ -411,7 +411,7 @@ func (e *executor) createService(
return nil, err
}
serviceSlug := strings.Replace(service, "/", "__", -1)
serviceSlug := strings.ReplaceAll(service, "/", "__")
containerName := fmt.Sprintf("%s-%s-%d", e.getProjectUniqRandomizedName(), serviceSlug, serviceIndex)
// this will fail potentially some builds if there's name collision
......
......@@ -149,7 +149,7 @@ func testServiceFromNamedImage(t *testing.T, description, imageName, serviceName
c := new(docker.MockClient)
defer c.AssertExpectations(t)
servicePart := fmt.Sprintf("-%s-0", strings.Replace(serviceName, "/", "__", -1))
servicePart := fmt.Sprintf("-%s-0", strings.ReplaceAll(serviceName, "/", "__"))
containerNameRegex, err := regexp.Compile("runner-abcdef12-project-0-concurrent-0-[^-]+" + servicePart)
require.NoError(t, err)
......
......@@ -4,10 +4,9 @@ import (
"errors"
"gitlab.com/gitlab-org/gitlab-runner/session/terminal"
terminalsession "gitlab.com/gitlab-org/gitlab-runner/session/terminal"
)
func (e *machineExecutor) Connect() (terminalsession.Conn, error) {
func (e *machineExecutor) Connect() (terminal.Conn, error) {
if term, ok := e.executor.(terminal.InteractiveTerminal); ok {
return term.Connect()
}
......
......@@ -167,7 +167,7 @@ func (p *ExecOptions) Run() error {
p.PodName, p.Namespace, pod.Status.Phase)
}
if len(p.ContainerName) == 0 {
if p.ContainerName == "" {
logrus.Infof("defaulting container name to '%s'", pod.Spec.Containers[0].Name)
p.ContainerName = pod.Spec.Containers[0].Name
}
......
......@@ -578,15 +578,16 @@ func (s *executor) getVolumeMounts() []api.VolumeMount {
// The configMap is nil when using legacy execution
if s.configMap != nil {
mounts = append(mounts, api.VolumeMount{
Name: "scripts",
MountPath: s.scriptsDir(),
})
mounts = append(mounts, api.VolumeMount{
Name: "logs",
MountPath: s.logsDir(),
})
mounts = append(
mounts,
api.VolumeMount{
Name: "scripts",
MountPath: s.scriptsDir(),
},
api.VolumeMount{
Name: "logs",
MountPath: s.logsDir(),
})
}
mounts = append(mounts, s.getVolumeMountsForConfig()...)
......@@ -655,25 +656,26 @@ func (s *executor) getVolumes() []api.Volume {
mode := int32(0777)
optional := false
volumes = append(volumes, api.Volume{
Name: "scripts",
VolumeSource: api.VolumeSource{
ConfigMap: &api.ConfigMapVolumeSource{
LocalObjectReference: api.LocalObjectReference{
Name: s.configMap.Name,
volumes = append(
volumes,
api.Volume{
Name: "scripts",
VolumeSource: api.VolumeSource{
ConfigMap: &api.ConfigMapVolumeSource{
LocalObjectReference: api.LocalObjectReference{
Name: s.configMap.Name,
},
DefaultMode: &mode,
Optional: &optional,
},
DefaultMode: &mode,
Optional: &optional,
},
},
})
volumes = append(volumes, api.Volume{
Name: "logs",
VolumeSource: api.VolumeSource{
EmptyDir: &api.EmptyDirVolumeSource{},
},
})
api.Volume{
Name: "logs",
VolumeSource: api.VolumeSource{
EmptyDir: &api.EmptyDirVolumeSource{},
},
})
return volumes
}
......
......@@ -214,7 +214,7 @@ func testKubernetesTimeoutRunFeatureFlag(t *testing.T, featureFlagName string, f
},
},
}
build.RunnerInfo.Timeout = 10 //seconds
build.RunnerInfo.Timeout = 10 // seconds
setBuildFeatureFlag(build, featureFlagName, featureFlagValue)
err = build.Run(&common.Config{}, &common.Trace{Writer: os.Stdout})
......
......@@ -234,7 +234,7 @@ func (l *kubernetesLogProcessor) scan(ctx context.Context, logs io.Reader) (*buf
// The line doesn't include the new line character.
// Lines without offset are acceptable and return -1 for offset.
func (l *kubernetesLogProcessor) parseLogLine(line string) (int64, string) {
if len(line) == 0 {
if line == "" {
return -1, ""
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment