Merge pull request #734 from cli/golangci-lint

Check code with golangci-lint on push

.github/workflows/go.yml

@@ -14,33 +14,14 @@ jobs:
         with:
           go-version: 1.13
 
-      - name: Check out code into the Go module directory
+      - name: Check out code
         uses: actions/checkout@v2
 
-      - name: Verify dependencies
-        shell: bash
-        run: |
-          go mod verify
-          go mod download
+      - name: Download dependencies
+        run: go mod download
 
       - name: Run tests
-        shell: bash
-        run: |
-          assert-nothing-changed() {
-            local diff
-            git checkout -- .
-            "$@" >/dev/null || true
-            if ! diff="$(git diff -U1 --color --exit-code)"; then
-              printf '\n\e[31mError: running `\e[1m%s\e[22m` results in modifications that you must check into version control:\e[0m\n%s\n' "$*" "$diff" >&2
-              return 1
-            fi
-          }
-
-          STATUS=0
-          go test -race ./... || STATUS=$?
-          assert-nothing-changed go fmt ./... || STATUS=$?
-          assert-nothing-changed go mod tidy || STATUS=$?
-          exit $STATUS
+        run: go test -race ./...
 
       - name: Build
         run: go build -v ./cmd/gh

.github/workflows/lint.yml

@@ -0,0 +1,54 @@
+name: Lint
+on:
+  push:
+    paths:
+    - '**.go'
+    - go.mod
+    - go.sum
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Set up Go 1.13
+        uses: actions/setup-go@v1
+        with:
+          go-version: 1.13
+
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: Verify dependencies
+        run: |
+          go mod verify
+          go mod download
+
+          LINT_VERSION=1.24.0
+          curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \
+            tar xz --strip-components 1 --wildcards \*/golangci-lint
+          mkdir -p bin && mv golangci-lint bin/
+
+      - name: Run checks
+        run: |
+          STATUS=0
+          assert-nothing-changed() {
+            local diff
+            "$@" >/dev/null || return 1
+            if ! diff="$(git diff -U1 --color --exit-code)"; then
+              printf '\e[31mError: running `\e[1m%s\e[22m` results in modifications that you must check into version control:\e[0m\n%s\n\n' "$*" "$diff" >&2
+              git checkout -- .
+              STATUS=1
+            fi
+          }
+
+          assert-nothing-changed go fmt ./...
+          assert-nothing-changed go mod tidy
+
+          while read -r file linter msg; do
+            IFS=: read -ra f <<<"$file"
+            printf '::error file=%s,line=%s,col=%s::%s\n' "${f[0]}" "${f[1]}" "${f[2]}" "[$linter] $msg"
+            STATUS=1
+          done < <(bin/golangci-lint run --out-format tab)
+
+          exit $STATUS

api/queries_issue_test.go

@@ -52,7 +52,7 @@ func TestIssueList(t *testing.T) {
 	}
 
 	bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 	if reqLimit := reqBody.Variables["limit"].(float64); reqLimit != 100 {
 		t.Errorf("expected 100, got %v", reqLimit)
 	}
@@ -61,7 +61,7 @@ func TestIssueList(t *testing.T) {
 	}
 
 	bodyBytes, _ = ioutil.ReadAll(http.Requests[1].Body)
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 	if endCursor := reqBody.Variables["endCursor"].(string); endCursor != "ENDCURSOR" {
 		t.Errorf("expected %q, got %q", "ENDCURSOR", endCursor)
 	}

api/queries_repo_test.go

@@ -35,7 +35,7 @@ func Test_RepoCreate(t *testing.T) {
 	}
 
 	bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 	if description := reqBody.Variables.Input["description"].(string); description != "roasted chesnuts" {
 		t.Errorf("expected description to be %q, got %q", "roasted chesnuts", description)
 	}

auth/oauth.go

@@ -65,7 +65,7 @@ func (oa *OAuthFlow) ObtainAccessToken() (accessToken string, err error) {
 		fmt.Fprintf(os.Stderr, "  2. Copy the contents of ~/.config/gh/config.yml to this system")
 	}
 
