diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 000000000..bd834a158 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -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 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..b6c345272 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,41 @@ +run: + timeout: 5m +linters: +#enabled by default: deadcode, errcheck, gosimple, govet, ineffasign, staticcheck, struccheck, typecheck, unused, varcheck + enable: + - whitespace #https://github.com/ultraware/whitespace + # - noctx #https://github.com/sonatard/noctx + - nilerr #https://github.com/gostaticanalysis/nilerr + - nestif #https://github.com/nakabonne/nestif + # - gocritic #https://github.com/go-critic/go-critic #TODO: reasses usefulness + # - goconst #https://github.com/jgautheron/goconst #TODO: reasses usefulness + - exportloopref #https://github.com/kyoh86/exportloopref + # - errname #https://github.com/Antonboom/errname #TODO: reasses usefulness + # - errorlint #https://github.com/polyfloyd/go-errorlint #TODO: reasses usefulness + - bodyclose #https://github.com/timakin/bodyclose + # - cyclop #https://github.com/bkielbasa/cyclop + - errcheck #https://github.com/kisielk/errcheck + - stylecheck #https://github.com/dominikh/go-tools/tree/master/stylecheck + - revive #golint is deprecated and golangci-lint recommends to use revive instead https://github.com/mgechev/revive + #other deprecated lint libraries: maligned, scopelint, interfacer +issues: + exclude-rules: + - path: _test\.go + linters: + - unused + - deadcode +linters-settings: + errcheck: + # https://github.com/kisielk/errcheck#excluding-functions + check-type-assertions: true + check-blank: true + + revive: + # see https://github.com/mgechev/revive#available-rules for details. + ignore-generated-header: false #recommended in their configuration + severity: warning + rules: + - name: indent-error-flow #Prevents redundant else statements + severity: warning + - name: useless-break + severity: warning \ No newline at end of file diff --git a/admin_setting_saml.go b/admin_setting_saml.go index 8e35dc765..3de700faa 100644 --- a/admin_setting_saml.go +++ b/admin_setting_saml.go @@ -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 } diff --git a/admin_user.go b/admin_user.go index b0500a71b..75009ce6e 100644 --- a/admin_user.go +++ b/admin_user.go @@ -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 diff --git a/helper_test.go b/helper_test.go index ebff5d2cf..6f9da74b6 100644 --- a/helper_test.go +++ b/helper_test.go @@ -16,7 +16,7 @@ import ( "github.com/hashicorp/go-uuid" ) -const badIdentifier = "! / nope" +const badIdentifier = "! / nope" //nolint // Memoize test account details var _testAccountDetails *TestAccountDetails @@ -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()) { diff --git a/ip_ranges.go b/ip_ranges.go index f0801532e..162a797f5 100644 --- a/ip_ranges.go +++ b/ip_ranges.go @@ -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 } diff --git a/logreader.go b/logreader.go index aee4472fe..96ffe840c 100644 --- a/logreader.go +++ b/logreader.go @@ -121,23 +121,22 @@ func (r *LogReader) read(l []byte) (int, error) { // Check if we need to continue the loop and wait 500 miliseconds // before checking if there is a new chunk available or that the // run is finished and we are done reading all chunks. - if written == 0 { - if (r.startOfText && r.endOfText) || // The logstream finished without issues. - (r.startOfText && r.reads%10 == 0) || // The logstream terminated unexpectedly. - (!r.startOfText && r.reads > 1) { // The logstream doesn't support STX/ETX. - done, err := r.done() - if err != nil { - return 0, err - } - if done { - return 0, io.EOF - } - } - return 0, io.ErrNoProgress + if written != 0 { + // Update the offset for the next read. + r.offset += int64(written) + return written, nil } - // Update the offset for the next read. - r.offset += int64(written) - - return written, nil + if (r.startOfText && r.endOfText) || // The logstream finished without issues. + (r.startOfText && r.reads%10 == 0) || // The logstream terminated unexpectedly. + (!r.startOfText && r.reads > 1) { // The logstream doesn't support STX/ETX. + done, err := r.done() + if err != nil { + return 0, err + } + if done { + return 0, io.EOF + } + } + return 0, io.ErrNoProgress } diff --git a/oauth_client.go b/oauth_client.go index 9b3ddcfca..33bb259a4 100644 --- a/oauth_client.go +++ b/oauth_client.go @@ -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 } diff --git a/oauth_client_integration_test.go b/oauth_client_integration_test.go index ec077f711..39f7c4ce2 100644 --- a/oauth_client_integration_test.go +++ b/oauth_client_integration_test.go @@ -322,7 +322,7 @@ func TestOAuthClientsCreateOptionsValid(t *testing.T) { } err := options.valid() - assert.EqualError(t, err, "Private Key can only be present with Azure DevOps Server service provider") + assert.EqualError(t, err, "private Key can only be present with Azure DevOps Server service provider") }) t.Run("with valid options including private key", func(t *testing.T) { diff --git a/policy_set_version.go b/policy_set_version.go index 30261e820..6c0284413 100644 --- a/policy_set_version.go +++ b/policy_set_version.go @@ -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 diff --git a/policy_set_version_integration_test.go b/policy_set_version_integration_test.go index 6181626f7..c3039bff2 100644 --- a/policy_set_version_integration_test.go +++ b/policy_set_version_integration_test.go @@ -102,7 +102,7 @@ func TestPolicySetVersionsUpload(t *testing.T) { *psv, "test-fixtures/policy-set-version", ) - assert.EqualError(t, err, "The Policy Set Version does not contain an upload link.") + assert.EqualError(t, err, "the Policy Set Version does not contain an upload link") }) } @@ -130,7 +130,7 @@ func TestPolicySetVersionsUploadURL(t *testing.T) { } _, err := psv.uploadURL() - assert.EqualError(t, err, "The Policy Set Version does not contain an upload link.") + assert.EqualError(t, err, "the Policy Set Version does not contain an upload link") }) t.Run("errors when the upload link is empty", func(t *testing.T) { @@ -142,6 +142,6 @@ func TestPolicySetVersionsUploadURL(t *testing.T) { } _, err := psv.uploadURL() - assert.EqualError(t, err, "The Policy Set Version upload URL is empty.") + assert.EqualError(t, err, "the Policy Set Version upload URL is empty") }) } diff --git a/registry_module.go b/registry_module.go index 084520b8f..45b9abae2 100644 --- a/registry_module.go +++ b/registry_module.go @@ -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) diff --git a/registry_module_integration_test.go b/registry_module_integration_test.go index 522d516a3..8da04b9e4 100644 --- a/registry_module_integration_test.go +++ b/registry_module_integration_test.go @@ -571,7 +571,7 @@ func TestRegistryModulesUpload(t *testing.T) { *rmv, "test-fixtures/config-version", ) - assert.EqualError(t, err, "Provided RegistryModuleVersion does not contain an upload link") + assert.EqualError(t, err, "provided RegistryModuleVersion does not contain an upload link") }) } diff --git a/tfe.go b/tfe.go index 9f50071f3..8c57e9e7b 100644 --- a/tfe.go +++ b/tfe.go @@ -1,6 +1,8 @@ package tfe import ( + "log" + "bytes" "context" "encoding/json" @@ -155,7 +157,7 @@ func NewClient(cfg *Config) (*Client, error) { config := DefaultConfig() // Layer in the provided config for any non-blank values. - if cfg != nil { + if cfg != nil { // nolint if cfg.Address != "" { config.Address = cfg.Address } @@ -356,14 +358,15 @@ func rateLimitBackoff(min, max time.Duration, attemptNum int, resp *http.Respons // First create some jitter bounded by the min and max durations. jitter := time.Duration(rnd.Float64() * float64(max-min)) - if resp != nil { - if v := resp.Header.Get(headerRateReset); v != "" { - if reset, _ := strconv.ParseFloat(v, 64); reset > 0 { - // Only update min if the given time to wait is longer. - if wait := time.Duration(reset * 1e9); wait > min { - min = wait - } - } + if resp != nil && resp.Header.Get(headerRateReset) != "" { + v := resp.Header.Get(headerRateReset) + reset, err := strconv.ParseFloat(v, 64) + if err != nil { + log.Fatal(err) + } + // Only update min if the given time to wait is longer + if reset > 0 && time.Duration(reset*1e9) > min { + min = time.Duration(reset * 1e9) } } @@ -417,13 +420,15 @@ func (c *Client) getRawAPIMetadata() (rawAPIMetadata, error) { // configureLimiter configures the rate limiter. func (c *Client) configureLimiter(rawLimit string) { - // Set default values for when rate limiting is disabled. limit := rate.Inf burst := 0 if v := rawLimit; v != "" { - if rateLimit, _ := strconv.ParseFloat(v, 64); rateLimit > 0 { + if rateLimit, err := strconv.ParseFloat(v, 64); rateLimit > 0 { + if err != nil { + log.Fatal(err) + } // Configure the limit and burst using a split of 2/3 for the limit and // 1/3 for the burst. This enables clients to burst 1/3 of the allowed // calls before the limiter kicks in. The remaining calls will then be @@ -531,18 +536,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 @@ -552,13 +557,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