diff --git a/.gitignore b/.gitignore index 6496f173e..7eb99fd71 100644 --- a/.gitignore +++ b/.gitignore @@ -78,3 +78,7 @@ tools/prerelease/registry.env hs_err_pid*.log replay_pid*.log core.[0-9]* + +# Throwaway projects the validation gate builds to drive the CLIs by hand. +.tmp-java-drive/ +.tmp-drive/ diff --git a/AGENTS.md b/AGENTS.md index 4832e9738..3dfb9bf0d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,7 +75,7 @@ PyPI has had no product change since `0.25.0` — nothing is broken. - **Kotlin** — `codegen-kotlin` (KotlinPoet on JVM): entity + Exposed table + Spring controller + payload + relations + filter allowlist + validator + stored-proc + output-parser generators. `integration-tests-kotlin` runs the persistence-conformance corpus through Exposed against Testcontainers Postgres. **Cross-port conformance corpora** (every port runs the shared corpus): -- Metamodel: `fixtures/conformance/` (363 fixtures; 25 shared corpora in total — per-corpus counts + the corpus x port matrix live in `docs/CONFORMANCE.md`). TS / C# / Java / Python all green. +- Metamodel: `fixtures/conformance/` (363 fixtures; 26 shared corpora in total — per-corpus counts + the corpus x port matrix live in `docs/CONFORMANCE.md`). TS / C# / Java / Python all green. - Render: `fixtures/render-conformance/`. TS / C# / Java / Kotlin / Python byte-identical. - Persistence: `fixtures/persistence-conformance/`. **Query** scenarios run on every port (TS / C# / Java / Kotlin / Python), each provisioning its test DB by executing the committed, TS-produced `canonical/schema.postgres.sql` (Postgres only — Derby dropped for the cross-port query corpus, ADR-0015). The **migration** scenarios are exercised by **TS only** (TS owns schema migrations). **The corpus now gates WRITES, not just reads (SP-H):** an `op: roundtrip` scenario type INSERTs through each port's runtime/ORM write codec (NOT raw SQL), reads the row back, and asserts the wire-normalized value. The `AllTypes` entity (`roundtrip-all-types.yaml`) carries one field of **every** persistable `field.*` subtype — string/int/long/double/float/decimal/boolean/date/time/timestamp(+tz)/currency/enum/uuid/object — plus an **array-of-VO** `field.object @isArray @storage:jsonb` column (`labels`, written as 2-element / empty-`[]` / single-element arrays across the three rows) — so every subtype write+read (incl. the array-of-value-object jsonb codec) round-trips through every port against Testcontainers PG. (`field.byte`/`field.short`/`field.class` were cut as non-functional registration-only stubs — the matrix tracks only genuinely-supported subtypes; see `fixtures/registry-conformance/README.md` → "Per-subtype write-round-trip matrix".) - API-contract: `fixtures/api-contract-conformance/`. TS / C# / Java / Kotlin / Python all green — each port runs **two lanes**: a hand-rolled reference server AND its **generated** API artifact booted over HTTP (the deployed controller/routes; TS+C# full-stack vs Testcontainers PG, Java/Kotlin/Python generated controller + in-memory repo behind the consumer seam). The generated fan-out found 10 real deployment bugs golden snapshots missed. Two sub-corpora run the **generated lane only, on all five ports** — `write-through/` and `projection/` (F22: a view-only `object.projection` serves GET list + GET by id and answers every write verb with `405 {"error": "method_not_allowed"}`). That is deliberate, not a gap: what is under test is whether a port's GENERATOR emits those routes, and a hand-rolled reference server would answer every scenario by construction. The `m2m/` sub-corpus also gates **TPH x M:N together** (base-declared, subtype-declared, abstract-mid-declared, a non-subtype source onto a subtype TARGET, and the cross-subtype source id answering `200 []`) — the two corpora were originally built disjoint (`tph/` had no relationships, `m2m/` no discriminators), which is precisely how that defect class survived. diff --git a/CHANGELOG.md b/CHANGELOG.md index 11136d113..7ce794df8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,18 @@ it until 1.1 ships._ validation". - **TypeScript: `runValidators` is exported from `@metaobjectsdev/runtime-ts`**, with `RunValidatorsOpts`. It was reachable only through `ObjectManager.validate()` before. +- **`verify` warns about two field mistakes the loader accepts, in every port.** An + `identity.reference` whose `@fields` names a field the object does not have gets + `WARN_REFERENCE_FIELD_NOT_FOUND`. A field name declared more than once in one object's + `children` list gets `WARN_DUPLICATE_FIELD_NAME`. Both load with no error today and still + do: these are advisory warnings, they never change the exit code, and nothing that loaded + before stops loading. A field inherited through `extends` or added by an overlay file + counts as present, and neither a subtype overriding an inherited field nor an overlay + redeclaring one is a duplicate. The lint runs on every `meta verify`, `dotnet meta verify`, + `mvn metaobjects:verify` and `metaobjects verify`, with the same codes and message text, + gated by the new `fixtures/field-lint-conformance/` corpus. Mute it with `--no-field-lint` + (`-Dmeta.verify.noFieldLint=true` in Maven) or `META_NO_FIELD_LINT=1`. In the Node `meta` + it is the `fields` section of `--format json|toon`. - **Metamodel 1.1: the reporting vocabulary (FR-044), loader-validated in all five ports.** Registered: `dimension.attribute`, `dimension.time` (`@grains`: `hour, day, week, month, diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index ec83f75ea..94b182ede 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -1,6 +1,6 @@ # Conformance coverage -The MetaObjects standard ships **25 shared conformance corpora** under +The MetaObjects standard ships **26 shared conformance corpora** under [`fixtures/`](../fixtures/). Every port runs every corpus that is *applicable to it* and asserts the same expected behaviour against the same fixtures. **This page is the inverse index**: fixture → feature doc + per-port pass status, and it is the @@ -49,6 +49,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/agent-context-conformance/`](../fixtures/agent-context-conformance/) | 4 | ✓ (the emitter is TS-owned) | — | — | — | — | | [`fixtures/metamodel-docs/`](../fixtures/metamodel-docs/) | 1 | ✓ (docs emit is TS-owned) | — | — | — | — | | [`fixtures/fmt-conformance/`](../fixtures/fmt-conformance/) (#304 — `meta fmt`) | 12 | ✓ (reference) | ✓ | inherits via Java | ✓ | ✓ | +| [`fixtures/field-lint-conformance/`](../fixtures/field-lint-conformance/) (the `verify` field authoring lint) | 14 | ✓ (reference) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/naming-conformance/`](../fixtures/naming-conformance/) | 8 cases | ✓ | ✓ | inherits via Java (`RouteNaming.pluralize`) | ✓ | ✓ | | [`fixtures/codegen-noop/`](../fixtures/codegen-noop/) (FR-044 — reporting vocabulary is inert) | 1 model pair (`reporting/with` vs `reporting/without`) | ✓ (codegen + migrate) | ✓ | ✓ | ✓ | ✓ | @@ -71,8 +72,9 @@ the corpora above do two different jobs. Only the first is a promise to adopters Mustache engine), `template-output-render-conformance/`, `output-prompt-conformance/`, `extract-conformance/`, `verify-conformance/`, `verify-strict-conformance/`, `persistence-conformance/` (runtime reads and writes, and the TS-owned migration - scenarios), `agent-context-conformance/`, `metamodel-docs/` and `fmt-conformance/` - (the canonical serializer, surfaced per-file — ADR-0034's "canonical format" is core). + scenarios), `agent-context-conformance/`, `metamodel-docs/`, `fmt-conformance/` + (the canonical serializer, surfaced per-file — ADR-0034's "canonical format" is core) + and `field-lint-conformance/` (the advisory field lint every port's `verify` prints). A red cell here is a MetaObjects bug. - **Template quality checks — not a promise.** The generated lane of `api-contract-conformance/`, `generator-registry-conformance/` (stable generator names), diff --git a/docs/features/cli.md b/docs/features/cli.md index 05b823536..5c99f44c5 100644 --- a/docs/features/cli.md +++ b/docs/features/cli.md @@ -121,6 +121,28 @@ Rules of the contract: `deprecated` inherited through the TARGET's own `extends` chain still counts. Advisory only, appears as `deprecations` in `--format json|toon`, and is muted with `--no-deprecation-lint` or `META_NO_DEPRECATION_LINT=1`. +- **The field authoring lint runs on every `verify`, in every port.** It reports two + mistakes that load with no error. `WARN_REFERENCE_FIELD_NOT_FOUND` flags an + `identity.reference` whose `@fields` names a field the object does not have; a field + inherited through `extends` or added by an overlay file counts as present. + `WARN_DUPLICATE_FIELD_NAME` flags a field name declared more than once in one object's + `children` list; a subtype overriding an inherited field and an overlay redeclaring a + field are not findings. This lint is advisory only and never changes the exit code. In + the Node `meta` it appears as `fields` in `--format json|toon`. The other ports print it + as text on stderr (Maven logs it as warnings). Mute it per port: + + | CLI | Flag | Environment | + |---|---|---| + | Node `meta verify` | `--no-field-lint` | `META_NO_FIELD_LINT=1` | + | `dotnet meta verify` | `--no-field-lint` | `META_NO_FIELD_LINT=1` | + | `mvn metaobjects:verify` | `-Dmeta.verify.noFieldLint=true` | `META_NO_FIELD_LINT=1` | + | `metaobjects verify` | `--no-field-lint` | `META_NO_FIELD_LINT=1` | + + The codes and message text are identical in every port, gated by + [`fixtures/field-lint-conformance/`](../../fixtures/field-lint-conformance/README.md). In + Python, for a multi-file collection whose roots declare different packages, the address + printed for a `::`-relative package is expanded against the merged root's package and can + differ from the other ports. ### The prompt directory: `--prompts` everywhere (F101) diff --git a/examples/showcase/site-payload.json b/examples/showcase/site-payload.json index ee3e186ce..351cf1838 100644 --- a/examples/showcase/site-payload.json +++ b/examples/showcase/site-payload.json @@ -8,7 +8,7 @@ }, "counts": { "fixtures": 363, - "corpora": 25, + "corpora": 26, "baseTypes": 17 }, "snippets": { diff --git a/fixtures/field-lint-conformance/README.md b/fixtures/field-lint-conformance/README.md new file mode 100644 index 000000000..36ff3d9d2 --- /dev/null +++ b/fixtures/field-lint-conformance/README.md @@ -0,0 +1,74 @@ +# `verify` field-lint conformance corpus + +Every port's `verify` command prints an advisory **field authoring lint**. It reports two +metadata mistakes that load with no error: + +| Code | What it reports | +|---|---| +| `WARN_REFERENCE_FIELD_NOT_FOUND` | An `identity.reference` whose `@fields` names a field its object does not have. | +| `WARN_DUPLICATE_FIELD_NAME` | A field name declared more than once in one object's `children` list. | + +Both are warnings and never a load error: the compatibility policy +(`docs/compatibility-policy.md`) does not allow a new load error for metadata that loads +today. Neither reaches an exit code. + +This corpus is the shared source of truth for the codes, the node addresses and the message +text. Each port's own test suite runs it, so the ports cannot drift. + +## Fixture format + +Each case is a directory: + +- `input/` holds one or more metadata documents (`.json` or `.yaml`). +- `expected.json` holds `{ "findings": [{ "code", "path", "message" }] }`. + +A runner does three things, in this order: + +1. Load `input/` with the port's loader, strict, and assert **no load errors**. This is + what proves each condition loads today. +2. Run the reference half over the loaded model and the duplicate half over the raw files + in `input/`. +3. Compare the findings with `expected.json` as an unordered set of + `(code, path, message)`. + +`path` is the declaring object's resolution key (`::`), a dot, then the +identity name or the field name. In Python, for a multi-file collection whose roots declare +different packages, the address printed for a `::`-relative package is expanded against the +merged root's package and can differ from the other ports. + +## The two halves read different things + +**The reference half reads the loaded model.** Whether an object has a field is a question +about its *effective* field set, so the lint counts: + +- a field **inherited** through `extends` (`reference-field-inherited-clean`); +- a field added by an **overlay** file (`reference-field-from-overlay-clean`). + +A reference is reported once, on the object that declares it, and is checked against that +object's effective fields. An inherited reference is not repeated on every subtype +(`reference-declared-on-base-reported-once`). + +**The duplicate half reads the raw documents.** The TypeScript, C# and Java loaders fold a +repeated field into the first declaration and drop one of a different subtype, so their +loaded model keeps no trace of the duplicate. The Python loader keeps both nodes. One scan +of the document gives every port the same answer. The scan covers root-level objects, and +accepts the YAML authoring sugar (a bare `field` key, a scalar body, a `[]` key suffix). + +The scope is **one `children` list**. These are not findings: + +- a subtype redeclaring an inherited field. That is an override + (`duplicate-inherited-override-clean`). +- an overlay file redeclaring a field of its base. That is the overlay merge + (`duplicate-across-overlay-clean`). + +## Who asserts it + +| Port | Runner | +|---|---| +| TypeScript (reference) | `server/typescript/packages/cli/test/field-lint-conformance.test.ts` | +| C# | `server/csharp/MetaObjects.Cli.Tests/FieldLintConformanceTests.cs` | +| Java / Kotlin | `server/java/maven-plugin/src/test/java/com/metaobjects/mojo/FieldLintConformanceTest.java` | +| Python | `server/python/tests/conformance/test_field_lint_conformance.py` | + +Kotlin has no CLI of its own. Its codegen runs through the Maven `metaobjects:verify` goal, +so the Java runner covers it. diff --git a/fixtures/field-lint-conformance/clean/expected.json b/fixtures/field-lint-conformance/clean/expected.json new file mode 100644 index 000000000..2ef564861 --- /dev/null +++ b/fixtures/field-lint-conformance/clean/expected.json @@ -0,0 +1,3 @@ +{ + "findings": [] +} diff --git a/fixtures/field-lint-conformance/clean/input/meta.app.json b/fixtures/field-lint-conformance/clean/input/meta.app.json new file mode 100644 index 000000000..8cf39ef89 --- /dev/null +++ b/fixtures/field-lint-conformance/clean/input/meta.app.json @@ -0,0 +1,61 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.long": { + "name": "ownerId" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerId" + ], + "@references": "Owner" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-across-overlay-clean/expected.json b/fixtures/field-lint-conformance/duplicate-across-overlay-clean/expected.json new file mode 100644 index 000000000..2ef564861 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-across-overlay-clean/expected.json @@ -0,0 +1,3 @@ +{ + "findings": [] +} diff --git a/fixtures/field-lint-conformance/duplicate-across-overlay-clean/input/meta.app.json b/fixtures/field-lint-conformance/duplicate-across-overlay-clean/input/meta.app.json new file mode 100644 index 000000000..e60520cd9 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-across-overlay-clean/input/meta.app.json @@ -0,0 +1,32 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-across-overlay-clean/input/meta.app.ui.json b/fixtures/field-lint-conformance/duplicate-across-overlay-clean/input/meta.app.ui.json new file mode 100644 index 000000000..ae423dadf --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-across-overlay-clean/input/meta.app.ui.json @@ -0,0 +1,21 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "overlay": true, + "children": [ + { + "field.string": { + "name": "label", + "@maxLength": 40 + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-field-different-subtype/expected.json b/fixtures/field-lint-conformance/duplicate-field-different-subtype/expected.json new file mode 100644 index 000000000..a4fe17d78 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-different-subtype/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_DUPLICATE_FIELD_NAME", + "path": "acme::app::Item.label", + "message": "acme::app::Item declares the field \"label\" 2 times in one children list. Nothing reports this at load, and only the first declaration is certain to take effect. Remove or rename the duplicate." + } + ] +} diff --git a/fixtures/field-lint-conformance/duplicate-field-different-subtype/input/meta.app.json b/fixtures/field-lint-conformance/duplicate-field-different-subtype/input/meta.app.json new file mode 100644 index 000000000..88749480b --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-different-subtype/input/meta.app.json @@ -0,0 +1,37 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "field.int": { + "name": "label" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-field-own-package/expected.json b/fixtures/field-lint-conformance/duplicate-field-own-package/expected.json new file mode 100644 index 000000000..2e6cbaa3c --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-own-package/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_DUPLICATE_FIELD_NAME", + "path": "acme::app::stock::Item.label", + "message": "acme::app::stock::Item declares the field \"label\" 2 times in one children list. Nothing reports this at load, and only the first declaration is certain to take effect. Remove or rename the duplicate." + } + ] +} diff --git a/fixtures/field-lint-conformance/duplicate-field-own-package/input/meta.app.json b/fixtures/field-lint-conformance/duplicate-field-own-package/input/meta.app.json new file mode 100644 index 000000000..ffe0f684b --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-own-package/input/meta.app.json @@ -0,0 +1,38 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "package": "::stock", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-field-same-subtype/expected.json b/fixtures/field-lint-conformance/duplicate-field-same-subtype/expected.json new file mode 100644 index 000000000..a4fe17d78 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-same-subtype/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_DUPLICATE_FIELD_NAME", + "path": "acme::app::Item.label", + "message": "acme::app::Item declares the field \"label\" 2 times in one children list. Nothing reports this at load, and only the first declaration is certain to take effect. Remove or rename the duplicate." + } + ] +} diff --git a/fixtures/field-lint-conformance/duplicate-field-same-subtype/input/meta.app.json b/fixtures/field-lint-conformance/duplicate-field-same-subtype/input/meta.app.json new file mode 100644 index 000000000..41f0feaa0 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-same-subtype/input/meta.app.json @@ -0,0 +1,37 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-field-three-times/expected.json b/fixtures/field-lint-conformance/duplicate-field-three-times/expected.json new file mode 100644 index 000000000..9a6250138 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-three-times/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_DUPLICATE_FIELD_NAME", + "path": "acme::app::Item.label", + "message": "acme::app::Item declares the field \"label\" 3 times in one children list. Nothing reports this at load, and only the first declaration is certain to take effect. Remove or rename the duplicate." + } + ] +} diff --git a/fixtures/field-lint-conformance/duplicate-field-three-times/input/meta.app.json b/fixtures/field-lint-conformance/duplicate-field-three-times/input/meta.app.json new file mode 100644 index 000000000..6273359c3 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-three-times/input/meta.app.json @@ -0,0 +1,42 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "field.string": { + "name": "label" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/duplicate-field-yaml/expected.json b/fixtures/field-lint-conformance/duplicate-field-yaml/expected.json new file mode 100644 index 000000000..a4fe17d78 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-yaml/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_DUPLICATE_FIELD_NAME", + "path": "acme::app::Item.label", + "message": "acme::app::Item declares the field \"label\" 2 times in one children list. Nothing reports this at load, and only the first declaration is certain to take effect. Remove or rename the duplicate." + } + ] +} diff --git a/fixtures/field-lint-conformance/duplicate-field-yaml/input/meta.app.yaml b/fixtures/field-lint-conformance/duplicate-field-yaml/input/meta.app.yaml new file mode 100644 index 000000000..85d443996 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-field-yaml/input/meta.app.yaml @@ -0,0 +1,13 @@ +metadata: + package: acme::app + children: + - object.entity: + name: Item + children: + - field.long: id + - field.string: label + - field.string[]: + name: label + - identity.primary: + name: pk + fields: [id] diff --git a/fixtures/field-lint-conformance/duplicate-inherited-override-clean/expected.json b/fixtures/field-lint-conformance/duplicate-inherited-override-clean/expected.json new file mode 100644 index 000000000..2ef564861 --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-inherited-override-clean/expected.json @@ -0,0 +1,3 @@ +{ + "findings": [] +} diff --git a/fixtures/field-lint-conformance/duplicate-inherited-override-clean/input/meta.app.json b/fixtures/field-lint-conformance/duplicate-inherited-override-clean/input/meta.app.json new file mode 100644 index 000000000..cb0dc6bba --- /dev/null +++ b/fixtures/field-lint-conformance/duplicate-inherited-override-clean/input/meta.app.json @@ -0,0 +1,48 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Base", + "abstract": true, + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.string": { + "name": "label", + "@maxLength": 10 + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "extends": "Base", + "children": [ + { + "field.string": { + "name": "label", + "@maxLength": 20 + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-composite-one-missing/expected.json b/fixtures/field-lint-conformance/reference-composite-one-missing/expected.json new file mode 100644 index 000000000..848b0871f --- /dev/null +++ b/fixtures/field-lint-conformance/reference-composite-one-missing/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_REFERENCE_FIELD_NOT_FOUND", + "path": "acme::app::Item.owner_fk", + "message": "identity.reference \"owner_fk\" lists \"region\" in @fields, but acme::app::Item has no field of that name, inherited and overlaid fields included. Nothing checks this at load, so the reference is built on a field that does not exist. Rename the entry to an existing field, or declare the field." + } + ] +} diff --git a/fixtures/field-lint-conformance/reference-composite-one-missing/input/meta.app.json b/fixtures/field-lint-conformance/reference-composite-one-missing/input/meta.app.json new file mode 100644 index 000000000..22cae4cc2 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-composite-one-missing/input/meta.app.json @@ -0,0 +1,62 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.long": { + "name": "ownerId" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerId", + "region" + ], + "@references": "Owner" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-declared-on-base-reported-once/expected.json b/fixtures/field-lint-conformance/reference-declared-on-base-reported-once/expected.json new file mode 100644 index 000000000..6cb7d3b89 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-declared-on-base-reported-once/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_REFERENCE_FIELD_NOT_FOUND", + "path": "acme::app::Owned.owner_fk", + "message": "identity.reference \"owner_fk\" lists \"ownerIdd\" in @fields, but acme::app::Owned has no field of that name, inherited and overlaid fields included. Nothing checks this at load, so the reference is built on a field that does not exist. Rename the entry to an existing field, or declare the field." + } + ] +} diff --git a/fixtures/field-lint-conformance/reference-declared-on-base-reported-once/input/meta.app.json b/fixtures/field-lint-conformance/reference-declared-on-base-reported-once/input/meta.app.json new file mode 100644 index 000000000..78a716f59 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-declared-on-base-reported-once/input/meta.app.json @@ -0,0 +1,83 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Owned", + "abstract": true, + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerIdd" + ], + "@references": "Owner" + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "extends": "Owned", + "children": [ + { + "field.string": { + "name": "label" + } + } + ] + } + }, + { + "object.entity": { + "name": "Crate", + "extends": "Owned", + "children": [ + { + "field.string": { + "name": "tag" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-field-from-overlay-clean/expected.json b/fixtures/field-lint-conformance/reference-field-from-overlay-clean/expected.json new file mode 100644 index 000000000..2ef564861 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-from-overlay-clean/expected.json @@ -0,0 +1,3 @@ +{ + "findings": [] +} diff --git a/fixtures/field-lint-conformance/reference-field-from-overlay-clean/input/meta.app.db.json b/fixtures/field-lint-conformance/reference-field-from-overlay-clean/input/meta.app.db.json new file mode 100644 index 000000000..24cada513 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-from-overlay-clean/input/meta.app.db.json @@ -0,0 +1,20 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Item", + "overlay": true, + "children": [ + { + "field.long": { + "name": "ownerId" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-field-from-overlay-clean/input/meta.app.json b/fixtures/field-lint-conformance/reference-field-from-overlay-clean/input/meta.app.json new file mode 100644 index 000000000..fc8dfd872 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-from-overlay-clean/input/meta.app.json @@ -0,0 +1,56 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerId" + ], + "@references": "Owner" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-field-inherited-clean/expected.json b/fixtures/field-lint-conformance/reference-field-inherited-clean/expected.json new file mode 100644 index 000000000..2ef564861 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-inherited-clean/expected.json @@ -0,0 +1,3 @@ +{ + "findings": [] +} diff --git a/fixtures/field-lint-conformance/reference-field-inherited-clean/input/meta.app.json b/fixtures/field-lint-conformance/reference-field-inherited-clean/input/meta.app.json new file mode 100644 index 000000000..cc8d3dd8c --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-inherited-clean/input/meta.app.json @@ -0,0 +1,75 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Owned", + "abstract": true, + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.long": { + "name": "ownerId" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "extends": "Owned", + "children": [ + { + "field.string": { + "name": "label" + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerId" + ], + "@references": "Owner" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-field-missing-own-package/expected.json b/fixtures/field-lint-conformance/reference-field-missing-own-package/expected.json new file mode 100644 index 000000000..f00f5c63e --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-missing-own-package/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_REFERENCE_FIELD_NOT_FOUND", + "path": "acme::app::stock::Item.owner_fk", + "message": "identity.reference \"owner_fk\" lists \"ownerIdd\" in @fields, but acme::app::stock::Item has no field of that name, inherited and overlaid fields included. Nothing checks this at load, so the reference is built on a field that does not exist. Rename the entry to an existing field, or declare the field." + } + ] +} diff --git a/fixtures/field-lint-conformance/reference-field-missing-own-package/input/meta.app.json b/fixtures/field-lint-conformance/reference-field-missing-own-package/input/meta.app.json new file mode 100644 index 000000000..b27e4228e --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-missing-own-package/input/meta.app.json @@ -0,0 +1,63 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "package": "::stock", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "package": "::stock", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.long": { + "name": "ownerId" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerIdd" + ], + "@references": "Owner" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/field-lint-conformance/reference-field-missing/expected.json b/fixtures/field-lint-conformance/reference-field-missing/expected.json new file mode 100644 index 000000000..963a686c0 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-missing/expected.json @@ -0,0 +1,9 @@ +{ + "findings": [ + { + "code": "WARN_REFERENCE_FIELD_NOT_FOUND", + "path": "acme::app::Item.owner_fk", + "message": "identity.reference \"owner_fk\" lists \"ownerIdd\" in @fields, but acme::app::Item has no field of that name, inherited and overlaid fields included. Nothing checks this at load, so the reference is built on a field that does not exist. Rename the entry to an existing field, or declare the field." + } + ] +} diff --git a/fixtures/field-lint-conformance/reference-field-missing/input/meta.app.json b/fixtures/field-lint-conformance/reference-field-missing/input/meta.app.json new file mode 100644 index 000000000..52509a3e1 --- /dev/null +++ b/fixtures/field-lint-conformance/reference-field-missing/input/meta.app.json @@ -0,0 +1,61 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.entity": { + "name": "Owner", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.entity": { + "name": "Item", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.long": { + "name": "ownerId" + } + }, + { + "identity.primary": { + "name": "pk", + "@fields": [ + "id" + ] + } + }, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [ + "ownerIdd" + ], + "@references": "Owner" + } + } + ] + } + } + ] + } +} diff --git a/server/csharp/MetaObjects.Cli.Tests/FieldLintConformanceTests.cs b/server/csharp/MetaObjects.Cli.Tests/FieldLintConformanceTests.cs new file mode 100644 index 000000000..6e9612fa0 --- /dev/null +++ b/server/csharp/MetaObjects.Cli.Tests/FieldLintConformanceTests.cs @@ -0,0 +1,59 @@ +using System.Text.Json; +using MetaObjects.Cli; +using MetaObjects.Loader; +using Xunit; + +namespace MetaObjects.Cli.Tests; + +/// +/// Cross-port field-lint conformance corpus — fixtures/field-lint-conformance/. See +/// that directory's README.md for the fixture format. Every case is LOADED strict +/// first, so "this loads with no error today" is proven by the fixture, and only then +/// linted. Mirrors server/typescript/packages/cli/test/field-lint-conformance.test.ts +/// exactly; a mismatch here is a bug in THIS port's lint, never in the fixture. +/// +public class FieldLintConformanceTests +{ + private static readonly string CorpusDir = ResolveCorpusDir(); + + private static string ResolveCorpusDir() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null) + { + var candidate = Path.Combine(dir.FullName, "fixtures", "field-lint-conformance"); + if (Directory.Exists(candidate)) return candidate; + dir = dir.Parent; + } + throw new DirectoryNotFoundException( + "Could not locate fixtures/field-lint-conformance/ by walking up from " + AppContext.BaseDirectory); + } + + public static IEnumerable Fixtures() => + Directory.GetDirectories(CorpusDir).OrderBy(d => d, StringComparer.Ordinal).Select(d => new object[] { Path.GetFileName(d) }); + + [Fact] + public void Discovers_the_corpus() => Assert.NotEmpty(Fixtures()); + + [Theory] + [MemberData(nameof(Fixtures))] + public void Fixture(string name) + { + var input = Path.Combine(CorpusDir, name, "input"); + using var doc = JsonDocument.Parse(File.ReadAllText(Path.Combine(CorpusDir, name, "expected.json"))); + var expected = doc.RootElement.GetProperty("findings").EnumerateArray() + .Select(f => (f.GetProperty("code").GetString()!, f.GetProperty("path").GetString()!, f.GetProperty("message").GetString()!)) + .Order().ToList(); + + var load = MetaDataLoader.FromDirectory(input, strict: true); + Assert.Empty(load.Errors.Select(e => e.Code + ": " + e.Message)); + + var files = Directory.GetFiles(input).OrderBy(f => f, StringComparer.Ordinal); + var actual = FieldLint.LintReferenceFields(load.Root) + .Concat(FieldLint.LintDuplicateFields(files)) + .Select(f => (f.Code, f.Path, f.Message)) + .Order().ToList(); + + Assert.Equal(expected, actual); + } +} diff --git a/server/csharp/MetaObjects.Cli.Tests/VerifyFieldLintTests.cs b/server/csharp/MetaObjects.Cli.Tests/VerifyFieldLintTests.cs new file mode 100644 index 000000000..a0bf9705c --- /dev/null +++ b/server/csharp/MetaObjects.Cli.Tests/VerifyFieldLintTests.cs @@ -0,0 +1,76 @@ +using Xunit; + +namespace MetaObjects.Cli.Tests; + +/// +/// dotnet meta verify — the field authoring lint, end to end: printed as its own +/// advisory section on stderr, never reaching the exit code, and muted by +/// --no-field-lint. Drives the BUILT CLI () so this proves +/// the wiring; the finding text is gated cross-port by . +/// +public sealed class VerifyFieldLintTests : IDisposable +{ + private readonly string _tmp = Directory.CreateTempSubdirectory("mo-field-lint-").FullName; + + public void Dispose() + { + try { Directory.Delete(_tmp, recursive: true); } catch { /* best effort */ } + } + + private (string Model, string Templates) Project(string referenceField, bool duplicateLabel) + { + const string label = """{ "field.string": { "name": "label" } },"""; + var model = Path.Combine(_tmp, "model"); + Directory.CreateDirectory(model); + File.WriteAllText(Path.Combine(model, "meta.app.json"), $$""" + { "metadata.root": { "package": "app", "children": [ + { "object.entity": { "name": "Owner", "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "name": "pk", "@fields": ["id"] } } ] } }, + { "object.entity": { "name": "Item", "children": [ + { "field.long": { "name": "id" } }, + { "field.long": { "name": "ownerId" } }, + {{label}} + {{(duplicateLabel ? label : "")}} + { "identity.primary": { "name": "pk", "@fields": ["id"] } }, + { "identity.reference": { "name": "owner_fk", "@fields": ["{{referenceField}}"], "@references": "Owner" } } ] } } + ] } } + """); + var templates = Path.Combine(_tmp, "templates"); + Directory.CreateDirectory(templates); + return (model, templates); + } + + [Fact] + public void Findings_are_advisory_and_do_not_change_the_exit_code() + { + var (model, templates) = Project("ownerIdd", duplicateLabel: true); + var (exitCode, stdout, stderr) = CliProcess.Run(_tmp, "verify", model, "--templates", "--prompts", templates); + + Assert.True(exitCode == 0, $"exit={exitCode}\nstdout={stdout}\nstderr={stderr}"); + Assert.Contains("dotnet meta verify — fields: 2 authoring warning(s) (advisory — does not fail the build):", stderr); + Assert.Contains("WARN_REFERENCE_FIELD_NOT_FOUND [app::Item.owner_fk]", stderr); + Assert.Contains("WARN_DUPLICATE_FIELD_NAME [app::Item.label]", stderr); + } + + [Fact] + public void Clean_metadata_prints_no_section() + { + var (model, templates) = Project("ownerId", duplicateLabel: false); + var (exitCode, _, stderr) = CliProcess.Run(_tmp, "verify", model, "--templates", "--prompts", templates); + + Assert.Equal(0, exitCode); + Assert.DoesNotContain("fields:", stderr); + } + + [Fact] + public void No_field_lint_silences_it() + { + var (model, templates) = Project("ownerIdd", duplicateLabel: true); + var (exitCode, _, stderr) = CliProcess.Run(_tmp, "verify", model, "--templates", "--prompts", templates, "--no-field-lint"); + + Assert.Equal(0, exitCode); + Assert.DoesNotContain("WARN_REFERENCE_FIELD_NOT_FOUND", stderr); + Assert.DoesNotContain("WARN_DUPLICATE_FIELD_NAME", stderr); + } +} diff --git a/server/csharp/MetaObjects.Cli/FieldLint.cs b/server/csharp/MetaObjects.Cli/FieldLint.cs new file mode 100644 index 000000000..294799e76 --- /dev/null +++ b/server/csharp/MetaObjects.Cli/FieldLint.cs @@ -0,0 +1,286 @@ +// `dotnet meta verify` — the field AUTHORING lint. +// +// Two metadata mistakes about an object's FIELDS load with no error on every port: +// +// 1. An `identity.reference` whose `@fields` names a field the object does not have. +// The loader resolves `@references` (the target) and never looks at `@fields`, so a +// typo there produces a foreign key over a column nothing declares. +// 2. Two `field.*` children with the same `name` in one object's `children` list. The +// later declaration is folded into the first, and one of a different subtype is +// dropped, both silently. +// +// WHY THESE ARE WARNINGS AND NOT LOAD ERRORS. docs/compatibility-policy.md does not allow +// a new load error for metadata that loads today, so every finding here is a warning by +// construction: nothing in this file reaches an exit code. +// +// THE TWO HALVES READ DIFFERENT THINGS, and have to. The reference half reads the LOADED +// model, because "does this object have that field" is a question about the EFFECTIVE +// field set — inherited through `extends` and merged from overlay files. The duplicate +// half reads the RAW DOCUMENTS, because the merge has already erased the duplicate from +// the model by the time anything can ask. +// +// Mirrors the TS reference (server/typescript/packages/cli/src/lib/field-lint.ts + +// packages/metadata/src/loader/declared-duplicate-fields.ts). The codes, the message +// text and the fixtures are shared: fixtures/field-lint-conformance/. + +using System.Text.Encodings.Web; +using System.Text.Json; +using MetaObjects.Loader; +using MetaObjects.Meta; +using YamlDotNet.RepresentationModel; +using static MetaObjects.Core.Identity.IdentityConstants; +using static MetaObjects.Shared.BaseTypes; +using static MetaObjects.Shared.Structural; + +namespace MetaObjects.Cli; + +/// The advisory field lint dotnet meta verify runs on every invocation. +public static class FieldLint +{ + /// An identity.reference lists a field its object does not have. + public const string WARN_REFERENCE_FIELD_NOT_FOUND = "WARN_REFERENCE_FIELD_NOT_FOUND"; + /// One children list declares the same field name more than once. + public const string WARN_DUPLICATE_FIELD_NAME = "WARN_DUPLICATE_FIELD_NAME"; + + /// Opt-out environment variable, beside --no-field-lint (Node meta parity). + public const string EnvOptOut = "META_NO_FIELD_LINT"; + + private const string KeyName = "name"; + private const string KeyPackage = "package"; + private const string KeyChildren = "children"; + private const char TypeSubTypeSeparator = '.'; + /// The YAML authoring sugar for isArray: true — a suffix on the wrapper key. + private const string ArraySuffix = "[]"; + + /// One advisory finding: its code, the node's address, and the message. + public sealed record Finding(string Code, string Path, string Message); + + // The JSON string form, so the text is byte-identical to the other ports'. The relaxed + // encoder keeps `"` as `\"` and leaves non-ASCII alone, as JSON.stringify does. + private static readonly JsonSerializerOptions QuoteOptions = new() { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; + + private static string Quote(string value) => JsonSerializer.Serialize(value, QuoteOptions); + + /// Report every identity.reference whose @fields names a field its object lacks. + public static List LintReferenceFields(MetaData root) + { + var findings = new List(); + // OWN-ONLY (ADR-0039 sanctioned case): a root has no super, and its own children + // are the declared objects. + foreach (var obj in root.OwnChildren()) + { + if (obj.Type != TYPE_OBJECT) continue; + var address = obj.ResolutionKey(); + // RESOLVING: the effective field set — a field inherited through `extends` or + // added by an overlay file is a field the object has. + var fields = obj.Children().Where(c => c.Type == TYPE_FIELD).Select(c => c.Name).ToHashSet(StringComparer.Ordinal); + // OWN-ONLY (ADR-0039 sanctioned case): report each DECLARATION once, on the + // object that declares it. The resolving Children() would repeat an inherited + // reference on every subtype. + foreach (var identity in obj.OwnChildren()) + { + if (identity.Type != TYPE_IDENTITY || identity.SubType != IDENTITY_SUBTYPE_REFERENCE) continue; + // RESOLVING: `@fields` may itself be inherited through the identity's `extends`. + foreach (var name in ListedFields(identity.Attr(IDENTITY_ATTR_FIELDS))) + { + if (fields.Contains(name)) continue; + findings.Add(new Finding( + WARN_REFERENCE_FIELD_NOT_FOUND, + $"{address}.{identity.Name}", + $"identity.reference {Quote(identity.Name)} lists {Quote(name)} in @fields, but " + + $"{address} has no field of that name, inherited and overlaid fields included. Nothing " + + "checks this at load, so the reference is built on a field that does not exist. Rename " + + "the entry to an existing field, or declare the field.")); + } + } + } + return findings; + } + + private static IEnumerable ListedFields(object? listed) => listed switch + { + string single => [single], + IEnumerable many => many, + IEnumerable many => many.OfType(), + _ => [], + }; + + /// + /// Structurally scan one document's raw content and report every field name a root-level + /// object declares more than once in its own children list. + /// + /// + /// Scope is ONE children list: a field redeclared by an overlay file, or by a subtype + /// overriding an inherited field, is not in one list and is not a finding. Malformed + /// shapes return an empty list; a syntax error throws, as the parser's own would. + /// + public static List DeclaredDuplicateFields(string content, MetaDataFormat format) + { + var findings = new List(); + var normalized = content.Length > 0 && content[0] == '' ? content[1..] : content; + var parsed = format == MetaDataFormat.Yaml ? ParseYaml(normalized) : ParseJson(normalized); + if (parsed is not List> document) return findings; + + // JSON fuses the subtype onto the root key; sigil-free YAML may write the bare type. + var rootBody = Get(document, $"{TYPE_METADATA}{TypeSubTypeSeparator}{SUBTYPE_ROOT}") ?? Get(document, TYPE_METADATA); + if (rootBody is not List> root) return findings; + var rootPkg = Get(root, KeyPackage) as string ?? ""; + if (Get(root, KeyChildren) is not List children) return findings; + + foreach (var child in children) + { + if (child is not List> wrapper) continue; + foreach (var (wrapperKey, bodyValue) in wrapper) + { + if (WrapperType(wrapperKey) != TYPE_OBJECT) continue; + if (bodyValue is not List> body) continue; + var name = DeclaredName(body); + if (name is null || Get(body, KeyChildren) is not List members) continue; + + // Insertion-ordered, so findings come out in document order. + var order = new List(); + var counts = new Dictionary(StringComparer.Ordinal); + foreach (var member in members) + { + if (member is not List> memberWrapper) continue; + foreach (var (memberKey, memberBody) in memberWrapper) + { + if (WrapperType(memberKey) != TYPE_FIELD) continue; + var fieldName = DeclaredName(memberBody); + if (fieldName is null) continue; + if (!counts.TryGetValue(fieldName, out var seen)) order.Add(fieldName); + counts[fieldName] = seen + 1; + } + } + + // The resolution key the parser gives a root-level node: its own `package` + // (a `::`-prefixed one is relative to the root's), else the root's. + var ownPkg = Get(body, KeyPackage) as string; + var pkg = string.IsNullOrEmpty(ownPkg) + ? rootPkg + : rootPkg.Trim() != "" && ownPkg.StartsWith(PACKAGE_SEPARATOR, StringComparison.Ordinal) + ? rootPkg + ownPkg + : ownPkg; + var address = pkg != "" ? $"{pkg}{PACKAGE_SEPARATOR}{name}" : name; + foreach (var fieldName in order) + { + var count = counts[fieldName]; + if (count < 2) continue; + findings.Add(new Finding( + WARN_DUPLICATE_FIELD_NAME, + $"{address}.{fieldName}", + $"{address} declares the field {Quote(fieldName)} {count} times in one children list. " + + "Nothing reports this at load, and only the first declaration is certain to take " + + "effect. Remove or rename the duplicate.")); + } + } + } + return findings; + } + + /// + /// Run over each metadata file. Unreadable or + /// unparsable files are skipped — the loader reports those itself. + /// + public static List LintDuplicateFields(IEnumerable files) + { + var findings = new List(); + foreach (var path in files) + { + try + { + var source = new FileSource(path); + findings.AddRange(DeclaredDuplicateFields(source.Read(), source.Format)); + } + catch (Exception) + { + // An advisory scan never breaks verify. + } + } + return findings; + } + + /// + /// Load the metadata verify was pointed at, lint it, and print the findings to + /// . Warnings ONLY: this never throws and never changes the + /// exit code. Loads LENIENT so the findings are the same with and without --lax; + /// a load failure is the gate's to report, so it stays silent here. + /// + public static void RunAdvisory(string metadataDir, IReadOnlyList? metadataFiles, IReadOnlyList? libraries, TextWriter stderr) + { + List findings; + try + { + var load = metadataFiles is { } resolved + ? MetaDataLoader.FromUris(resolved.Select(f => new Uri(f)).ToList(), libraries, false) + : MetaDataLoader.FromDirectory(metadataDir, libraries, strict: false); + if (load.Errors.Count > 0) return; + var files = metadataFiles ?? new DirectorySource(metadataDir).Expand().Select(f => f.FilePath).ToList(); + findings = [.. LintReferenceFields(load.Root), .. LintDuplicateFields(files)]; + } + catch (Exception) + { + return; + } + if (findings.Count == 0) return; + stderr.WriteLine($"dotnet meta verify — fields: {findings.Count} authoring warning(s) (advisory — does not fail the build):"); + foreach (var f in findings) stderr.WriteLine($" {f.Code} [{f.Path}]: {f.Message}"); + } + + // -- raw document model --------------------------------------------------- + // A mapping is an ORDERED list of pairs, a sequence a List, a scalar a string. + + private static object? Get(List> mapping, string key) + { + foreach (var (k, v) in mapping) + if (k == key) return v; + return null; + } + + /// The TYPE segment of a wrapper key: field.string[] and bare field are both field. + private static string WrapperType(string key) + { + var dot = key.IndexOf(TypeSubTypeSeparator); + var head = dot < 0 ? key : key[..dot]; + return head.EndsWith(ArraySuffix, StringComparison.Ordinal) ? head[..^ArraySuffix.Length] : head; + } + + /// A node's declared name: the body's name, or the body itself when YAML wrote a scalar. + private static string? DeclaredName(object? body) + { + var name = body is List> mapping ? Get(mapping, KeyName) : body; + return name is string s && s != "" ? s : null; + } + + private static object? ParseJson(string text) + { + using var doc = JsonDocument.Parse(text); + return FromJson(doc.RootElement); + } + + private static object? FromJson(JsonElement element) => element.ValueKind switch + { + JsonValueKind.Object => element.EnumerateObject() + .Select(p => new KeyValuePair(p.Name, FromJson(p.Value))).ToList(), + JsonValueKind.Array => element.EnumerateArray().Select(FromJson).ToList(), + JsonValueKind.String => element.GetString(), + _ => null, + }; + + private static object? ParseYaml(string text) + { + var stream = new YamlStream(); + stream.Load(new StringReader(text)); + return stream.Documents.Count == 0 ? null : FromYaml(stream.Documents[0].RootNode); + } + + private static object? FromYaml(YamlNode node) => node switch + { + YamlMappingNode mapping => mapping.Children + .Where(p => p.Key is YamlScalarNode) + .Select(p => new KeyValuePair(((YamlScalarNode)p.Key).Value ?? "", FromYaml(p.Value))).ToList(), + YamlSequenceNode sequence => sequence.Children.Select(FromYaml).ToList(), + YamlScalarNode scalar => scalar.Value, + _ => null, + }; +} diff --git a/server/csharp/MetaObjects.Cli/Program.cs b/server/csharp/MetaObjects.Cli/Program.cs index 47e7c116a..eea1dd603 100644 --- a/server/csharp/MetaObjects.Cli/Program.cs +++ b/server/csharp/MetaObjects.Cli/Program.cs @@ -457,7 +457,7 @@ static int RunVerify(string[] rest) string? generatorsCsv = null; string? templateRoot = null; string? columnNamingRaw = null; - bool templates = false, codegen = false, db = false, lax = false; + bool templates = false, codegen = false, db = false, lax = false, noFieldLint = false; for (int i = 0; i < rest.Length; i++) { @@ -482,6 +482,9 @@ static int RunVerify(string[] rest) // --lax (#96 / ADR-0023): restore the legacy open-attr load. verify is // strict-by-default — an undeclared/typo'd own @attr is ERR_UNKNOWN_ATTR. else if (a == "--lax") lax = true; + // Mutes the advisory field AUTHORING lint (FieldLint) — never a gate, so this + // changes what is printed and nothing else. META_NO_FIELD_LINT=1 does the same. + else if (a == "--no-field-lint") noFieldLint = true; else if (a == "--out" && i + 1 < rest.Length) outDir = rest[++i]; else if (a == "--namespace" && i + 1 < rest.Length) { ns = rest[++i]; nsExplicit = true; } else if (a == "--generators" && i + 1 < rest.Length) generatorsCsv = rest[++i]; @@ -495,7 +498,7 @@ static int RunVerify(string[] rest) else if (a.StartsWith('-')) { Console.Error.WriteLine($"dotnet meta verify: unknown option \"{a}\""); - Console.Error.WriteLine("usage: dotnet meta verify [--templates [--prompts ]] [--codegen --out [--namespace ] [--column-naming literal|snake_case|kebab-case]] [--db] [--lax]"); + Console.Error.WriteLine("usage: dotnet meta verify [--templates [--prompts ]] [--codegen --out [--namespace ] [--column-naming literal|snake_case|kebab-case]] [--db] [--lax] [--no-field-lint]"); return 2; } else if (metadataDir is null) metadataDir = a; @@ -636,6 +639,12 @@ static int RunVerify(string[] rest) if (result.DbRejectionMessage is not null) Console.Error.WriteLine($" {result.DbRejectionMessage}"); + // The field AUTHORING lint — a reference identity over a field the object lacks, a + // field name declared twice in one children list. Both load clean on every port. Runs + // on every `verify`, whichever gates were selected; warnings ONLY, never the exit code. + if (!noFieldLint && Environment.GetEnvironmentVariable(FieldLint.EnvOptOut) != "1") + FieldLint.RunAdvisory(resolvedMeta.Directory, resolvedMeta.Files, resolvedMeta.Libraries, Console.Error); + // The handed-off codegen gate already printed its own verdict (inherited console); // fold its exit code into the aggregate the same way every other subverb does — max, // non-zero on any drift. diff --git a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/FieldLint.java b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/FieldLint.java new file mode 100644 index 000000000..a669f9919 --- /dev/null +++ b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/FieldLint.java @@ -0,0 +1,214 @@ +package com.metaobjects.mojo; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.metaobjects.field.MetaField; +import com.metaobjects.identity.MetaIdentity; +import com.metaobjects.loader.MetaDataLoader; +import com.metaobjects.object.MetaObject; +import org.yaml.snakeyaml.LoaderOptions; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.SafeConstructor; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; + +/** + * {@code metaobjects:verify} — the field AUTHORING lint. + * + *

