diff --git a/internal/agent/preview_test.go b/internal/agent/preview_test.go index f0b157651..4e3864f95 100644 --- a/internal/agent/preview_test.go +++ b/internal/agent/preview_test.go @@ -140,6 +140,34 @@ func TestWhyExcluded_ExtensionFilter(t *testing.T) { }, expected: ExcludeNone, }, + { + name: "supported Starlark bzl extension", + diff: model.Diff{ + NewPath: "rules/library.bzl", + }, + expected: ExcludeNone, + }, + { + name: "supported Starlark star extension", + diff: model.Diff{ + NewPath: "config.star", + }, + expected: ExcludeNone, + }, + { + name: "supported Bazel extension", + diff: model.Diff{ + NewPath: "MODULE.bazel", + }, + expected: ExcludeNone, + }, + { + name: "extensionless Starlark build file", + diff: model.Diff{ + NewPath: "pkg/BUILD", + }, + expected: ExcludeNone, + }, { name: "file without extension", diff: model.Diff{ diff --git a/internal/config/allowlist/allowed_ext_test.go b/internal/config/allowlist/allowed_ext_test.go index e3cee2f71..cde900e88 100644 --- a/internal/config/allowlist/allowed_ext_test.go +++ b/internal/config/allowlist/allowed_ext_test.go @@ -86,6 +86,12 @@ func TestIsAllowedExt(t *testing.T) { {".THRIFT", true}, {".capnp", true}, {".CAPNP", true}, + {".bzl", true}, + {".BZL", true}, + {".bazel", true}, + {".BAZEL", true}, + {".star", true}, + {".STAR", true}, {".sol", true}, {".SOL", true}, {".vy", true}, diff --git a/internal/config/allowlist/supported_file_types.json b/internal/config/allowlist/supported_file_types.json index c50002c89..1f0bf1246 100644 --- a/internal/config/allowlist/supported_file_types.json +++ b/internal/config/allowlist/supported_file_types.json @@ -95,6 +95,9 @@ ".zig", ".thrift", ".capnp", + ".bzl", + ".bazel", + ".star", ".sol", ".vy" ] diff --git a/internal/config/rules/rule_docs/starlark.md b/internal/config/rules/rule_docs/starlark.md new file mode 100644 index 000000000..b6c60896e --- /dev/null +++ b/internal/config/rules/rule_docs/starlark.md @@ -0,0 +1,226 @@ +# Starlark Code Review Guide + +> Guidance for AI reviewers. Starlark is a deterministic, hermetic configuration +> language (a dialect of Python) used primarily by Bazel. It is **not** Python: +> the syntax is a strict subset, but the semantics differ, and many Python +> features are intentionally absent. Unless noted, this guide assumes Bazel's +> Starlark dialect. + +## Obvious Typos or Spelling Errors + +- Spelling errors in variable, function, provider, rule, attribute, and target + names **at their declaration sites**; do not report spelling errors at call + sites. +- Spelling errors in `fail()` or `print()` messages, comments, and docstrings + that affect readability. +- Typos in `load()` symbol names and label strings (these can cause load or + analysis failures). +- Misspelled attribute names in rule or macro calls (for example `sorce` instead + of `srcs`), because they are usually a runtime error or are silently ignored. + +## Determinism and Purity + +Starlark is designed to be deterministic and side-effect free. Review code that +could break hermeticity. + +**Key checks:** + +- No reliance on wall-clock time, randomness, host filesystem state, + environment variables, or network access in ordinary `.bzl` or `BUILD` code. +- Rule implementations must produce the same result for the same inputs and + `ctx`. +- Repository rules may perform network or filesystem access, but their result + must still be deterministic for a given set of inputs. +- No `print()` in production code (see Error Reporting). + +## Recursion and Termination + +**Key checks:** + +- No direct or mutual recursion; Starlark rejects recursion at runtime. +- No `while` loops; Starlark has only `for` loops over finite sequences. +- Starlark `for` loops always terminate because they iterate over finite + collections; `break` and `continue` cannot make a loop non-terminating, so + flag excessive iteration cost instead (see Depsets and Efficiency). +- Avoid deeply nested comprehensions that make termination or cost hard to + reason about. + +## Frozen Values and Mutation + +Starlark favors immutability. Only `list` and `dict` are mutable, and only +inside the evaluation context that created them. Bazel 8.1 also added a +mutable core `set` type, so `set` values are subject to the same rules. + +**Key checks:** + +- Do not mutate a value loaded from another `.bzl` file, a rule attribute, or a + value returned by a rule or provider; those values are frozen. +- Do not mutate a collection while iterating over it (a dynamic error). +- Avoid mutable default arguments; defaults are evaluated once and shared + across calls. +- Do not rely on object identity; Starlark has no `is` operator. + +**Example:** + +```python +# Bad: mutates a frozen value +# other.bzl +load(":defs.bzl", "registry") + +def use(): + registry.append("x") # runtime error: registry is frozen + +# Good: the function owns the mutable value; loaded module globals are frozen +# after evaluation and cannot be mutated from another file +def register(name): + registry = [] + registry.append(name) + return registry +``` + +```python +# Bad: mutable default argument is shared between calls +def add(name, names=[]): + names.append(name) + return names + +# Good: use None and create the list inside +def add(name, names=None): + names = names or [] + names.append(name) + return names +``` + +## Safe Data Handling (Python Differences) + +**Key checks:** + +- Ordered comparisons (`<`, `<=`, `>`, `>=`) are defined only within a single + value type; `==` and `!=` may compare across types. +- Strings are not iterable; use `s.elems()` to iterate over characters. +- Dictionary literals cannot contain duplicate keys. +- Dictionary iteration order is deterministic. +- There is no implicit string concatenation; use `+`. +- In Bazel, `int` is limited to 32-bit signed values and overflow is an error. +- Use `struct()` instead of `class`; use `provider()` for rule outputs. +- Define named functions with `def`; Bazel's dialect has no `lambda`. +- There is no `import`; use `load()`, placed at the top of the file. +- There is no `global` or `nonlocal`. + +**Example:** + +```python +# Bad: Python-style exception handling does not exist +def lookup(data, key): + try: + return data[key] + except KeyError: + return None + +# Good: check first and fail with a clear message +def lookup(data, key): + if key not in data: + fail("missing key %r in %r" % (key, data)) + return data[key] +``` + +## Depsets and Efficiency + +**Key checks:** + +- Aggregate transitive information with `depset`; do not flatten transitive + depsets with `to_list()` inside a loop, which is O(N^2) over the dependency + graph. +- Avoid `depset.to_list()` except for debugging. +- Build a depset in a single call; do not construct depsets inside a loop. +- Use `ctx.actions.args()` for command lines instead of flattening depsets or + building giant strings. +- Pass depsets directly as action inputs; do not convert them to lists. + +**Example:** + +```python +# Bad: repeatedly flattening depsets with to_list() is O(N^2) +files = [] +for dep in ctx.attr.deps: + files += dep[MyProvider].files.to_list() + +# Good: keep the depsets nested and merge once +files = depset( + direct = ctx.files.srcs, + transitive = [dep[MyProvider].files for dep in ctx.attr.deps], +) +``` + +## Error Reporting + +**Key checks:** + +- Use `fail()` to report validation errors and unrecoverable conditions. +- `print()` is for debugging only; it must not appear in production code, or it + must be gated behind a hardcoded `DEBUG` flag. +- Error messages should name the offending value and the expected condition. +- Do not silently swallow errors, and do not return `None` where the caller + expects a value without documenting that behavior. + +## Loads, Symbols, and Encapsulation + +**Key checks:** + +- Use `load()` (not `import`), with all loads at the top of the file. +- Minimize exported symbols per `.bzl` file; mark file-private top-level + symbols with a leading `_`. +- Remove unused loads, variables, and parameters. +- Prefer rules over macros; macros expand before graph analysis and hide the + real build graph. +- Flag macro-generated helper targets that are not prefixed with the main + target's `name` (for example `name_bar` or `_name_bar`). +- Flag helper targets that lack `visibility = ["//visibility:private"]` or a + `manual` tag; without `manual`, wildcard patterns such as `:all` and `:...` + expand them. +- Keep `BUILD` files simple and explicit (DAMP over DRY); avoid top-level list + comprehensions and shared `deps` variables. + +## Naming Conventions + +**Requirements:** + +- Variables, functions, rules, attributes, and targets use `snake_case`. +- Constants use `UPPER_SNAKE_CASE`. +- File-private top-level symbols start with `_`. +- Rule and macro implementation functions are named `__impl`. +- Rule names are nouns describing the produced artifact; common suffixes + include `_library`, `_binary`, `_test`, and `_import`. +- Common attributes follow standard names and types: `srcs` is a `label_list` + (typically source files), `deps` is a `label_list` (typically *not* files), + `data` is a `label_list` (runtime/test data), and `runtime_deps` is a + `label_list` for dependencies not needed at compile time. +- A macro's `name` parameter is first, and generated targets are prefixed with + `name` plus `_`. +- Call rules and macros with keyword arguments, spaces around `=`, and `name` + first. + +## Documentation + +- Add a docstring at the top of each `.bzl` file and for each public function. +- Document rules, aspects, providers, and attributes using the `doc` argument. +- Rules pass data through well-defined providers; flag provider fields that + are undeclared or undocumented. +- Use comments to explain *why*, not what the code does. + +## Formatting + +- Formatting must match `buildifier`; do not nitpick formatting it would + produce. +- Prefer double quotes for strings. +- Use four-space indentation. +- Use a single blank line between top-level definitions. + +## Do Not Report + +- Code that already avoids Python-isms correctly, such as using `==` instead of + `is`, or `fail()` instead of `try`/`except`. +- Formatting that `buildifier` is responsible for. +- Spelling errors at call sites rather than declaration sites. +- The absence of features Starlark intentionally omits (`class`, `while`, + `import`, recursion, and so on). diff --git a/internal/config/rules/system_rules.json b/internal/config/rules/system_rules.json index 21f4b02ec..f252ca37f 100644 --- a/internal/config/rules/system_rules.json +++ b/internal/config/rules/system_rules.json @@ -42,6 +42,7 @@ "**/*.zig": "zig.md", "**/*.thrift": "thrift.md", "**/*.capnp": "capnp.md", + "**/{BUILD,WORKSPACE,*.bzl,*.bazel,*.star}": "starlark.md", "**/*.m": "matlab.md", "**/*.sol": "solidity.md", "**/*.vy": "vyper.md" diff --git a/internal/config/rules/system_rules_test.go b/internal/config/rules/system_rules_test.go index 74d144d54..1ebf2859b 100644 --- a/internal/config/rules/system_rules_test.go +++ b/internal/config/rules/system_rules_test.go @@ -135,6 +135,11 @@ func TestResolve_DefaultRules(t *testing.T) { {"if/common.thrift", "Field IDs and Wire Compatibility"}, {"schema/addressbook.capnp", "Ordinals and Wire Compatibility"}, {"src/rpc.capnp", "Ordinals and Wire Compatibility"}, + {"rules/library.bzl", "Starlark Code Review Guide"}, + {"config.star", "Starlark Code Review Guide"}, + {"MODULE.bazel", "Starlark Code Review Guide"}, + {"pkg/BUILD", "Starlark Code Review Guide"}, + {"WORKSPACE", "Starlark Code Review Guide"}, {"Models/main.m", "Indexing, Shapes, and Implicit Expansion"}, {"src/Counter.sol", "Checks-Effects-Interactions"}, {"contracts/Vault.sol", "Delegatecall and Proxy Upgradeability"}, diff --git a/internal/scan/coverage_test.go b/internal/scan/coverage_test.go index 06b18f44d..7979d8664 100644 --- a/internal/scan/coverage_test.go +++ b/internal/scan/coverage_test.go @@ -182,6 +182,11 @@ func TestWhyExcluded_AllBranches(t *testing.T) { item: model.ScanItem{Path: "main.go", Content: "x"}, want: model.ExcludeNone, }, + { + name: "Starlark file passes", + item: model.ScanItem{Path: "rules/library.bzl", Content: "x"}, + want: model.ExcludeNone, + }, } for _, tt := range tests { diff --git a/internal/tool/file_find.go b/internal/tool/file_find.go index 31b8dcf6a..e38a0e6d9 100644 --- a/internal/tool/file_find.go +++ b/internal/tool/file_find.go @@ -202,7 +202,7 @@ func (p *FileFindProvider) listWalkFiles(ctx context.Context) ([]string, error) // shouldSkipFile returns true if a git ls-files output path should be skipped. // Keeps only widely useful files (those with recognizable extensions). func shouldSkipFile(path string) bool { - // Keep extensionless build/config files like Makefile, Dockerfile, LICENSE + // Keep well-known extensionless build/config files. base := path if idx := strings.LastIndex(path, "/"); idx != -1 { base = path[idx+1:] @@ -211,7 +211,7 @@ func shouldSkipFile(path string) bool { if !hasExt { // Allow well-known extensionless files switch base { - case "Makefile", "Dockerfile", "LICENSE", "Vagrantfile", "Containerfile": + case "Makefile", "Dockerfile", "LICENSE", "Vagrantfile", "Containerfile", "BUILD", "WORKSPACE": return false } return true // skip other extensionless files diff --git a/internal/tool/file_find_test.go b/internal/tool/file_find_test.go index 6861ad28d..39359c89b 100644 --- a/internal/tool/file_find_test.go +++ b/internal/tool/file_find_test.go @@ -118,6 +118,8 @@ func setupFileFindRepo(t *testing.T) string { write("Makefile", "all:\n") write("Dockerfile", "FROM scratch\n") write("LICENSE", "MIT\n") + write("pkg/BUILD", "filegroup(name = \"all\")\n") + write("WORKSPACE", "workspace(name = \"example\")\n") write("data_binary", "binary\n") run("git", "add", ".") @@ -138,6 +140,30 @@ func TestFileFind_GitRepo_WorkspaceMode(t *testing.T) { } } +func TestFileFind_GitRepo_StarlarkBuildFiles(t *testing.T) { + dir := setupFileFindRepo(t) + p := NewFileFind(&FileReader{RepoDir: dir, Mode: ModeWorkspace}) + + tests := []struct { + query string + want string + }{ + {query: "BUILD", want: "pkg/BUILD"}, + {query: "WORKSPACE", want: "WORKSPACE"}, + } + for _, tt := range tests { + t.Run(tt.query, func(t *testing.T) { + got, err := p.Execute(context.Background(), map[string]any{"query_name": tt.query}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(got, tt.want) { + t.Errorf("expected %s, got: %s", tt.want, got) + } + }) + } +} + func TestFileFind_GitRepo_CommitMode(t *testing.T) { dir := setupFileFindRepo(t) cmd := exec.Command("git", "rev-parse", "HEAD") @@ -287,6 +313,9 @@ func TestShouldSkipFile(t *testing.T) { {"LICENSE", false}, {"Vagrantfile", false}, {"Containerfile", false}, + {"BUILD", false}, + {"pkg/BUILD", false}, + {"WORKSPACE", false}, {"some_binary", true}, {"dir/unknown_file", true}, } diff --git a/pages/src/content/docs/en/review-rules.md b/pages/src/content/docs/en/review-rules.md index a8791dca2..37c2d07b2 100644 --- a/pages/src/content/docs/en/review-rules.md +++ b/pages/src/content/docs/en/review-rules.md @@ -182,6 +182,7 @@ matching order: | `**/*.{jsonnet,libsonnet}` | `jsonnet.md` — Jsonnet configuration templates and libraries. | | `**/*.thrift` | `thrift.md` — Apache Thrift IDL wire compatibility. | | `**/*.capnp` | `capnp.md` — Cap'n Proto schema wire compatibility. | +| `**/{BUILD,WORKSPACE,*.bzl,*.bazel,*.star}` | `starlark.md` — Starlark and Bazel build, macro, and rule design. | | `**/*.m` | `matlab.md` (or `objc.md` via [content sniffing](#content-sniffing-for-m-files)) | | `**/*.sol` | `solidity.md` — Solidity smart contracts. | | `**/*.vy` | `vyper.md` — Vyper smart contracts. | diff --git a/pages/src/content/docs/en/tools.md b/pages/src/content/docs/en/tools.md index 4db64e91f..90eff0195 100644 --- a/pages/src/content/docs/en/tools.md +++ b/pages/src/content/docs/en/tools.md @@ -253,7 +253,8 @@ Find files in the repo by repository-relative path or filename keyword (substrin The candidate set comes from `git ls-files --cached --others --exclude-standard` in workspace mode, or `git ls-tree -r --name-only ` in range / commit mode. Extensionless files are skipped except -for `Makefile`, `Dockerfile`, `LICENSE`, `Vagrantfile`, `Containerfile`. +for `Makefile`, `Dockerfile`, `LICENSE`, `Vagrantfile`, `Containerfile`, +`BUILD`, and `WORKSPACE`. ### Output diff --git a/pages/src/content/docs/ja/review-rules.md b/pages/src/content/docs/ja/review-rules.md index bc8c02ce9..75e047782 100644 --- a/pages/src/content/docs/ja/review-rules.md +++ b/pages/src/content/docs/ja/review-rules.md @@ -144,6 +144,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest | `**/*.{jsonnet,libsonnet}` | `jsonnet.md`: Jsonnet の設定テンプレートとライブラリ。 | | `**/*.thrift` | `thrift.md`: Apache Thrift IDL のワイヤ互換性。 | | `**/*.capnp` | `capnp.md`: Cap'n Proto スキーマのワイヤ互換性。 | +| `**/{BUILD,WORKSPACE,*.bzl,*.bazel,*.star}` | `starlark.md`: Starlark と Bazel のビルド、マクロ、ルール設計。 | | `**/*.m` | `matlab.md`（または[コンテンツスニッフィング](#content-sniffing-for-m-files)により `objc.md`） | | `**/*.sol` | `solidity.md`: Solidity スマートコントラクト。 | | `**/*.vy` | `vyper.md`: Vyper スマートコントラクト。 | diff --git a/pages/src/content/docs/ja/tools.md b/pages/src/content/docs/ja/tools.md index 90a890734..8b47ad630 100644 --- a/pages/src/content/docs/ja/tools.md +++ b/pages/src/content/docs/ja/tools.md @@ -225,7 +225,7 @@ hunk ヘッダー `@@ -x,y +m,n @@` から範囲を計算すべきです—— 候補セットは、ワークスペースモードでは `git ls-files --cached --others --exclude-standard` から、 区間 / commit モードでは `git ls-tree -r --name-only ` から得られます。拡張子のないファイルは スキップされますが、`Makefile`、`Dockerfile`、`LICENSE`、`Vagrantfile`、 -`Containerfile` は例外です。 +`Containerfile`、`BUILD`、`WORKSPACE` は例外です。 ### 出力 diff --git a/pages/src/content/docs/ko/review-rules.md b/pages/src/content/docs/ko/review-rules.md index 4bc8a6a14..c1a7f4b80 100644 --- a/pages/src/content/docs/ko/review-rules.md +++ b/pages/src/content/docs/ko/review-rules.md @@ -173,6 +173,7 @@ diff 단계에서 일어납니다. | `**/*.{jsonnet,libsonnet}` | `jsonnet.md` — Jsonnet 설정 템플릿과 라이브러리. | | `**/*.thrift` | `thrift.md` — Apache Thrift IDL 통신 호환성. | | `**/*.capnp` | `capnp.md` — Cap'n Proto 스키마 통신 호환성. | +| `**/{BUILD,WORKSPACE,*.bzl,*.bazel,*.star}` | `starlark.md` — Starlark 및 Bazel 빌드, 매크로, 규칙 설계. | | `**/*.m` | `matlab.md`(또는 [내용 탐지](#content-sniffing-for-m-files)로 `objc.md`) | | `**/*.sol` | `solidity.md` — Solidity 스마트 컨트랙트. | | `**/*.vy` | `vyper.md` — Vyper 스마트 컨트랙트. | diff --git a/pages/src/content/docs/ko/tools.md b/pages/src/content/docs/ko/tools.md index a72ad58c0..7656143e9 100644 --- a/pages/src/content/docs/ko/tools.md +++ b/pages/src/content/docs/ko/tools.md @@ -237,7 +237,7 @@ hunk 머리글 `@@ -x,y +m,n @@`에서 범위를 계산해야 합니다. 보통 후보 목록은 워크스페이스 모드에서는 `git ls-files --cached --others --exclude-standard`, range·commit 모드에서는 `git ls-tree -r --name-only `에서 옵니다. 확장자가 없는 파일은 건너뛰되 `Makefile`, `Dockerfile`, `LICENSE`, -`Vagrantfile`, `Containerfile`은 예외입니다. +`Vagrantfile`, `Containerfile`, `BUILD`, `WORKSPACE`는 예외입니다. ### 출력 {#output} diff --git a/pages/src/content/docs/ru/review-rules.md b/pages/src/content/docs/ru/review-rules.md index 19dc3e251..d87deb4bb 100644 --- a/pages/src/content/docs/ru/review-rules.md +++ b/pages/src/content/docs/ru/review-rules.md @@ -184,6 +184,7 @@ OCR использует [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com | `**/*.{jsonnet,libsonnet}` | `jsonnet.md` — шаблоны конфигурации и библиотеки Jsonnet. | | `**/*.thrift` | `thrift.md` — совместимость Apache Thrift IDL на уровне wire. | | `**/*.capnp` | `capnp.md` — совместимость схем Cap'n Proto на уровне wire. | +| `**/{BUILD,WORKSPACE,*.bzl,*.bazel,*.star}` | `starlark.md` — Starlark и проектирование сборок, макросов и правил Bazel. | | `**/*.m` | `matlab.md` (или `objc.md` через [определение содержимого](#content-sniffing-for-m-files)) | | `**/*.sol` | `solidity.md` — смарт-контракты Solidity. | | `**/*.vy` | `vyper.md` — смарт-контракты Vyper. | diff --git a/pages/src/content/docs/ru/tools.md b/pages/src/content/docs/ru/tools.md index d7d0ecb3b..e27101838 100644 --- a/pages/src/content/docs/ru/tools.md +++ b/pages/src/content/docs/ru/tools.md @@ -255,8 +255,8 @@ LINE_RANGE: 10-80 Набор кандидатов формируется командой `git ls-files --cached --others --exclude-standard` в режиме рабочей области или командой `git ls-tree -r --name-only ` в режиме диапазона / коммита. Файлы без -расширения пропускаются, кроме `Makefile`, `Dockerfile`, `LICENSE`, `Vagrantfile` -и `Containerfile`. +расширения пропускаются, кроме `Makefile`, `Dockerfile`, `LICENSE`, `Vagrantfile`, +`Containerfile`, `BUILD` и `WORKSPACE`. ### Вывод diff --git a/pages/src/content/docs/zh/review-rules.md b/pages/src/content/docs/zh/review-rules.md index ee790999e..073c83f30 100644 --- a/pages/src/content/docs/zh/review-rules.md +++ b/pages/src/content/docs/zh/review-rules.md @@ -165,6 +165,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest | `**/*.{jsonnet,libsonnet}` | `jsonnet.md`——Jsonnet 配置模板与库。 | | `**/*.thrift` | `thrift.md`——Apache Thrift IDL 线协议兼容性。 | | `**/*.capnp` | `capnp.md`——Cap'n Proto schema 线协议兼容性。 | +| `**/{BUILD,WORKSPACE,*.bzl,*.bazel,*.star}` | `starlark.md`——Starlark 与 Bazel 构建、宏和规则设计。 | | `**/*.m` | `matlab.md`（或通过[内容嗅探](#针对-m-文件的内容嗅探)使用 `objc.md`） | | `**/*.sol` | `solidity.md`——Solidity 智能合约。 | | `**/*.vy` | `vyper.md`——Vyper 智能合约。 | diff --git a/pages/src/content/docs/zh/tools.md b/pages/src/content/docs/zh/tools.md index 8f8f7806f..8703e7483 100644 --- a/pages/src/content/docs/zh/tools.md +++ b/pages/src/content/docs/zh/tools.md @@ -224,7 +224,7 @@ hunk 头 `@@ -x,y +m,n @@` 计算范围——通常 `m-50` 到 `m+n+50`。 候选集在工作区模式下来自 `git ls-files --cached --others --exclude-standard`， 在区间 / commit 模式下来自 `git ls-tree -r --name-only `。无扩展名的文件 被跳过，但 `Makefile`、`Dockerfile`、`LICENSE`、`Vagrantfile`、 -`Containerfile` 例外。 +`Containerfile`、`BUILD`、`WORKSPACE` 例外。 ### 输出