From 97d00e12d0ab5fd6a767d347ab8f7929cde47330 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 7 Oct 2026 23:24:06 +0200 Subject: [PATCH 1/5] structpath: build computed paths and patterns from parts; lint rule Co-authored-by: Isaac --- .agents/rules/style-guide-go.md | 4 +++ .../mutator/paths/artifact_paths_visitor.go | 4 +-- .../paths/job_libraries_paths_visitor.go | 10 +++--- .../config/mutator/paths/job_paths_visitor.go | 23 ++++++++------ .../mutator/paths/pipeline_paths_visitor.go | 10 +++--- .../mutator/resolve_variable_references.go | 3 +- .../apply_bundle_permissions.go | 2 +- bundle/config/mutator/rewrite_sync_paths.go | 2 +- .../config/validate/validate_volume_path.go | 13 ++++---- bundle/libraries/remote_path.go | 16 +++++----- bundle/libraries/same_name_libraries.go | 16 +++++----- libs/gorules/rule_structpath_parse.go | 26 ++++++++++++++++ libs/structs/structpath/path.go | 31 +++++++++++++++++++ libs/structs/structpath/path_test.go | 10 ++++++ 14 files changed, 122 insertions(+), 48 deletions(-) create mode 100644 libs/gorules/rule_structpath_parse.go diff --git a/.agents/rules/style-guide-go.md b/.agents/rules/style-guide-go.md index bedd86132ca..07847641fdc 100644 --- a/.agents/rules/style-guide-go.md +++ b/.agents/rules/style-guide-go.md @@ -117,6 +117,10 @@ return fieldPaths **RULE: Be careful with `encoding/csv` `Writer.UseCRLF = true`.** It rewrites both record terminators AND embedded newlines inside quoted fields to `\r\n`, so tests for quoted multiline fields must expect `\r\n`, not just the line endings between rows. +### Structpath + +**RULE: Build computed structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern`); never format or concatenate a string to parse it.** `MustParsePath`/`MustParsePattern`/`MustParsePaths` are for string literals and tests; `ParsePath` with the error handled is for user input. The `NoComputedStructpathParse` ruleguard rule enforces this. + ### Environment variables **RULE: In library and product code, use `github.com/databricks/cli/libs/env` for reading environment variables, not `os.Getenv`.** `env.Get(ctx, name)` and `env.Lookup(ctx, name)` can be overridden per-context in tests, so you don't have to mutate process-wide state to exercise a code path. `os.Getenv` is still fine in `main`, tests, and acceptance/integration harnesses where no `ctx` is available and overrides aren't needed. diff --git a/bundle/config/mutator/paths/artifact_paths_visitor.go b/bundle/config/mutator/paths/artifact_paths_visitor.go index 4f934e42ba8..017d8d1835a 100644 --- a/bundle/config/mutator/paths/artifact_paths_visitor.go +++ b/bundle/config/mutator/paths/artifact_paths_visitor.go @@ -12,12 +12,12 @@ type artifactRewritePattern struct { func artifactRewritePatterns() []artifactRewritePattern { // Base pattern to match all artifacts. - base := "artifacts.*" + base := structpath.NewPattern(nil, "artifacts", structpath.AnyKey) // Compile list of configuration paths to rewrite. return []artifactRewritePattern{ { - pattern: structpath.MustParsePattern(base + ".path"), + pattern: structpath.NewPattern(base, "path"), mode: TranslateModeLocalAbsoluteDirectory, }, } diff --git a/bundle/config/mutator/paths/job_libraries_paths_visitor.go b/bundle/config/mutator/paths/job_libraries_paths_visitor.go index e96e136f814..0e20ac565c0 100644 --- a/bundle/config/mutator/paths/job_libraries_paths_visitor.go +++ b/bundle/config/mutator/paths/job_libraries_paths_visitor.go @@ -6,15 +6,15 @@ import ( "github.com/databricks/cli/libs/structs/structvar" ) -func jobTaskLibrariesRewritePatterns(base string) []jobRewritePattern { +func jobTaskLibrariesRewritePatterns(base *structpath.PatternNode) []jobRewritePattern { return []jobRewritePattern{ { - structpath.MustParsePattern(base + ".libraries[*].whl"), + structpath.NewPattern(base, "libraries", structpath.AnyIndex, "whl"), TranslateModeLocalRelative, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".libraries[*].jar"), + structpath.NewPattern(base, "libraries", structpath.AnyIndex, "jar"), TranslateModeLocalRelative, noSkipRewrite, }, @@ -23,7 +23,7 @@ func jobTaskLibrariesRewritePatterns(base string) []jobRewritePattern { func jobLibrariesRewritePatterns() []jobRewritePattern { // Base pattern to match all tasks in all jobs. - base := "resources.jobs.*.tasks[*]" + base := jobTasksPattern // Compile list of patterns and their respective rewrite functions. jobEnvironmentsPatterns := []jobRewritePattern{ @@ -48,7 +48,7 @@ func jobLibrariesRewritePatterns() []jobRewritePattern { } taskPatterns := jobTaskLibrariesRewritePatterns(base) - forEachPatterns := jobTaskLibrariesRewritePatterns(base + ".for_each_task.task") + forEachPatterns := jobTaskLibrariesRewritePatterns(structpath.NewPattern(base, "for_each_task", "task")) allPatterns := append(taskPatterns, jobEnvironmentsPatterns...) allPatterns = append(allPatterns, jobEnvironmentsWithRequirementsPatterns...) allPatterns = append(allPatterns, forEachPatterns...) diff --git a/bundle/config/mutator/paths/job_paths_visitor.go b/bundle/config/mutator/paths/job_paths_visitor.go index 9b8c3ea5a94..9d643cf0080 100644 --- a/bundle/config/mutator/paths/job_paths_visitor.go +++ b/bundle/config/mutator/paths/job_paths_visitor.go @@ -11,39 +11,42 @@ type jobRewritePattern struct { skipRewrite func(string) bool } +// jobTasksPattern matches all tasks in all jobs. +var jobTasksPattern = structpath.NewPattern(nil, "resources", "jobs", structpath.AnyKey, "tasks", structpath.AnyIndex) + func noSkipRewrite(string) bool { return false } -func jobTaskRewritePatterns(base string) []jobRewritePattern { +func jobTaskRewritePatterns(base *structpath.PatternNode) []jobRewritePattern { return []jobRewritePattern{ { - structpath.MustParsePattern(base + ".notebook_task.notebook_path"), + structpath.NewPattern(base, "notebook_task", "notebook_path"), TranslateModeNotebook, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".spark_python_task.python_file"), + structpath.NewPattern(base, "spark_python_task", "python_file"), TranslateModeFile, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".dbt_task.project_directory"), + structpath.NewPattern(base, "dbt_task", "project_directory"), TranslateModeDirectory, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".sql_task.file.path"), + structpath.NewPattern(base, "sql_task", "file", "path"), TranslateModeFile, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".alert_task.workspace_path"), + structpath.NewPattern(base, "alert_task", "workspace_path"), TranslateModeFile, noSkipRewrite, }, { - structpath.MustParsePattern(base + ".libraries[*].requirements"), + structpath.NewPattern(base, "libraries", structpath.AnyIndex, "requirements"), TranslateModeFile, noSkipRewrite, }, @@ -51,7 +54,7 @@ func jobTaskRewritePatterns(base string) []jobRewritePattern { // The AI Runtime task runs this bash script on each node; the backend // reads it as a workspace file, so translate the local path to its // remote (or immutable-snapshot) location like any other file. - structpath.MustParsePattern(base + ".ai_runtime_task.deployments[*].command_path"), + structpath.NewPattern(base, "ai_runtime_task", "deployments", structpath.AnyIndex, "command_path"), TranslateModeFile, noSkipRewrite, }, @@ -60,10 +63,10 @@ func jobTaskRewritePatterns(base string) []jobRewritePattern { func jobRewritePatterns() []jobRewritePattern { // Task-level base pattern for all jobs. - base := "resources.jobs.*.tasks[*]" + base := jobTasksPattern taskPatterns := jobTaskRewritePatterns(base) - forEachPatterns := jobTaskRewritePatterns(base + ".for_each_task.task") + forEachPatterns := jobTaskRewritePatterns(structpath.NewPattern(base, "for_each_task", "task")) patterns := append(taskPatterns, forEachPatterns...) return append(patterns, jobRewritePattern{ diff --git a/bundle/config/mutator/paths/pipeline_paths_visitor.go b/bundle/config/mutator/paths/pipeline_paths_visitor.go index ff50c637db8..23b7c171462 100644 --- a/bundle/config/mutator/paths/pipeline_paths_visitor.go +++ b/bundle/config/mutator/paths/pipeline_paths_visitor.go @@ -16,28 +16,28 @@ type pipelineRewritePattern struct { } // Base pattern to match all libraries in all pipelines. -var base = "resources.pipelines.*" +var base = structpath.NewPattern(nil, "resources", "pipelines", structpath.AnyKey) func pipelineRewritePatterns() []pipelineRewritePattern { // Compile list of configuration paths to rewrite. allPatterns := []pipelineRewritePattern{ { - pattern: structpath.MustParsePattern(base + ".libraries[*].notebook.path"), + pattern: structpath.NewPattern(base, "libraries", structpath.AnyIndex, "notebook", "path"), mode: TranslateModeNotebook, skipRewrite: noSkipRewrite, }, { - pattern: structpath.MustParsePattern(base + ".libraries[*].file.path"), + pattern: structpath.NewPattern(base, "libraries", structpath.AnyIndex, "file", "path"), mode: TranslateModeFile, skipRewrite: noSkipRewrite, }, { - pattern: structpath.MustParsePattern(base + ".libraries[*].glob.include"), + pattern: structpath.NewPattern(base, "libraries", structpath.AnyIndex, "glob", "include"), mode: TranslateModeGlob, skipRewrite: noSkipRewrite, }, { - pattern: structpath.MustParsePattern(base + ".root_path"), + pattern: structpath.NewPattern(base, "root_path"), mode: TranslateModeDirectory, skipRewrite: noSkipRewrite, }, diff --git a/bundle/config/mutator/resolve_variable_references.go b/bundle/config/mutator/resolve_variable_references.go index f1e779ad832..85babc91d5a 100644 --- a/bundle/config/mutator/resolve_variable_references.go +++ b/bundle/config/mutator/resolve_variable_references.go @@ -47,6 +47,7 @@ var defaultPrefixes = []string{ var artifactPath = structpath.MustParsePath("artifacts") type resolveVariableReferences struct { + // prefixes are top-level config keys. prefixes []string pattern *structpath.PatternNode lookupFn func(structvar.View, *structpath.PathNode, *bundle.Bundle) (structvar.View, error) @@ -169,7 +170,7 @@ func (m *resolveVariableReferences) Validate(ctx context.Context, b *bundle.Bund func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { prefixes := make([]*structpath.PathNode, len(m.prefixes)) for i, prefix := range m.prefixes { - prefixes[i] = structpath.MustParsePath(prefix) + prefixes[i] = structpath.NewPath(nil, prefix) } // The path ${var.foo} is a shorthand for ${variables.foo.value}. diff --git a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go index bfc9e373616..96a819c57f1 100644 --- a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go +++ b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go @@ -116,7 +116,7 @@ func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Di slices.Sort(keys) for _, key := range keys { - pattern := structpath.MustParsePattern("resources." + key + ".*") + pattern := structpath.NewPattern(nil, "resources", key, structpath.AnyKey) err = structvar.ForEach(b.Config.View(), pattern, func(p *structpath.PathNode, v structvar.View) error { had := v.Get("permissions").IsValid() diff --git a/bundle/config/mutator/rewrite_sync_paths.go b/bundle/config/mutator/rewrite_sync_paths.go index c4eefebae2e..91a3451e4b6 100644 --- a/bundle/config/mutator/rewrite_sync_paths.go +++ b/bundle/config/mutator/rewrite_sync_paths.go @@ -48,7 +48,7 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string, v structvar.View) (string func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { rewrite := func(field string, toSlash bool) error { - pattern := structpath.MustParsePattern("sync." + field + "[*]") + pattern := structpath.NewPattern(nil, "sync", field, structpath.AnyIndex) return structvar.ForEach(b.Config.View(), pattern, func(p *structpath.PathNode, v structvar.View) error { path, err := m.makeRelativeTo(b.BundleRootPath, v) if err != nil { diff --git a/bundle/config/validate/validate_volume_path.go b/bundle/config/validate/validate_volume_path.go index c440dfbf06f..802034bbbee 100644 --- a/bundle/config/validate/validate_volume_path.go +++ b/bundle/config/validate/validate_volume_path.go @@ -23,13 +23,12 @@ func (m *validateVolumePath) Apply(ctx context.Context, b *bundle.Bundle) diag.D // Define paths to check and their corresponding config field names pathChecks := []struct { path string - configName string configPath *structpath.PathNode }{ - {b.Config.Workspace.RootPath, "workspace.root_path", structpath.NewPath(nil, "workspace", "root_path")}, - {b.Config.Workspace.FilePath, "workspace.file_path", structpath.NewPath(nil, "workspace", "file_path")}, - {b.Config.Workspace.StatePath, "workspace.state_path", structpath.NewPath(nil, "workspace", "state_path")}, - {b.Config.Workspace.ResourcePath, "workspace.resource_path", structpath.NewPath(nil, "workspace", "resource_path")}, + {b.Config.Workspace.RootPath, structpath.NewPath(nil, "workspace", "root_path")}, + {b.Config.Workspace.FilePath, structpath.NewPath(nil, "workspace", "file_path")}, + {b.Config.Workspace.StatePath, structpath.NewPath(nil, "workspace", "state_path")}, + {b.Config.Workspace.ResourcePath, structpath.NewPath(nil, "workspace", "resource_path")}, } // Check each path @@ -37,14 +36,14 @@ func (m *validateVolumePath) Apply(ctx context.Context, b *bundle.Bundle) diag.D if check.path != "" && strings.HasPrefix(check.path, "/Volumes/") { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, - Summary: fmt.Sprintf("%s %s starts with /Volumes. /Volumes can only be used with workspace.artifact_path.", check.configName, check.path), + Summary: fmt.Sprintf("%s %s starts with /Volumes. /Volumes can only be used with workspace.artifact_path.", check.configPath, check.path), Detail: "For more information, see https://docs.databricks.com/aws/en/dev-tools/bundles/settings#workspace", Locations: b.Config.GetLocationsOf(check.configPath), Paths: []*structpath.PathNode{check.configPath}, }) // Return early for root path validation - if check.configName == "workspace.root_path" { + if check.configPath.String() == "workspace.root_path" { return diags } } diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go index d28e21c10c1..9c890d98d4a 100644 --- a/bundle/libraries/remote_path.go +++ b/bundle/libraries/remote_path.go @@ -131,14 +131,14 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]LocationToUpdate, err libs := make(map[string]([]LocationToUpdate)) patterns := []*structpath.PatternNode{ - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(envDepsPattern.String() + "[*]"), - structpath.MustParsePattern(pipelineEnvDepsPattern.String() + "[*]"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(envDepsPattern, structpath.AnyIndex), + structpath.NewPattern(pipelineEnvDepsPattern, structpath.AnyIndex), // The AI Runtime task's code_source_path is a local archive (typically an // artifact-built .tar.gz) that must be uploaded and referenced by its remote // path, exactly like a wheel or jar library. diff --git a/bundle/libraries/same_name_libraries.go b/bundle/libraries/same_name_libraries.go index a2ba9591a4f..3dcb4ba03b0 100644 --- a/bundle/libraries/same_name_libraries.go +++ b/bundle/libraries/same_name_libraries.go @@ -14,14 +14,14 @@ import ( type checkForSameNameLibraries struct{} var patterns = []*structpath.PatternNode{ - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(taskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(forEachTaskLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].whl"), - structpath.MustParsePattern(clusterLibrariesPattern.String() + "[*].jar"), - structpath.MustParsePattern(envDepsPattern.String() + "[*]"), - structpath.MustParsePattern(pipelineEnvDepsPattern.String() + "[*]"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(taskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(forEachTaskLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "whl"), + structpath.NewPattern(clusterLibrariesPattern, structpath.AnyIndex, "jar"), + structpath.NewPattern(envDepsPattern, structpath.AnyIndex), + structpath.NewPattern(pipelineEnvDepsPattern, structpath.AnyIndex), } type libData struct { diff --git a/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go new file mode 100644 index 00000000000..672aabd5a73 --- /dev/null +++ b/libs/gorules/rule_structpath_parse.go @@ -0,0 +1,26 @@ +package gorules + +import "github.com/quasilyte/go-ruleguard/dsl" + +// NoComputedStructpathParse forbids parsing computed structpath strings in production +// code. Parsing re-validates the string, can panic on user keys with special characters +// and costs allocations; build the path from parts with structpath.NewPath, +// NewPathSlice or NewPattern instead. Parsing string literals and parsing in tests is fine. +func NoComputedStructpathParse(m dsl.Matcher) { + m.Match(`structpath.MustParsePath($s)`, `structpath.MustParsePattern($s)`). + Where(!m["s"].Const && !m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). + Report(`build computed paths with structpath.NewPath / NewPattern instead of parsing them`) + + m.Match(`structpath.MustParsePaths($*_, $s, $*_)`). + Where(!m["s"].Const && !m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). + Report(`build computed paths with structpath.NewPathSlice instead of parsing them`) + + m.Match( + `structpath.ParsePath(fmt.Sprintf($*_))`, + `structpath.ParsePath($a + $b)`, + `structpath.ParsePattern(fmt.Sprintf($*_))`, + `structpath.ParsePattern($a + $b)`, + ). + Where(!m.File().Name.Matches(`_test\.go$`)). + Report(`build the path with structpath.NewPath / NewPattern instead of formatting and parsing a string`) +} diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index e87b35afadc..2d02187b58d 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -369,6 +369,37 @@ func NewPatternBracketStar(prev *PatternNode) *PatternNode { }) } +// wildcard is a NewPattern part that matches any key or element. +type wildcard int + +// Wildcards for [NewPattern]: AnyKey matches any map key or field (rendered ".*") and +// AnyIndex matches any sequence element (rendered "[*]"). +const ( + AnyKey wildcard = tagDotStar + AnyIndex wildcard = tagBracketStar +) + +// NewPattern appends parts to prev like [NewPath]; AnyKey and AnyIndex append wildcards. +func NewPattern(prev *PatternNode, parts ...any) *PatternNode { + for _, part := range parts { + switch v := part.(type) { + case string: + prev = NewPatternStringKey(prev, v) + case int: + prev = NewPatternIndex(prev, v) + case wildcard: + if v == AnyKey { + prev = NewPatternDotStar(prev) + } else { + prev = NewPatternBracketStar(prev) + } + default: + panic(fmt.Sprintf("structpath.NewPattern: unsupported part %#v", part)) + } + } + return prev +} + func NewPatternKeyValue(prev *PatternNode, key, value string) *PatternNode { return (*PatternNode)(NewKeyValue((*PathNode)(prev), key, value)) } diff --git a/libs/structs/structpath/path_test.go b/libs/structs/structpath/path_test.go index 4e9901e6a46..6901d8381e1 100644 --- a/libs/structs/structpath/path_test.go +++ b/libs/structs/structpath/path_test.go @@ -1304,3 +1304,13 @@ func TestNewPathSlice(t *testing.T) { assert.Equal(t, "resources.jobs['${var.env}_job']", paths[0].String()) assert.Equal(t, "sync.paths[3]", NewPathSlice("sync", "paths", 3)[0].String()) } + +func TestNewPattern(t *testing.T) { + p := NewPattern(nil, "resources", "jobs", AnyKey, "tasks", AnyIndex, "libraries") + assert.Equal(t, MustParsePattern("resources.jobs.*.tasks[*].libraries").String(), p.String()) + + ext := NewPattern(p, AnyIndex, "whl", 0) + assert.Equal(t, "resources.jobs.*.tasks[*].libraries[*].whl[0]", ext.String()) + + assert.Panics(t, func() { NewPattern(nil, 1.5) }) +} From af44f19e392c76268211082853d07bc983e7b2bc Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 8 Oct 2026 13:48:43 +0200 Subject: [PATCH 2/5] config: GetLocationsOf/GetLocationOf/DefinitionLocationOf for path nodes Co-authored-by: Isaac --- .agents/rules/style-guide-go.md | 2 +- bundle/artifacts/prepare.go | 5 +++-- bundle/config/loader/process_root_includes.go | 3 ++- .../mutator/resolve_job_run_file_triggers.go | 21 ++++++++++--------- .../model_serving_endpoint_fixups.go | 5 +++-- .../mutator/validate_job_run_triggers.go | 15 ++++++------- bundle/config/root.go | 7 ++++++- bundle/deploy/metadata/compute.go | 7 ++++--- libs/gorules/rule_structpath_parse.go | 14 +++++++++++++ 9 files changed, 52 insertions(+), 27 deletions(-) diff --git a/.agents/rules/style-guide-go.md b/.agents/rules/style-guide-go.md index 07847641fdc..ad5c87c4888 100644 --- a/.agents/rules/style-guide-go.md +++ b/.agents/rules/style-guide-go.md @@ -119,7 +119,7 @@ return fieldPaths ### Structpath -**RULE: Build computed structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern`); never format or concatenate a string to parse it.** `MustParsePath`/`MustParsePattern`/`MustParsePaths` are for string literals and tests; `ParsePath` with the error handled is for user input. The `NoComputedStructpathParse` ruleguard rule enforces this. +**RULE: Build computed structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern`); never format or concatenate a string to parse it (for location lookups pass the node to `GetLocationsOf`).** `MustParsePath`/`MustParsePattern`/`MustParsePaths` are for string literals and tests; `ParsePath` with the error handled is for user input. The `NoComputedStructpathParse` ruleguard rule enforces this. ### Environment variables diff --git a/bundle/artifacts/prepare.go b/bundle/artifacts/prepare.go index d5330208eeb..bfd836121fa 100644 --- a/bundle/artifacts/prepare.go +++ b/bundle/artifacts/prepare.go @@ -17,6 +17,7 @@ import ( "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/logdiag" "github.com/databricks/cli/libs/python" + "github.com/databricks/cli/libs/structs/structpath" ) func Prepare() bundle.Mutator { @@ -38,7 +39,7 @@ func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for _, artifactName := range slices.Sorted(maps.Keys(b.Config.Artifacts)) { artifact := b.Config.Artifacts[artifactName] if artifact == nil { - l := b.Config.GetLocation("artifacts." + artifactName) + l := b.Config.GetLocationOf(structpath.NewPath(nil, "artifacts", artifactName)) logdiag.LogDiag(ctx, diag.Diagnostic{ Severity: diag.Error, Summary: "Artifact not properly configured", @@ -61,7 +62,7 @@ func (m *prepare) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics logdiag.LogError(ctx, fmt.Errorf("artifact %q: a tgz artifact needs a `files` entry naming the output path", artifactName)) } - l := b.Config.DefinitionLocation("artifacts." + artifactName) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "artifacts", artifactName)) dirPath := filepath.Dir(l.File) // Check if source paths are absolute, if not, make them absolute diff --git a/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 36166fa8512..db942390c73 100644 --- a/bundle/config/loader/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/logdiag" + "github.com/databricks/cli/libs/structs/structpath" ) type processRootIncludes struct{} @@ -112,7 +113,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. Summary: "Files in the 'include' configuration section must be YAML or JSON files.", Detail: fmt.Sprintf("The file %s in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead.", rel), // The match's index within the glob is unrelated to the entry's position in the include list. - Locations: b.Config.GetLocations(fmt.Sprintf("include[%d]", entryIndex)), + Locations: b.Config.GetLocationsOf(structpath.NewPath(nil, "include", entryIndex)), }) continue } diff --git a/bundle/config/mutator/resolve_job_run_file_triggers.go b/bundle/config/mutator/resolve_job_run_file_triggers.go index 35cffd0dcbe..3e097afbb61 100644 --- a/bundle/config/mutator/resolve_job_run_file_triggers.go +++ b/bundle/config/mutator/resolve_job_run_file_triggers.go @@ -15,6 +15,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" libsync "github.com/databricks/cli/libs/sync" ) @@ -65,7 +66,7 @@ func (*resolveJobRunFileTriggers) Apply(ctx context.Context, b *bundle.Bundle) d if t.OnFileChange == nil { continue } - path := fmt.Sprintf("resources.job_runs.%s.lifecycle.triggers[%d].on_file_change", name, i) + path := structpath.NewPath(nil, "resources", "job_runs", name, "lifecycle", "triggers", i, "on_file_change") pattern, fingerprint, d := resolveFileTrigger(b, path, *t.OnFileChange, syncable) diags = diags.Extend(d) if !d.HasError() { @@ -105,7 +106,7 @@ func listSyncableRelPaths(ctx context.Context, b *bundle.Bundle) ([]string, erro return out, nil } -func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string) (string, string, diag.Diagnostics) { +func resolveFileTrigger(b *bundle.Bundle, loc *structpath.PathNode, pattern string, syncable []string) (string, string, diag.Diagnostics) { relPattern, diags := validateFileTriggerPattern(b, loc, pattern) if diags.HasError() { return "", "", diags @@ -119,7 +120,7 @@ func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("invalid pattern %q: %s", pattern, err), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) continue } @@ -131,7 +132,7 @@ func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("hash %q: %s", rel, err), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) continue } @@ -145,20 +146,20 @@ func resolveFileTrigger(b *bundle.Bundle, loc, pattern string, syncable []string diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fileTriggerPrefix + fmt.Sprintf("no synced files match %q", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } return relPattern, hex.EncodeToString(h.Sum(nil)), diags } -func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, diag.Diagnostics) { +func validateFileTriggerPattern(b *bundle.Bundle, loc *structpath.PathNode, pattern string) (string, diag.Diagnostics) { var diags diag.Diagnostics // A double star looks recursive but path.Match treats it as two ordinary stars. if strings.Contains(pattern, "**") { return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("** in %q is not supported; use * for a single directory level", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } // Reject a genuinely absolute path; Join would otherwise silently reinterpret it @@ -170,7 +171,7 @@ func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("pattern %q must be relative to the defining YAML file", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } // NormalizePaths has already rewritten YAML-relative globs to be bundle-root @@ -182,7 +183,7 @@ func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("pattern %q is not under the sync root", pattern), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } relPattern = filepath.ToSlash(relPattern) @@ -191,7 +192,7 @@ func validateFileTriggerPattern(b *bundle.Bundle, loc, pattern string) (string, return "", diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: fileTriggerPrefix + fmt.Sprintf("invalid pattern %q: %s", pattern, err), - Locations: b.Config.GetLocations(loc), + Locations: b.Config.GetLocationsOf(loc), }) } return relPattern, diags diff --git a/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go b/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go index 6c25146e6bd..47ad68d9bcc 100644 --- a/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go +++ b/bundle/config/mutator/resourcemutator/model_serving_endpoint_fixups.go @@ -5,6 +5,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go/service/serving" ) @@ -56,7 +57,7 @@ func (m *modelServingEndpointFixups) Apply(ctx context.Context, b *bundle.Bundle Summary: "Cannot use both served_models and served_entities", Detail: "Model serving endpoint cannot specify both served_models and served_entities at the same time.", Locations: []diag.Location{ - b.Config.GetLocation("resources.model_serving_endpoints." + key), + b.Config.GetLocationOf(structpath.NewPath(nil, "resources", "model_serving_endpoints", key)), }, }) continue @@ -71,7 +72,7 @@ func (m *modelServingEndpointFixups) Apply(ctx context.Context, b *bundle.Bundle Summary: "Using served_models is deprecated", Detail: "The served_models field is deprecated. Please use served_entities instead.", Locations: []diag.Location{ - b.Config.GetLocation("resources.model_serving_endpoints." + key + ".config.served_models"), + b.Config.GetLocationOf(structpath.NewPath(nil, "resources", "model_serving_endpoints", key, "config", "served_models")), }, }) diff --git a/bundle/config/mutator/validate_job_run_triggers.go b/bundle/config/mutator/validate_job_run_triggers.go index d33a7263bde..3d9df745261 100644 --- a/bundle/config/mutator/validate_job_run_triggers.go +++ b/bundle/config/mutator/validate_job_run_triggers.go @@ -2,11 +2,11 @@ package mutator import ( "context" - "fmt" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" ) type validateJobRunTriggers struct{} @@ -27,12 +27,12 @@ func (*validateJobRunTriggers) Apply(_ context.Context, b *bundle.Bundle) diag.D continue } for i, t := range jr.Lifecycle.Triggers { - path := fmt.Sprintf("resources.job_runs.%s.lifecycle.triggers[%d]", name, i) + path := structpath.NewPath(nil, "resources", "job_runs", name, "lifecycle", "triggers", i) if t.OnBundleDeploy == nil && t.OnFileChange == nil { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers entry must set on_bundle_deploy or on_file_change", - Locations: b.Config.GetLocations(path), + Locations: b.Config.GetLocationsOf(path), }) continue } @@ -40,7 +40,7 @@ func (*validateJobRunTriggers) Apply(_ context.Context, b *bundle.Bundle) diag.D diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers entry must set only one of on_bundle_deploy or on_file_change", - Locations: b.Config.GetLocations(path), + Locations: b.Config.GetLocationsOf(path), }) continue } @@ -48,20 +48,21 @@ func (*validateJobRunTriggers) Apply(_ context.Context, b *bundle.Bundle) diag.D diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers.on_bundle_deploy must be true when set", - Locations: b.Config.GetLocations(path + ".on_bundle_deploy"), + Locations: b.Config.GetLocationsOf(structpath.NewPath(path, "on_bundle_deploy")), }) } if t.OnFileChange != nil { + onFileChange := structpath.NewPath(path, "on_file_change") if strings.TrimSpace(*t.OnFileChange) == "" { diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "lifecycle.triggers.on_file_change must be non-empty when set", - Locations: b.Config.GetLocations(path + ".on_file_change"), + Locations: b.Config.GetLocationsOf(onFileChange), }) continue } // Report bad patterns at validate time; hashing only runs on deploy. - _, patternDiags := validateFileTriggerPattern(b, path+".on_file_change", *t.OnFileChange) + _, patternDiags := validateFileTriggerPattern(b, onFileChange, *t.OnFileChange) diags = diags.Extend(patternDiags) } } diff --git a/bundle/config/root.go b/bundle/config/root.go index 0ed17499e08..17a363b411d 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -601,7 +601,12 @@ func (r Root) DefinitionLocation(path string) diag.Location { if err != nil { return diag.Location{} } - locs := r.locations.At(p) + return r.DefinitionLocationOf(p) +} + +// DefinitionLocationOf is [Root.DefinitionLocation] for a path node. +func (r Root) DefinitionLocationOf(path *structpath.PathNode) diag.Location { + locs := r.locations.At(path) if len(locs) == 0 { return diag.Location{} } diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index c8775748151..d4db537ded0 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle/metadata" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/structs/structpath" ) type compute struct{} @@ -46,7 +47,7 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for name, job := range b.Config.Resources.Jobs { // Compute config file path the job is defined in, relative to the bundle // root - l := b.Config.DefinitionLocation("resources.jobs." + name) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "resources", "jobs", name)) if l.File == "" { // Skip resources that exist only in the deployment state: statemgmt.Load, // which runs before this mutator, injects them into the config without a @@ -73,7 +74,7 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for name, pipeline := range b.Config.Resources.Pipelines { // Compute config file path the pipeline is defined in, relative to the bundle // root - l := b.Config.DefinitionLocation("resources.pipelines." + name) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "resources", "pipelines", name)) if l.File == "" { // Skip resources that exist only in the deployment state: statemgmt.Load, // which runs before this mutator, injects them into the config without a @@ -98,7 +99,7 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics for name, dashboard := range b.Config.Resources.Dashboards { // Compute config file path the dashboard is defined in, relative to the bundle // root - l := b.Config.DefinitionLocation("resources.dashboards." + name) + l := b.Config.DefinitionLocationOf(structpath.NewPath(nil, "resources", "dashboards", name)) if l.File == "" { // Skip resources that exist only in the deployment state: statemgmt.Load, // which runs before this mutator, injects them into the config without a diff --git a/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go index 672aabd5a73..74b6ca6edd9 100644 --- a/libs/gorules/rule_structpath_parse.go +++ b/libs/gorules/rule_structpath_parse.go @@ -23,4 +23,18 @@ func NoComputedStructpathParse(m dsl.Matcher) { ). Where(!m.File().Name.Matches(`_test\.go$`)). Report(`build the path with structpath.NewPath / NewPattern instead of formatting and parsing a string`) + + m.Match( + `$c.GetLocations($a + $b)`, + `$c.GetLocation($a + $b)`, + `$c.DefinitionLocation($a + $b)`, + `$c.GetLocations(fmt.Sprintf($*_))`, + `$c.GetLocation(fmt.Sprintf($*_))`, + `$c.DefinitionLocation(fmt.Sprintf($*_))`, + `$c.GetLocations($p.String())`, + `$c.GetLocation($p.String())`, + `$c.DefinitionLocation($p.String())`, + ). + Where(!m.File().Name.Matches(`_test\.go$`)). + Report(`pass a *structpath.PathNode to GetLocationsOf / GetLocationOf / DefinitionLocationOf instead of a built string`) } From 7242a563c2b7bdecdd49617911081443250a076b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 9 Oct 2026 13:28:16 +0200 Subject: [PATCH 3/5] lint rule: match structpath by import, check every MustParsePaths argument, type-check String() and receivers Co-authored-by: Isaac --- libs/gorules/rule_structpath_parse.go | 31 +++++++++++++++++++-------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go index 74b6ca6edd9..ba04581b862 100644 --- a/libs/gorules/rule_structpath_parse.go +++ b/libs/gorules/rule_structpath_parse.go @@ -6,14 +6,14 @@ import "github.com/quasilyte/go-ruleguard/dsl" // code. Parsing re-validates the string, can panic on user keys with special characters // and costs allocations; build the path from parts with structpath.NewPath, // NewPathSlice or NewPattern instead. Parsing string literals and parsing in tests is fine. +// Values passed through a variable are not tracked. func NoComputedStructpathParse(m dsl.Matcher) { - m.Match(`structpath.MustParsePath($s)`, `structpath.MustParsePattern($s)`). - Where(!m["s"].Const && !m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). - Report(`build computed paths with structpath.NewPath / NewPattern instead of parsing them`) + m.Import("github.com/databricks/cli/libs/structs/structpath") + m.Import("github.com/databricks/cli/bundle/config") - m.Match(`structpath.MustParsePaths($*_, $s, $*_)`). + m.Match(`structpath.MustParsePath($s)`, `structpath.MustParsePattern($s)`, `structpath.MustParsePaths($*s)`). Where(!m["s"].Const && !m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). - Report(`build computed paths with structpath.NewPathSlice instead of parsing them`) + Report(`build computed paths with structpath.NewPath / NewPathSlice / NewPattern instead of parsing them`) m.Match( `structpath.ParsePath(fmt.Sprintf($*_))`, @@ -24,6 +24,15 @@ func NoComputedStructpathParse(m dsl.Matcher) { Where(!m.File().Name.Matches(`_test\.go$`)). Report(`build the path with structpath.NewPath / NewPattern instead of formatting and parsing a string`) + // Re-parsing a node as its own kind is a no-op; parsing a pattern as a path is fine + // (it rejects wildcards). + m.Match(`structpath.ParsePath($p.String())`). + Where(!m.File().Name.Matches(`_test\.go$`) && m["p"].Type.Is("*structpath.PathNode")). + Report(`use the structpath node instead of rendering and parsing it`) + m.Match(`structpath.ParsePattern($p.String())`). + Where(!m.File().Name.Matches(`_test\.go$`) && m["p"].Type.Is("*structpath.PatternNode")). + Report(`use the structpath node instead of rendering and parsing it`) + m.Match( `$c.GetLocations($a + $b)`, `$c.GetLocation($a + $b)`, @@ -31,10 +40,14 @@ func NoComputedStructpathParse(m dsl.Matcher) { `$c.GetLocations(fmt.Sprintf($*_))`, `$c.GetLocation(fmt.Sprintf($*_))`, `$c.DefinitionLocation(fmt.Sprintf($*_))`, - `$c.GetLocations($p.String())`, - `$c.GetLocation($p.String())`, - `$c.DefinitionLocation($p.String())`, ). - Where(!m.File().Name.Matches(`_test\.go$`)). + Where(!m.File().Name.Matches(`_test\.go$`) && + (m["c"].Type.Is("config.Root") || m["c"].Type.Is("*config.Root"))). Report(`pass a *structpath.PathNode to GetLocationsOf / GetLocationOf / DefinitionLocationOf instead of a built string`) + + m.Match(`$c.GetLocations($p.String())`, `$c.GetLocation($p.String())`, `$c.DefinitionLocation($p.String())`). + Where(!m.File().Name.Matches(`_test\.go$`) && + (m["c"].Type.Is("config.Root") || m["c"].Type.Is("*config.Root")) && + m["p"].Type.Is("*structpath.PathNode")). + Report(`pass the *structpath.PathNode to GetLocationsOf / GetLocationOf / DefinitionLocationOf instead of its string`) } From f1728e783492834c9220eef6f992df09657983ad Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 14 Oct 2026 23:03:49 +0200 Subject: [PATCH 4/5] apply_default_task_source: look up the source definition by path node Co-authored-by: Isaac --- .../apply_default_task_source.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/apply_default_task_source.go b/bundle/config/mutator/resourcemutator/apply_default_task_source.go index df0ca1fec68..4e43b08d258 100644 --- a/bundle/config/mutator/resourcemutator/apply_default_task_source.go +++ b/bundle/config/mutator/resourcemutator/apply_default_task_source.go @@ -2,10 +2,10 @@ package resourcemutator import ( "context" - "fmt" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/databricks-sdk-go/service/jobs" ) @@ -50,9 +50,9 @@ func (a *applyDefaultTaskSource) Apply(ctx context.Context, b *bundle.Bundle) di for i := range job.Tasks { task := &job.Tasks[i] - taskPath := fmt.Sprintf("resources.jobs.%s.tasks[%d]", name, i) + taskPath := structpath.NewPath(nil, "resources", "jobs", name, "tasks", i) if task.ForEachTask != nil { - setGitTaskSource(b, taskPath+".for_each_task.task", &task.ForEachTask.Task) + setGitTaskSource(b, structpath.NewPath(taskPath, "for_each_task", "task"), &task.ForEachTask.Task) } setGitTaskSource(b, taskPath, task) } @@ -64,32 +64,32 @@ func (a *applyDefaultTaskSource) Apply(ctx context.Context, b *bundle.Bundle) di // supports the field, unless the user already set it. // The task types are the ones that support the `source` field: // https://docs.databricks.com/api/workspace/jobs/create -func setGitTaskSource(b *bundle.Bundle, taskPath string, task *jobs.Task) { +func setGitTaskSource(b *bundle.Bundle, taskPath *structpath.PathNode, task *jobs.Task) { type taskSource struct { - key string + path []any source *jobs.Source } var sources []taskSource if task.DbtTask != nil { - sources = append(sources, taskSource{"dbt_task", &task.DbtTask.Source}) + sources = append(sources, taskSource{[]any{"dbt_task", "source"}, &task.DbtTask.Source}) } if task.GenAiComputeTask != nil { - sources = append(sources, taskSource{"gen_ai_compute_task", &task.GenAiComputeTask.Source}) + sources = append(sources, taskSource{[]any{"gen_ai_compute_task", "source"}, &task.GenAiComputeTask.Source}) } if task.NotebookTask != nil { - sources = append(sources, taskSource{"notebook_task", &task.NotebookTask.Source}) + sources = append(sources, taskSource{[]any{"notebook_task", "source"}, &task.NotebookTask.Source}) } if task.SparkPythonTask != nil { - sources = append(sources, taskSource{"spark_python_task", &task.SparkPythonTask.Source}) + sources = append(sources, taskSource{[]any{"spark_python_task", "source"}, &task.SparkPythonTask.Source}) } if task.SqlTask != nil && task.SqlTask.File != nil { - sources = append(sources, taskSource{"sql_task.file", &task.SqlTask.File.Source}) + sources = append(sources, taskSource{[]any{"sql_task", "file", "source"}, &task.SqlTask.File.Source}) } for _, s := range sources { // An explicit `source: ""` is set by the user, although the typed value is // empty; not every task type has ForceSendFields to record it, but it is // defined in the configuration. - if *s.source != "" || b.Config.DefinitionLocation(taskPath+"."+s.key+".source").File != "" { + if *s.source != "" || b.Config.DefinitionLocationOf(structpath.NewPath(taskPath, s.path...)).File != "" { continue } *s.source = jobs.SourceGit From 0e76b8d1b34e6f0c4c7009f1b5e5ced6b2513e76 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 15 Oct 2026 17:11:23 +0200 Subject: [PATCH 5/5] Build nested patterns with NewPattern; lint against nesting constructors Co-authored-by: Isaac --- bundle/artifacts/expand_globs.go | 2 +- bundle/config/mutator/python/apply_python_output.go | 6 +++--- libs/gorules/rule_structpath_parse.go | 11 +++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/bundle/artifacts/expand_globs.go b/bundle/artifacts/expand_globs.go index 6b1c46a8f43..9b08566eb33 100644 --- a/bundle/artifacts/expand_globs.go +++ b/bundle/artifacts/expand_globs.go @@ -46,7 +46,7 @@ func (e expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnosti base := structpath.NewPath(nil, "artifacts", e.name, "files") // Pattern to match the source key in the files sequence. - pattern := structpath.NewPatternStringKey(structpath.NewPatternBracketStar(structpath.NewPatternStringKey(structpath.NewPatternStringKey(structpath.NewPatternStringKey(nil, "artifacts"), e.name), "files")), "source") + pattern := structpath.NewPattern(nil, "artifacts", e.name, "files", structpath.AnyIndex, "source") artifact := b.Config.Artifacts[e.name] if artifact == nil { diff --git a/bundle/config/mutator/python/apply_python_output.go b/bundle/config/mutator/python/apply_python_output.go index a3b165a1384..5c39f7f3b41 100644 --- a/bundle/config/mutator/python/apply_python_output.go +++ b/bundle/config/mutator/python/apply_python_output.go @@ -70,7 +70,7 @@ func createOverrideVisitor(leftRoot, rightRoot structvar.View) (applyPythonOutpu // rightRoot: {"resources": { "jobs": ...}}}, return deleted.AddPattern( - structpath.NewPatternDotStar(structpath.NewPatternStringKey(structpath.MustParsePattern("resources"), np.KeyAt(1))), + structpath.NewPattern(nil, "resources", np.KeyAt(1), structpath.AnyKey), leftRoot, ) } else if np.Len() == 3 { @@ -113,7 +113,7 @@ func createOverrideVisitor(leftRoot, rightRoot structvar.View) (applyPythonOutpu // rightRoot: {"resources": { "jobs": {...} }} return added.AddPattern( - structpath.NewPatternDotStar(structpath.NewPatternStringKey(structpath.MustParsePattern("resources"), np.KeyAt(1))), + structpath.NewPattern(nil, "resources", np.KeyAt(1), structpath.AnyKey), rightRoot, ) } else if np.Len() == 3 { @@ -160,7 +160,7 @@ func createOverrideVisitor(leftRoot, rightRoot structvar.View) (applyPythonOutpu // rightRoot: {"resources": { "jobs": {...} }} return added.AddPattern( - structpath.NewPatternDotStar(structpath.NewPatternStringKey(structpath.MustParsePattern("resources"), np.KeyAt(1))), + structpath.NewPattern(nil, "resources", np.KeyAt(1), structpath.AnyKey), rightRoot, ) } else if np.Len() == 3 { diff --git a/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go index ba04581b862..72cde2b50aa 100644 --- a/libs/gorules/rule_structpath_parse.go +++ b/libs/gorules/rule_structpath_parse.go @@ -50,4 +50,15 @@ func NoComputedStructpathParse(m dsl.Matcher) { (m["c"].Type.Is("config.Root") || m["c"].Type.Is("*config.Root")) && m["p"].Type.Is("*structpath.PathNode")). Report(`pass the *structpath.PathNode to GetLocationsOf / GetLocationOf / DefinitionLocationOf instead of its string`) + + // Nested single-step constructors are hard to read; NewPath and NewPattern take the + // parts in order (with AnyKey / AnyIndex for wildcards). + m.Match(`structpath.$f(structpath.$g($*_), $*_)`). + Where(m["f"].Text.Matches(`^NewPattern(StringKey|DotString|BracketString|Index|DotStar|BracketStar|KeyValue)$`) && + m["g"].Text.Matches(`^(NewPattern|MustParsePattern)`) && !m.File().PkgPath.Matches(`libs/structs/structpath$`)). + Report(`build the pattern with structpath.NewPattern(prev, parts...) instead of nesting constructors`) + m.Match(`structpath.$f(structpath.$g($*_), $*_)`). + Where(m["f"].Text.Matches(`^New(StringKey|DotString|BracketString|Index|KeyValue)$`) && + m["g"].Text.Matches(`^(New(StringKey|DotString|BracketString|Index|KeyValue|Path)|MustParsePath)$`) && !m.File().PkgPath.Matches(`libs/structs/structpath$`)). + Report(`build the path with structpath.NewPath(prev, parts...) instead of nesting constructors`) }