Skip to content

Commit 07b0446

Browse files
gmlewisgmlewis
authored
chore!: Add sliceofpointers custom linter (#3447)
BREAKING CHANGE: `ListOAuthApps` now returns `([]*OAuthApp, error)` instead of `([]OAuthApp, error)`.
1 parent 04070d2 commit 07b0446

File tree

17 files changed

+215
-40
lines changed

17 files changed

+215
-40
lines changed

‎.custom-gcl.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
version: v1.62.0
2+
plugins:
3+
- module: "github.com/google/go-github/v68/tools/sliceofpointers"
4+
path: ./tools/sliceofpointers

‎.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,6 @@ vendor/
1414

1515
# goenv local version. See https://github.com/syndbg/goenv/blob/master/COMMANDS.md#goenv-local for more info.
1616
.go-version
17+
18+
# golangci-lint -v custom generates the following local file:
19+
custom-gcl

‎.golangci.yml

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,18 @@ linters:
1818
- perfsprint
1919
- paralleltest
2020
- revive
21+
- sliceofpointers
2122
- stylecheck
2223
- tparallel
2324
- unconvert
2425
- unparam
2526
- whitespace
2627
linters-settings:
28+
custom:
29+
sliceofpointers:
30+
type: module
31+
description: Reports usage of []*string and slices of structs without pointers.
32+
original-url: github.com/google/go-github/v68/tools/sliceofpointers
2733
gocritic:
2834
disable-all: true
2935
enabled-checks:
@@ -38,7 +44,7 @@ linters-settings:
3844
misspell:
3945
locale: US
4046
ignore-words:
41-
- "analyses" # returned by the GitHub API
47+
- "analyses" # returned by the GitHub API
4248
- "cancelled" # returned by the GitHub API
4349
# extra words from https://go.dev//wiki/Spelling
4450
extra-words:
@@ -102,28 +108,28 @@ issues:
102108
path: _test\.go
103109

104110
# We need to pass nil Context in order to test DoBare erroring on nil ctx.
105-
- linters: [ staticcheck ]
106-
text: 'SA1012: do not pass a nil Context'
111+
- linters: [staticcheck]
112+
text: "SA1012: do not pass a nil Context"
107113
path: _test\.go
108114

109115
# We need to use sha1 for validating signatures
110-
- linters: [ gosec ]
111-
text: 'G505: Blocklisted import crypto/sha1: weak cryptographic primitive'
116+
- linters: [gosec]
117+
text: "G505: Blocklisted import crypto/sha1: weak cryptographic primitive"
112118

113119
# This is adapted from golangci-lint's default exclusions. It disables linting for error checks on
114120
# os.RemoveAll, fmt.Fprint*, fmt.Scanf, and any function ending in "Close".
115-
- linters: [ errcheck ]
121+
- linters: [errcheck]
116122
text: Error return value of .(.*Close|fmt\.Fprint.*|fmt\.Scanf|os\.Remove(All)?). is not checked
117123

118124
# We don't care about file inclusion via variable in examples or internal tools.
119-
- linters: [ gosec ]
120-
text: 'G304: Potential file inclusion via variable'
125+
- linters: [gosec]
126+
text: "G304: Potential file inclusion via variable"
121127
path: '^(example|tools)\/'
122128

123129
# We don't run parallel integration tests
124-
- linters: [ paralleltest, tparallel ]
125-
path: '^test/integration'
130+
- linters: [paralleltest, tparallel]
131+
path: "^test/integration"
126132

127133
# Because fmt.Sprint(reset.Unix())) is more readable than strconv.FormatInt(reset.Unix(), 10).
128-
- linters: [ perfsprint]
129-
text: 'fmt.Sprint.* can be replaced with faster strconv.FormatInt'
134+
- linters: [perfsprint]
135+
text: "fmt.Sprint.* can be replaced with faster strconv.FormatInt"

‎github/codespaces_secrets_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestCodespacesService_ListSecrets(t *testing.T) {
2525
methodName string
2626
}
2727
opts := &ListOptions{Page: 2, PerPage: 2}
28-
tests := []test{
28+
tests := []*test{
2929
{
3030
name: "User",
3131
handleFunc: func(mux *http.ServeMux) {
@@ -128,7 +128,7 @@ func TestCodespacesService_GetSecret(t *testing.T) {
128128
badCall func(context.Context, *Client) (*Secret, *Response, error)
129129
methodName string
130130
}
131-
tests := []test{
131+
tests := []*test{
132132
{
133133
name: "User",
134134
handleFunc: func(mux *http.ServeMux) {
@@ -222,7 +222,7 @@ func TestCodespacesService_CreateOrUpdateSecret(t *testing.T) {
222222
badCall func(context.Context, *Client, *EncryptedSecret) (*Response, error)
223223
methodName string
224224
}
225-
tests := []test{
225+
tests := []*test{
226226
{
227227
name: "User",
228228
handleFunc: func(mux *http.ServeMux) {
@@ -318,7 +318,7 @@ func TestCodespacesService_DeleteSecret(t *testing.T) {
318318
badCall func(context.Context, *Client) (*Response, error)
319319
methodName string
320320
}
321-
tests := []test{
321+
tests := []*test{
322322
{
323323
name: "User",
324324
handleFunc: func(mux *http.ServeMux) {
@@ -401,7 +401,7 @@ func TestCodespacesService_GetPublicKey(t *testing.T) {
401401
methodName string
402402
}
403403

404-
tests := []test{
404+
tests := []*test{
405405
{
406406
name: "User",
407407
handleFunc: func(mux *http.ServeMux) {
@@ -496,7 +496,7 @@ func TestCodespacesService_ListSelectedReposForSecret(t *testing.T) {
496496
methodName string
497497
}
498498
opts := &ListOptions{Page: 2, PerPage: 2}
499-
tests := []test{
499+
tests := []*test{
500500
{
501501
name: "User",
502502
handleFunc: func(mux *http.ServeMux) {
@@ -581,7 +581,7 @@ func TestCodespacesService_SetSelectedReposForSecret(t *testing.T) {
581581
methodName string
582582
}
583583
ids := SelectedRepoIDs{64780797}
584-
tests := []test{
584+
tests := []*test{
585585
{
586586
name: "User",
587587
handleFunc: func(mux *http.ServeMux) {
@@ -653,7 +653,7 @@ func TestCodespacesService_AddSelectedReposForSecret(t *testing.T) {
653653
methodName string
654654
}
655655
repo := &Repository{ID: Ptr(int64(1234))}
656-
tests := []test{
656+
tests := []*test{
657657
{
658658
name: "User",
659659
handleFunc: func(mux *http.ServeMux) {
@@ -721,7 +721,7 @@ func TestCodespacesService_RemoveSelectedReposFromSecret(t *testing.T) {
721721
methodName string
722722
}
723723
repo := &Repository{ID: Ptr(int64(1234))}
724-
tests := []test{
724+
tests := []*test{
725725
{
726726
name: "User",
727727
handleFunc: func(mux *http.ServeMux) {

‎github/github.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1117,7 +1117,8 @@ GitHub API docs: https://docs.github.com/rest/#client-errors
11171117
type ErrorResponse struct {
11181118
Response *http.Response `json:"-"` // HTTP response that caused this error
11191119
Message string `json:"message"` // error message
1120-
Errors []Error `json:"errors"` // more detail on individual errors
1120+
//nolint:sliceofpointers
1121+
Errors []Error `json:"errors"` // more detail on individual errors
11211122
// Block is only populated on certain types of errors such as code 451.
11221123
Block *ErrorBlock `json:"block,omitempty"`
11231124
// Most errors will also include a documentation_url field pointing

‎github/strings_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func TestStringify(t *testing.T) {
4949
{Ptr(123), `123`},
5050
{Ptr(false), `false`},
5151
{
52+
//nolint:sliceofpointers
5253
[]*string{Ptr("a"), Ptr("b")},
5354
`["a" "b"]`,
5455
},

‎scrape/apps.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ func (c *Client) AppRestrictionsEnabled(org string) (bool, error) {
4646

4747
// ListOAuthApps lists the reviewed OAuth Applications for the
4848
// specified organization (whether approved or denied).
49-
func (c *Client) ListOAuthApps(org string) ([]OAuthApp, error) {
49+
func (c *Client) ListOAuthApps(org string) ([]*OAuthApp, error) {
5050
doc, err := c.get("/organizations/%s/settings/oauth_application_policy", org)
5151
if err != nil {
5252
return nil, err
5353
}
5454

55-
var apps []OAuthApp
55+
var apps []*OAuthApp
5656
doc.Find(".oauth-application-allowlist ul > li").Each(func(i int, s *goquery.Selection) {
5757
var app OAuthApp
5858
app.Name = s.Find(".request-info strong").First().Text()
@@ -73,7 +73,7 @@ func (c *Client) ListOAuthApps(org string) ([]OAuthApp, error) {
7373
} else if r := s.Find(".request-indicator .denied-request"); r.Length() > 0 {
7474
app.State = OAuthAppDenied
7575
}
76-
apps = append(apps, app)
76+
apps = append(apps, &app)
7777
})
7878

7979
return apps, nil

‎scrape/apps_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func Test_ListOAuthApps(t *testing.T) {
6161
if err != nil {
6262
t.Fatalf("ListOAuthApps(e) returned err: %v", err)
6363
}
64-
want := []OAuthApp{
64+
want := []*OAuthApp{
6565
{
6666
ID: 22222,
6767
Name: "Coveralls",

‎scrape/forms.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ type htmlForm struct {
3535
//
3636
// In the future, we might want to allow a custom selector to be passed in to
3737
// further restrict what forms will be returned.
38-
func parseForms(node *html.Node) (forms []htmlForm) {
38+
func parseForms(node *html.Node) (forms []*htmlForm) {
3939
if node == nil {
4040
return nil
4141
}
@@ -71,7 +71,7 @@ func parseForms(node *html.Node) (forms []htmlForm) {
7171
value := s.Text()
7272
form.Values.Add(name, value)
7373
})
74-
forms = append(forms, form)
74+
forms = append(forms, &form)
7575
})
7676

7777
return forms

‎scrape/forms_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,22 @@ func Test_ParseForms(t *testing.T) {
1616
tests := []struct {
1717
description string
1818
html string
19-
forms []htmlForm
19+
forms []*htmlForm
2020
}{
2121
{"no forms", `<html></html>`, nil},
22-
{"empty form", `<html><form></form></html>`, []htmlForm{{Values: url.Values{}}}},
22+
{"empty form", `<html><form></form></html>`, []*htmlForm{{Values: url.Values{}}}},
2323
{
2424
"single form with one value",
2525
`<html><form action="a" method="m"><input name="n1" value="v1"></form></html>`,
26-
[]htmlForm{{Action: "a", Method: "m", Values: url.Values{"n1": {"v1"}}}},
26+
[]*htmlForm{{Action: "a", Method: "m", Values: url.Values{"n1": {"v1"}}}},
2727
},
2828
{
2929
"two forms",
3030
`<html>
3131
<form action="a1" method="m1"><input name="n1" value="v1"></form>
3232
<form action="a2" method="m2"><input name="n2" value="v2"></form>
3333
</html>`,
34-
[]htmlForm{
34+
[]*htmlForm{
3535
{Action: "a1", Method: "m1", Values: url.Values{"n1": {"v1"}}},
3636
{Action: "a2", Method: "m2", Values: url.Values{"n2": {"v2"}}},
3737
},
@@ -43,7 +43,7 @@ func Test_ParseForms(t *testing.T) {
4343
<input type="radio" name="n1" value="v2">
4444
<input type="radio" name="n1" value="v3">
4545
</form></html>`,
46-
[]htmlForm{{Values: url.Values{}}},
46+
[]*htmlForm{{Values: url.Values{}}},
4747
},
4848
{
4949
"form with radio buttons",
@@ -52,7 +52,7 @@ func Test_ParseForms(t *testing.T) {
5252
<input type="radio" name="n1" value="v2">
5353
<input type="radio" name="n1" value="v3" checked>
5454
</form></html>`,
55-
[]htmlForm{{Values: url.Values{"n1": {"v3"}}}},
55+
[]*htmlForm{{Values: url.Values{"n1": {"v3"}}}},
5656
},
5757
{
5858
"form with checkboxes",
@@ -61,12 +61,12 @@ func Test_ParseForms(t *testing.T) {
6161
<input type="checkbox" name="n2" value="v2">
6262
<input type="checkbox" name="n3" value="v3" checked>
6363
</form></html>`,
64-
[]htmlForm{{Values: url.Values{"n1": {"v1"}, "n3": {"v3"}}}},
64+
[]*htmlForm{{Values: url.Values{"n1": {"v1"}, "n3": {"v3"}}}},
6565
},
6666
{
6767
"single form with textarea",
6868
`<html><form><textarea name="n1">v1</textarea></form></html>`,
69-
[]htmlForm{{Values: url.Values{"n1": {"v1"}}}},
69+
[]*htmlForm{{Values: url.Values{"n1": {"v1"}}}},
7070
},
7171
}
7272

‎script/lint.sh

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ fail() {
1919
EXIT_CODE=1
2020
}
2121

22-
# install golangci-lint bin/golangci-lint doesn't exist with the correct version
23-
if ! "$BIN"/golangci-lint --version 2> /dev/null | grep -q "$GOLANGCI_LINT_VERSION"; then
22+
# install golangci-lint and custom-gcl in ./bin if they don't exist with the correct version
23+
if ! "$BIN"/custom-gcl --version 2> /dev/null | grep -q "$GOLANGCI_LINT_VERSION"; then
2424
GOBIN="$BIN" go install "github.com/golangci/golangci-lint/cmd/golangci-lint@v$GOLANGCI_LINT_VERSION"
25+
"$BIN"/golangci-lint custom && mv ./custom-gcl "$BIN"
2526
fi
2627

2728
MOD_DIRS="$(git ls-files '*go.mod' | xargs dirname | sort)"
@@ -33,9 +34,9 @@ for dir in $MOD_DIRS; do
3334
cd "$dir"
3435
# github actions output when running in an action
3536
if [ -n "$GITHUB_ACTIONS" ]; then
36-
"$BIN"/golangci-lint run --path-prefix "$dir" --out-format colored-line-number
37+
"$BIN"/custom-gcl run --path-prefix "$dir" --out-format colored-line-number
3738
else
38-
"$BIN"/golangci-lint run --path-prefix "$dir"
39+
"$BIN"/custom-gcl run --path-prefix "$dir"
3940
fi
4041
) || fail "failed linting $dir"
4142
done

‎tools/sliceofpointers/go.mod

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
module tools/sliceofpointers
2+
3+
go 1.22.0
4+
5+
require (
6+
github.com/golangci/plugin-module-register v0.1.1
7+
golang.org/x/tools v0.29.0
8+
)
9+
10+
require (
11+
golang.org/x/mod v0.22.0 // indirect
12+
golang.org/x/sync v0.10.0 // indirect
13+
)

‎tools/sliceofpointers/go.sum

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c=
2+
github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc=
3+
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
4+
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
5+
golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
6+
golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
7+
golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
8+
golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
9+
golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE=
10+
golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588=

0 commit comments

Comments
 (0)