Two metadata mistakes about an object's FIELDS load with no error on every port:

+ *
    + *
  1. An {@code identity.reference} whose {@code @fields} names a field the object does + * not have. The loader resolves {@code @references} (the target) and never looks at + * {@code @fields}, so a typo there produces a foreign key over a column nothing + * declares.
    2. + *
  2. Two {@code field.*} children with the same {@code name} in one object's + * {@code children} list. The later declaration is folded into the first, and one of + * a different subtype is dropped, both silently.
    3. + *
+ * + *

Why these are warnings and not load errors. {@code docs/compatibility-policy.md} + * does not allow a new load error for metadata that loads today, so every finding here is a + * warning by construction: nothing in this class fails a build.

+ * + *

The two halves read different things, and have to. The reference half reads the + * LOADED model, because "does this object have that field" is a question about the EFFECTIVE + * field set — inherited through {@code extends} and merged from overlay files. The duplicate + * half reads the RAW DOCUMENTS, because the merge has already erased the duplicate from the + * model by the time anything can ask.

+ * + *

Mirrors the TS reference ({@code packages/cli/src/lib/field-lint.ts} + + * {@code packages/metadata/src/loader/declared-duplicate-fields.ts}). The codes, the message + * text and the fixtures are shared: {@code fixtures/field-lint-conformance/}. Kotlin runs + * through this same goal.

