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

golangci-lint workflow #262

Merged
merged 19 commits into from
Sep 29, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3dd8ec4
draft golangci-lint workflow
uturunku1 Sep 22, 2021
222fe15
fix gh action error that says every step must define a or key
uturunku1 Sep 22, 2021
e8279f2
1.42.1 is an invalid version
uturunku1 Sep 22, 2021
6bfaa45
temporarily commenting the option only-new-issues to correct old issues
uturunku1 Sep 22, 2021
e14e068
apply golangci-lint recommendations: remove unused code which is ever…
uturunku1 Sep 22, 2021
08332ca
add back file because its functions are being used, is just that gola…
uturunku1 Sep 22, 2021
9f58003
add flag to ignore test files
uturunku1 Sep 22, 2021
5b21f13
add yml for golangci configuration and add setting to ignore unused f…
uturunku1 Sep 23, 2021
13c20e5
remove extra lines
uturunku1 Sep 28, 2021
413f623
pin to version 1.29 to compare behaviour with version 1.40
uturunku1 Sep 28, 2021
3c44683
enable some useful lint libraries and use go version 1.16 and golangc…
uturunku1 Sep 28, 2021
be0ed9d
golangci-lint library recommends to stop using golint because it is d…
uturunku1 Sep 28, 2021
771ab13
fix new issues point by golangci-lint
uturunku1 Sep 28, 2021
cabf350
clarify comment on why we are not using golint
uturunku1 Sep 28, 2021
1689acd
enable other lint libraries that I liked their description and add ad…
uturunku1 Sep 29, 2021
a573060
add configuration to golangci-lint and fix errors mentioned by enable…
uturunku1 Sep 29, 2021
2e17d84
remove not useful comments
uturunku1 Sep 29, 2021
1cb63d4
match lowercase in tests
uturunku1 Sep 29, 2021
8b80493
missing test
uturunku1 Sep 29, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: golangci-lint
on:
push:
branches:
- main
pull_request:
jobs:
golangci:
name: lint
runs-on: ubuntu-latest
strategy:
matrix:
go-version: [1.16.x]
steps:
- uses: actions/checkout@v2
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
version: v1.42
#only-new-issues: true
14 changes: 14 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
run:
timeout: 5m
linters:
enable:
- gosimple
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the docs say that this gosimple plus others are enabled by default. Part of that list includes errcheck. But when I run errcheck -verbose ./... locally, i get

go-tfe % errcheck -verbose ./...
checking github.com/hashicorp/go-tfe
checking github.com/hashicorp/go-tfe/examples/workspaces
checking github.com/hashicorp/go-tfe
checking github.com/hashicorp/go-tfe.test
checking github.com/hashicorp/go-tfe/examples/organizations
ip_ranges.go:83:23:     (io.Closer).Close       defer resp.Body.Close()
logreader.go:76:23:     (io.Closer).Close       defer resp.Body.Close()
tfe.go:410:17:  (io.Closer).Close       resp.Body.Close()
tfe.go:594:23:  (io.Closer).Close       defer resp.Body.Close()

It doesn't seem like these are enabled by default then? I think we should add all those that are supposed to be enabled by default explicitly here. So

linters:
  enable:
   - deadcode
   - errcheck
   - gosimple
   - govet
   - ineffassign
   - staticcheck
   - structcheck
   - typecheck
   - unused
   - varcheck

- stylecheck
- revive #golint is deprecated and golangci-lint recommends to use revive instead
#other deprecated lint libraries: maligned, scopelint, interfacer
issues:
exclude-rules:
- path: _test\.go
linters:
- unused
- deadcode
6 changes: 3 additions & 3 deletions admin_setting_saml.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ type AdminSAMLSetting struct {
}

