diff --git a/crates/allium-parser/src/analysis.rs b/crates/allium-parser/src/analysis.rs index 0877c46..94f72fb 100644 --- a/crates/allium-parser/src/analysis.rs +++ b/crates/allium-parser/src/analysis.rs @@ -96,6 +96,7 @@ fn run_checks(mut ctx: Ctx<'_>, source: &str) -> Vec { ctx.check_config_undefined_references(); ctx.check_list_literal_homogeneity(); ctx.check_qualified_default_aliases(); + ctx.check_default_field_schemas(); apply_suppressions(ctx.diagnostics, source) } @@ -4596,6 +4597,132 @@ impl Ctx<'_> { } } } + + /// Validates `default Type x = { ... }` object literals against the + /// declared schema of `Type` (and, recursively, of nested value/entity + /// types). Catches drift — an object-literal field that the entity no + /// longer declares (`allium.default.unknownField`) — and the rule-14c case + /// of an empty list literal whose target field is not a `List`, so it + /// has no element type to infer (`allium.list.emptyListNoElementType`). + /// + /// Only unqualified (local) types are validated: a qualified + /// `default alias/Type` names an entity in an imported module whose field + /// schema this single-module pass cannot see. + fn check_default_field_schemas(&mut self) { + let schemas = collect_local_type_schemas(self.module); + let mut diagnostics = Vec::new(); + for d in &self.module.declarations { + let Decl::Default(def) = d else { continue }; + if def.type_alias.is_some() { + continue; // imported type — schema not visible here + } + let (Some(type_name), Expr::ObjectLiteral { fields, .. }) = + (&def.type_name, &def.value) + else { + continue; + }; + validate_object_literal(fields, &type_name.name, &schemas, &mut diagnostics); + } + for diag in diagnostics { + self.push(diag); + } + } +} + +/// entity/value type name → (field name → declared type expression). +fn collect_local_type_schemas(module: &Module) -> HashMap<&str, HashMap<&str, &Expr>> { + let mut schemas: HashMap<&str, HashMap<&str, &Expr>> = HashMap::new(); + for d in &module.declarations { + let Decl::Block(b) = d else { continue }; + if !matches!( + b.kind, + BlockKind::Entity | BlockKind::ExternalEntity | BlockKind::Value + ) { + continue; + } + let Some(name) = &b.name else { continue }; + let mut fields: HashMap<&str, &Expr> = HashMap::new(); + for item in &b.items { + match &item.kind { + BlockItemKind::Assignment { name: f, value } + | BlockItemKind::FieldWithWhen { name: f, value, .. } => { + fields.insert(f.name.as_str(), value); + } + _ => {} + } + } + schemas.insert(name.name.as_str(), fields); + } + schemas +} + +/// Whether a field's declared type expression is a `List` (optionally +/// wrapped as `List?`). +fn is_list_type(expr: &Expr) -> bool { + match expr { + Expr::GenericType { name, .. } => matches!(name.as_ref(), Expr::Ident(id) if id.name == "List"), + Expr::TypeOptional { inner, .. } => is_list_type(inner), + _ => false, + } +} + +/// The base entity/value type name a field declaration refers to, if it is a +/// direct (optionally optional) named-type reference — used to recurse into +/// nested object literals. Collection and primitive types yield `None`. +fn base_type_name(expr: &Expr) -> Option<&str> { + match expr { + Expr::Ident(id) => Some(id.name.as_str()), + Expr::TypeOptional { inner, .. } => base_type_name(inner), + _ => None, + } +} + +fn validate_object_literal<'a>( + fields: &'a [NamedArg], + type_name: &str, + schemas: &HashMap<&'a str, HashMap<&'a str, &'a Expr>>, + out: &mut Vec, +) { + // Unknown type (e.g. a primitive, or a type declared elsewhere) — nothing + // to validate against. + let Some(schema) = schemas.get(type_name) else { return }; + for field in fields { + let Some(field_type) = schema.get(field.name.name.as_str()) else { + out.push( + Diagnostic::error( + field.name.span, + format!( + "Default sets field '{}' which is not declared on '{}'.", + field.name.name, type_name + ), + ) + .with_code("allium.default.unknownField"), + ); + continue; + }; + // Rule 14c: an empty list literal needs a `List` target to supply + // the element type. + if let Expr::ListLiteral { elements, span } = &field.value { + if elements.is_empty() && !is_list_type(field_type) { + out.push( + Diagnostic::error( + *span, + format!( + "Empty list literal has no inferable element type: target field '{}' is not a List.", + field.name.name + ), + ) + .with_code("allium.list.emptyListNoElementType"), + ); + } + } + // Recurse into a nested object literal against the field's declared type. + if let Expr::ObjectLiteral { fields: nested, .. } = &field.value { + if let Some(nested_type) = base_type_name(field_type) { + validate_object_literal(nested, nested_type, schemas, out); + } + } + } } fn collect_list_literals_from_item<'a>(kind: &'a BlockItemKind, out: &mut Vec<(&'a [Expr], Span)>) { @@ -6740,6 +6867,59 @@ mod tests { assert!(has_code(&ds, "allium.default.undefinedImportedAlias")); } + // -- Default field-schema validation (drift) + rule 14c -- + + #[test] + fn default_unknown_field_flagged() { + let ds = analyze_src( + "entity Policy { id: String }\ndefault Policy p = { id: \"x\", naem: \"typo\" }", + ); + assert!(has_code(&ds, "allium.default.unknownField")); + } + + #[test] + fn default_known_fields_ok() { + let ds = analyze_src( + "entity Policy { id: String\n label: String }\ndefault Policy p = { id: \"x\", label: \"y\" }", + ); + assert!(!has_code(&ds, "allium.default.unknownField")); + } + + #[test] + fn default_nested_object_unknown_field_flagged() { + // Drift in a nested value-type literal is caught recursively. + let ds = analyze_src( + "value Predicate { clause_order: List }\nentity Policy { id: String\n predicate: Predicate }\ndefault Policy p = { id: \"x\", predicate: { bogus: 5 } }", + ); + assert!(has_code(&ds, "allium.default.unknownField")); + } + + #[test] + fn empty_list_in_list_field_ok() { + let ds = analyze_src( + "entity E { tags: List }\ndefault E e = { tags: [] }", + ); + assert!(!has_code(&ds, "allium.list.emptyListNoElementType")); + } + + #[test] + fn empty_list_in_non_list_field_flagged() { + let ds = analyze_src( + "entity E { id: String }\ndefault E e = { id: [] }", + ); + assert!(has_code(&ds, "allium.list.emptyListNoElementType")); + } + + #[test] + fn qualified_default_fields_not_validated() { + // Imported type's schema isn't visible to a single-module pass; no + // unknown-field false positives on a qualified default. + let ds = analyze_src( + "use \"./p.allium\" as gp\n\ndefault gp/Policy p = { anything: 1, goes: 2 }", + ); + assert!(!has_code(&ds, "allium.default.unknownField")); + } + // -- Duplicate let -- #[test] diff --git a/docs/project/rust-checker-parity.md b/docs/project/rust-checker-parity.md index 90dc8db..a673f28 100644 --- a/docs/project/rust-checker-parity.md +++ b/docs/project/rust-checker-parity.md @@ -223,6 +223,8 @@ double-firing on the body. | `allium.let.duplicateBinding` | error | Yes | Yes | | `allium.config.undefinedReference` | warning | Yes | Yes | | `allium.list.mixedElementTypes` | error | Yes | Yes | +| `allium.list.emptyListNoElementType` | error | Yes | Yes | +| `allium.default.unknownField` | error | Yes | Yes | | `allium.surface.unusedPath` | info | Disabled | Yes | Two language features were added for juxt/allium#43: @@ -243,11 +245,17 @@ Two language features were added for juxt/allium#43: qualified triggers and config references. Both implementations validate that the alias is a known `use` import and emit `allium.default.undefinedImportedAlias` otherwise (Rust: `check_qualified_default_aliases`; TypeScript: - `findDefaultTypeReferenceIssues`). Note this does not yet validate the - literal's field set against the imported entity's schema — the checker has no - default-field-set validation in either implementation, so the cross-module - drift detection envisioned in the issue is not implemented; the qualified form - parses and resolves, removing the need to redeclare the type locally. + `findDefaultTypeReferenceIssues`). Default literals are validated against the + declared schema of **local** entity/value types (`check_default_field_schemas` + / `findDefaultFieldSchemaIssues`): a field the type does not declare is + reported as `allium.default.unknownField` (recursing into nested object + literals), and an empty list literal whose target field is not a `List` is + reported as `allium.list.emptyListNoElementType` (language-reference rule 14c). + Qualified (imported) default types are **not** validated this way — their field + schema is not visible to a single-module pass; cross-module drift detection + would require plumbing imported entity field sets through `CrossModuleContext` + (as is done for triggers) and would be CLI-multi-file-only, since the + single-file TypeScript analyzer cannot see other modules. `allium.surface.requiresWithoutDeferred` is TypeScript-only (no Rust equivalent yet). When porting it, note the deferred-name matching semantics fixed in issue #26: a named requires block matches a deferred declaration by its full name, by a trailing `.`-separated segment, or — for module-qualified declarations like `deferred billing/InvoiceWorkflow` — by the unqualified name after the `alias/` prefix. The alias alone must not satisfy the match. diff --git a/extensions/allium/src/language-tools/analyzer.ts b/extensions/allium/src/language-tools/analyzer.ts index 99cf78c..286d071 100644 --- a/extensions/allium/src/language-tools/analyzer.ts +++ b/extensions/allium/src/language-tools/analyzer.ts @@ -122,6 +122,7 @@ export function analyzeAllium( } findings.push(...findInvalidTriggerIssues(lineStarts, blocks)); findings.push(...findListLiteralHomogeneityIssues(text, lineStarts)); + findings.push(...findDefaultFieldSchemaIssues(text, lineStarts)); findings.push(...findDuplicateConfigKeys(maskedText, lineStarts, blocks)); findings.push(...findDuplicateDefaultNames(maskedText, lineStarts)); @@ -2725,6 +2726,183 @@ function findListLiteralHomogeneityIssues( return findings; } +type AstNode = Record; +type TypeSchema = Map>; + +// The single variant key of an externally-tagged AST node (e.g. "ListLiteral"). +function nodeVariant(node: unknown): string | undefined { + if (!node || typeof node !== "object" || Array.isArray(node)) { + return undefined; + } + return Object.keys(node as object)[0]; +} + +function isListType(typeExpr: unknown): boolean { + const variant = nodeVariant(typeExpr); + const body = (typeExpr as AstNode)?.[variant ?? ""] as AstNode | undefined; + if (variant === "GenericType") { + const name = body?.name as AstNode | undefined; + const ident = name?.Ident as AstNode | undefined; + return ident?.name === "List"; + } + if (variant === "TypeOptional") { + return isListType(body?.inner); + } + return false; +} + +function baseTypeName(typeExpr: unknown): string | undefined { + const variant = nodeVariant(typeExpr); + const body = (typeExpr as AstNode)?.[variant ?? ""] as AstNode | undefined; + if (variant === "Ident") { + return body?.name as string | undefined; + } + if (variant === "TypeOptional") { + return baseTypeName(body?.inner); + } + return undefined; +} + +/** + * Validates `default Type x = { ... }` literals against the declared schema of + * local entity/value types. Catches drift (`allium.default.unknownField`) and + * rule 14c (`allium.list.emptyListNoElementType`, an empty `[]` whose target + * field is not a `List`), recursing into nested object literals. Mirrors the + * Rust `check_default_field_schemas`. Qualified (imported) types are skipped — + * their schema is not visible to this single-module pass. + */ +function findDefaultFieldSchemaIssues( + text: string, + lineStarts: number[], +): Finding[] { + const findings: Finding[] = []; + let declarations: unknown; + try { + declarations = (parseAllium(text).module as unknown as AstNode).declarations; + } catch { + return findings; + } + if (!Array.isArray(declarations)) { + return findings; + } + + const schemas: TypeSchema = new Map(); + for (const decl of declarations) { + const block = (decl as AstNode)?.Block as AstNode | undefined; + if (!block) { + continue; + } + if ( + block.kind !== "Entity" && + block.kind !== "ExternalEntity" && + block.kind !== "Value" + ) { + continue; + } + const typeName = (block.name as AstNode | undefined)?.name as + | string + | undefined; + if (!typeName) { + continue; + } + const fields = new Map(); + for (const item of (block.items as unknown[]) ?? []) { + const kind = (item as AstNode)?.kind as AstNode | undefined; + const assignment = (kind?.Assignment ?? kind?.FieldWithWhen) as + | AstNode + | undefined; + if (assignment) { + const fname = (assignment.name as AstNode | undefined)?.name as + | string + | undefined; + if (fname) { + fields.set(fname, assignment.value as AstNode); + } + } + } + schemas.set(typeName, fields); + } + + const validate = (fields: unknown[], typeName: string): void => { + const schema = schemas.get(typeName); + if (!schema) { + return; + } + for (const field of fields) { + const nameNode = (field as AstNode)?.name as AstNode | undefined; + const fieldName = nameNode?.name as string | undefined; + if (!fieldName) { + continue; + } + const fieldType = schema.get(fieldName); + if (!fieldType) { + const span = (nameNode?.span ?? { start: 0, end: 0 }) as { + start: number; + end: number; + }; + findings.push( + rangeFinding( + lineStarts, + span.start, + span.end, + "allium.default.unknownField", + `Default sets field '${fieldName}' which is not declared on '${typeName}'.`, + "error", + ), + ); + continue; + } + const value = (field as AstNode).value; + const valueVariant = nodeVariant(value); + if (valueVariant === "ListLiteral") { + const listBody = (value as AstNode).ListLiteral as AstNode; + const elements = (listBody.elements as unknown[]) ?? []; + if (elements.length === 0 && !isListType(fieldType)) { + const span = (listBody.span ?? { start: 0, end: 0 }) as { + start: number; + end: number; + }; + findings.push( + rangeFinding( + lineStarts, + span.start, + span.end, + "allium.list.emptyListNoElementType", + `Empty list literal has no inferable element type: target field '${fieldName}' is not a List.`, + "error", + ), + ); + } + } else if (valueVariant === "ObjectLiteral") { + const nestedType = baseTypeName(fieldType); + if (nestedType) { + const nestedFields = + ((value as AstNode).ObjectLiteral as AstNode).fields ?? []; + validate(nestedFields as unknown[], nestedType); + } + } + } + }; + + for (const decl of declarations) { + const def = (decl as AstNode)?.Default as AstNode | undefined; + if (!def || def.type_alias) { + continue; + } + const typeName = (def.type_name as AstNode | undefined)?.name as + | string + | undefined; + const value = def.value as AstNode | undefined; + if (!typeName || nodeVariant(value) !== "ObjectLiteral") { + continue; + } + const fields = ((value as AstNode).ObjectLiteral as AstNode).fields ?? []; + validate(fields as unknown[], typeName); + } + + return findings; +} + function rangeFinding( lineStarts: number[], startOffset: number, diff --git a/extensions/allium/test/analyzer.test.ts b/extensions/allium/test/analyzer.test.ts index cfb1f9e..6403144 100644 --- a/extensions/allium/test/analyzer.test.ts +++ b/extensions/allium/test/analyzer.test.ts @@ -1307,3 +1307,48 @@ test("accepts a qualified type name in a default declaration", () => { false, ); }); + +// Default field-schema drift + rule 14c (allium/#43 follow-up) +test("flags unknown fields in a default (drift), including nested", () => { + const top = analyzeAllium( + `entity Policy {\n id: String\n}\ndefault Policy p = { id: "x", naem: "typo" }`, + ); + assert.ok(top.some((f) => f.code === "allium.default.unknownField")); + + const nested = analyzeAllium( + `value P {\n clause_order: List\n}\nentity Policy {\n id: String\n predicate: P\n}\ndefault Policy p = { id: "x", predicate: { bogus: 5 } }`, + ); + assert.ok(nested.some((f) => f.code === "allium.default.unknownField")); +}); + +test("does not flag known fields, or fields on a qualified (imported) default", () => { + const known = analyzeAllium( + `entity E {\n id: String\n label: String\n}\ndefault E e = { id: "x", label: "y" }`, + ); + assert.equal( + known.some((f) => f.code === "allium.default.unknownField"), + false, + ); + const qualified = analyzeAllium( + `use "./p.allium" as gp\n\ndefault gp/Policy p = { anything: 1, goes: 2 }`, + ); + assert.equal( + qualified.some((f) => f.code === "allium.default.unknownField"), + false, + ); +}); + +test("rule 14c: empty list literal must target a List field", () => { + const bad = analyzeAllium( + `entity E {\n id: String\n}\ndefault E e = { id: [] }`, + ); + assert.ok(bad.some((f) => f.code === "allium.list.emptyListNoElementType")); + + const ok = analyzeAllium( + `entity E {\n tags: List\n}\ndefault E e = { tags: [] }`, + ); + assert.equal( + ok.some((f) => f.code === "allium.list.emptyListNoElementType"), + false, + ); +});