mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-10-02 09:24:52 +08:00
Route `.jsonnet` and `.libsonnet` to a new rule doc. `.libsonnet` is a naming convention for importable libraries, not a separate language, so both share one doc (as .hs/.lhs and .nim/.nims/.nimble already do). The vendor exclude is extension-scoped rather than a bare `**/vendor/**`: IsExcludedPath applies every pattern to every path with no language dispatch, so an unscoped directory pattern would also drop vendored Go and PHP sources, whose extensions are allowlisted. No test-file pattern is added — real Jsonnet projects split between `test_*.libsonnet`, `tests/*.jsonnet` and `*_test.jsonnet`, and an over-broad glob silently drops handwritten files. Verified: go test ./... and gofmt -l internal/config clean.
This commit is contained in:
@@ -71,6 +71,10 @@ func TestIsAllowedExt(t *testing.T) {
|
||||
{".PO", true},
|
||||
{".pot", true},
|
||||
{".POT", true},
|
||||
{".jsonnet", true},
|
||||
{".JSONNET", true},
|
||||
{".libsonnet", true},
|
||||
{".LIBSONNET", true},
|
||||
{".txt", false},
|
||||
{".md", false},
|
||||
{".png", false},
|
||||
@@ -187,6 +191,16 @@ func TestIsExcludedPath(t *testing.T) {
|
||||
{"elm non-test", "src/Parser.elm", false},
|
||||
{"elm tests in filename", "src/TestsHelper.elm", false},
|
||||
|
||||
// Jsonnet vendored dependencies (written by `jb install`, wiped by `rm -rf vendor`).
|
||||
// The pattern is extension-scoped: IsExcludedPath applies every pattern to every
|
||||
// path, so a bare **/vendor/** would also drop vendored Go and PHP sources.
|
||||
{"jsonnet vendor root", "vendor/github.com/grafana/jsonnet-libs/ksonnet-util/kausal.libsonnet", true},
|
||||
{"jsonnet vendor nested dir", "jsonnet/vendor/foo/main.jsonnet", true},
|
||||
{"jsonnet non-vendor lib", "lib/config.libsonnet", false},
|
||||
{"jsonnet non-vendor env", "environments/prod/main.jsonnet", false},
|
||||
{"go under vendor still reviewed", "vendor/github.com/pkg/errors/errors.go", false},
|
||||
{"php under vendor still reviewed", "vendor/monolog/monolog/src/Logger.php", false},
|
||||
|
||||
// Snapshot files
|
||||
{"jest snapshot dir", "src/__snapshots__/App.test.js.snap", true},
|
||||
{"snap file", "src/components/Button.snap", true},
|
||||
|
||||
@@ -33,5 +33,6 @@
|
||||
"**/*Test.swift",
|
||||
"**/*Tests.swift",
|
||||
"**/Tests/**/*.swift",
|
||||
"**/tests/**/*.elm"
|
||||
"**/tests/**/*.elm",
|
||||
"**/vendor/**/*.{jsonnet,libsonnet}"
|
||||
]
|
||||
|
||||
@@ -88,5 +88,7 @@
|
||||
".elm",
|
||||
".properties",
|
||||
".po",
|
||||
".pot"
|
||||
".pot",
|
||||
".jsonnet",
|
||||
".libsonnet"
|
||||
]
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
> Favor precision over recall: only raise an issue when you are confident it is a real defect, and stay silent when the surrounding context is unclear — a false alarm costs more reviewer trust than a missed minor issue. Review only what is observable in the Jsonnet under review; do not infer the contents of libraries imported from outside the diff, the values supplied for external variables or top-level arguments, or how the rendered output is consumed downstream. Do not flag formatting that `jsonnetfmt` would silently fix.
|
||||
|
||||
#### Late Binding: self, $, and super
|
||||
- `$` used where the enclosing object was meant. `$` is pinned to the outermost object of the *file it is written in*, so a library field that reaches for `$._config` resolves against the library's own root, not the caller's tree, the moment the object is merged into someone else's configuration
|
||||
- A field reading `self.x` from inside a nested object where `self` has already rebound to that nested object rather than the one holding `x`. The established fix is capturing the intended scope once at the top of the object — `local this = self,` / `local defaults = self,` — and referring to `this.x`; flag a nested `self` reference where such a capture already exists in the same object and was clearly meant to be used
|
||||
- `super.f` referenced in an object that is not the right operand of a `+` in any reachable composition, which is a runtime error rather than a silent default
|
||||
- A `local` shadowing a field name that later code refers to unqualified, so a change to the field no longer affects the reference
|
||||
|
||||
#### Object Composition and Overrides
|
||||
- `+` between two objects merges only the top level: a field present in both is taken wholesale from the right operand, and its nested contents are replaced rather than merged. Flag a nested field written with `:` where the surrounding overrides use `+:` and the intent is clearly to add to the inherited value, not to discard it
|
||||
- `f+: v` evaluates to `super.f + v`, so the operator's meaning follows the type: objects merge one level, arrays concatenate, strings concatenate as text. An override that means "replace this list" written as `+:` silently appends instead, producing duplicate containers, volumes, or arguments
|
||||
- `f+:` where `f` does not exist in the inherited object — a renamed or misspelled field — silently defines a new field instead of overriding anything, and nothing reads it
|
||||
- An override applied to an object that is not on the right-hand side of the composition it was written for, so the later operand wins and the override is dropped from the output
|
||||
|
||||
#### Hidden Fields and Rendered Output
|
||||
- A `::` field the rendered manifest is expected to contain. Hidden fields are absent from the output with no error, so the omission surfaces only where the artifact is applied
|
||||
- A `:` field holding an internal helper, a partially built template, a raw function argument, or a credential, which leaks verbatim into the rendered YAML/JSON
|
||||
- `:::` used to force visibility on a field that the library deliberately hid, without a stated reason
|
||||
- The required-argument idiom `x:: error 'must provide x'`: the error only fires when something reads `x`, so an override that misspells the field name leaves the default in place and the failure appears far from its cause, or not at all if nothing reads it
|
||||
- Null versus omission when the target is a Kubernetes manifest: an explicit `field: null` is a delete/reset in a strategic-merge patch, while an omitted field inherits the server default. `std.prune` and an explicit `null` are different requests, not stylistic variants — flag one substituted for the other
|
||||
|
||||
#### Imports and External Inputs
|
||||
- `import` used on a file that is not Jsonnet, where `importstr` (raw text) or `importbin` (raw bytes) was meant, and the reverse: `importstr` on a Jsonnet file, yielding source text rather than a value
|
||||
- Imports resolve at compile time against the `-J`/jpath search path, so a file added earlier in that path shadows the intended one and changes the output with no diagnostic. Flag an import whose relative path reaches into a vendored tree directly rather than through the library's documented entry point
|
||||
- `std.extVar('name')` or a top-level argument read without a documented default or any validation — output then depends on state that is invisible in the file
|
||||
- External variable values arrive as strings; flag one compared to a number, used in arithmetic, or treated as an object without `std.parseInt`/`std.parseJson`/`std.parseYaml`
|
||||
- `std.extVar` inside a computed field name (`[if std.extVar('x') then 'k']`), where an unset or falsy value changes which keys exist in the output at all
|
||||
|
||||
#### Termination and Manifestation
|
||||
- A recursive function or self-referential object with no argument that provably shrinks toward its base case. Evaluation is lazy, so an infinite structure is built without complaint and only exhausts the stack when a consumer forces it
|
||||
- `std.manifestYamlDoc` quotes keys by default; `quote_keys=false` is what produces unquoted YAML keys. Flag its output being fed to a consumer that requires plain keys, and flag the result — a string — being re-parsed or indexed as if it were structured data
|
||||
- A manifested string interpolated into a CLI flag, a ConfigMap entry, or an annotation where indentation, a multi-document `---` separator, or a non-string scalar changes how the receiver parses it
|
||||
- `std.toString` or `std.manifestJson` used as the input to a hash, a checksum annotation, or an equality check, where field ordering or the representation of numbers is not guaranteed to be stable across evaluator versions
|
||||
- `assert` used to validate an input at a point that is never forced, so the check silently never runs
|
||||
@@ -36,6 +36,7 @@
|
||||
"**/*.{hs,lhs}": "haskell.md",
|
||||
"**/*.{nim,nims,nimble}": "nim.md",
|
||||
"**/*.swift": "swift.md",
|
||||
"**/*.elm": "elm.md"
|
||||
"**/*.elm": "elm.md",
|
||||
"**/*.{jsonnet,libsonnet}": "jsonnet.md"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -122,6 +122,9 @@ func TestResolve_DefaultRules(t *testing.T) {
|
||||
{"ChattyFit/ChattyFit/Views/WorkoutSessionView.swift", "SwiftUI State and Lifecycle"},
|
||||
{"src/Main.elm", "Elm Architecture"},
|
||||
{"app/Page/Home.elm", "Elm Architecture"},
|
||||
{"lib/config.libsonnet", "Late Binding"},
|
||||
{"environments/prod/main.jsonnet", "Late Binding"},
|
||||
{"jsonnet/kube-prometheus/components/grafana.libsonnet", "Late Binding"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
@@ -177,6 +177,7 @@ matching order:
|
||||
| `**/*.jl` | `julia.md` — Julia source. |
|
||||
| `**/*.{tf,hcl,tfvars}` | `terraform.md` — Terraform / HCL. |
|
||||
| `**/*.bicep` | `bicep.md` — Bicep (Azure) templates. |
|
||||
| `**/*.{jsonnet,libsonnet}` | `jsonnet.md` — Jsonnet configuration templates and libraries. |
|
||||
| *(fallback)* | `default.md` |
|
||||
|
||||
The resolved rule body becomes the `{{system_rule}}` placeholder in the
|
||||
|
||||
@@ -139,6 +139,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
|
||||
| `**/*.jl` | `julia.md`: Julia ソースコード。 |
|
||||
| `**/*.{tf,hcl,tfvars}` | `terraform.md`: Terraform / HCL。 |
|
||||
| `**/*.bicep` | `bicep.md`: Bicep(Azure)テンプレート。 |
|
||||
| `**/*.{jsonnet,libsonnet}` | `jsonnet.md`: Jsonnet の設定テンプレートとライブラリ。 |
|
||||
| *(fallback)* | `default.md` |
|
||||
|
||||
解決されたルール本文は、plan および main task prompt 内の `{{system_rule}}` プレースホルダーの内容になります。
|
||||
|
||||
@@ -179,6 +179,7 @@ OCR использует [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com
|
||||
| `**/*.jl` | `julia.md` — исходный код Julia. |
|
||||
| `**/*.{tf,hcl,tfvars}` | `terraform.md` — Terraform / HCL. |
|
||||
| `**/*.bicep` | `bicep.md` — шаблоны Bicep (Azure). |
|
||||
| `**/*.{jsonnet,libsonnet}` | `jsonnet.md` — шаблоны конфигурации и библиотеки Jsonnet. |
|
||||
| *(fallback)* | `default.md` |
|
||||
|
||||
Разрешённое тело правила становится значением плейсхолдера `{{system_rule}}`
|
||||
|
||||
@@ -160,6 +160,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
|
||||
| `**/*.jl` | `julia.md`——Julia 源代码。 |
|
||||
| `**/*.{tf,hcl,tfvars}` | `terraform.md`——Terraform / HCL。 |
|
||||
| `**/*.bicep` | `bicep.md`——Bicep(Azure)模板。 |
|
||||
| `**/*.{jsonnet,libsonnet}` | `jsonnet.md`——Jsonnet 配置模板与库。 |
|
||||
| *(fallback)* | `default.md` |
|
||||
|
||||
解析出的规则正文成为 plan 和 main task prompt 中 `{{system_rule}}` 占位符的内容。
|
||||
|
||||
Reference in New Issue
Block a user