// Read returns the SAML settings.
func (s *adminSAMLSettings) Read(ctx context.Context) (*AdminSAMLSetting, error) {
req, err := s.client.newRequest("GET", "admin/saml-settings", nil)
func (a *adminSAMLSettings) Read(ctx context.Context) (*AdminSAMLSetting, error) {
req, err := a.client.newRequest("GET", "admin/saml-settings", nil)
if err != nil {
return nil, err
}

saml := &AdminSAMLSetting{}
err = s.client.do(ctx, req, saml)
err = a.client.do(ctx, req, saml)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion admin_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type AdminUserListOptions struct {

// List all user accounts in the Terraform Enterprise installation
func (a *adminUsers) List(ctx context.Context, options AdminUserListOptions) (*AdminUserList, error) {
u := fmt.Sprintf("admin/users")
u := "admin/users"
req, err := a.client.newRequest("GET", u, &options)
if err != nil {
return nil, err
Expand Down
5 changes: 2 additions & 3 deletions helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/hashicorp/go-uuid"
)

const badIdentifier = "! / nope"
const badIdentifier = "! / nope" //nolint

// Memoize test account details
var _testAccountDetails *TestAccountDetails
Expand Down Expand Up @@ -561,9 +561,8 @@ func createRunWithStatus(t *testing.T, client *Client, w *Workspace, timeout int
func createPlannedRun(t *testing.T, client *Client, w *Workspace) (*Run, func()) {
if paidFeaturesDisabled() {
return createRunWithStatus(t, client, w, 45, RunPlanned)
} else {
return createRunWithStatus(t, client, w, 45, RunCostEstimated)
}
return createRunWithStatus(t, client, w, 45, RunCostEstimated)
}

func createCostEstimatedRun(t *testing.T, client *Client, w *Workspace) (*Run, func()) {
Expand Down
2 changes: 1 addition & 1 deletion ip_ranges.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (i *ipRanges) customDo(ctx context.Context, req *retryablehttp.Request, ir
defer resp.Body.Close()

if resp.StatusCode < 200 && resp.StatusCode >= 400 {
return fmt.Errorf("Error HTTP response while retrieving IP ranges: %d", resp.StatusCode)
return fmt.Errorf("error HTTP response while retrieving IP ranges: %d", resp.StatusCode)
} else if resp.StatusCode == 304 {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion oauth_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (o OAuthClientCreateOptions) valid() error {
return errors.New("service provider is required")
}
if validString(o.PrivateKey) && *o.ServiceProvider != *ServiceProvider(ServiceProviderAzureDevOpsServer) {
return errors.New("Private Key can only be present with Azure DevOps Server service provider")
return errors.New("private Key can only be present with Azure DevOps Server service provider")
}
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions policy_set_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ type PolicySetVersion struct {
func (p PolicySetVersion) uploadURL() (string, error) {
uploadURL, ok := p.Links["upload"].(string)
if !ok {
return uploadURL, fmt.Errorf("The Policy Set Version does not contain an upload link.")
return uploadURL, fmt.Errorf("the Policy Set Version does not contain an upload link")
}

if uploadURL == "" {
return uploadURL, fmt.Errorf("The Policy Set Version upload URL is empty.")
return uploadURL, fmt.Errorf("the Policy Set Version upload URL is empty")
}

return uploadURL, nil
Expand Down
2 changes: 1 addition & 1 deletion registry_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type RegistryModuleVersion struct {
func (r *registryModules) Upload(ctx context.Context, rmv RegistryModuleVersion, path string) error {
uploadURL, ok := rmv.Links["upload"].(string)
if !ok {
return fmt.Errorf("Provided RegistryModuleVersion does not contain an upload link")
return fmt.Errorf("provided RegistryModuleVersion does not contain an upload link")
}

body, err := packContents(path)
Expand Down
17 changes: 8 additions & 9 deletions tfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,18 +531,18 @@ func serializeRequestBody(v interface{}) (interface{}, error) {

// Infer whether the request uses jsonapi or regular json
// serialization based on how the fields are tagged.
jsonApiFields := 0
jsonAPIFields := 0
jsonFields := 0
for i := 0; i < modelType.NumField(); i++ {
structField := modelType.Field(i)
if structField.Tag.Get("jsonapi") != "" {
jsonApiFields++
jsonAPIFields++
}
if structField.Tag.Get("json") != "" {
jsonFields++
}
}
if jsonApiFields > 0 && jsonFields > 0 {
if jsonAPIFields > 0 && jsonFields > 0 {
// Defining a struct with both json and jsonapi tags doesn't
// make sense, because a struct can only be serialized
// as one or another. If this does happen, it's a bug
Expand All @@ -552,13 +552,12 @@ func serializeRequestBody(v interface{}) (interface{}, error) {

if jsonFields > 0 {
return json.Marshal(v)
} else {
buf := bytes.NewBuffer(nil)
if err := jsonapi.MarshalPayloadWithoutIncluded(buf, v); err != nil {
return nil, err
}
return buf, nil
}
buf := bytes.NewBuffer(nil)
if err := jsonapi.MarshalPayloadWithoutIncluded(buf, v); err != nil {
return nil, err
}
return buf, nil
}

// do sends an API request and returns the API response. The API response
Expand Down