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

Use https://staticcheck.io/ for linting #640

Merged
merged 2 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ RUN \
&& go get gotest.tools/gotestsum \
# --> Go symbols and outline for go to symbol support and test support
&& go get github.com/acroca/go-symbols@v0.1.1 && go get github.com/ramya-rao-a/go-outline@7182a932836a71948db4a81991a494751eccfe77 \
# --> GolangCI-lint
&& go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.25.0 \
# --> Static checker
&& go install honnef.co/go/tools/cmd/staticcheck@latest \
Copy link
Contributor

Choose a reason for hiding this comment

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

in the push hook we have just go install honnef.co/go/tools/cmd/staticcheck - that this version was reason for failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah! yes, i'll change it in a separate PR


USER developer
2 changes: 1 addition & 1 deletion .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
go get gotest.tools/gotestsum
go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.25.0
go install honnef.co/go/tools/cmd/staticcheck

- name: Pull external libraries
run: make vendor
Expand Down
9 changes: 5 additions & 4 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ jobs:
uses: actions/setup-go@v1
with:
go-version: 1.13.x
- name: Download gotestsum and golangci
- name: Download gotestsum and staticcheck
run: |
# Install
curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
go get gotest.tools/gotestsum
go install honnef.co/go/tools/cmd/staticcheck
- name: Import GPG key
id: import_gpg
uses: crazy-max/ghaction-import-gpg@v2
Expand Down
35 changes: 0 additions & 35 deletions .golangci.yml

This file was deleted.

3 changes: 0 additions & 3 deletions .goreleaser.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# This is an example goreleaser.yaml file with some sane defaults.
# Make sure to check the documentation at http://goreleaser.com
before:
hooks:
# You may remove this if you don't use go modules.
- go mod download
builds:
- env:
Expand Down
9 changes: 4 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,16 @@ Most likely, `terraform init -upgrade -verify-plugins=false -lock=false` would b

## Developing provider

After installing necessary software for building provider from sources, you should install `golangci-lint` and `gotestsum` in order to run `make test`.
After installing necessary software for building provider from sources, you should install `staticcheck` and `gotestsum` in order to run `make test`.

Make sure you have `$GOPATH/bin` in your `$PATH`:
```
echo "export PATH=\$PATH:$(go env GOPATH)/bin" >> ~/.bash_profile
```

Installing `golangci-lint`:
Installing `staticcheck`:
```bash
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.27.0
$(go env GOPATH)/bin/golangci-lint
go install honnef.co/go/tools/cmd/staticcheck@latest
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above, although it works for me on the command-line

```