-	http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	_ = http.Serve(listener, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		oa.logf("server handler: %s\n", r.URL.Path)
 		if r.URL.Path != "/callback" {
 			w.WriteHeader(404)

command/issue.go

@@ -170,7 +170,7 @@ func issueStatus(cmd *cobra.Command, args []string) error {
 	if issuePayload.Assigned.TotalCount > 0 {
 		printIssues(out, "  ", issuePayload.Assigned.TotalCount, issuePayload.Assigned.Issues)
 	} else {
-		message := fmt.Sprintf("  There are no issues assigned to you")
+		message := "  There are no issues assigned to you"
 		printMessage(out, message)
 	}
 	fmt.Fprintln(out)
@@ -427,7 +427,7 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue)
 		table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
 		table.EndRow()
 	}
-	table.Render()
+	_ = table.Render()
 	remaining := totalCount - len(issues)
 	if remaining > 0 {
 		fmt.Fprintf(w, utils.Gray("%sAnd %d more\n"), prefix, remaining)

command/issue_test.go

@@ -162,7 +162,7 @@ No issues match your search in OWNER/REPO
 			Author   string
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.Assignee, "probablyCher")
 	eq(t, reqBody.Variables.Labels, []string{"web", "bug"})
@@ -191,7 +191,7 @@ func TestIssueList_nullAssigneeLabels(t *testing.T) {
 	reqBody := struct {
 		Variables map[string]interface{}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	_, assigneeDeclared := reqBody.Variables["assignee"]
 	_, labelsDeclared := reqBody.Variables["labels"]
@@ -471,7 +471,7 @@ func TestIssueCreate(t *testing.T) {
 			}
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.Input.RepositoryID, "REPOID")
 	eq(t, reqBody.Variables.Input.Title, "hello")

command/pr_checkout_test.go

@@ -167,7 +167,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
 			Repo  string
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.Owner, "OTHER")
 	eq(t, reqBody.Variables.Repo, "POE")
@@ -420,7 +420,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
 	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
 		switch strings.Join(cmd.Args, " ") {
 		case "git config branch.feature.merge":
-			return &test.OutputStub{[]byte("refs/heads/feature\n"), nil}
+			return &test.OutputStub{Out: []byte("refs/heads/feature\n")}
 		default:
 			ranCommands = append(ranCommands, cmd.Args)
 			return &test.OutputStub{}
@@ -471,7 +471,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
 	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
 		switch strings.Join(cmd.Args, " ") {
 		case "git config branch.feature.merge":
-			return &test.OutputStub{[]byte("refs/heads/feature\n"), nil}
+			return &test.OutputStub{Out: []byte("refs/heads/feature\n")}
 		default:
 			ranCommands = append(ranCommands, cmd.Args)
 			return &test.OutputStub{}

command/pr_create_test.go

@@ -54,7 +54,7 @@ func TestPRCreate(t *testing.T) {
 			}
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.Input.RepositoryID, "REPOID")
 	eq(t, reqBody.Variables.Input.Title, "my title")
@@ -316,7 +316,7 @@ func TestPRCreate_cross_repo_same_branch(t *testing.T) {
 			}
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.Input.RepositoryID, "REPOID0")
 	eq(t, reqBody.Variables.Input.Title, "cross repo")
@@ -392,7 +392,7 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) {
 			}
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	expectedBody := "- commit 0\n- commit 1\n"
 
@@ -469,7 +469,7 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) {
 			}
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	expectedBody := "was the color of a television, turned to a dead channel"
 
@@ -527,7 +527,7 @@ func TestPRCreate_survey_autofill(t *testing.T) {
 			}
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	expectedBody := "was the color of a television, turned to a dead channel"
 

command/pr_test.go

