diff --git a/crates/allium-parser/src/analysis.rs b/crates/allium-parser/src/analysis.rs index 29c5711..447178b 100644 --- a/crates/allium-parser/src/analysis.rs +++ b/crates/allium-parser/src/analysis.rs @@ -4485,8 +4485,14 @@ fn collect_bound_names<'a>(expr: &'a Expr, out: &mut HashSet<&'a str>) { } Expr::Call { args, .. } => { for arg in args { - if let CallArg::Positional(Expr::Ident(id)) = arg { - out.insert(&id.name); + match arg { + CallArg::Positional(Expr::Ident(id)) => { + out.insert(&id.name); + } + CallArg::Named(named) if is_type_annotation_expr(&named.value) => { + out.insert(&named.name.name); + } + _ => {} } } } @@ -4498,6 +4504,21 @@ fn collect_bound_names<'a>(expr: &'a Expr, out: &mut HashSet<&'a str>) { } } +fn is_type_annotation_expr(expr: &Expr) -> bool { + match expr { + Expr::Ident(id) => starts_uppercase(&id.name), + Expr::QualifiedName(q) => starts_uppercase(&q.name), + Expr::GenericType { name, args, .. } => { + is_type_annotation_expr(name) && args.iter().all(is_type_annotation_expr) + } + Expr::Pipe { left, right, .. } => { + is_type_annotation_expr(left) && is_type_annotation_expr(right) + } + Expr::TypeOptional { inner, .. } => is_type_annotation_expr(inner), + _ => false, + } +} + fn check_unbound_roots( expr: &Expr, bound: &HashSet<&str>, @@ -5092,6 +5113,39 @@ mod tests { assert!(!has_code(&ds, "allium.surface.unusedBinding")); } + // -- Rule bindings -- + + #[test] + fn typed_call_trigger_param_is_bound() { + let ds = analyze_src( + "entity Account {\n name: String\n}\n\n\ + entity Greeting {\n label: String\n}\n\n\ + rule TypedParam {\n when: AccountSeen(account: Account)\n \ + ensures: Greeting.created(label: account.name)\n}\n", + ); + assert!(!has_code(&ds, "allium.rule.undefinedBinding")); + } + + #[test] + fn generic_typed_call_trigger_param_is_bound() { + let ds = analyze_src( + "entity Account {\n name: String\n}\n\n\ + entity Greeting {\n label: String\n}\n\n\ + rule GenericTypedParam {\n when: AccountsSeen(accounts: List)\n \ + ensures: Greeting.created(label: accounts.count)\n}\n", + ); + assert!(!has_code(&ds, "allium.rule.undefinedBinding")); + } + + #[test] + fn named_literal_trigger_arg_is_not_binding() { + let ds = analyze_src( + "rule LiteralFilter {\n when: CommandInvoked(name: \"npm run test\")\n \ + requires: name.status = active\n ensures: Done()\n}\n", + ); + assert!(has_code(&ds, "allium.rule.undefinedBinding")); + } + // -- Status state machine -- #[test] diff --git a/extensions/allium/src/language-tools/analyzer.ts b/extensions/allium/src/language-tools/analyzer.ts index d8b10ad..4838040 100644 --- a/extensions/allium/src/language-tools/analyzer.ts +++ b/extensions/allium/src/language-tools/analyzer.ts @@ -2437,11 +2437,16 @@ function collectRuleBoundNames( /^\s*when\s*:\s*[A-Za-z_][A-Za-z0-9_]*(?:\/[A-Za-z_][A-Za-z0-9_]*)?\s*\(([^)]*)\)/m; const whenCallMatch = ruleBody.match(whenCallPattern); if (whenCallMatch) { - for (const raw of whenCallMatch[1].split(",")) { + for (const raw of splitTopLevelArgs(whenCallMatch[1])) { const name = raw.trim(); if (name.length === 0 || name === "_") { continue; } + const typed = name.match(/^([A-Za-z_][A-Za-z0-9_]*)\s*:\s*(.+)$/); + if (typed && isTypeAnnotationText(typed[2])) { + bound.add(typed[1]); + continue; + } if (/^[A-Za-z_][A-Za-z0-9_]*\??$/.test(name)) { bound.add(name.replace(/\?$/, "")); } @@ -2487,6 +2492,128 @@ function collectRuleBoundNames( return bound; } +function splitTopLevelArgs(argsText: string): string[] { + const parts: string[] = []; + let start = 0; + let angleDepth = 0; + let parenDepth = 0; + let bracketDepth = 0; + let braceDepth = 0; + let stringQuote: string | undefined; + let escaped = false; + + for (let i = 0; i < argsText.length; i += 1) { + const ch = argsText[i]; + + if (stringQuote) { + if (escaped) { + escaped = false; + } else if (ch === "\\") { + escaped = true; + } else if (ch === stringQuote) { + stringQuote = undefined; + } + continue; + } + + if (ch === '"' || ch === "'") { + stringQuote = ch; + continue; + } + + if (ch === "<") { + angleDepth += 1; + } else if (ch === ">" && angleDepth > 0) { + angleDepth -= 1; + } else if (ch === "(") { + parenDepth += 1; + } else if (ch === ")" && parenDepth > 0) { + parenDepth -= 1; + } else if (ch === "[") { + bracketDepth += 1; + } else if (ch === "]" && bracketDepth > 0) { + bracketDepth -= 1; + } else if (ch === "{") { + braceDepth += 1; + } else if (ch === "}" && braceDepth > 0) { + braceDepth -= 1; + } else if ( + ch === "," && + angleDepth === 0 && + parenDepth === 0 && + bracketDepth === 0 && + braceDepth === 0 + ) { + parts.push(argsText.slice(start, i)); + start = i + 1; + } + } + + parts.push(argsText.slice(start)); + return parts; +} + +function isTypeAnnotationText(text: string): boolean { + const trimmed = text.trim(); + if (trimmed.length === 0) { + return false; + } + if (trimmed.endsWith("?")) { + return isTypeAnnotationText(trimmed.slice(0, -1)); + } + + const generic = parseGenericTypeText(trimmed); + if (generic) { + const args = splitTopLevelArgs(generic.args).map((arg) => arg.trim()); + return ( + isTypeAnnotationText(generic.name) && + args.length > 0 && + args.every((arg) => arg.length > 0 && isTypeAnnotationText(arg)) + ); + } + + return /^(?:[A-Za-z_][A-Za-z0-9_]*\/)?[A-Z][A-Za-z0-9_]*$/.test(trimmed); +} + +function parseGenericTypeText( + text: string, +): { name: string; args: string } | undefined { + let depth = 0; + let genericStart = -1; + let genericEnd = -1; + + for (let i = 0; i < text.length; i += 1) { + const ch = text[i]; + if (ch === "<") { + if (depth === 0 && genericStart === -1) { + genericStart = i; + } + depth += 1; + } else if (ch === ">") { + depth -= 1; + if (depth < 0) { + return undefined; + } + if (depth === 0) { + genericEnd = i; + } + } + } + + if ( + depth !== 0 || + genericStart === -1 || + genericEnd !== text.length - 1 + ) { + return undefined; + } + + return { + name: text.slice(0, genericStart).trim(), + args: text.slice(genericStart + 1, genericEnd).trim(), + }; +} + function isInsideDoubleQuotedStringAtIndex( text: string, index: number, diff --git a/extensions/allium/test/analyzer.test.ts b/extensions/allium/test/analyzer.test.ts index 55e6ba2..e4518d5 100644 --- a/extensions/allium/test/analyzer.test.ts +++ b/extensions/allium/test/analyzer.test.ts @@ -779,6 +779,40 @@ test("does not report binding defined by trigger parameter", () => { ); }); +test("does not report binding defined by typed trigger parameter", () => { + const findings = analyzeAllium( + `entity User {\n status: String\n}\n\nrule Notify {\n when: UserUpdated(user: User)\n requires: user.status = active\n ensures: Done()\n}\n`, + ); + assert.equal( + findings.some((f) => f.code === "allium.rule.undefinedBinding"), + false, + ); +}); + +test("does not report binding defined by generic typed trigger parameter", () => { + const findings = analyzeAllium( + `entity User {\n status: String\n}\n\nrule Notify {\n when: UsersUpdated(users: List, metadata: Map)\n requires: users.count > 0\n requires: metadata.count > 0\n ensures: Done()\n}\n`, + ); + assert.equal( + findings.some((f) => f.code === "allium.rule.undefinedBinding"), + false, + ); +}); + +test("does not treat named literal trigger argument as binding", () => { + const findings = analyzeAllium( + `rule Notify {\n when: CommandInvoked(name: "npm run test")\n requires: name.status = active\n ensures: Done()\n}\n`, + ); + assert.ok(findings.some((f) => f.code === "allium.rule.undefinedBinding")); +}); + +test("does not treat lowercase named trigger argument as typed binding", () => { + const findings = analyzeAllium( + `rule Notify {\n when: FieldSelected(field: course_name)\n requires: field.status = active\n ensures: Done()\n}\n`, + ); + assert.ok(findings.some((f) => f.code === "allium.rule.undefinedBinding")); +}); + test("does not report binding defined in context block", () => { const findings = analyzeAllium( `entity User {\n status: String\n}\n\ngiven {\n user: User\n}\n\nrule Notify {\n when: Ping()\n requires: user.status = active\n ensures: Done()\n}\n`,