Installing `gotestsum`:
Expand Down Expand Up @@ -298,7 +297,7 @@ func TestPreviewAccPipelineResource_CreatePipeline(t *testing.T) {

## Linting

Please use makefile for linting. If you run `golangci-lint` by itself it will fail due to different tags containing same functions.
Please use makefile for linting. If you run `staticcheck` by itself it will fail due to different tags containing same functions.
So please run `make lint` instead.

## Unit testing resources
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ fmt:
@gofmt -w $(shell find . -type f -name '*.go' -not -path "./vendor/*")

lint: vendor
@echo "✓ Linting source code with golangci-lint make sure you run make fmt ..."
@golangci-lint run --skip-dirs-use-default --timeout 5m
@echo "✓ Linting source code with https://staticcheck.io/ ..."
@staticcheck ./...

test: lint
@echo "✓ Running tests ..."
Expand Down
6 changes: 3 additions & 3 deletions access/resource_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (a PermissionsAPI) put(objectID string, objectACL AccessControlChangeList)

// Update updates object permissions. Technically, it's using method named SetOrDelete, but here we do more
func (a PermissionsAPI) Update(objectID string, objectACL AccessControlChangeList) error {
if "/authorization/tokens" == objectID {
if objectID == "/authorization/tokens" {
// Cannot remove admins's CAN_MANAGE permission on tokens
objectACL.AccessControlList = append(objectACL.AccessControlList, AccessControlChange{
GroupName: "admins",
Expand Down Expand Up @@ -279,7 +279,7 @@ func (oa *ObjectACL) ToPermissionsEntity(ctx context.Context, d *schema.Resource
}
return entity, nil
}
return entity, fmt.Errorf("Unknown object type %s", oa.ObjectType)
return entity, fmt.Errorf("unknown object type %s", oa.ObjectType)
}

// ResourcePermissions definition
Expand All @@ -304,7 +304,7 @@ func ResourcePermissions() *schema.Resource {
"access_control", "group_name"); err == nil {
groupNameSchema.ValidateDiagFunc = func(i interface{}, p cty.Path) diag.Diagnostics {
if v, ok := i.(string); ok {
if "admins" == strings.ToLower(v) {
if strings.ToLower(v) == "admins" {
return diag.Diagnostics{
{
Summary: "It is not possible to restrict any permissions from `admins`.",
Expand Down
8 changes: 4 additions & 4 deletions access/resource_permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func TestResourcePermissionsCreate_no_access_control(t *testing.T) {
State: map[string]interface{}{
"cluster_id": "abc",
},
}.ExpectError(t, "Invalid config supplied. [access_control] Missing required argument")
}.ExpectError(t, "invalid config supplied. [access_control] Missing required argument")
}

func TestResourcePermissionsCreate_conflicting_fields(t *testing.T) {
Expand All @@ -338,7 +338,7 @@ func TestResourcePermissionsCreate_conflicting_fields(t *testing.T) {
},
},
}.Apply(t)
qa.AssertErrorStartsWith(t, err, "Invalid config supplied. cluster_id: conflicts with notebook_path. notebook_path: conflicts with cluster_id")
qa.AssertErrorStartsWith(t, err, "invalid config supplied. cluster_id: conflicts with notebook_path. notebook_path: conflicts with cluster_id")
}

func TestResourcePermissionsCreate_AdminsThrowError(t *testing.T) {
Expand All @@ -354,7 +354,7 @@ func TestResourcePermissionsCreate_AdminsThrowError(t *testing.T) {
}
`,
}.Apply(t)
assert.EqualError(t, err, "Invalid config supplied. [access_control] "+
assert.EqualError(t, err, "invalid config supplied. [access_control] "+
"It is not possible to restrict any permissions from `admins`.")
}

Expand Down Expand Up @@ -716,7 +716,7 @@ func TestResourcePermissionsUpdate(t *testing.T) {
func permissionsTestHelper(t *testing.T,
cb func(permissionsAPI PermissionsAPI, user, group string,
ef func(string) PermissionsEntity)) {
if "" == os.Getenv("CLOUD_ENV") {
if os.Getenv("CLOUD_ENV") == "" {
t.Skip("Acceptance tests skipped unless env 'CLOUD_ENV' is set")
}
randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
Expand Down
2 changes: 2 additions & 0 deletions access/resource_secret_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ func (a SecretScopesAPI) Create(s SecretScope) error {
return err
}
if !a.client.IsAzure() {
//lint:ignore ST1005 Azure is a valid capitalized string
return fmt.Errorf("Azure KeyVault is not available")
}
if a.client.AzureAuth.IsClientSecretSet() {
//lint:ignore ST1005 Azure is a valid capitalized string
return fmt.Errorf("Azure KeyVault cannot yet be configured for Service Principal authorization")
}
req.BackendType = "AZURE_KEYVAULT"
Expand Down
2 changes: 0 additions & 2 deletions codecov.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
ignore:
- "vendor/**/*"
- "website/**/*"
- "dist/**/*"
- "examples/**/*"

coverage:
status:
Expand Down
6 changes: 3 additions & 3 deletions common/azure_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ func (aa *AzureAuth) addSpManagementTokenVisitor(r *http.Request, management aut
log.Printf("[DEBUG] Setting 'X-Databricks-Azure-SP-Management-Token' header")
ba, ok := management.(*autorest.BearerAuthorizer)
if !ok {
return fmt.Errorf("Supposed to get BearerAuthorizer, but got %#v", management)
return fmt.Errorf("supposed to get BearerAuthorizer, but got %#v", management)
}
tokenProvider := ba.TokenProvider()
if tokenProvider == nil {
return fmt.Errorf("Token provider is nil")
return fmt.Errorf("token provider is nil")
}
accessToken := tokenProvider.OAuthToken()
if accessToken == "" {
Expand Down Expand Up @@ -280,7 +280,7 @@ func (aa *AzureAuth) ensureWorkspaceURL(ctx context.Context,
}
resourceID := aa.resourceID()
if resourceID == "" {
return fmt.Errorf("Somehow resource id is not set")
return fmt.Errorf("somehow resource id is not set")
}
log.Println("[DEBUG] Getting Workspace ID via management token.")
env, err := aa.getAzureEnvironment()
Expand Down
6 changes: 3 additions & 3 deletions common/azure_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestAddSpManagementTokenVisitor(t *testing.T) {
aa := AzureAuth{}
r := httptest.NewRequest("GET", "/a/b/c", http.NoBody)
err := aa.addSpManagementTokenVisitor(r, &autorest.BearerAuthorizer{})
assert.EqualError(t, err, "Token provider is nil")
assert.EqualError(t, err, "token provider is nil")
}

func TestAddSpManagementTokenVisitor_Refreshed(t *testing.T) {
Expand Down Expand Up @@ -75,7 +75,7 @@ func TestAddSpManagementTokenVisitor_RefreshedError(t *testing.T) {
refreshMinutes: 6,
}
err := aa.addSpManagementTokenVisitor(r, autorest.NewBearerAuthorizer(&rct))
require.EqualError(t, err, "Cannot get access token: This is just a failing script.\n")
require.EqualError(t, err, "cannot get access token: This is just a failing script.\n")

err = aa.addSpManagementTokenVisitor(r, autorest.NewBasicAuthorizer("a", "b"))
require.Error(t, err)
Expand All @@ -102,7 +102,7 @@ func TestEnsureWorkspaceURL_CornerCases(t *testing.T) {

aa.databricksClient = &DatabricksClient{}
err = aa.ensureWorkspaceURL(context.Background(), nil)
assert.EqualError(t, err, "Somehow resource id is not set")
assert.EqualError(t, err, "somehow resource id is not set")

aa = AzureAuth{
Environment: "xyz",
Expand Down
8 changes: 4 additions & 4 deletions common/azure_cli_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ func (rct *refreshableCliToken) RefreshExchangeWithContext(ctx context.Context,
func (rct *refreshableCliToken) refreshInternal(resource string) (err error) {
out, err := exec.Command("az", "account", "get-access-token", "--resource", resource).Output()
if ee, ok := err.(*exec.ExitError); ok {
err = fmt.Errorf("Cannot get access token: %s", string(ee.Stderr))
err = fmt.Errorf("cannot get access token: %s", string(ee.Stderr))
return
}
if err != nil {
err = fmt.Errorf("Cannot get access token: %v", err)
err = fmt.Errorf("cannot get access token: %v", err)
return
}
var cliToken cli.Token
Expand Down Expand Up @@ -109,8 +109,8 @@ func (aa *AzureAuth) configureWithAzureCLI() (func(r *http.Request) error, error
_, err := cli.GetTokenFromCLI(AzureDatabricksResourceID)
if err != nil {
if err.Error() == "Invoking Azure CLI failed with the following error: " {
return nil, fmt.Errorf("Most likely Azure CLI is not installed. " +
"See https://docs.microsoft.com/en-us/cli/azure/?view=azure-cli-latest for details.")
return nil, fmt.Errorf("most likely Azure CLI is not installed. " +
"See https://docs.microsoft.com/en-us/cli/azure/?view=azure-cli-latest for details")
}
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions common/azure_cli_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestInternalRefresh_ExitError(t *testing.T) {
lock: &sync.RWMutex{},
}
err := rct.refreshInternal("a")
assert.EqualError(t, err, "Cannot get access token: This is just a failing script.\n")
assert.EqualError(t, err, "cannot get access token: This is just a failing script.\n")
}

func TestInternalRefresh_OtherError(t *testing.T) {
Expand All @@ -135,7 +135,7 @@ func TestInternalRefresh_OtherError(t *testing.T) {
lock: &sync.RWMutex{},
}
err := rct.refreshInternal("a")
assert.EqualError(t, err, "Cannot get access token: exec: \"az\": executable file not found in $PATH")
assert.EqualError(t, err, "cannot get access token: exec: \"az\": executable file not found in $PATH")
}

func TestInternalRefresh_Corrupt(t *testing.T) {
Expand Down Expand Up @@ -239,7 +239,7 @@ func TestConfigureWithAzureCLI_NotInstalled(t *testing.T) {

_, err := client.AzureAuth.configureWithAzureCLI()
require.Error(t, err)
assert.True(t, strings.HasPrefix(err.Error(), "Most likely Azure CLI is not installed"),
assert.True(t, strings.HasPrefix(err.Error(), "most likely Azure CLI is not installed"),
"Actual message: %s", err.Error())
}

Expand All @@ -249,6 +249,6 @@ func TestCliAuthorizer_Error(t *testing.T) {
aa := AzureAuth{}
_, err := aa.cliAuthorizer("x")
require.Error(t, err)
assert.True(t, strings.HasPrefix(err.Error(), `Cannot get access token: exec: "az"`),
assert.True(t, strings.HasPrefix(err.Error(), `cannot get access token: exec: "az"`),
"Actual message: %s", err.Error())
}
4 changes: 2 additions & 2 deletions common/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (c *DatabricksClient) Authenticate() error {
c.fixHost()
return nil
}
return fmt.Errorf("Authentication is not configured for provider. Please configure it\n" +
return fmt.Errorf("authentication is not configured for provider. Please configure it\n" +
"through one of the following options:\n" +
"1. DATABRICKS_HOST + DATABRICKS_TOKEN environment variables.\n" +
"2. host + token provider arguments.\n" +
Expand Down Expand Up @@ -120,7 +120,7 @@ func (c *DatabricksClient) configureAuthWithDirectParams() (func(r *http.Request
needsHostBecause = "token"
}
if needsHostBecause != "" && c.Host == "" {
return nil, fmt.Errorf("Host is empty, but is required by %s", needsHostBecause)
return nil, fmt.Errorf("host is empty, but is required by %s", needsHostBecause)
}
if c.Token == "" || c.Host == "" {
return nil, nil
Expand Down
6 changes: 3 additions & 3 deletions common/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestDatabricksClientConfigure_Nothing(t *testing.T) {
os.Setenv("PATH", "testdata:/bin")

_, err := configureAndAuthenticate(&DatabricksClient{})
AssertErrorStartsWith(t, err, "Authentication is not configured for provider")
AssertErrorStartsWith(t, err, "authentication is not configured for provider")
}

func TestDatabricksClientConfigure_BasicAuth_NoHost(t *testing.T) {
Expand All @@ -34,7 +34,7 @@ func TestDatabricksClientConfigure_BasicAuth_NoHost(t *testing.T) {
Password: "bar",
})

AssertErrorStartsWith(t, err, "Host is empty, but is required by basic_auth")
AssertErrorStartsWith(t, err, "host is empty, but is required by basic_auth")
assert.Equal(t, "Zm9vOmJhcg==", dc.Token)
}

Expand Down Expand Up @@ -65,7 +65,7 @@ func TestDatabricksClientConfigure_Token_NoHost(t *testing.T) {
Token: "dapi345678",
})

AssertErrorStartsWith(t, err, "Host is empty, but is required by token")
AssertErrorStartsWith(t, err, "host is empty, but is required by token")
assert.Equal(t, "dapi345678", dc.Token)
}

Expand Down
6 changes: 3 additions & 3 deletions common/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (c *DatabricksClient) unmarshall(path string, body []byte, response interfa

func (c *DatabricksClient) api2(r *http.Request) error {
if r.URL == nil {
return fmt.Errorf("No URL found in request")
return fmt.Errorf("no URL found in request")
}
r.URL.Path = fmt.Sprintf("/api/2.0%s", r.URL.Path)
r.Header.Set("Content-Type", "application/json")
Expand All @@ -302,7 +302,7 @@ func (c *DatabricksClient) api2(r *http.Request) error {

func (c *DatabricksClient) api12(r *http.Request) error {
if r.URL == nil {
return fmt.Errorf("No URL found in request")
return fmt.Errorf("no URL found in request")
}
r.URL.Path = fmt.Sprintf("/api/1.2%s", r.URL.Path)
r.Header.Set("Content-Type", "application/json")
Expand Down Expand Up @@ -499,7 +499,7 @@ func makeRequestBody(method string, requestURL *string, data interface{}, marsha
}
*requestURL += "?" + params.Encode()
default:
return requestBody, fmt.Errorf("Unsupported request data: %#v", data)
return requestBody, fmt.Errorf("unsupported request data: %#v", data)
}
} else {
if marshalJSON {
Expand Down
Loading