+ */ +public final class FieldLint { + + /** An {@code identity.reference} lists a field its object does not have. */ + public static final String WARN_REFERENCE_FIELD_NOT_FOUND = "WARN_REFERENCE_FIELD_NOT_FOUND"; + /** One {@code children} list declares the same field name more than once. */ + public static final String WARN_DUPLICATE_FIELD_NAME = "WARN_DUPLICATE_FIELD_NAME"; + + /** Opt-out environment variable, beside {@code -Dmeta.verify.noFieldLint} (Node {@code meta} parity). */ + public static final String ENV_OPT_OUT = "META_NO_FIELD_LINT"; + + private static final String KEY_NAME = "name"; + private static final String KEY_PACKAGE = "package"; + private static final String KEY_CHILDREN = "children"; + private static final String ROOT_KEY_BARE = "metadata"; + private static final String ROOT_KEY_FUSED = "metadata.root"; + private static final String TYPE_OBJECT = MetaObject.TYPE_OBJECT; + private static final String TYPE_FIELD = MetaField.TYPE_FIELD; + private static final String PACKAGE_SEPARATOR = "::"; + private static final char TYPE_SUBTYPE_SEPARATOR = '.'; + /** The YAML authoring sugar for {@code isArray: true} — a suffix on the wrapper key. */ + private static final String ARRAY_SUFFIX = "[]"; + + // The JSON string form, so the text is byte-identical to the other ports'. + private static final Gson QUOTER = new GsonBuilder().disableHtmlEscaping().create(); + + /** One advisory finding: its code, the node's address, and the message. */ + public record Finding(String code, String path, String message) {} + + private FieldLint() {} + + private static String quote(String value) { + return QUOTER.toJson(value); + } + + /** Report every {@code identity.reference} whose {@code @fields} names a field its object lacks. */ + public static List lintReferenceFields(MetaDataLoader loader) { + List findings = new ArrayList<>(); + // ADR-0039: own — the root's own children in declaration order (a root has no super). + for (MetaObject object : loader.getRoot().getChildren(MetaObject.class, false)) { + String address = object.getName(); + // RESOLVING: the effective field set — a field inherited through `extends` or + // added by an overlay file is a field the object has. + Set fields = new HashSet<>(); + for (MetaField field : object.getMetaFields()) fields.add(field.getName()); + // ADR-0039: own (sanctioned case) — report each DECLARATION once, on the object + // that declares it. The resolving read would repeat an inherited reference on + // every subtype. + for (MetaIdentity identity : object.getChildren(MetaIdentity.class, false)) { + if (!identity.isReference()) continue; + // RESOLVING: getFields() reads @fields through the identity's own `extends`. + for (String name : identity.getFields()) { + if (fields.contains(name)) continue; + findings.add(new Finding( + WARN_REFERENCE_FIELD_NOT_FOUND, + address + "." + identity.getName(), + "identity.reference " + quote(identity.getName()) + " lists " + quote(name) + + " in @fields, but " + address + " has no field of that name, inherited and " + + "overlaid fields included. Nothing checks this at load, so the reference is " + + "built on a field that does not exist. Rename the entry to an existing field, " + + "or declare the field.")); + } + } + } + return findings; + } + + /** + * Structurally scan one document's raw content and report every field name a root-level + * object declares more than once in its own {@code children} list. + * + *

Scope is ONE children list: a field redeclared by an overlay file, or by a subtype + * overriding an inherited field, is not in one list and is not a finding. Malformed + * shapes return an empty list; a syntax error throws, as the parser's own would.

+ * + * @param yaml {@code true} for a YAML document, {@code false} for JSON + */ + public static List declaredDuplicateFields(String content, boolean yaml) { + List findings = new ArrayList<>(); + String normalized = !content.isEmpty() && content.charAt(0) == '' ? content.substring(1) : content; + Object parsed = yaml + ? new Yaml(new SafeConstructor(new LoaderOptions())).load(normalized) + : QUOTER.fromJson(normalized, Object.class); + if (!(parsed instanceof Map document)) return findings; + + // JSON fuses the subtype onto the root key; sigil-free YAML may write the bare type. + Object rootBody = document.containsKey(ROOT_KEY_FUSED) ? document.get(ROOT_KEY_FUSED) : document.get(ROOT_KEY_BARE); + if (!(rootBody instanceof Map root)) return findings; + String rootPkg = root.get(KEY_PACKAGE) instanceof String s ? s : ""; + if (!(root.get(KEY_CHILDREN) instanceof List children)) return findings; + + for (Object child : children) { + if (!(child instanceof Map wrapper)) continue; + for (Map.Entry entry : wrapper.entrySet()) { + if (!(entry.getKey() instanceof String wrapperKey) || !TYPE_OBJECT.equals(wrapperType(wrapperKey))) continue; + if (!(entry.getValue() instanceof Map body)) continue; + String name = declaredName(body); + if (name == null || !(body.get(KEY_CHILDREN) instanceof List members)) continue; + + // Insertion-ordered, so findings come out in document order. + Map counts = new LinkedHashMap<>(); + for (Object member : members) { + if (!(member instanceof Map memberWrapper)) continue; + for (Map.Entry memberEntry : memberWrapper.entrySet()) { + if (!(memberEntry.getKey() instanceof String memberKey) || !TYPE_FIELD.equals(wrapperType(memberKey))) continue; + String fieldName = declaredName(memberEntry.getValue()); + if (fieldName != null) counts.merge(fieldName, 1, Integer::sum); + } + } + + // The resolution key the parser gives a root-level node: its own `package` + // (a `::`-prefixed one is relative to the root's), else the root's. + String pkg = rootPkg; + if (body.get(KEY_PACKAGE) instanceof String ownPkg && !ownPkg.isEmpty()) { + boolean relative = !rootPkg.trim().isEmpty() && ownPkg.startsWith(PACKAGE_SEPARATOR); + pkg = relative ? rootPkg + ownPkg : ownPkg; + } + String address = pkg.isEmpty() ? name : pkg + PACKAGE_SEPARATOR + name; + for (Map.Entry count : counts.entrySet()) { + if (count.getValue() < 2) continue; + findings.add(new Finding( + WARN_DUPLICATE_FIELD_NAME, + address + "." + count.getKey(), + address + " declares the field " + quote(count.getKey()) + " " + count.getValue() + + " times in one children list. Nothing reports this at load, and only the first " + + "declaration is certain to take effect. Remove or rename the duplicate.")); + } + } + } + return findings; + } + + /** + * Run {@link #declaredDuplicateFields} over each metadata file. Unreadable or unparsable + * files are skipped — the loader reports those itself. + */ + public static List lintDuplicateFields(List files) { + List findings = new ArrayList<>(); + for (Path file : files) { + try { + String lower = file.getFileName().toString().toLowerCase(Locale.ROOT); + boolean yaml = lower.endsWith(".yaml") || lower.endsWith(".yml"); + findings.addAll(declaredDuplicateFields(Files.readString(file, StandardCharsets.UTF_8), yaml)); + } catch (Exception e) { + // An advisory scan never breaks verify. + } + } + return findings; + } + + /** The TYPE segment of a wrapper key: {@code field.string[]} and bare {@code field} are both {@code field}. */ + private static String wrapperType(String key) { + int dot = key.indexOf(TYPE_SUBTYPE_SEPARATOR); + String head = dot < 0 ? key : key.substring(0, dot); + return head.endsWith(ARRAY_SUFFIX) ? head.substring(0, head.length() - ARRAY_SUFFIX.length()) : head; + } + + /** A node's declared name: the body's {@code name}, or the body itself when YAML wrote a scalar. */ + private static String declaredName(Object body) { + Object name = body instanceof Map mapping ? mapping.get(KEY_NAME) : body; + return name instanceof String s && !s.isEmpty() ? s : null; + } +} diff --git a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java index f7f814999..795ea45ae 100644 --- a/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java +++ b/server/java/maven-plugin/src/main/java/com/metaobjects/mojo/MetaDataVerifyMojo.java @@ -105,6 +105,18 @@ public class MetaDataVerifyMojo extends AbstractMetaDataMojo { public void setTemplateRoot(String templateRoot) { this.templateRoot = templateRoot; } public String getTemplateRoot() { return templateRoot; } + /** + * Mute the advisory field AUTHORING lint ({@link FieldLint}) — a reference identity + * over a field the object lacks, and a field name declared twice in one children + * list. Never a gate: it only prints warnings, so this changes what is logged and + * nothing else. {@code META_NO_FIELD_LINT=1} does the same. + */ + @Parameter(property = "meta.verify.noFieldLint", defaultValue = "false") + private boolean noFieldLint = false; + + public void setNoFieldLint(boolean noFieldLint) { this.noFieldLint = noFieldLint; } + public boolean isNoFieldLint() { return noFieldLint; } + @Override public void execute() throws MojoExecutionException, MojoFailureException { // #233: warm the global registry singletons before verify builds its loader, @@ -133,6 +145,47 @@ public void execute() throws MojoExecutionException, MojoFailureException { verifyCodegen(); } + // ------------------------------------------------------------------------ + // the field authoring lint — advisory, every mode + // ------------------------------------------------------------------------ + + /** + * Runs on every {@code verify}, whichever mode was selected, as soon as the metadata + * has loaded — so its warnings are printed even when the gate then fails the build. + * Warnings ONLY: this never throws and never changes the build result. + */ + private void runFieldLintAdvisory(MetaDataLoader loader) { + if (noFieldLint || "1".equals(System.getenv(FieldLint.ENV_OPT_OUT))) return; + List findings = new ArrayList<>(); + try { + findings.addAll(FieldLint.lintReferenceFields(loader)); + findings.addAll(FieldLint.lintDuplicateFields(sourceFiles(loader))); + } catch (RuntimeException e) { + return; // an advisory scan never breaks verify + } + if (findings.isEmpty()) return; + getLog().warn("metaobjects:verify — fields: " + findings.size() + + " authoring warning(s) (advisory — does not fail the build):"); + for (FieldLint.Finding f : findings) { + getLog().warn(" " + f.code() + " [" + f.path() + "]: " + f.message()); + } + } + + /** The on-disk metadata files this loader read — the documents the duplicate scan reads raw. */ + private static List sourceFiles(MetaDataLoader loader) { + List files = new ArrayList<>(); + if (loader.getSourceURIs() == null) return files; + for (java.net.URI uri : loader.getSourceURIs()) { + com.metaobjects.loader.uri.URIModel model = com.metaobjects.loader.uri.URIHelper.toURIModel(uri); + if (!com.metaobjects.loader.uri.URIHelper.URI_SOURCE_FILE.equals(model.getUriSourceType())) continue; + // A relative carries its as a URI argument — resolve it the + // way URIHelper's own stream opener does, or the scan reads nothing. + String sourceDir = model.getUriArg(com.metaobjects.loader.uri.URIHelper.URI_ARG_SOURCEDIR); + files.add(sourceDir != null ? Paths.get(sourceDir, model.getUriSource()) : Paths.get(model.getUriSource())); + } + return files; + } + // ------------------------------------------------------------------------ // mode=templates — template/prompt {{field}}<->payload drift (ADR-0021 D2) // ------------------------------------------------------------------------ @@ -146,6 +199,7 @@ private void verifyTemplates() throws MojoExecutionException, MojoFailureExcepti ClassLoader projectClassLoader = createProjectClassLoader(); MetaDataLoader loader = createLoader(projectClassLoader); + runFieldLintAdvisory(loader); TemplateVerify.Outcome outcome = TemplateVerify.run(loader, Paths.get(templateRoot)); @@ -178,6 +232,7 @@ private void verifyTemplates() throws MojoExecutionException, MojoFailureExcepti private void verifyCodegen() throws MojoExecutionException, MojoFailureException { ClassLoader projectClassLoader = createProjectClassLoader(); MetaDataLoader loader = createLoader(projectClassLoader); + runFieldLintAdvisory(loader); // Per-generator: resolve its committed (real) outputDir from the merged args, then // stage an arg-override so the regenerate writes to a temp dir instead. Keep the diff --git a/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/FieldLintConformanceTest.java b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/FieldLintConformanceTest.java new file mode 100644 index 000000000..c0bc15f1e --- /dev/null +++ b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/FieldLintConformanceTest.java @@ -0,0 +1,93 @@ +package com.metaobjects.mojo; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; +import com.metaobjects.loader.MetaDataLoader; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.stream.Stream; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Cross-port field-lint conformance corpus — {@code fixtures/field-lint-conformance/}. See + * that directory's README.md for the fixture format. Every case is LOADED first, so "this + * loads with no error today" is proven by the fixture, and only then linted. Mirrors the + * TypeScript reference ({@code field-lint-conformance.test.ts}) and the C#/Python ports + * exactly; a mismatch here is a bug in THIS port's lint, never in the fixture. + */ +@RunWith(Parameterized.class) +public class FieldLintConformanceTest { + + private static final Path CORPUS = locateCorpus(); + + private static Path locateCorpus() { + Path dir = Paths.get("").toAbsolutePath(); + while (dir != null) { + Path candidate = dir.resolve("fixtures/field-lint-conformance"); + if (Files.isDirectory(candidate)) return candidate; + dir = dir.getParent(); + } + throw new AssertionError("Could not locate fixtures/field-lint-conformance/ by walking up from " + + Paths.get("").toAbsolutePath()); + } + + @Parameterized.Parameters(name = "{0}") + public static Collection fixtures() throws IOException { + List params = new ArrayList<>(); + try (Stream dirs = Files.list(CORPUS)) { + List sorted = dirs.filter(Files::isDirectory).sorted(Comparator.comparing(Path::toString)).toList(); + for (Path d : sorted) params.add(new Object[]{d.getFileName().toString(), d}); + } + return params; + } + + private final String name; + private final Path dir; + + public FieldLintConformanceTest(String name, Path dir) { + this.name = name; + this.dir = dir; + } + + @Test + public void fixture() throws IOException { + Path input = dir.resolve("input"); + MetaDataLoader loader = MetaDataLoader.fromDirectory("field-lint-" + name, input); + assertTrue(name + ": expected a clean load, got: " + loader.getErrors(), loader.getErrors().isEmpty()); + + List files; + try (Stream listed = Files.list(input)) { + files = listed.sorted(Comparator.comparing(Path::toString)).toList(); + } + List actual = new ArrayList<>(); + for (FieldLint.Finding f : FieldLint.lintReferenceFields(loader)) actual.add(row(f.code(), f.path(), f.message())); + for (FieldLint.Finding f : FieldLint.lintDuplicateFields(files)) actual.add(row(f.code(), f.path(), f.message())); + + List expected = new ArrayList<>(); + JsonObject doc = JsonParser.parseString(Files.readString(dir.resolve("expected.json"), StandardCharsets.UTF_8)).getAsJsonObject(); + for (JsonElement e : doc.getAsJsonArray("findings")) { + JsonObject f = e.getAsJsonObject(); + expected.add(row(f.get("code").getAsString(), f.get("path").getAsString(), f.get("message").getAsString())); + } + + assertEquals(name, expected.stream().sorted().toList(), actual.stream().sorted().toList()); + } + + private static String row(String code, String path, String message) { + return code + " [" + path + "]: " + message; + } +} diff --git a/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyFieldLintTest.java b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyFieldLintTest.java new file mode 100644 index 000000000..387abcc16 --- /dev/null +++ b/server/java/maven-plugin/src/test/java/com/metaobjects/mojo/MetaDataVerifyFieldLintTest.java @@ -0,0 +1,93 @@ +package com.metaobjects.mojo; + +import org.apache.maven.plugin.logging.SystemStreamLog; +import org.junit.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * {@code metaobjects:verify} — the field authoring lint, end to end: logged as warnings in + * its own section, never failing the build, and muted by {@code meta.verify.noFieldLint}. + * The finding text is gated cross-port by {@link FieldLintConformanceTest}; this proves + * the goal runs the lint and that a finding does not throw. + */ +public class MetaDataVerifyFieldLintTest { + + /** Captures the lint's {@code warn} lines so the test can read what the goal logged. */ + private static final class CapturingLog extends SystemStreamLog { + final List warnings = new ArrayList<>(); + + @Override + public void warn(CharSequence content) { + // Only the lint's own lines: the goal also warns about things a bare, + // unconfigured mojo lacks (a MojoExecution phase), which are not under test. + String line = content.toString(); + if (line.contains("— fields:") || line.startsWith(" WARN_")) warnings.add(line); + } + } + + private static Path corpusCase(String name) { + Path dir = Paths.get("").toAbsolutePath(); + while (dir != null) { + Path candidate = dir.resolve("fixtures/field-lint-conformance").resolve(name).resolve("input"); + if (Files.isDirectory(candidate)) return candidate; + dir = dir.getParent(); + } + throw new AssertionError("Could not locate fixtures/field-lint-conformance/" + name); + } + + private static CapturingLog verify(String corpusCase, boolean noFieldLint) throws Exception { + MetaDataVerifyMojo mojo = new MetaDataVerifyMojo(); + LoaderParam loader = LoaderParam.builder("verify-field-lint-test") + .withClassname("com.metaobjects.loader.MetaDataLoader") + .withSourceDir(corpusCase(corpusCase).toString()) + .withSource("meta.app.json") + .build(); + mojo.setLoader(loader); + mojo.setGenerators(Collections.emptyList()); + mojo.setGlobals(Collections.emptyMap()); + // The templates gate over a model with no templates is clean, so anything that + // throws here would be the lint failing the build. + mojo.setMode("templates"); + mojo.setTemplateRoot(Files.createTempDirectory("verify-field-lint").toString()); + mojo.setNoFieldLint(noFieldLint); + CapturingLog log = new CapturingLog(); + mojo.setLog(log); + mojo.execute(); + return log; + } + + @Test + public void aMissingReferenceFieldIsAWarningAndTheBuildPasses() throws Exception { + CapturingLog log = verify("reference-field-missing", false); + assertEquals("metaobjects:verify — fields: 1 authoring warning(s) (advisory — does not fail the build):", + log.warnings.get(0)); + assertTrue(log.warnings.get(1), log.warnings.get(1).startsWith( + " WARN_REFERENCE_FIELD_NOT_FOUND [acme::app::Item.owner_fk]: ")); + } + + @Test + public void aDuplicateFieldIsReportedFromTheRawDocument() throws Exception { + CapturingLog log = verify("duplicate-field-same-subtype", false); + assertTrue(log.warnings.toString(), log.warnings.stream().anyMatch( + w -> w.startsWith(" WARN_DUPLICATE_FIELD_NAME [acme::app::Item.label]: "))); + } + + @Test + public void cleanMetadataLogsNoSection() throws Exception { + assertTrue(verify("clean", false).warnings.isEmpty()); + } + + @Test + public void noFieldLintSilencesIt() throws Exception { + assertTrue(verify("reference-field-missing", true).warnings.isEmpty()); + } +} diff --git a/server/python/src/metaobjects/cli.py b/server/python/src/metaobjects/cli.py index e5cbe39ef..d3403307e 100644 --- a/server/python/src/metaobjects/cli.py +++ b/server/python/src/metaobjects/cli.py @@ -65,6 +65,7 @@ refuse_unowned_packages, ) from metaobjects.config.neutral_config import read_neutral_config +from metaobjects.field_lint import FieldLintFinding, lint_duplicate_fields, lint_reference_fields from metaobjects.loader.meta_data_loader import LoadResult from metaobjects.loader.sources import FileSource from metaobjects.meta.core.object.meta_object import MetaObject @@ -2294,6 +2295,69 @@ def _verify_db(_args: argparse.Namespace) -> int: return 2 +#: Opt-out for the advisory field lint, beside ``--no-field-lint`` (Node `meta` parity). +FIELD_LINT_ENV = "META_NO_FIELD_LINT" + + +def _field_lint_findings(args: argparse.Namespace) -> "list[FieldLintFinding] | None": + """Load the metadata ``verify`` was pointed at and run the field lint over it. + + Returns ``None`` when there is nothing to lint: the metadata could not be located + or did not load. Every such failure is already reported by the gate that ran, with + its own message and exit code, so this stays silent. + + Loads LENIENT, deliberately: the lint must report the same findings under + ``--lax`` as without it, and an unknown attr is the strict gate's finding, not + this one's. + """ + from metaobjects.loader.sources import DirectorySource + + providers, _errors = _resolve_providers(getattr(args, "provider", None)) + if args.metadata_dir is not None: + root, _ = _load_root(args.metadata_dir, providers=providers) + files = [source.path for source in DirectorySource(args.metadata_dir).expand()] + else: + config_path = _find_config(args) + config = load_project_config(config_path) if config_path is not None else None + libraries: "list[str] | None" = None + if config is not None: + # Quiet on purpose: a bad provider is the gate's error to print, once. + providers, _errors = _resolve_providers(config.providers) + libraries = config.libraries + start = project_root_for(config.metadata_dir()) + else: + start = Path.cwd() + collection = resolve_metadata_location(config=config, root=start) + root, _ = _load_root_from_collection(collection, providers=providers, libraries=libraries) + # The project's OWN files — a dependency artifact is not the adopter's to edit. + files = list(collection.own_files) + if root is None: + return None + return [*lint_reference_fields(root), *lint_duplicate_fields(files)] + + +def _run_field_lint_advisory(args: argparse.Namespace) -> None: + """The field AUTHORING lint (see :mod:`metaobjects.field_lint`) — runs on every + ``verify``, whichever gates were selected. Warnings ONLY: it prints to stderr and + never changes the exit code. Muted by ``--no-field-lint`` or ``META_NO_FIELD_LINT=1``. + """ + if getattr(args, "no_field_lint", False) or os.environ.get(FIELD_LINT_ENV) == "1": + return + try: + findings = _field_lint_findings(args) + except Exception: # noqa: BLE001 — an advisory scan never breaks verify + return + if not findings: + return + print( + f"metaobjects verify — fields: {len(findings)} authoring warning(s) " + "(advisory — does not fail the build):", + file=sys.stderr, + ) + for finding in findings: + print(f" {finding.code} [{finding.path}]: {finding.message}", file=sys.stderr) + + def _cmd_verify(args: argparse.Namespace) -> int: """Subverb dispatch (ADR-0021 D2). Run each requested mode; aggregate exit = max (non-zero if ANY mode drifts). Bare ``verify`` (no subverb) keeps the @@ -2322,6 +2386,7 @@ def _cmd_verify(args: argparse.Namespace) -> int: exit_code = max(exit_code, _verify_codegen(args)) if run_templates: exit_code = max(exit_code, _verify_templates(args)) + _run_field_lint_advisory(args) return exit_code @@ -2579,6 +2644,15 @@ def _build_parser() -> argparse.ArgumentParser: default=None, help="comma-separated entity allowlist for --codegen drift (match `gen --entities`)", ) + verify.add_argument( + "--no-field-lint", + action="store_true", + help=( + "suppress the advisory field AUTHORING lint (a reference identity over a " + "missing field; a duplicate field name) — never a gate, it cannot fail the " + "build. META_NO_FIELD_LINT=1 does the same." + ), + ) verify.add_argument( "--lax", action="store_true", diff --git a/server/python/src/metaobjects/field_lint.py b/server/python/src/metaobjects/field_lint.py new file mode 100644 index 000000000..9d66c108c --- /dev/null +++ b/server/python/src/metaobjects/field_lint.py @@ -0,0 +1,219 @@ +"""``metaobjects verify`` — the field AUTHORING lint. + +Two metadata mistakes about an object's FIELDS load with no error on every port: + +1. An ``identity.reference`` whose ``@fields`` names a field the object does not + have. The loader resolves ``@references`` (the target) and never looks at + ``@fields``, so a typo there produces a foreign key over a column nothing declares. +2. Two ``field.*`` children with the same ``name`` in one object's ``children`` list. + +WHY THESE ARE WARNINGS AND NOT LOAD ERRORS. ``docs/compatibility-policy.md`` does not +allow a new load error for metadata that loads today, so every finding here is a +warning by construction: nothing in this module reaches an exit code. + +THE TWO HALVES READ DIFFERENT THINGS, and have to. The reference half reads the LOADED +model, because "does this object have that field" is a question about the EFFECTIVE +field set — inherited through ``extends`` and merged from overlay files. The duplicate +half reads the RAW DOCUMENTS: the TypeScript, C# and Java loaders fold a repeated +field into the first declaration, so their model keeps no trace of it, and one scan of +the document gives every port the same answer. + +Mirrors the TS reference (``packages/cli/src/lib/field-lint.ts`` + +``packages/metadata/src/loader/declared-duplicate-fields.ts``). The codes, the message +text and the fixtures are shared: ``fixtures/field-lint-conformance/``. +""" + +from __future__ import annotations + +import json +from dataclasses import dataclass +from pathlib import Path +from typing import Iterable + +from metaobjects.loader.sources import FileSource +from metaobjects.meta.core.identity.identity_constants import ( + IDENTITY_ATTR_FIELDS, + IDENTITY_SUBTYPE_REFERENCE, +) +from metaobjects.meta.meta_data import MetaData +from metaobjects.shared.base_types import ( + SUBTYPE_ROOT, + TYPE_FIELD, + TYPE_IDENTITY, + TYPE_METADATA, + TYPE_OBJECT, +) +from metaobjects.shared.separators import PACKAGE_SEP +from metaobjects.source.yaml_positions import parse_yaml_with_positions + +#: An ``identity.reference`` lists a field its object does not have. +WARN_REFERENCE_FIELD_NOT_FOUND = "WARN_REFERENCE_FIELD_NOT_FOUND" +#: One ``children`` list declares the same field name more than once. +WARN_DUPLICATE_FIELD_NAME = "WARN_DUPLICATE_FIELD_NAME" + +_KEY_NAME = "name" +_KEY_PACKAGE = "package" +_KEY_CHILDREN = "children" +_TYPE_SUBTYPE_SEP = "." +#: The YAML authoring sugar for ``isArray: true`` — a suffix on the wrapper key. +_ARRAY_SUFFIX = "[]" + + +@dataclass(frozen=True) +class FieldLintFinding: + """One advisory finding: its code, the node's address, and the message.""" + + code: str + path: str + message: str + + +def _quote(value: str) -> str: + # The JSON string form, so the text is byte-identical to the other ports'. + return json.dumps(value, ensure_ascii=False) + + +def lint_reference_fields(root: MetaData) -> list[FieldLintFinding]: + """Report every ``identity.reference`` whose ``@fields`` names a field its object lacks.""" + out: list[FieldLintFinding] = [] + # OWN-ONLY (ADR-0039 sanctioned case): a root has no super, and its own children + # are the declared objects. + root_pkg = root.package or "" + for obj in root.own_children(): + if obj.type != TYPE_OBJECT: + continue + own_pkg = obj.package + if own_pkg and own_pkg.startswith(PACKAGE_SEP) and root_pkg.strip() != "": + address = f"{root_pkg}{own_pkg}{PACKAGE_SEP}{obj.name}" + else: + address = obj.resolution_key() + # RESOLVING: the effective field set — a field inherited through ``extends`` or + # added by an overlay file is a field the object has. + fields = {c.name for c in obj.children() if c.type == TYPE_FIELD} + # OWN-ONLY (ADR-0039 sanctioned case): report each DECLARATION once, on the + # object that declares it. The resolving ``children()`` would repeat an + # inherited reference on every subtype. + for identity in obj.own_children(): + if identity.type != TYPE_IDENTITY or identity.sub_type != IDENTITY_SUBTYPE_REFERENCE: + continue + # RESOLVING: ``@fields`` may itself be inherited through the identity's + # ``extends`` (``attrs()`` resolves; ``attr()`` is own-only in this port). + listed = identity.attrs().get(IDENTITY_ATTR_FIELDS) + names = listed if isinstance(listed, list) else [listed] if isinstance(listed, str) else [] + for name in names: + if not isinstance(name, str) or name in fields: + continue + out.append( + FieldLintFinding( + WARN_REFERENCE_FIELD_NOT_FOUND, + f"{address}.{identity.name}", + f"identity.reference {_quote(identity.name)} lists {_quote(name)} in @fields, " + f"but {address} has no field of that name, inherited and overlaid fields " + "included. Nothing checks this at load, so the reference is built on a field " + "that does not exist. Rename the entry to an existing field, or declare the " + "field.", + ) + ) + return out + + +def _wrapper_type(key: str) -> str: + """The TYPE segment of a wrapper key: ``field.string[]`` and bare ``field`` are both ``field``.""" + head = key.split(_TYPE_SUBTYPE_SEP, 1)[0] + return head[: -len(_ARRAY_SUFFIX)] if head.endswith(_ARRAY_SUFFIX) else head + + +def _declared_name(body: object) -> str | None: + """A node's declared name: the body's ``name``, or the body itself when YAML wrote a scalar.""" + name = body.get(_KEY_NAME) if isinstance(body, dict) else body + return name if isinstance(name, str) and name != "" else None + + +def declared_duplicate_fields(content: str, format: str) -> list[FieldLintFinding]: + """Structurally scan one document's raw content and report every field name a + root-level object declares more than once in its own ``children`` list. + + Scope is ONE children list: a field redeclared by an overlay file, or by a subtype + overriding an inherited field, is not in one list and is not a finding. Malformed + shapes return ``[]``; a syntax error raises, as the parser's own would. + """ + normalized = content[1:] if content.startswith("") else content + if format == "json": + parsed = json.loads(normalized) + elif format == "yaml": + parsed = parse_yaml_with_positions(normalized) + else: + return [] + if not isinstance(parsed, dict): + return [] + # JSON fuses the subtype onto the root key; sigil-free YAML may write the bare type. + root_body = parsed.get(f"{TYPE_METADATA}{_TYPE_SUBTYPE_SEP}{SUBTYPE_ROOT}", parsed.get(TYPE_METADATA)) + if not isinstance(root_body, dict): + return [] + raw_root_pkg = root_body.get(_KEY_PACKAGE) + root_pkg = raw_root_pkg if isinstance(raw_root_pkg, str) else "" + children = root_body.get(_KEY_CHILDREN) + if not isinstance(children, list): + return [] + + out: list[FieldLintFinding] = [] + for child in children: + if not isinstance(child, dict): + continue + for wrapper_key, body in child.items(): + if not isinstance(wrapper_key, str) or _wrapper_type(wrapper_key) != TYPE_OBJECT: + continue + if not isinstance(body, dict): + continue + name = _declared_name(body) + members = body.get(_KEY_CHILDREN) + if name is None or not isinstance(members, list): + continue + + counts: dict[str, int] = {} + for member in members: + if not isinstance(member, dict): + continue + for member_key, member_body in member.items(): + if not isinstance(member_key, str) or _wrapper_type(member_key) != TYPE_FIELD: + continue + field_name = _declared_name(member_body) + if field_name is not None: + counts[field_name] = counts.get(field_name, 0) + 1 + + # The resolution key the parser gives a root-level node: its own ``package`` + # (a ``::``-prefixed one is relative to the root's), else the root's. + raw_own_pkg = body.get(_KEY_PACKAGE) + if isinstance(raw_own_pkg, str) and raw_own_pkg != "": + relative = root_pkg.strip() != "" and raw_own_pkg.startswith(PACKAGE_SEP) + pkg = root_pkg + raw_own_pkg if relative else raw_own_pkg + else: + pkg = root_pkg + address = f"{pkg}{PACKAGE_SEP}{name}" if pkg != "" else name + for field_name, count in counts.items(): + if count > 1: + out.append( + FieldLintFinding( + WARN_DUPLICATE_FIELD_NAME, + f"{address}.{field_name}", + f"{address} declares the field {_quote(field_name)} {count} times in one " + "children list. Nothing reports this at load, and only the first " + "declaration is certain to take effect. Remove or rename the duplicate.", + ) + ) + return out + + +def lint_duplicate_fields(files: Iterable[Path | str]) -> list[FieldLintFinding]: + """Run :func:`declared_duplicate_fields` over each metadata file. + + Unreadable or unparsable files are skipped — the loader reports those itself. + """ + out: list[FieldLintFinding] = [] + for path in files: + try: + source = FileSource(path) + out.extend(declared_duplicate_fields(source.read(), source.format)) + except Exception: # noqa: BLE001 — an advisory scan never breaks verify + continue + return out diff --git a/server/python/tests/codegen/test_cli_verify_field_lint.py b/server/python/tests/codegen/test_cli_verify_field_lint.py new file mode 100644 index 000000000..57859faf0 --- /dev/null +++ b/server/python/tests/codegen/test_cli_verify_field_lint.py @@ -0,0 +1,100 @@ +"""``metaobjects verify`` — the field authoring lint, end to end. + +It prints its own advisory section on stderr, never reaches the exit code, and is +muted by ``--no-field-lint`` / ``META_NO_FIELD_LINT=1``. The finding text itself is +gated cross-port by ``tests/conformance/test_field_lint_conformance.py``. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from metaobjects.cli import main +from tests.codegen.gen_suite import GEN_SUITE + + +def _meta_dir(tmp_path: Path, reference_field: str, duplicate_label: bool) -> str: + label = {"field.string": {"name": "label"}} + pk = {"identity.primary": {"name": "pk", "@fields": ["id"]}} + doc = { + "metadata.root": { + "package": "app", + "children": [ + {"object.entity": {"name": "Owner", "children": [{"field.long": {"name": "id"}}, pk]}}, + { + "object.entity": { + "name": "Item", + "children": [ + {"field.long": {"name": "id"}}, + {"field.long": {"name": "ownerId"}}, + label, + *([label] if duplicate_label else []), + pk, + { + "identity.reference": { + "name": "owner_fk", + "@fields": [reference_field], + "@references": "Owner", + } + }, + ], + } + }, + ], + } + } + d = tmp_path / "meta" + d.mkdir() + (d / "meta.app.json").write_text(json.dumps(doc)) + return str(d) + + +def _verify(tmp_path: Path, meta_dir: str, *extra: str) -> int: + out = tmp_path / "out" + assert main(["gen", "--generators", GEN_SUITE, meta_dir, "--out", str(out)]) == 0 + return main(["verify", "--codegen", "--generators", GEN_SUITE, meta_dir, "--out", str(out), *extra]) + + +def test_findings_are_advisory_and_do_not_change_the_exit_code( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + meta_dir = _meta_dir(tmp_path, "ownerIdd", duplicate_label=False) + capsys.readouterr() + assert _verify(tmp_path, meta_dir) == 0 + err = capsys.readouterr().err + assert "metaobjects verify — fields: 1 authoring warning(s) (advisory — does not fail the build):" in err + assert "WARN_REFERENCE_FIELD_NOT_FOUND [app::Item.owner_fk]" in err + + +def test_duplicate_field_is_reported_from_the_raw_document( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + meta_dir = _meta_dir(tmp_path, "ownerId", duplicate_label=True) + capsys.readouterr() + assert _verify(tmp_path, meta_dir) == 0 + err = capsys.readouterr().err + assert "WARN_DUPLICATE_FIELD_NAME [app::Item.label]" in err + + +def test_clean_metadata_prints_no_section(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + meta_dir = _meta_dir(tmp_path, "ownerId", duplicate_label=False) + assert _verify(tmp_path, meta_dir) == 0 + assert "fields:" not in capsys.readouterr().err + + +def test_no_field_lint_flag_silences_it(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + meta_dir = _meta_dir(tmp_path, "ownerIdd", duplicate_label=False) + assert _verify(tmp_path, meta_dir, "--no-field-lint") == 0 + assert "WARN_REFERENCE_FIELD_NOT_FOUND" not in capsys.readouterr().err + + +def test_env_var_silences_it( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("META_NO_FIELD_LINT", "1") + meta_dir = _meta_dir(tmp_path, "ownerIdd", duplicate_label=False) + assert _verify(tmp_path, meta_dir) == 0 + assert "WARN_REFERENCE_FIELD_NOT_FOUND" not in capsys.readouterr().err diff --git a/server/python/tests/conformance/test_field_lint_conformance.py b/server/python/tests/conformance/test_field_lint_conformance.py new file mode 100644 index 000000000..964a3b805 --- /dev/null +++ b/server/python/tests/conformance/test_field_lint_conformance.py @@ -0,0 +1,49 @@ +"""Cross-port field-lint conformance corpus — fixtures/field-lint-conformance/. + +See that directory's README.md for the fixture format. Every case is LOADED strict +first, so "this loads with no error today" is proven by the fixture, and only then +linted. Mirrors the TS reference (field-lint-conformance.test.ts) and the C#/Java +ports exactly; a mismatch here is a bug in THIS port's lint, never in the fixture. +""" +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from metaobjects import MetaDataLoader +from metaobjects.field_lint import lint_duplicate_fields, lint_reference_fields + + +def _corpus_root() -> Path: + here = Path(__file__).resolve() + for parent in [here, *here.parents]: + candidate = parent / "fixtures" / "field-lint-conformance" + if candidate.is_dir(): + return candidate + raise RuntimeError("could not locate fixtures/field-lint-conformance from " + str(here)) + + +CORPUS = _corpus_root() +FIXTURES = sorted(p.name for p in CORPUS.iterdir() if p.is_dir()) + + +def test_found_fixtures() -> None: + assert FIXTURES, "fixtures/field-lint-conformance/ discovered no fixture directories" + + +@pytest.mark.parametrize("name", FIXTURES) +def test_fixture(name: str) -> None: + input_dir = CORPUS / name / "input" + expected = json.loads((CORPUS / name / "expected.json").read_text(encoding="utf-8")) + + result = MetaDataLoader.from_directory(str(input_dir), strict=True) + assert [str(e) for e in result.errors] == [] + + findings = [ + *lint_reference_fields(result.root), + *lint_duplicate_fields(sorted(input_dir.iterdir())), + ] + actual = sorted((f.code, f.path, f.message) for f in findings) + assert actual == sorted((f["code"], f["path"], f["message"]) for f in expected["findings"]) diff --git a/server/typescript/packages/cli/src/commands/verify.ts b/server/typescript/packages/cli/src/commands/verify.ts index b065e3705..c3c2cdee7 100644 --- a/server/typescript/packages/cli/src/commands/verify.ts +++ b/server/typescript/packages/cli/src/commands/verify.ts @@ -41,6 +41,7 @@ import { lintRequirements } from "../lib/requirement-lint.js"; import { lintOverlays } from "../lib/overlay-lint.js"; import { lintNodeNames } from "../lib/name-lint.js"; import { lintDeprecatedReferences } from "../lib/deprecation-lint.js"; +import { lintDuplicateFields, lintReferenceFields } from "../lib/field-lint.js"; import { FileSource } from "@metaobjectsdev/metadata/core"; import { resolveD1Config, resolveMigrateConfig } from "../lib/config.js"; import { @@ -360,6 +361,8 @@ export async function verifyCommand( // #305 — the deprecated-reference authoring lint. Its own section, same reason. let deprecationSection: AdvisorySection = skippedSection("the deprecated-reference lint did not run"); + let fieldSection: AdvisorySection = + skippedSection("the field lint did not run"); // The ledger counts `meta verify` prints on every run. Undefined for a project // declaring no requirement.* node at all (opt-in by declaration) — the payload // then omits the block rather than reporting zeroes that would read as an empty @@ -405,6 +408,11 @@ export async function verifyCommand( // warnings ONLY, never changes the exit code. runDeprecationLintAdvisory(); + // A reference identity over a field the object lacks, and a field name declared + // twice in one children list — both load clean on every port. Runs on every + // `meta verify`; warnings ONLY, never changes the exit code. + await runFieldLintAdvisory(); + // Advisory verify-as-teacher pass: surface hand-rolled work the metadata could // model. Warnings ONLY — never changes the exit code (bias to under-flagging). // Suppressed with --no-antipatterns or META_NO_ANTIPATTERNS=1 for the rare @@ -455,6 +463,7 @@ export async function verifyCommand( overlays: overlaySection, names: nameSection, deprecations: deprecationSection, + fields: fieldSection, }), fmt, ); @@ -852,6 +861,41 @@ export async function verifyCommand( } } + // -- field authoring lint (advisory) ---------------------------------------- + // Reads the LOADED model for the reference half and the project's OWN raw files + // for the duplicate half (see field-lint.ts) — a dependency artifact is not the + // adopter's to edit. Its own section, its own cap, and a skip that carries its + // reason rather than looking like a clean scan. + async function runFieldLintAdvisory(): Promise { + if (flags.noFieldLint) { + fieldSection = skippedSection("suppressed by --no-field-lint"); + return; + } + if (process.env.META_NO_FIELD_LINT === "1") { + fieldSection = skippedSection("suppressed by META_NO_FIELD_LINT=1"); + return; + } + let findings: Diagnostic[]; + try { + findings = [ + ...lintReferenceFields(root), + ...(await lintDuplicateFields(collection.ownFiles, (path) => new FileSource(path))), + ]; + } catch (err) { + // Never let an advisory scan break verify — and never report it as clean. + fieldSection = skippedSection(`the field lint failed: ${describeError(err)}`); + return; + } + fieldSection = ranSection(findings.map((d) => toDiagnosticRow(d, "lint"))); + if (findings.length > 0) { + log.warn( + `meta verify — fields: ${findings.length} authoring warning(s) ` + + `(advisory — does not fail the build):`, + ); + warnCapped(findings.map(formatDiagnostic), flags.limit, { structured }); + } + } + // -- deprecated-reference authoring lint (advisory) ------------------------- // #305 — reads the LOADED model (see deprecation-lint.ts). Its own section, // its own cap, and a skip that carries its reason rather than looking clean. @@ -1890,6 +1934,7 @@ function buildVerifyPayload(input: { overlays: AdvisorySection; names: AdvisorySection; deprecations: AdvisorySection; + fields: AdvisorySection; }): Record { const ran = input.gates.filter((g) => g.ran); const failed = ran.filter((g) => !g.ok); @@ -1915,6 +1960,9 @@ function buildVerifyPayload(input: { if (input.deprecations.status === "ran" && input.deprecations.total > 0) { parts.push(`${input.deprecations.total} deprecated-reference finding(s)`); } + if (input.fields.status === "ran" && input.fields.total > 0) { + parts.push(`${input.fields.total} field authoring finding(s)`); + } const help: string[] = []; if (failed.length > 0) { @@ -1940,13 +1988,19 @@ function buildVerifyPayload(input: { `${input.deprecations.total} reference(s) to a deprecated node — see deprecations.rows[]; each names the deprecated target and, where declared, its replacement`, ); } + if (input.fields.total > 0) { + help.push( + `${input.fields.total} field authoring finding(s) — see fields.rows[]; each names a reference identity listing a field its object lacks, or a field name declared twice in one children list`, + ); + } if ( failed.length === 0 && input.antiPatterns.total === 0 && input.requirements.total === 0 && input.overlays.total === 0 && input.names.total === 0 && - input.deprecations.total === 0 + input.deprecations.total === 0 && + input.fields.total === 0 ) { help.push("no drift and nothing advisory to answer — nothing to do"); } @@ -1961,6 +2015,7 @@ function buildVerifyPayload(input: { overlays: input.overlays, names: input.names, deprecations: input.deprecations, + fields: input.fields, ...(input.requirementCounts !== undefined ? { requirementCounts: input.requirementCounts } : {}), // The honest boundary. Everything named here is REACHABLE — it is printed as // text on stderr — but it is not in this document, and a reader must not have diff --git a/server/typescript/packages/cli/src/index.ts b/server/typescript/packages/cli/src/index.ts index 0d09a562e..7bc7f6517 100644 --- a/server/typescript/packages/cli/src/index.ts +++ b/server/typescript/packages/cli/src/index.ts @@ -147,6 +147,9 @@ VERIFY FLAGS (ADR-0021 D2 — explicit subverbs; combine any; exit 1 on ANY drif in a name; never a gate — this lint can't fail the build) --no-deprecation-lint Suppress the advisory deprecated-reference AUTHORING lint (never a gate — this lint can't fail the build) + --no-field-lint Suppress the advisory field AUTHORING lint (a reference + identity over a missing field; a duplicate field name — + never a gate; this lint can't fail the build) --limit How many advisory lines TEXT output prints per section before it truncates (default 20). Never applies to --format toon/json, which carry every finding and every diagnostic. @@ -344,6 +347,9 @@ FLAGS: a name) — never a gate; this lint can't fail the build --no-deprecation-lint Suppress the advisory deprecated-reference AUTHORING lint — never a gate; this lint can't fail the build + --no-field-lint Suppress the advisory field AUTHORING lint (a reference + identity over a missing field; a duplicate field name) — + never a gate; this lint can't fail the build --limit Advisory lines TEXT output prints PER SECTION before truncating (default 20; per-section so the authoring lint can never push the gate's own warnings off the end) @@ -389,6 +395,12 @@ the deprecated node also carries @replacedBy, the finding names the replacement. A node referencing itself (a recursive FK, a same-entity passthrough) is not a finding. Warnings only — it can never fail the build. Opt out with --no-deprecation-lint or META_NO_DEPRECATION_LINT=1. + +verify also prints an ADVISORY field authoring lint, in its own section. It reports +an identity.reference whose @fields names a field the object does not have +(inherited and overlaid fields count as present), and a field name declared more +than once in one object's children list. Both load with no error. Warnings only — +it can never fail the build. Opt out with --no-field-lint or META_NO_FIELD_LINT=1. `, export: `meta export — flatten loaded metadata to one canonical JSON artifact diff --git a/server/typescript/packages/cli/src/lib/args.ts b/server/typescript/packages/cli/src/lib/args.ts index a5f3e3dab..5b33f547c 100644 --- a/server/typescript/packages/cli/src/lib/args.ts +++ b/server/typescript/packages/cli/src/lib/args.ts @@ -405,6 +405,13 @@ export interface VerifyFlags { * and can never fail the build. */ noDeprecationLint: boolean; + /** + * Suppress the advisory FIELD authoring lint — a reference identity whose + * `@fields` names a field the object lacks, and a field name declared twice in + * one children list. Advisory only, like its siblings: it has no gate half and + * can never fail the build. + */ + noFieldLint: boolean; /** * ADR-0023 strict-attr load opt-OUT (#96). `verify` is strict-by-default — an * undeclared/typo'd own `@attr` fails verify (ERR_UNKNOWN_ATTR). `--lax` @@ -448,6 +455,7 @@ export const VERIFY_OPTIONS = { "no-overlay-lint": { type: "boolean", default: false }, "no-name-lint": { type: "boolean", default: false }, "no-deprecation-lint": { type: "boolean", default: false }, + "no-field-lint": { type: "boolean", default: false }, lax: { type: "boolean", default: false }, "d1": { type: "string" }, "remote": { type: "boolean", default: false }, @@ -534,6 +542,7 @@ export function parseVerifyArgs(argv: string[]): VerifyFlags { noOverlayLint: !!values["no-overlay-lint"], noNameLint: !!values["no-name-lint"], noDeprecationLint: !!values["no-deprecation-lint"], + noFieldLint: !!values["no-field-lint"], lax: !!values.lax, d1: values.d1 as string | undefined, remote: !!values.remote, diff --git a/server/typescript/packages/cli/src/lib/field-lint.ts b/server/typescript/packages/cli/src/lib/field-lint.ts new file mode 100644 index 000000000..f19644c44 --- /dev/null +++ b/server/typescript/packages/cli/src/lib/field-lint.ts @@ -0,0 +1,122 @@ +// `meta verify` — the field AUTHORING lint. +// +// Two metadata mistakes about an object's FIELDS load with no error on every port: +// +// 1. An `identity.reference` whose `@fields` names a field the object does not have. +// The loader resolves `@references` (the target) and never looks at `@fields`, so a +// typo there produces a foreign key over a column nothing declares. +// 2. Two `field.*` children with the same `name` in one object's `children` list. The +// later declaration is folded into the first, and one of a different subtype is +// dropped, both silently. +// +// WHY THESE ARE WARNINGS AND NOT LOAD ERRORS. docs/compatibility-policy.md does not allow +// a new load error for metadata that loads today — name-lint.ts's header has the same +// ruling for whitespace names. So, like every sibling lint in this directory: +// +// EVERY FINDING IS A WARNING, BY CONSTRUCTION. Nothing here reaches the exit code. +// +// THE TWO HALVES READ DIFFERENT THINGS, and have to. The reference half reads the LOADED +// model, because "does this object have that field" is a question about the EFFECTIVE +// field set — inherited through `extends` and merged from overlay files. The duplicate +// half reads the RAW DOCUMENTS, because the merge has already erased the duplicate from +// the model by the time anything can ask (overlay-lint.ts reads files for the same reason). +// +// The codes, the message text and the fixtures are shared with the C#, Java and Python +// CLIs: fixtures/field-lint-conformance/. + +import { + IDENTITY_ATTR_FIELDS, + IDENTITY_SUBTYPE_REFERENCE, + TYPE_FIELD, + TYPE_IDENTITY, + TYPE_OBJECT, + type MetaData, + type MetaDataSource, +} from "@metaobjectsdev/metadata"; +import { declaredDuplicateFields, type DeclaredDuplicateField } from "@metaobjectsdev/metadata/core"; +import type { Diagnostic } from "./requirement-check.js"; + +/** An `identity.reference` lists a field its object does not have. */ +export const WARN_REFERENCE_FIELD_NOT_FOUND = "WARN_REFERENCE_FIELD_NOT_FOUND"; +/** One `children` list declares the same field name more than once. */ +export const WARN_DUPLICATE_FIELD_NAME = "WARN_DUPLICATE_FIELD_NAME"; + +/** Resolve one metadata file path to the source the duplicate scan reads it through. */ +export type ReadSource = (path: string) => MetaDataSource; + +function warn(path: string, code: string, message: string): Diagnostic { + return { severity: "warn", code, path, message }; +} + +/** + * Report every `identity.reference` whose `@fields` names a field its object lacks. + * Returns `[]` for a model where every listed field exists. + */ +export function lintReferenceFields(root: MetaData): Diagnostic[] { + const out: Diagnostic[] = []; + // OWN-ONLY (ADR-0039 sanctioned case): a root has no super, and its own children are + // the declared objects. + for (const object of root.ownChildren()) { + if (object.type !== TYPE_OBJECT) continue; + const address = object.resolutionKey(); + // RESOLVING: the effective field set — a field inherited through `extends` or added + // by an overlay file is a field the object has. + const fields = new Set(object.children().filter((c) => c.type === TYPE_FIELD).map((c) => c.name)); + // OWN-ONLY (ADR-0039 sanctioned case): report each DECLARATION once, on the object + // that declares it. The resolving `children()` would repeat an inherited reference + // on every subtype. + for (const identity of object.ownChildren()) { + if (identity.type !== TYPE_IDENTITY || identity.subType !== IDENTITY_SUBTYPE_REFERENCE) continue; + // RESOLVING: `@fields` may itself be inherited through the identity's `extends`. + const listed = identity.attr(IDENTITY_ATTR_FIELDS); + const names = Array.isArray(listed) ? listed : typeof listed === "string" ? [listed] : []; + for (const name of names) { + if (typeof name !== "string" || fields.has(name)) continue; + out.push(warn(`${address}.${identity.name}`, WARN_REFERENCE_FIELD_NOT_FOUND, referenceFieldMessage(identity.name, name, address))); + } + } + } + return out; +} + +function referenceFieldMessage(identity: string, field: string, object: string): string { + return ( + `identity.reference ${JSON.stringify(identity)} lists ${JSON.stringify(field)} in @fields, but ` + + `${object} has no field of that name, inherited and overlaid fields included. Nothing checks ` + + `this at load, so the reference is built on a field that does not exist. Rename the entry to ` + + `an existing field, or declare the field.` + ); +} + +/** One raw-scan finding as a diagnostic. Exported so the scan and its text are testable + * without a filesystem. */ +export function duplicateFieldDiagnostic(dup: DeclaredDuplicateField): Diagnostic { + return warn( + `${dup.object}.${dup.field}`, + WARN_DUPLICATE_FIELD_NAME, + `${dup.object} declares the field ${JSON.stringify(dup.field)} ${dup.count} times in one children ` + + `list. Nothing reports this at load, and only the first declaration is certain to take effect. ` + + `Remove or rename the duplicate.`, + ); +} + +/** + * Report every field name declared more than once in one object's `children` list, + * scanning each of `files` as raw content. + * + * Unreadable or unparsable files are skipped — the loader reports those itself. + */ +export async function lintDuplicateFields(files: readonly string[], readSource: ReadSource): Promise { + const out: Diagnostic[] = []; + for (const path of files) { + let declared: ReadonlyArray; + try { + const source = readSource(path); + declared = declaredDuplicateFields(await source.read(), source.format); + } catch { + continue; + } + out.push(...declared.map(duplicateFieldDiagnostic)); + } + return out; +} diff --git a/server/typescript/packages/cli/test/__snapshots__/cli.test.ts.snap b/server/typescript/packages/cli/test/__snapshots__/cli.test.ts.snap index 7ad4ee0d6..9dd93b382 100644 --- a/server/typescript/packages/cli/test/__snapshots__/cli.test.ts.snap +++ b/server/typescript/packages/cli/test/__snapshots__/cli.test.ts.snap @@ -127,6 +127,9 @@ VERIFY FLAGS (ADR-0021 D2 — explicit subverbs; combine any; exit 1 on ANY drif in a name; never a gate — this lint can't fail the build) --no-deprecation-lint Suppress the advisory deprecated-reference AUTHORING lint (never a gate — this lint can't fail the build) + --no-field-lint Suppress the advisory field AUTHORING lint (a reference + identity over a missing field; a duplicate field name — + never a gate; this lint can't fail the build) --limit How many advisory lines TEXT output prints per section before it truncates (default 20). Never applies to --format toon/json, which carry every finding and every diagnostic. diff --git a/server/typescript/packages/cli/test/field-lint-conformance.test.ts b/server/typescript/packages/cli/test/field-lint-conformance.test.ts new file mode 100644 index 000000000..6d1e53c1f --- /dev/null +++ b/server/typescript/packages/cli/test/field-lint-conformance.test.ts @@ -0,0 +1,45 @@ +// Cross-port field-lint conformance corpus — fixtures/field-lint-conformance/. +// +// Every case is LOADED strict first, so "this loads with no error today" is proven by +// the fixture rather than asserted in prose, and only then linted. The C#, Java and +// Python runners assert the same `expected.json`, so the codes, addresses and message +// text cannot drift between the ports. +import { describe, test, expect } from "bun:test"; +import { readdirSync, readFileSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { MetaDataLoader } from "@metaobjectsdev/metadata"; +import { FileSource } from "@metaobjectsdev/metadata/core"; +import { lintDuplicateFields, lintReferenceFields } from "../src/lib/field-lint.js"; + +const CORPUS_DIR = join(import.meta.dir, "../../../../../fixtures/field-lint-conformance"); + +interface Finding { code: string; path: string; message: string } + +const order = (a: Finding, b: Finding): number => + a.code.localeCompare(b.code) || a.path.localeCompare(b.path) || a.message.localeCompare(b.message); + +const cases = readdirSync(CORPUS_DIR).filter((n) => statSync(join(CORPUS_DIR, n)).isDirectory()).sort(); + +describe("field-lint conformance corpus", () => { + test("discovers the corpus", () => { + expect(cases.length).toBeGreaterThan(0); + }); + + for (const name of cases) { + test(name, async () => { + const input = join(CORPUS_DIR, name, "input"); + const expected = JSON.parse(readFileSync(join(CORPUS_DIR, name, "expected.json"), "utf8")) as { findings: Finding[] }; + + const result = await MetaDataLoader.fromDirectory(input, { strict: true }); + expect(result.errors.map(String)).toEqual([]); + + const files = readdirSync(input).sort().map((f) => join(input, f)); + const actual = [ + ...lintReferenceFields(result.root), + ...(await lintDuplicateFields(files, (path) => new FileSource(path))), + ].map(({ code, path, message }) => ({ code, path: path ?? "", message })); + + expect(actual.sort(order)).toEqual([...expected.findings].sort(order)); + }); + } +}); diff --git a/server/typescript/packages/cli/test/unit/args-verify.test.ts b/server/typescript/packages/cli/test/unit/args-verify.test.ts index 00275f9d7..f5cbd2cfb 100644 --- a/server/typescript/packages/cli/test/unit/args-verify.test.ts +++ b/server/typescript/packages/cli/test/unit/args-verify.test.ts @@ -6,7 +6,7 @@ describe("parseVerifyArgs", () => { test("defaults: prompts/db/dialect undefined, allow empty, skipSchema false, no explicit subverb", () => { expect(parseVerifyArgs([])).toEqual({ prompts: undefined, db: undefined, dialect: undefined, allow: [], skipSchema: false, - templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: false, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, lax: false, + templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: false, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, noFieldLint: false, lax: false, replay: false, replaySnapshot: false, d1: undefined, remote: false, limit: DEFAULT_ADVISORY_LIMIT, }); @@ -14,7 +14,7 @@ describe("parseVerifyArgs", () => { test("--prompts is captured", () => { expect(parseVerifyArgs(["--prompts", "templates"])).toEqual({ prompts: "templates", db: undefined, dialect: undefined, allow: [], skipSchema: false, - templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: false, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, lax: false, + templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: false, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, noFieldLint: false, lax: false, replay: false, replaySnapshot: false, d1: undefined, remote: false, limit: DEFAULT_ADVISORY_LIMIT, }); @@ -22,7 +22,7 @@ describe("parseVerifyArgs", () => { test("--db / --dialect / --skip-schema are captured", () => { expect(parseVerifyArgs(["--db", "file:x.db", "--dialect", "sqlite", "--skip-schema"])).toEqual({ prompts: undefined, db: "file:x.db", dialect: "sqlite", allow: [], skipSchema: true, - templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: true, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, lax: false, + templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: true, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, noFieldLint: false, lax: false, replay: false, replaySnapshot: false, d1: undefined, remote: false, limit: DEFAULT_ADVISORY_LIMIT, }); @@ -32,7 +32,7 @@ describe("parseVerifyArgs", () => { test("--dialect d1 --d1 --remote are captured; --dialect d1 alone is an explicit subverb", () => { expect(parseVerifyArgs(["--dialect", "d1", "--d1", "DB", "--remote"])).toEqual({ prompts: undefined, db: undefined, dialect: "d1", allow: [], skipSchema: false, - templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: true, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, lax: false, + templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: true, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, noFieldLint: false, lax: false, replay: false, replaySnapshot: false, d1: "DB", remote: true, limit: DEFAULT_ADVISORY_LIMIT, }); @@ -61,7 +61,7 @@ describe("parseVerifyArgs", () => { expect(parseVerifyArgs(["--allow", "drop-column,drop-table"])).toEqual({ prompts: undefined, db: undefined, dialect: undefined, allow: ["drop-column", "drop-table"], skipSchema: false, - templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: false, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, lax: false, + templates: false, codegen: false, forbidHandEdits: false, docs: false, deps: false, anyExplicit: false, noAntipatterns: false, noRequirementLint: false, noOverlayLint: false, noNameLint: false, noDeprecationLint: false, noFieldLint: false, lax: false, replay: false, replaySnapshot: false, d1: undefined, remote: false, limit: DEFAULT_ADVISORY_LIMIT, }); diff --git a/server/typescript/packages/cli/test/verify-field-lint.test.ts b/server/typescript/packages/cli/test/verify-field-lint.test.ts new file mode 100644 index 000000000..4f0fb9e7a --- /dev/null +++ b/server/typescript/packages/cli/test/verify-field-lint.test.ts @@ -0,0 +1,126 @@ +// `meta verify` — the field authoring lint, end to end: printed as its own advisory +// section, carried in full in the structured payload, never reaching the exit code, and +// muted by its own flag/env pair the way every sibling advisory is. +import { describe, test, expect, beforeEach, afterEach, afterAll } from "bun:test"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { run } from "../src/index.js"; + +const dirs: string[] = []; +afterAll(() => { + for (const d of dirs) rmSync(d, { recursive: true, force: true }); +}); + +/** A project whose `Item` references `Owner` through `referenceField`, and declares + * `label` once or twice. */ +function project(referenceField: string, duplicateLabel: boolean): string { + const root = mkdtempSync(join(tmpdir(), "vfl-")); + dirs.push(root); + mkdirSync(join(root, "metaobjects"), { recursive: true }); + mkdirSync(join(root, ".metaobjects"), { recursive: true }); + writeFileSync(join(root, ".metaobjects", "config.json"), JSON.stringify({ schema_version: 1, sources: [] })); + const label = { "field.string": { name: "label" } }; + writeFileSync(join(root, "metaobjects", "meta.app.json"), JSON.stringify({ + "metadata.root": { + package: "app", + children: [ + { + "object.entity": { + name: "Owner", + children: [ + { "source.rdb": { "@table": "owners" } }, + { "field.long": { name: "id" } }, + { "identity.primary": { name: "pk", "@fields": ["id"] } }, + ], + }, + }, + { + "object.entity": { + name: "Item", + children: [ + { "source.rdb": { "@table": "items" } }, + { "field.long": { name: "id" } }, + { "field.long": { name: "ownerId" } }, + label, + ...(duplicateLabel ? [label] : []), + { "identity.primary": { name: "pk", "@fields": ["id"] } }, + { "identity.reference": { name: "owner_fk", "@fields": [referenceField], "@references": "Owner" } }, + ], + }, + }, + ], + }, + })); + return root; +} + +let out: string[]; +let err: string[]; +let origLog: typeof console.log; +let origErr: typeof console.error; +beforeEach(() => { + out = []; + err = []; + origLog = console.log; + origErr = console.error; + console.log = (...a: unknown[]) => { out.push(a.map(String).join(" ")); }; + console.error = (...a: unknown[]) => { err.push(a.map(String).join(" ")); }; +}); +afterEach(() => { + console.log = origLog; + console.error = origErr; +}); + +describe("meta verify — the field authoring lint", () => { + test("both findings are advisory in text output, exit 0", async () => { + const exit = await run(["verify", "--format", "text", "--cwd", project("ownerIdd", true)]); + expect(exit).toBe(0); + const all = [...out, ...err].join("\n"); + expect(all).toContain("meta verify — fields: 2 authoring warning(s) (advisory — does not fail the build):"); + expect(all).toContain("WARN_REFERENCE_FIELD_NOT_FOUND [app::Item.owner_fk]"); + expect(all).toContain("WARN_DUPLICATE_FIELD_NAME [app::Item.label]"); + }); + + test("the structured payload carries the findings in their own `fields` section", async () => { + const exit = await run(["verify", "--format", "json", "--cwd", project("ownerIdd", true)]); + expect(exit).toBe(0); + const payload = JSON.parse(out.join("\n")) as { + fields: { status: string; total: number; rows: { code: string; path: string; source: string }[] }; + summary: string; + }; + expect(payload.fields.status).toBe("ran"); + expect(payload.fields.rows.map((r) => [r.code, r.path, r.source])).toEqual([ + ["WARN_REFERENCE_FIELD_NOT_FOUND", "app::Item.owner_fk", "lint"], + ["WARN_DUPLICATE_FIELD_NAME", "app::Item.label", "lint"], + ]); + expect(payload.summary).toContain("2 field authoring finding(s)"); + }); + + test("a clean model reports nothing and the section still says it ran", async () => { + const exit = await run(["verify", "--format", "json", "--cwd", project("ownerId", false)]); + expect(exit).toBe(0); + const payload = JSON.parse(out.join("\n")) as { fields: { status: string; total: number } }; + expect(payload.fields).toMatchObject({ status: "ran", total: 0 }); + }); + + test("--no-field-lint silences it", async () => { + const exit = await run(["verify", "--format", "text", "--cwd", project("ownerIdd", true), "--no-field-lint"]); + expect(exit).toBe(0); + const all = [...out, ...err].join("\n"); + expect(all).not.toContain("WARN_REFERENCE_FIELD_NOT_FOUND"); + expect(all).not.toContain("WARN_DUPLICATE_FIELD_NAME"); + }); + + test("META_NO_FIELD_LINT=1 silences it", async () => { + const prev = process.env.META_NO_FIELD_LINT; + process.env.META_NO_FIELD_LINT = "1"; + try { + expect(await run(["verify", "--format", "text", "--cwd", project("ownerIdd", true)])).toBe(0); + } finally { + if (prev === undefined) delete process.env.META_NO_FIELD_LINT; + else process.env.META_NO_FIELD_LINT = prev; + } + expect([...out, ...err].join("\n")).not.toContain("WARN_REFERENCE_FIELD_NOT_FOUND"); + }); +}); diff --git a/server/typescript/packages/metadata/src/core/index.ts b/server/typescript/packages/metadata/src/core/index.ts index 4bd2f511b..a273e07f5 100644 --- a/server/typescript/packages/metadata/src/core/index.ts +++ b/server/typescript/packages/metadata/src/core/index.ts @@ -16,3 +16,8 @@ export { UriSource } from "../loader/sources/uri-source.js"; export { parseYaml } from "./parser-yaml.js"; export { loadAndExportJson } from "./export-json.js"; export type { ExportResult } from "./export-json.js"; + +// The `meta verify` field lint's structural pre-parse walk. Lives here, not on the +// root entry, because it parses YAML. +export { declaredDuplicateFields } from "../loader/declared-duplicate-fields.js"; +export type { DeclaredDuplicateField } from "../loader/declared-duplicate-fields.js"; diff --git a/server/typescript/packages/metadata/src/loader/declared-duplicate-fields.ts b/server/typescript/packages/metadata/src/loader/declared-duplicate-fields.ts new file mode 100644 index 000000000..c0aa684cb --- /dev/null +++ b/server/typescript/packages/metadata/src/loader/declared-duplicate-fields.ts @@ -0,0 +1,114 @@ +// declaredDuplicateFields — structural pre-parse walk for the `meta verify` field lint. +// +// Two `field.*` children with the same `name` in ONE object's `children` list load +// with no error on every port, and the loaded model keeps no trace of it: the parser's +// merge rule folds the later declaration into the first (a different subtype is +// discarded outright). So the question "was this field declared twice?" can only be +// answered from the document, before the merge — the same reason +// `declaredTopLevelKeys` exists for the overlay lint. +// +// Scope, deliberately narrow: +// - ONE children list. The same field declared in a base file and again in an +// overlay file is the overlay merge working as designed, and a subtype +// redeclaring an inherited field is an override. Neither is in one list. +// - Root-level objects only, addressed by the resolution key the parser gives them. + +import { TYPE_FIELD, TYPE_METADATA, TYPE_OBJECT, SUBTYPE_ROOT } from "../shared/base-types.js"; +import { + PACKAGE_SEPARATOR, + RESERVED_KEY_CHILDREN, + RESERVED_KEY_NAME, + RESERVED_KEY_PACKAGE, + TYPE_SUBTYPE_SEPARATOR, +} from "../shared/structural.js"; +import { expandPackageForPath } from "../parser-core.js"; +import { parseYamlWithPositions } from "../core/yaml-positions-walker.js"; +import type { MetaDataFormat } from "./meta-data-source.js"; + +/** The YAML authoring sugar for `isArray: true` — a suffix on the wrapper key. */ +const ARRAY_SUFFIX = "[]"; + +/** One field name declared more than once in one object's `children` list. */ +export interface DeclaredDuplicateField { + /** The declaring object's resolution key (`::`, or the bare name). */ + object: string; + /** The repeated field name. */ + field: string; + /** How many times the list declares it (always 2 or more). */ + count: number; +} + +function isRecord(v: unknown): v is Record { + return typeof v === "object" && v !== null && !Array.isArray(v); +} + +/** The TYPE segment of a wrapper key: `field.string[]` and the bare `field` are both `field`. */ +function wrapperType(key: string): string { + const dot = key.indexOf(TYPE_SUBTYPE_SEPARATOR); + const head = dot < 0 ? key : key.slice(0, dot); + return head.endsWith(ARRAY_SUFFIX) ? head.slice(0, -ARRAY_SUFFIX.length) : head; +} + +/** A node's declared name: the body's `name`, or the body itself when YAML wrote a scalar. */ +function declaredName(body: unknown): string | undefined { + const name = isRecord(body) ? body[RESERVED_KEY_NAME] : body; + return typeof name === "string" && name !== "" ? name : undefined; +} + +/** + * Structurally scan a source's raw content and report every field name a root-level + * object declares more than once in its own `children` list, in document order. + * + * Malformed shapes return `[]` rather than throwing — the real parse is where a + * structural error surfaces. A syntax error does throw, as `declaredTopLevelKeys` does. + */ +export function declaredDuplicateFields( + content: string, + format: MetaDataFormat, +): ReadonlyArray { + // Strip UTF-8 BOM if present (mirrors parseJson / parseYaml). + const normalized = content.charCodeAt(0) === 0xfeff ? content.slice(1) : content; + let parsed: unknown; + if (format === "json") parsed = JSON.parse(normalized); + else if (format === "yaml") parsed = parseYamlWithPositions(normalized).value; + else return []; + if (!isRecord(parsed)) return []; + + // JSON fuses the subtype onto the root key; sigil-free YAML may write the bare type. + const rootBody = parsed[`${TYPE_METADATA}${TYPE_SUBTYPE_SEPARATOR}${SUBTYPE_ROOT}`] ?? parsed[TYPE_METADATA]; + if (!isRecord(rootBody)) return []; + const rawRootPkg = rootBody[RESERVED_KEY_PACKAGE]; + const rootPkg = typeof rawRootPkg === "string" ? rawRootPkg : ""; + const children = rootBody[RESERVED_KEY_CHILDREN]; + if (!Array.isArray(children)) return []; + + const out: DeclaredDuplicateField[] = []; + for (const child of children) { + if (!isRecord(child)) continue; + for (const [wrapperKey, body] of Object.entries(child)) { + if (wrapperType(wrapperKey) !== TYPE_OBJECT || !isRecord(body)) continue; + const name = declaredName(body); + const members = body[RESERVED_KEY_CHILDREN]; + if (name === undefined || !Array.isArray(members)) continue; + + const counts = new Map(); + for (const member of members) { + if (!isRecord(member)) continue; + for (const [memberKey, memberBody] of Object.entries(member)) { + if (wrapperType(memberKey) !== TYPE_FIELD) continue; + const fieldName = declaredName(memberBody); + if (fieldName !== undefined) counts.set(fieldName, (counts.get(fieldName) ?? 0) + 1); + } + } + + const rawOwnPkg = body[RESERVED_KEY_PACKAGE]; + const pkg = + typeof rawOwnPkg === "string" && rawOwnPkg !== "" ? expandPackageForPath(rootPkg, rawOwnPkg) : rootPkg; + const object = pkg !== "" ? `${pkg}${PACKAGE_SEPARATOR}${name}` : name; + for (const [field, count] of counts) { + if (count > 1) out.push({ object, field, count }); + } + } + } + return out; +}