@@ -55,11 +55,11 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) {
 	cmd.Flags().VisitAll(func(f *pflag.Flag) {
 		switch v := f.Value.(type) {
 		case pflag.SliceValue:
-			v.Replace([]string{})
+			_ = v.Replace([]string{})
 		default:
 			switch v.Type() {
 			case "bool", "string", "int":
-				v.Set(f.DefValue)
+				_ = v.Set(f.DefValue)
 			}
 		}
 	})
@@ -111,8 +111,8 @@ func TestPRStatus_fork(t *testing.T) {
 	defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
 		switch strings.Join(cmd.Args, " ") {
 		case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`:
-			return &test.OutputStub{[]byte(`branch.blueberries.remote origin
-branch.blueberries.merge refs/heads/blueberries`), nil}
+			return &test.OutputStub{Out: []byte(`branch.blueberries.remote origin
+branch.blueberries.merge refs/heads/blueberries`)}
 		default:
 			panic("not implemented")
 		}
@@ -312,7 +312,7 @@ No pull requests match your search in OWNER/REPO
 			Labels []string
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.State, []string{"OPEN", "CLOSED", "MERGED"})
 	eq(t, reqBody.Variables.Labels, []string{"one", "two", "three"})
@@ -357,7 +357,7 @@ func TestPRList_filteringClosed(t *testing.T) {
 			State []string
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.State, []string{"CLOSED", "MERGED"})
 }
@@ -381,7 +381,7 @@ func TestPRList_filteringAssignee(t *testing.T) {
 			Q string
 		}
 	}{}
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 
 	eq(t, reqBody.Variables.Q, `repo:OWNER/REPO assignee:hubot is:pr sort:created-desc is:merged label:"needs tests" base:"develop"`)
 }

command/repo_test.go

@@ -466,7 +466,7 @@ func TestRepoCreate(t *testing.T) {
 	}
 
 	bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body)
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 	if repoName := reqBody.Variables.Input["name"].(string); repoName != "REPO" {
 		t.Errorf("expected %q, got %q", "REPO", repoName)
 	}
@@ -533,7 +533,7 @@ func TestRepoCreate_org(t *testing.T) {
 	eq(t, http.Requests[0].URL.Path, "/users/ORG")
 
 	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 	if orgID := reqBody.Variables.Input["ownerId"].(string); orgID != "ORGID" {
 		t.Errorf("expected %q, got %q", "ORGID", orgID)
 	}
@@ -598,7 +598,7 @@ func TestRepoCreate_orgWithTeam(t *testing.T) {
 	eq(t, http.Requests[0].URL.Path, "/orgs/ORG/teams/monkeys")
 
 	bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
-	json.Unmarshal(bodyBytes, &reqBody)
+	_ = json.Unmarshal(bodyBytes, &reqBody)
 	if orgID := reqBody.Variables.Input["ownerId"].(string); orgID != "ORGID" {
 		t.Errorf("expected %q, got %q", "ORGID", orgID)
 	}

command/root.go

@@ -85,7 +85,7 @@ var versionCmd = &cobra.Command{
 	Use:    "version",
 	Hidden: true,
 	Run: func(cmd *cobra.Command, args []string) {
-		fmt.Printf(versionOutput)
+		fmt.Print(versionOutput)
 	},
 }
 

command/root_test.go

@@ -1,41 +1,40 @@
 package command
 
 import (
-	"fmt"
 	"testing"
 )
 
 func TestChangelogURL(t *testing.T) {
 	tag := "0.3.2"
-	url := fmt.Sprintf("https://github.com/cli/cli/releases/tag/v0.3.2")
+	url := "https://github.com/cli/cli/releases/tag/v0.3.2"
 	result := changelogURL(tag)
 	if result != url {
 		t.Errorf("expected %s to create url %s but got %s", tag, url, result)
 	}
 
 	tag = "v0.3.2"
-	url = fmt.Sprintf("https://github.com/cli/cli/releases/tag/v0.3.2")
+	url = "https://github.com/cli/cli/releases/tag/v0.3.2"
 	result = changelogURL(tag)
 	if result != url {
 		t.Errorf("expected %s to create url %s but got %s", tag, url, result)
 	}
 
 	tag = "0.3.2-pre.1"
-	url = fmt.Sprintf("https://github.com/cli/cli/releases/tag/v0.3.2-pre.1")
+	url = "https://github.com/cli/cli/releases/tag/v0.3.2-pre.1"
 	result = changelogURL(tag)
 	if result != url {
 		t.Errorf("expected %s to create url %s but got %s", tag, url, result)
 	}
 
 	tag = "0.3.5-90-gdd3f0e0"
-	url = fmt.Sprintf("https://github.com/cli/cli/releases/latest")
+	url = "https://github.com/cli/cli/releases/latest"
 	result = changelogURL(tag)
 	if result != url {
 		t.Errorf("expected %s to create url %s but got %s", tag, url, result)
 	}
 
 	tag = "deadbeef"
-	url = fmt.Sprintf("https://github.com/cli/cli/releases/latest")
+	url = "https://github.com/cli/cli/releases/latest"
 	result = changelogURL(tag)
 	if result != url {
 		t.Errorf("expected %s to create url %s but got %s", tag, url, result)

command/testing.go

@@ -38,9 +38,9 @@ func initAskStubber() (*askStubber, func()) {
 			}
 			if sq.Default {
 				defaultValue := reflect.ValueOf(q.Prompt).Elem().FieldByName("Default")
-				core.WriteAnswer(response, q.Name, defaultValue)
+				_ = core.WriteAnswer(response, q.Name, defaultValue)
 			} else {
-				core.WriteAnswer(response, q.Name, sq.Value)
+				_ = core.WriteAnswer(response, q.Name, sq.Value)
 			}
 		}
 

context/config_setup.go

@@ -44,7 +44,7 @@ func setupConfigFile(filename string) (*configEntry, error) {
 
 	fmt.Fprintln(os.Stderr, "Notice: authentication required")
 	fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname)
-	waitForEnter(os.Stdin)
+	_ = waitForEnter(os.Stdin)
 	token, err := flow.ObtainAccessToken()
 	if err != nil {
 		return nil, err
@@ -83,7 +83,7 @@ func setupConfigFile(filename string) (*configEntry, error) {
 
 	if err == nil {
 		fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ")
-		waitForEnter(os.Stdin)
+		_ = waitForEnter(os.Stdin)
 	}
 
 	return &entry, err

git/git_test.go

@@ -27,7 +27,7 @@ func Test_UncommittedChangeCount(t *testing.T) {
 
 	for _, v := range cases {
 		_ = run.SetPrepareCmd(func(*exec.Cmd) run.Runnable {
-			return &test.OutputStub{[]byte(v.Output), nil}
+			return &test.OutputStub{Out: []byte(v.Output)}
 		})
 		ucc, _ := UncommittedChangeCount()
 

git/ssh_config.go

@@ -72,7 +72,7 @@ func ParseSSHConfig() SSHAliasMap {
 func sshParse(r ...io.Reader) SSHAliasMap {
 	config := make(SSHAliasMap)
 	for _, file := range r {
-		sshParseConfig(config, file)
+		_ = sshParseConfig(config, file)
 	}
 	return config
 }

pkg/githubtemplate/github_template_test.go

@@ -108,7 +108,7 @@ func TestFind(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			for _, p := range tt.prepare {
 				fp := path.Join(tmpdir, p)
-				os.MkdirAll(path.Dir(fp), 0700)
+				_ = os.MkdirAll(path.Dir(fp), 0700)
 				file, err := os.Create(fp)
 				if err != nil {
 					t.Fatal(err)
@@ -179,7 +179,7 @@ about: This is how you report bugs
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ioutil.WriteFile(tmpfile.Name(), []byte(tt.prepare), 0600)
+			_ = ioutil.WriteFile(tmpfile.Name(), []byte(tt.prepare), 0600)
 			if got := ExtractName(tt.args.filePath); got != tt.want {
 				t.Errorf("ExtractName() = %v, want %v", got, tt.want)
 			}
@@ -244,7 +244,7 @@ Even more
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ioutil.WriteFile(tmpfile.Name(), []byte(tt.prepare), 0600)
+			_ = ioutil.WriteFile(tmpfile.Name(), []byte(tt.prepare), 0600)
 			if got := ExtractContents(tt.args.filePath); string(got) != tt.want {
 				t.Errorf("ExtractContents() = %v, want %v", string(got), tt.want)
 			}

pkg/surveyext/editor.go

@@ -79,8 +79,8 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int
 
 	// start reading runes from the standard in
 	rr := e.NewRuneReader()
-	rr.SetTermMode()
-	defer rr.RestoreTermMode()
+	_ = rr.SetTermMode()
+	defer func() { _ = rr.RestoreTermMode() }()
 
 	cursor := e.NewCursor()
 	cursor.Hide()

pkg/text/truncate.go

@@ -28,7 +28,7 @@ func Truncate(max int, s string) string {
 	useEllipsis := false
 	if max >= minWidthForEllipsis {
 		useEllipsis = true
-		max -= 3
+		max -= ellipsisWidth
 	}
 
 	cw := 0

test/helpers.go

@@ -53,7 +53,11 @@ func (cs *CmdStubber) StubError(errText string) {
 	stderrBuff := bytes.NewBufferString(errText)
 	args := []string{"stub"} // TODO make more real?
 	err := errors.New(errText)
-	cs.Stubs = append(cs.Stubs, &OutputStub{[]byte{}, &run.CmdError{stderrBuff, args, err}})
+	cs.Stubs = append(cs.Stubs, &OutputStub{Error: &run.CmdError{
+		Stderr: stderrBuff,
+		Args:   args,
+		Err:    err,
+	}})
 }
 
 func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) run.Runnable {

update/update.go

@@ -76,7 +76,7 @@ func setStateEntry(stateFilePath string, t time.Time, r ReleaseInfo) error {
 	if err != nil {
 		return err
 	}
-	ioutil.WriteFile(stateFilePath, content, 0600)
+	_ = ioutil.WriteFile(stateFilePath, content, 0600)
 
 	return nil
 }

utils/table_printer.go

@@ -69,7 +69,7 @@ func (t *ttyTablePrinter) AddField(s string, truncateFunc func(int, string) stri
 		truncateFunc = text.Truncate
 	}
 	if t.rows == nil {
-		t.rows = make([][]tableField, 1, 1)
+		t.rows = make([][]tableField, 1)
 	}
 	rowI := len(t.rows) - 1
 	field := tableField{