Merge pull request #386 from ankitm123/lint-repo

refactor: add lint step and fix linting issues

.golangci.yml

@@ -0,0 +1,121 @@
+linters-settings:
+  depguard:
+    list-type: blacklist
+    packages:
+      - github.com/jenkins-x/jx/v2/pkg/log/
+      - github.com/satori/go.uuid
+      - github.com/pborman/uuid
+    packages-with-error-message:
+      - github.com/jenkins-x/jx/v2/pkg/log/: "use jenkins-x/jx-logging instead"
+      - github.com/satori/go.uuid: "use github.com/google/uuid instead"
+      - github.com/pborman/uuid: "use github.com/google/uuid instead"
+  dupl:
+    threshold: 100
+  exhaustive:
+    default-signifies-exhaustive: false
+  funlen:
+    lines: 200
+    statements: 150
+  goconst:
+    min-len: 3
+    min-occurrences: 3
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - ifElseChain
+      - octalLiteral
+      - whyNoLint
+      - wrapperFunc
+      - importShadow # not important for now
+      - unnamedResult # not important
+  gocyclo:
+    min-complexity: 15
+  goimports: {}
+  golint:
+    min-confidence: 0
+  gofmt:
+    simplify: true
+  gomnd:
+    settings:
+      mnd:
+        # don't include the "operation" and "assign"
+        checks: [argument, case, condition, return]
+  govet:
+    check-shadowing: true
+    settings:
+      printf:
+        funcs:
+          - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Debugf
+          - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Infof
+          - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Warnf
+          - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Errorf
+          - (github.com/jenkins-x/jx-logging/v3/pkg/log/Logger()).Fatalf
+  lll:
+    line-length: 140
+  maligned:
+    suggest-new: true
+  misspell: {}
+  nolintlint:
+    allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
+    allow-unused: false # report any unused nolint directives
+    require-explanation: false # don't require an explanation for nolint directives
+    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+linters:
+  # please, do not use `enable-all`: it's deprecated and will be removed soon.
+  # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
+  disable-all: true
+  enable:
+    - asciicheck
+    - bodyclose
+    - deadcode
+    - depguard
+    - errcheck
+    - gofmt
+    - goimports
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - ineffassign
+    - misspell
+    - nakedret
+    - rowserrcheck
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+    - revive
+    - gocritic
+    - govet
+issues:
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    #    - path: _test\.go
+    #      linters:
+    #        - gomnd
+    #    https://github.com/go-critic/go-critic/issues/926
+    - linters:
+        - gocritic
+      text: "unnecessaryDefer:"
+  exclude:
+    - 'shadow: declaration of "err" shadows declaration at'
+  max-same-issues: 0
+
+run:
+  timeout: 30m
+  skip-dirs:
+    - cmd/docs
+# golangci.com configuration
+# https://github.com/golangci/golangci/wiki/Configuration
+service:
+  golangci-lint-version: 1.42.x # use the fixed version to not introduce new linters unexpectedly
+  prepare:
+    - echo "here I can run custom commands, but no preparation needed for this repo"

.lighthouse/jenkins-x/lint.yaml

@@ -0,0 +1,30 @@
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  creationTimestamp: null
+  name: lint
+spec:
+  pipelineSpec:
+    tasks:
+    - name: jx-promote-lint
+      resources: {}
+      taskSpec:
+        metadata: {}
+        stepTemplate:
+          image: uses:jenkins-x/jx3-pipeline-catalog/tasks/go/pullrequest.yaml@versionStream
+          name: ""
+          resources:
+            requests:
+              cpu: 400m
+              memory: 600Mi
+          workingDir: /workspace/source
+        steps:
+        - image: uses:jenkins-x/jx3-pipeline-catalog/tasks/git-clone/git-clone-pr.yaml@versionStream
+          name: ""
+          resources: {}
+        - name: make-lint
+          resources: {}
+  podTemplate: {}
+  serviceAccountName: tekton-bot
+  timeout: 30m0s
+status: {}

.lighthouse/jenkins-x/triggers.yaml

@@ -6,6 +6,13 @@ spec:
     context: "pr"
     always_run: true
     optional: false
-    trigger: "/test"
-    rerun_command: "/retest"
+    trigger: (?m)^/test( all| pr),?(s+|$)
+    rerun_command: /test pr
     source: "pullrequest.yaml"
+  - name: lint
+    context: "lint"
+    always_run: true
+    optional: false
+    trigger: (?m)^/test( all| lint),?(s+|$)
+    rerun_command: /test lint
+    source: "lint.yaml"

cmd/app/main.go

@@ -1,3 +1,4 @@
+//nolint
 // +build !windows
 
 package app

hack/linter.sh

@@ -11,36 +11,10 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 
 if ! [ -x "$(command -v golangci-lint)" ]; then
 	echo "Installing GolangCI-Lint"
-	${DIR}/install_golint.sh -b $GOPATH/bin v1.20.0
+	${DIR}/install_golint.sh -b $GOPATH/bin v1.42.1
 fi
 
 export GO111MODULE=on
 golangci-lint run \
