Merge pull request #262 from hashicorp/add-golangcilint

golangci-lint workflow

.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

.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

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
 	}

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

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()) {

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
 	}

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
 }

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
 }

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) {

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

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")
 	})
 }

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)

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")
 	})
 }
 

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