diff --git a/.agents/rules/style-guide-go.md b/.agents/rules/style-guide-go.md index bedd86132ca..271466f2eb2 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 structpath paths and patterns from parts (`structpath.NewPath`, `NewPathSlice`, `NewPattern` with `AnyKey` / `AnyIndex`); never parse a string you could build, and never format or concatenate one to parse it.** `MustParsePath` / `MustParsePattern` / `MustParsePaths` are for tests; `ParsePath` with the error handled is for strings that are data (user input, map keys). Location lookups on `config.Root` take the node: `NearestLocations` for diagnostics, `ExactLocations` for where a value is written. 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/libs/gorules/rule_structpath_parse.go b/libs/gorules/rule_structpath_parse.go new file mode 100644 index 00000000000..0f48907af65 --- /dev/null +++ b/libs/gorules/rule_structpath_parse.go @@ -0,0 +1,46 @@ +package gorules + +import "github.com/quasilyte/go-ruleguard/dsl" + +// NoComputedStructpathParse forbids building structpath paths and patterns by parsing +// 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. ParsePath / ParsePattern with +// the error handled remain for strings that are data (user input, map keys). Tests may +// parse. Values passed through a variable are not tracked. +func NoComputedStructpathParse(m dsl.Matcher) { + m.Import("github.com/databricks/cli/libs/structs/structpath") + + m.Match(`structpath.MustParsePath($*_)`, `structpath.MustParsePattern($*_)`, `structpath.MustParsePaths($*_)`). + Where(!m.File().Name.Matches(`_test\.go$`) && !m.File().PkgPath.Matches(`internal/bundletest`)). + Report(`build paths with structpath.NewPath / NewPathSlice / NewPattern 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`) + + // 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`) + + // 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`) +}