-	--no-config \
-  --disable-all \
-  -E golint \
-  -E varcheck \
-  -E errcheck \
-	-E misspell \
-	-E unconvert \
-  -E deadcode \
-  -E unconvert \
-  -E gosec \
-  -E gofmt \
-  -E goimports \
-  -E structcheck \
-  -E interfacer \
-  -E govet \
-  -E unparam \
-  -E megacheck \
-  -E goconst \
-  -E ineffassign \
-  -E unparam \
-  -E gocritic \
-  -E maligned \
-  -E typecheck \
-  --skip-dirs vendor \
-  --deadline 15m0s \
   --verbose \
   --build-tags build
-

pkg/envctx/env_context.go

@@ -56,7 +56,7 @@ type ChartDetails struct {
 // ChartDetails resolves the chart details from a full or local name and an optional repository URL.
 // this function can handle an empty repository but the chart name "foo/bar" and resolve the prefix "foo" to a repository
 // URL - or taking chart name "bar" and a repository URL and defaulting the prefix to "foo/bar"
-func (c *EnvironmentContext) ChartDetails(chartName string, repo string) (*ChartDetails, error) {
+func (c *EnvironmentContext) ChartDetails(chartName, repo string) (*ChartDetails, error) {
 	prefix := ""
 	localName := chartName
 	name := chartName

pkg/envctx/load.go

@@ -107,7 +107,6 @@ func (e *EnvironmentContext) LazyLoad(gclient gitclient.Interface, jxClient vers
 			if err != nil {
 				return errors.Wrapf(err, "failed to create version stream dir %s", versionsDir)
 			}
-			//return errors.Errorf("dev environment git repository %s does not have a versionStream dir", url)
 		}
 
 		e.VersionResolver = &versionstream.VersionResolver{

pkg/environments/fork.go

@@ -2,6 +2,7 @@ package environments
 
 import (
 	"context"
+
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/jx-helpers/v3/pkg/gitclient"
 	"github.com/jenkins-x/jx-helpers/v3/pkg/scmhelpers"
@@ -40,7 +41,7 @@ func (o *EnvironmentPullRequestOptions) EnsureForked(client *scm.Client, repoNam
 	return repo.Clone, nil
 }
 
-func (o *EnvironmentPullRequestOptions) rebaseForkFromUpstream(dir string, gitURL string) error {
+func (o *EnvironmentPullRequestOptions) rebaseForkFromUpstream(dir, gitURL string) error {
 	g := o.Git()
 	branch, err := gitclient.Branch(g, dir)
 	if err != nil {

pkg/environments/fork_test.go

@@ -2,12 +2,13 @@ package environments_test
 
 import (
 	"context"
+	"testing"
+
 	"github.com/jenkins-x-plugins/jx-promote/pkg/environments"
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/go-scm/scm/driver/fake"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
 func TestFork(t *testing.T) {

pkg/environments/gitops.go

@@ -48,7 +48,6 @@ func (o *EnvironmentPullRequestOptions) Create(gitURL, prDir string, pullRequest
 		if err != nil {
 			return nil, err
 		}
-		prDir = tempDir
 		defer os.RemoveAll(tempDir)
 	}
 

pkg/environments/pr.go

@@ -32,7 +32,7 @@ func (o *EnvironmentPullRequestOptions) Git() gitclient.Interface {
 }
 
 // CreatePullRequest crates a pull request if there are git changes
-func (o *EnvironmentPullRequestOptions) CreatePullRequest(scmClient *scm.Client, gitURL string, repoFullName, dir string, doneCommit bool, existingPR *scm.PullRequest) (*scm.PullRequest, error) {
+func (o *EnvironmentPullRequestOptions) CreatePullRequest(scmClient *scm.Client, gitURL, repoFullName, dir string, doneCommit bool, existingPR *scm.PullRequest) (*scm.PullRequest, error) {
 	gitter := o.Git()
 	changes, err := gitclient.HasChanges(gitter, dir)
 	if err != nil {
@@ -150,7 +150,7 @@ func (o *EnvironmentPullRequestOptions) CreatePullRequest(scmClient *scm.Client,
 	return o.addLabelsToPullRequest(ctx, scmClient, repoFullName, pr)
 }
 
-func (o *EnvironmentPullRequestOptions) GetScmClient(gitURL string, kind string) (*scm.Client, string, error) {
+func (o *EnvironmentPullRequestOptions) GetScmClient(gitURL, kind string) (*scm.Client, string, error) {
 	if gitURL == "" {
 		log.Logger().Infof("no git URL specified so cannot create a Pull Request")
 		return nil, "", nil

pkg/environments/types.go

@@ -12,7 +12,7 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 )
 
-//ValuesFiles is a wrapper for a slice of values files to allow them to be passed around as a pointer
+// ValuesFiles is a wrapper for a slice of values files to allow them to be passed around as a pointer
 type ValuesFiles struct {
 	Items []string
 }