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

Enable gofumpt and goimports in golangci-lint #1999

Merged
merged 3 commits into from
Dec 12, 2024
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
7 changes: 7 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ linters:
- staticcheck
- unused
- gofmt
- gofumpt
- goimports
linters-settings:
govet:
enable-all: true
Expand All @@ -27,5 +29,10 @@ linters-settings:
- (*github.com/spf13/cobra.Command).MarkFlagRequired
- (*github.com/spf13/pflag.FlagSet).MarkDeprecated
- (*github.com/spf13/pflag.FlagSet).MarkHidden
gofumpt:
module-path: github.com/databricks/cli
extra-rules: true
#goimports:
# local-prefixes: github.com/databricks/cli
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this superfluous?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a suggestion for a followup. If I do this here, it touches every file. We can decide if we want it and apply in follow-up PR.

issues:
exclude-dirs-use-default: false # recommended by docs https://golangci-lint.run/usage/false-positives/
1 change: 0 additions & 1 deletion bundle/artifacts/all.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package artifacts
import (
"context"
"fmt"

"slices"

"github.com/databricks/cli/bundle"
Expand Down
3 changes: 1 addition & 2 deletions bundle/artifacts/autodetect.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ func DetectPackages() bundle.Mutator {
return &autodetect{}
}

type autodetect struct {
}
type autodetect struct{}

func (m *autodetect) Name() string {
return "artifacts.DetectPackages"
Expand Down
1 change: 0 additions & 1 deletion bundle/artifacts/expand_globs.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ func (m *expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnost
// Set the expanded globs back into the configuration.
return dyn.SetByPath(v, base, dyn.V(output))
})

if err != nil {
return diag.FromErr(err)
}
Expand Down
3 changes: 1 addition & 2 deletions bundle/artifacts/whl/autodetect.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ import (
"github.com/databricks/cli/libs/log"
)

type detectPkg struct {
}
type detectPkg struct{}

func DetectPackage() bundle.Mutator {
return &detectPkg{}
Expand Down
4 changes: 2 additions & 2 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error)

// Make directory if it doesn't exist yet.
dir := filepath.Join(parts...)
err := os.MkdirAll(dir, 0700)
err := os.MkdirAll(dir, 0o700)
if err != nil {
return "", err
}
Expand All @@ -203,7 +203,7 @@ func (b *Bundle) InternalDir(ctx context.Context) (string, error) {
}

dir := filepath.Join(cacheDir, internalFolder)
err = os.MkdirAll(dir, 0700)
err = os.MkdirAll(dir, 0o700)
if err != nil {
return dir, err
}
Expand Down
6 changes: 4 additions & 2 deletions bundle/config/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ type PyDABs struct {
Import []string `json:"import,omitempty"`
}

type Command string
type ScriptHook string
type (
Command string
ScriptHook string
)

// These hook names are subject to change and currently experimental
const (
Expand Down
6 changes: 4 additions & 2 deletions bundle/config/generate/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import (
"github.com/databricks/databricks-sdk-go/service/jobs"
)

var jobOrder = yamlsaver.NewOrder([]string{"name", "job_clusters", "compute", "tasks"})
var taskOrder = yamlsaver.NewOrder([]string{"task_key", "depends_on", "existing_cluster_id", "new_cluster", "job_cluster_key"})
var (
jobOrder = yamlsaver.NewOrder([]string{"name", "job_clusters", "compute", "tasks"})
taskOrder = yamlsaver.NewOrder([]string{"task_key", "depends_on", "existing_cluster_id", "new_cluster", "job_cluster_key"})
)

func ConvertJobToValue(job *jobs.Job) (dyn.Value, error) {
value := make(map[string]dyn.Value)
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/loader/process_root_includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
var out []bundle.Mutator

// Map with files we've already seen to avoid loading them twice.
var seen = map[string]bool{}
seen := map[string]bool{}

for _, file := range config.FileNames {
seen[file] = true
Expand Down
1 change: 0 additions & 1 deletion bundle/config/mutator/apply_presets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,5 +481,4 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) {
require.Equal(t, tt.expectedValue, b.Config.Presets.SourceLinkedDeployment)
})
}

}
1 change: 0 additions & 1 deletion bundle/config/mutator/compute_id_compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func rewriteComputeIdToClusterId(v dyn.Value, p dyn.Path) (dyn.Value, diag.Diagn
var diags diag.Diagnostics
computeIdPath := p.Append(dyn.Key("compute_id"))
computeId, err := dyn.GetByPath(v, computeIdPath)

// If the "compute_id" key is not set, we don't need to do anything.
if err != nil {
return v, nil
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/expand_pipeline_glob_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

func touchEmptyFile(t *testing.T, path string) {
err := os.MkdirAll(filepath.Dir(path), 0700)
err := os.MkdirAll(filepath.Dir(path), 0o700)
require.NoError(t, err)
f, err := os.Create(path)
require.NoError(t, err)
Expand Down
5 changes: 2 additions & 3 deletions bundle/config/mutator/initialize_urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import (
"github.com/databricks/cli/libs/diag"
)

type initializeURLs struct {
}
type initializeURLs struct{}

// InitializeURLs makes sure the URL field of each resource is configured.
// NOTE: since this depends on an extra API call, this mutator adds some extra
Expand Down Expand Up @@ -39,7 +38,7 @@ func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
return nil
}

func initializeForWorkspace(b *bundle.Bundle, orgId string, host string) error {
func initializeForWorkspace(b *bundle.Bundle, orgId, host string) error {
baseURL, err := url.Parse(host)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/override_compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (m *overrideCompute) Name() string {

func overrideJobCompute(j *resources.Job, compute string) {
for i := range j.Tasks {
var task = &j.Tasks[i]
task := &j.Tasks[i]

if task.ForEachTask != nil {
task = &task.ForEachTask.Task
Expand Down
3 changes: 1 addition & 2 deletions bundle/config/mutator/paths/job_paths_visitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func jobRewritePatterns() []jobRewritePattern {
// VisitJobPaths visits all paths in job resources and applies a function to each path.
func VisitJobPaths(value dyn.Value, fn VisitFunc) (dyn.Value, error) {
var err error
var newValue = value
newValue := value

for _, rewritePattern := range jobRewritePatterns() {
newValue, err = dyn.MapByPattern(newValue, rewritePattern.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
Expand All @@ -105,7 +105,6 @@ func VisitJobPaths(value dyn.Value, fn VisitFunc) (dyn.Value, error) {

return fn(p, rewritePattern.kind, v)
})

if err != nil {
return dyn.InvalidValue, err
}
Expand Down
2 changes: 0 additions & 2 deletions bundle/config/mutator/prepend_workspace_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,12 @@ func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di

return dyn.NewValue(fmt.Sprintf("/Workspace%s", path), v.Locations()), nil
})

if err != nil {
return dyn.InvalidValue, err
}
}
return v, nil
})

if err != nil {
return diag.FromErr(err)
}
Expand Down
1 change: 0 additions & 1 deletion bundle/config/mutator/python/python_diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ type parsePythonDiagnosticsTest struct {
}

func TestParsePythonDiagnostics(t *testing.T) {

testCases := []parsePythonDiagnosticsTest{
{
name: "short error with location",
Expand Down
18 changes: 8 additions & 10 deletions bundle/config/mutator/python/python_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (
"io"
"os"
"path/filepath"
"strings"

"github.com/databricks/databricks-sdk-go/logger"
"github.com/fatih/color"

"strings"

"github.com/databricks/cli/libs/python"

"github.com/databricks/cli/bundle/env"
Expand Down Expand Up @@ -94,11 +93,10 @@ func (m *pythonMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno

// mutateDiags is used because Mutate returns 'error' instead of 'diag.Diagnostics'
var mutateDiags diag.Diagnostics
var mutateDiagsHasError = errors.New("unexpected error")
mutateDiagsHasError := errors.New("unexpected error")

err := b.Config.Mutate(func(leftRoot dyn.Value) (dyn.Value, error) {
pythonPath, err := detectExecutable(ctx, experimental.PyDABs.VEnvPath)

if err != nil {
return dyn.InvalidValue, fmt.Errorf("failed to get Python interpreter path: %w", err)
}
Expand Down Expand Up @@ -141,7 +139,7 @@ func createCacheDir(ctx context.Context) (string, error) {
// use 'default' as target name
cacheDir := filepath.Join(tempDir, "default", "pydabs")

err := os.MkdirAll(cacheDir, 0700)
err := os.MkdirAll(cacheDir, 0o700)
if err != nil {
return "", err
}
Expand All @@ -152,7 +150,7 @@ func createCacheDir(ctx context.Context) (string, error) {
return os.MkdirTemp("", "-pydabs")
}

func (m *pythonMutator) runPythonMutator(ctx context.Context, cacheDir string, rootPath string, pythonPath string, root dyn.Value) (dyn.Value, diag.Diagnostics) {
func (m *pythonMutator) runPythonMutator(ctx context.Context, cacheDir, rootPath, pythonPath string, root dyn.Value) (dyn.Value, diag.Diagnostics) {
inputPath := filepath.Join(cacheDir, "input.json")
outputPath := filepath.Join(cacheDir, "output.json")
diagnosticsPath := filepath.Join(cacheDir, "diagnostics.json")
Expand Down Expand Up @@ -263,10 +261,10 @@ func writeInputFile(inputPath string, input dyn.Value) error {
return fmt.Errorf("failed to marshal input: %w", err)
}

return os.WriteFile(inputPath, rootConfigJson, 0600)
return os.WriteFile(inputPath, rootConfigJson, 0o600)
}

func loadOutputFile(rootPath string, outputPath string) (dyn.Value, diag.Diagnostics) {
func loadOutputFile(rootPath, outputPath string) (dyn.Value, diag.Diagnostics) {
outputFile, err := os.Open(outputPath)
if err != nil {
return dyn.InvalidValue, diag.FromErr(fmt.Errorf("failed to open output file: %w", err))
Expand Down Expand Up @@ -381,7 +379,7 @@ func createLoadOverrideVisitor(ctx context.Context) merge.OverrideVisitor {

return right, nil
},
VisitUpdate: func(valuePath dyn.Path, left dyn.Value, right dyn.Value) (dyn.Value, error) {
VisitUpdate: func(valuePath dyn.Path, left, right dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, fmt.Errorf("unexpected change at %q (update)", valuePath.String())
},
}
Expand Down Expand Up @@ -430,7 +428,7 @@ func createInitOverrideVisitor(ctx context.Context) merge.OverrideVisitor {

return right, nil
},
VisitUpdate: func(valuePath dyn.Path, left dyn.Value, right dyn.Value) (dyn.Value, error) {
VisitUpdate: func(valuePath dyn.Path, left, right dyn.Value) (dyn.Value, error) {
if !valuePath.HasPrefix(jobsPath) {
return dyn.InvalidValue, fmt.Errorf("unexpected change at %q (update)", valuePath.String())
}
Expand Down
15 changes: 7 additions & 8 deletions bundle/config/mutator/python/python_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ func TestPythonMutator_load(t *testing.T) {
Column: 5,
},
}, diags[0].Locations)

}

func TestPythonMutator_load_disallowed(t *testing.T) {
Expand Down Expand Up @@ -588,7 +587,7 @@ or activate the environment before running CLI commands:
assert.Equal(t, expected, out)
}

func withProcessStub(t *testing.T, args []string, output string, diagnostics string) context.Context {
func withProcessStub(t *testing.T, args []string, output, diagnostics string) context.Context {
ctx := context.Background()
ctx, stub := process.WithStub(ctx)

Expand All @@ -611,10 +610,10 @@ func withProcessStub(t *testing.T, args []string, output string, diagnostics str
assert.NoError(t, err)

if reflect.DeepEqual(actual.Args, args) {
err := os.WriteFile(outputPath, []byte(output), 0600)
err := os.WriteFile(outputPath, []byte(output), 0o600)
require.NoError(t, err)

err = os.WriteFile(diagnosticsPath, []byte(diagnostics), 0600)
err = os.WriteFile(diagnosticsPath, []byte(diagnostics), 0o600)
require.NoError(t, err)

return nil
Expand All @@ -626,7 +625,7 @@ func withProcessStub(t *testing.T, args []string, output string, diagnostics str
return ctx
}

func loadYaml(name string, content string) *bundle.Bundle {
func loadYaml(name, content string) *bundle.Bundle {
v, diag := config.LoadFromBytes(name, []byte(content))

if diag.Error() != nil {
Expand All @@ -650,17 +649,17 @@ func withFakeVEnv(t *testing.T, venvPath string) {

interpreterPath := interpreterPath(venvPath)

err = os.MkdirAll(filepath.Dir(interpreterPath), 0755)
err = os.MkdirAll(filepath.Dir(interpreterPath), 0o755)
if err != nil {
panic(err)
}

err = os.WriteFile(interpreterPath, []byte(""), 0755)
err = os.WriteFile(interpreterPath, []byte(""), 0o755)
if err != nil {
panic(err)
}

err = os.WriteFile(filepath.Join(venvPath, "pyvenv.cfg"), []byte(""), 0755)
err = os.WriteFile(filepath.Join(venvPath, "pyvenv.cfg"), []byte(""), 0o755)
if err != nil {
panic(err)
}
Expand Down
13 changes: 6 additions & 7 deletions bundle/config/mutator/resolve_variable_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ func ResolveVariableReferencesInLookup() bundle.Mutator {
}

func ResolveVariableReferencesInComplexVariables() bundle.Mutator {
return &resolveVariableReferences{prefixes: []string{
"bundle",
"workspace",
"variables",
},
return &resolveVariableReferences{
prefixes: []string{
"bundle",
"workspace",
"variables",
},
pattern: dyn.NewPattern(dyn.Key("variables"), dyn.AnyKey(), dyn.Key("value")),
lookupFn: lookupForComplexVariables,
skipFn: skipResolvingInNonComplexVariables,
Expand Down Expand Up @@ -173,7 +174,6 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
return dyn.InvalidValue, dynvar.ErrSkipResolution
})
})

if err != nil {
return dyn.InvalidValue, err
}
Expand All @@ -184,7 +184,6 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
diags = diags.Extend(normaliseDiags)
return root, nil
})

if err != nil {
diags = diags.Extend(diag.FromErr(err))
}
Expand Down
1 change: 0 additions & 1 deletion bundle/config/mutator/rewrite_workspace_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func (m *rewriteWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di
return v, nil
})
})

if err != nil {
return diag.FromErr(err)
}
Expand Down
1 change: 0 additions & 1 deletion bundle/config/mutator/rewrite_workspace_prefix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,4 @@ func TestNoWorkspacePrefixUsed(t *testing.T) {
require.Equal(t, "${workspace.artifact_path}/jar1.jar", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[1].Libraries[0].Jar)
require.Equal(t, "${workspace.file_path}/notebook2", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[2].NotebookTask.NotebookPath)
require.Equal(t, "${workspace.artifact_path}/jar2.jar", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[2].Libraries[0].Jar)

}
Loading
Loading