diff --git a/docs/ddlint-design.md b/docs/ddlint-design.md index 8d84754a..1a893015 100644 --- a/docs/ddlint-design.md +++ b/docs/ddlint-design.md @@ -711,7 +711,18 @@ The semantic model records: - rule-local bindings introduced by rule heads, assignment patterns, and `for`-loop patterns; - explicit scope records with parent links; and -- relation and variable use sites together with a final resolution result. +- relation and variable use sites together with a final resolution result and + use-site provenance. + +Relation use-site provenance is part of the contract for correctness rules: + +- rule-head relation atoms are recorded as write sites; +- rule-body atoms are recorded as read sites; and +- `for` iterable and guard relation atoms are recorded as read sites. + +This distinction is what allows `unused-relation` to warn on head-only sink +relations while still treating resolved body, iterable, and guard references as +genuine reads. Resolution is deliberately tri-state: @@ -743,16 +754,18 @@ immediate value to users, the following catalog of rules is proposed. This list prioritizes correctness checks, followed by performance hints, and stylistic suggestions, establishing a solid foundation of essential lints. -| Rule Name | Group | Default Level | Autofixable | Description | -| ---------------------- | ----------- | ------------- | ----------- | ---------------------------------------------------------------------------------------------------------------------------------- | -| unused-relation | correctness | warn | No | Detects relations that are defined but never read from. | -| unused-variable | correctness | warn | No | Detects variables bound in a rule head that are not used in the body. | -| shadowed-variable | correctness | warn | No | Detects when a variable binding in a literal shadows one from a preceding literal in the same rule body. | -| recursive-negation | correctness | error | No | Detects rules with recursion through negation, which leads to unsafe, non-monotonic programs. | -| inefficient-join-order | performance | hint | No | Evaluates rule bodies and suggests reordering atoms for a more efficient join plan, e.g., placing more restrictive literals first. | -| superfluous-group-by | performance | warn | Yes | Detects group_by clauses where the aggregation is trivial (e.g., grouping by all variables) and can be removed. | -| consistent-casing | style | allow | Yes | Enforces a consistent casing style for relation and type identifiers (e.g., PascalCase) and variables (e.g., snake_case). | -| no-magic-numbers | style | allow | No | Flags the use of unnamed numeric literals in rule bodies where a named constant might be clearer. | +Table: DDLint rule catalogue and metadata. + +| Rule Name | Group | Default Level | Autofixable | Description | +| ---------------------- | ----------- | ------------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------- | +| unused-relation | correctness | warn | No | Detects declared relations with no resolved read-like uses in rule bodies, `for` iterables, or `for` guards; rule heads count only as writes. | +| unused-variable | correctness | warn | No | Detects variables bound in a rule head that are not used in the body. | +| shadowed-variable | correctness | warn | No | Detects when a variable binding in a literal shadows one from a preceding literal in the same rule body. | +| recursive-negation | correctness | error | No | Detects rules with recursion through negation, which leads to unsafe, non-monotonic programs. | +| inefficient-join-order | performance | hint | No | Evaluates rule bodies and suggests reordering atoms for a more efficient join plan, e.g., placing more restrictive literals first. | +| superfluous-group-by | performance | warn | Yes | Detects group_by clauses where the aggregation is trivial (e.g., grouping by all variables) and can be removed. | +| consistent-casing | style | allow | Yes | Enforces a consistent casing style for relation and type identifiers (e.g., PascalCase) and variables (e.g., snake_case). | +| no-magic-numbers | style | allow | No | Flags the use of unnamed numeric literals in rule bodies where a named constant might be clearer. | This table serves as a concrete work breakdown for the engineering team and clearly communicates the linter's initial capabilities and priorities to early diff --git a/docs/execplans/4-1-1-implement-unused-relation-diagnostics.md b/docs/execplans/4-1-1-implement-unused-relation-diagnostics.md new file mode 100644 index 00000000..ed095846 --- /dev/null +++ b/docs/execplans/4-1-1-implement-unused-relation-diagnostics.md @@ -0,0 +1,417 @@ +# Implement `unused-relation` diagnostics + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: Implemented + +## Purpose / big picture + +Roadmap item `4.1.1` is the first production lint rule in the initial +correctness catalogue. After this change, `ddlint` exports an `unused-relation` +lint rule (`UnusedRelationRule`) that callers must explicitly register in a +`CstRuleStore` before running the `Runner`. Once registered, the rule emits an +`unused-relation` warning for each declared relation that has no resolved +read-like uses anywhere in the analysed program. + +For this milestone, "read from" means a resolved relation-position use in a +rule body, a `for` iterable, or a `for` guard. A relation named in a rule head +is a write site, not a read site, so head-only relations must still be warned +about. Observable success is: + +- `ddlint` exports a concrete `unused-relation` rule (`UnusedRelationRule`) + that callers register in `CstRuleStore`; the rule is not enabled by default. +- Running `Runner` with that rule registered emits one warning per unread + relation declaration and no warnings for relations with at least one resolved + read. +- The semantic model exposes enough provenance to distinguish relation reads + from writes without re-walking the concrete syntax tree inside the rule. +- Unit tests cover semantic read-versus-write classification and the rule's + positive and negative cases. +- Behavioural tests prove end-to-end diagnostics through `Runner`. +- `docs/ddlint-design.md` records the final rule contract and the semantic-use + provenance added for it. +- `docs/roadmap.md` marks `4.1.1` done only after all quality gates pass. + +## Approval gate + +Approved by the user on 2026-03-22 when they requested implementation of this +ExecPlan. + +## Context and orientation + +The current repository already contains the semantic substrate this rule will +build on: + +- `src/sema/build.rs` builds one `SemanticModel` from a parsed program. +- `src/sema/builder.rs` records top-level relation, function, and type + declarations. +- `src/sema/traverse.rs` records relation and variable uses while walking rule + heads and rule bodies. +- `src/sema/model.rs` stores `Symbol` and `UseSite` records, but today a + `UseSite` does not record whether it came from a rule head or a read-like + body position. +- `src/linter/rule.rs`, `src/linter/store.rs`, and `src/linter/runner.rs` + provide the rule trait surface, dispatch store, and parallel runner. +- `src/linter/macros.rs` provides `declare_lint!`, which should be used for + the new rule unless a documented blocker appears. + +There is no shipped rule-catalogue module yet. Current behavioural tests +register ad hoc rules directly in `CstRuleStore`. This milestone therefore +needs to add the first exported production rule module and corresponding tests, +but it does not need to invent a full Command-Line Interface (CLI)-configured +default ruleset. + +The key design gap is semantic provenance. `docs/ddlint-design.md` says +`unused-relation` detects relations "defined but never read from", yet the +current semantic layer records relation uses from both rule heads and rule +bodies with the same `UseKind::Relation`. Counting all relation-position uses +would therefore under-report unused relations by treating writes as reads. + +## Constraints + +- Treat `docs/ddlint-design.md` section `3.2.1` and section `3.3` plus + `docs/roadmap.md` item `4.1.1` as the normative design basis for this + milestone. +- Keep scope limited to implementing `unused-relation`, the additive semantic + provenance needed for it, its tests, and the required documentation and + roadmap updates. +- Do not implement `unused-variable`, `shadowed-variable`, command-line + interface (CLI) rule listing, configuration-file loading, or rich `miette` + conversion in this milestone. +- Keep parser grammar and parse-stage diagnostics unchanged unless a bug blocks + the rule and there is no narrower fix. +- Extend the semantic model additively. Existing `RuleCtx`, `Runner`, and + parser APIs must remain source-compatible. +- Preserve the current owned-data and `Send + Sync` guarantees of + `SemanticModel`; do not store `rowan` nodes inside semantic records. +- Use `declare_lint!` for the production rule unless a documented limitation + of the macro makes that impossible. +- Every new Rust module must begin with a `//!` module comment. +- Keep files below 400 lines by splitting rule modules, helpers, and tests as + needed. +- Update `docs/ddlint-design.md` with any rule-semantics decision taken during + implementation. +- Mark `docs/roadmap.md` item `4.1.1` done only after all gates pass. +- Run quality gates through Make targets, using `set -o pipefail` and `tee`. + +## Tolerances (exception triggers) + +- Scope: if implementation requires more than 10 changed files or more than + 650 net new lines, stop and re-evaluate the module split before continuing. +- Interface: if the rule cannot be implemented without a breaking change to + `RuleCtx`, `Runner`, `SemanticModel`, or `declare_lint!`, stop and escalate. +- Semantics: if the team decides that `output relation` declarations should be + exempt because of external consumers, stop and get explicit confirmation + before encoding that policy. +- Provenance: if read-versus-write classification cannot be represented + additively in `UseSite`, stop and redesign the semantic query surface before + proceeding. +- Iterations: if targeted tests or Clippy fixes still fail after three focused + rounds, stop and escalate with the exact failing test names or lint IDs. + +## Risks + +- Risk: head relation atoms are currently recorded as generic relation uses, so + a naïve implementation would treat writes as reads and miss real unused + relations. Severity: high. Likelihood: high. Mitigation: add explicit use + provenance and write failing tests before adding the rule. + +- Risk: the roadmap wording says "declared relations with no usage sites", + while the design catalogue says "never read from". Severity: medium. + Likelihood: medium. Mitigation: document and implement the narrower + read-based rule semantics, because that is the more precise statement and + avoids counting rule-head writes as reads. + +- Risk: `output relation` declarations may represent externally consumed data, + but the current design documents do not define an exemption. Severity: + medium. Likelihood: medium. Mitigation: keep the milestone scoped to internal + program analysis and document that no external-consumer exemption is applied + unless the user approves a policy change. + +- Risk: the current runner API has only per-node and per-token hooks, so a + rule that repeatedly scans the full semantic model could become noisy or + awkward to maintain. Severity: low. Likelihood: medium. Mitigation: add small + semantic query helpers that make the rule implementation direct and testable, + but avoid introducing a broader precomputed lint-cache layer. + +## Decision log + +- Decision: treat rule-head relation atoms as write sites, not read sites, for + `unused-relation`. Rationale: `docs/ddlint-design.md` defines the rule as + "defined but never read from", and counting heads as reads would make sink + relations appear used when they are only written. Date/Author: 2026-03-21 / + Codex. + +- Decision: do not add a special exemption for `output relation` declarations + in this milestone. Rationale: no current design document defines such an + exemption, and roadmap item `4.1.1` is phrased in terms of declarations plus + internal usage sites. Date/Author: 2026-03-21 / Codex. + +- Decision: expose the first production rule as a normal exported rule type + that tests register explicitly in `CstRuleStore`, rather than inventing a + global default ruleset now. Rationale: the current repository has no shipped + rule-catalogue registration surface, and adding one would broaden scope + beyond `4.1.1`. Date/Author: 2026-03-21 / Codex. + +## Proposed design + +Add a small production-rules namespace under `src/linter/` so the rule has a +stable home and future correctness rules can follow the same pattern. A minimal +layout is: + +- `src/linter/rules/mod.rs` +- `src/linter/rules/correctness/mod.rs` +- `src/linter/rules/correctness/unused_relation.rs` + +Export the new module from `src/linter/mod.rs` so integration tests and later +CLI wiring can import the rule cleanly. + +Extend the semantic model with additive relation-use provenance. The shipped +implementation uses a `UseOrigin` enum stored on every `UseSite` with the +following variants: + +- `UseOrigin::RelationHead` — relation use from a rule head (write site); +- `UseOrigin::RelationBody` — relation use from a rule-body atom or + semantic-rule body atom (read site); +- `UseOrigin::ForIterable` — relation use from a `for` iterable expression + (read site); +- `UseOrigin::ForGuard` — relation use from a `for` guard expression (read + site); and +- `UseOrigin::Variable` — variable use recorded while traversing expressions. + +The `UseOrigin::is_relation_read()` method returns `true` for `RelationBody`, +`ForIterable`, and `ForGuard`, allowing the rule to answer one clear question +without inspecting syntax nodes: "does this relation declaration have any +resolved read-like uses?" + +Add semantic helper methods that keep rule code simple. The shipped API +provides: + +```rust +model.relation_symbols() // Iterator over (SymbolId, &Symbol) for relations +model.relation_symbol_at_span(span) // Lookup symbol by span +model.has_resolved_relation_read(symbol_id) // Check if symbol has reads +``` + +These helpers allow the rule to iterate over relation declarations and check +whether each has resolved read-like uses, without inspecting syntax nodes or +filtering the full symbol table. + +Implement the rule itself with `declare_lint!`. It should target +`SyntaxKind::N_RELATION_DECL`, use metadata `name: "unused-relation"`, +`group: "correctness"`, and `level: warn`, then emit one diagnostic per unread +relation declaration with a message equivalent to: + +```plaintext +relation `Foo` is declared but never read from +``` + +The diagnostic span should be the declaration node range, which already matches +the recorded relation declaration span in the semantic model. + +The rule must ignore malformed declarations that do not map cleanly to a named +relation symbol. Silent non-emission is preferable to a panic when semantic +facts are incomplete because of parse recovery. + +## Implementation plan + +### Milestone 1: add semantic use provenance + +Update `src/sema/model.rs`, `src/sema/traverse.rs`, and any supporting helpers +so relation uses carry enough provenance to distinguish reads from writes. Keep +the existing `UseKind::Relation` and `UseKind::Variable` split; this milestone +only needs extra origin metadata, not a new top-level use-kind taxonomy. + +Adjust semantic-model unit tests in `src/sema/tests.rs` and behavioural tests +in `tests/semantic_scope_resolution.rs` so they assert the new provenance +contract. At minimum, add coverage proving: + +- a relation in a rule head is recorded as a write; +- the same relation name in a rule body is recorded as a read; and +- top-level `for` semantic rules preserve the same distinction. + +### Milestone 2: add semantic query helpers for the rule + +Add focused helper methods on `SemanticModel` that answer the questions +`unused-relation` actually needs. Keep them additive and deterministic. The +goal is that the rule module reads as straightforward policy code rather than a +manually repeated scan over `symbols()` and `uses()`. + +Write unit tests for those helpers near the semantic-model tests. Include at +least these cases: + +- a relation with one resolved body read returns true for "has resolved read"; +- a relation mentioned only in rule heads returns false; and +- an unresolved relation-position use does not count as a read. + +### Milestone 3: implement the production rule module + +Create the production rule module under `src/linter/rules/correctness/` and +export it through `src/linter/mod.rs`. Use `declare_lint!` for metadata and +dispatch boilerplate. + +In the rule body: + +1. Read the declaration node's span. +2. Resolve that span to the corresponding relation symbol via a semantic-model + helper. +3. Ask whether the symbol has any resolved read-like uses. +4. Emit a warning diagnostic only when the answer is no. + +Keep helper functions in the same rule module if they are specific to this +rule. If they become reusable across multiple correctness rules, move them to a +small sibling helper module only after the first rule works and the need is +real. + +### Milestone 4: add rule-focused tests + +Add unit tests close to the rule module and behavioural tests under `tests/` +that run the real parser and runner. Use `rstest` fixtures and parameterized +cases where they reduce repetition. + +The minimum coverage set is: + +- positive: a declared relation with no reads emits exactly one + `unused-relation` diagnostic; +- negative: a relation read in a rule body emits no diagnostic; +- head-only write: a relation that appears only in rule heads still emits a + diagnostic; +- unresolved name safety: an unresolved relation-position use does not mark a + declaration as used; +- multi-relation ordering: diagnostics are deterministic when several unused + relations exist. + +Use a dedicated behavioural test file, for example +`tests/unused_relation_rule.rs`, rather than burying these cases inside the +generic runner tests. + +### Milestone 5: update docs and roadmap + +Update `docs/ddlint-design.md` in the semantic-model section and the initial +lint catalogue section so the implemented read-versus-write distinction is +explicit. If the semantic-model contract section already describes relation +uses too loosely, tighten that wording there rather than spreading the rule +semantics across multiple unrelated docs. + +If semantic provenance becomes a durable invariant rather than a rule-specific +detail, add a short note to `docs/parser-implementation-notes.md` explaining +that relation use sites now record whether they are reads or writes. This is +the right place for implementation-level semantic invariants that future rules +will rely on. + +After all tests and gates pass, change `docs/roadmap.md` item `4.1.1` from +unchecked to done. + +## Validation plan + +Start with targeted tests that go red before the implementation is complete, +then finish with the full repository gates. + +Suggested targeted commands during development: + +```bash +set -o pipefail; cargo test sema::tests 2>&1 | tee /tmp/4-1-1-sema-unit.log +set -o pipefail; cargo test --test semantic_scope_resolution 2>&1 | tee /tmp/4-1-1-sema-behaviour.log +set -o pipefail; cargo test unused_relation 2>&1 | tee /tmp/4-1-1-unused-relation-targeted.log +``` + +Required final commands: + +```bash +set -o pipefail; make fmt 2>&1 | tee /tmp/4-1-1-make-fmt.log +set -o pipefail; make markdownlint 2>&1 | tee /tmp/4-1-1-make-markdownlint.log +set -o pipefail; make nixie 2>&1 | tee /tmp/4-1-1-make-nixie.log +set -o pipefail; make check-fmt 2>&1 | tee /tmp/4-1-1-make-check-fmt.log +set -o pipefail; make lint 2>&1 | tee /tmp/4-1-1-make-lint.log +set -o pipefail; make test 2>&1 | tee /tmp/4-1-1-make-test.log +``` + +Acceptance evidence for the rule should include at least one behavioural test +with source equivalent to: + +```plaintext +input relation Source(x: u32) +relation Sink(x: u32) +Sink(x) :- Source(x). +``` + +Expected outcome: `Source` is not warned because it is read in the body; `Sink` +is warned because it is only written in the head. + +## Progress + +- [x] (2026-03-21 00:00Z) Reviewed roadmap item `4.1.1`, the `execplans` + skill, current semantic-analysis code, and adjacent ExecPlans. +- [x] (2026-03-21 00:10Z) Identified the key semantic gap: relation uses do + not yet distinguish reads from writes. +- [x] (2026-03-21 00:20Z) Wrote this draft ExecPlan to + `docs/execplans/4-1-1-implement-unused-relation-diagnostics.md`. +- [x] (2026-03-22 00:05Z) User approved implementation by requesting execution + of this plan. +- [x] (2026-03-22 00:40Z) Implemented semantic use-site provenance, + relation-read query helpers, the exported `unused-relation` rule module, and + unit plus behavioural coverage. +- [x] (2026-03-22 00:50Z) Updated `docs/ddlint-design.md`, + `docs/parser-implementation-notes.md`, and `docs/roadmap.md` to reflect the + shipped contract. +- [x] (2026-03-22 01:25Z) Ran `make fmt`, `make markdownlint`, `make nixie`, + `make check-fmt`, `make lint`, and `CI=1 make test`; all passed. + +## Surprises & discoveries + +- Observation: roadmap prerequisites `3.3.2` and `3.3.4` are already complete, + and the semantic model does record relation declarations and relation uses. + Impact: the missing work is rule-policy plumbing and provenance, not parser + or symbol-table construction. + +- Observation: `src/sema/traverse.rs` currently records relation uses from rule + heads via `collect_head_expr`, so the semantic model does not yet encode the + "read from" language used by the rule catalogue. Impact: `unused-relation` + cannot be implemented correctly without extending `UseSite`. + +- Observation: the current linter module exports engine primitives only; there + is no production rule namespace yet. Impact: this milestone should introduce + a small `src/linter/rules/` module tree, but should not broaden into a full + default ruleset or CLI registry. + +- Observation: early experiments with top-level `for` desugaring recorded + iterable relation reads as semantic-rule body reads rather than a dedicated + `ForIterable` origin. The shipped contract treats those uses as read-like, + and tests should assert read-versus-write semantics per the durable rule + contract rather than overfitting to any particular desugaring detail. + Historical note: the `ForIterable` origin variant was introduced to + distinguish iterable positions from rule-body positions. + +## Outcomes & retrospective + +- Final rule modules: + `src/linter/rules/mod.rs`, `src/linter/rules/correctness/mod.rs`, and + `src/linter/rules/correctness/unused_relation.rs`. +- Shipped semantic provenance shape: `UseSite` now carries `UseOrigin` with + `RelationHead`, `RelationBody`, `ForIterable`, `ForGuard`, and `Variable`. + `SemanticModel` now exposes `relation_symbols()`, + `relation_symbol_at_span()`, and `has_resolved_relation_read()`. +- Test coverage: + `src/sema/tests.rs` covers relation use origins and helper queries; + `tests/semantic_scope_resolution.rs` covers end-to-end semantic provenance; + `src/linter/rules/correctness/unused_relation.rs` contains focused rule unit + tests; and `tests/unused_relation_rule.rs` covers end-to-end diagnostics and + deterministic ordering through `Runner`. +- Documentation updates: + `docs/ddlint-design.md` now documents relation read-versus-write provenance + and the exact `unused-relation` contract; + `docs/parser-implementation-notes.md` records relation-use provenance as a + current semantic invariant; and `docs/roadmap.md` marks item `4.1.1` done. +- Passed gate commands: + + ```bash + set -o pipefail; make fmt 2>&1 | tee /tmp/4-1-1-final-make-fmt.log + set -o pipefail; make markdownlint 2>&1 | tee /tmp/4-1-1-make-markdownlint.log + set -o pipefail; make nixie 2>&1 | tee /tmp/4-1-1-make-nixie.log + set -o pipefail; make check-fmt 2>&1 | tee /tmp/4-1-1-final-check-fmt.log + set -o pipefail; make lint 2>&1 | tee /tmp/4-1-1-final-lint.log + set -o pipefail; CI=1 make test 2>&1 | tee /tmp/4-1-1-final-test.log + ``` diff --git a/docs/parser-implementation-notes.md b/docs/parser-implementation-notes.md index 8c1980d0..348636df 100644 --- a/docs/parser-implementation-notes.md +++ b/docs/parser-implementation-notes.md @@ -202,6 +202,8 @@ Current guarantees are intentionally narrow: scopes. - Top-level relation, function, and type declarations are recorded in source order. +- Relation use sites record provenance that distinguishes rule-head writes from + rule-body reads and `for` iterable/guard reads. - Rule-head bindings are visible from the start of their rule scope. - Assignment-pattern bindings become visible only after the literal that introduces them. diff --git a/docs/roadmap.md b/docs/roadmap.md index e44136e9..41288300 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -356,8 +356,9 @@ catalog. ### 4.1. Correctness rules -- [ ] 4.1.1. Implement `unused-relation` diagnostics for declared relations with - no usage sites. Requires 3.3.2 and 3.3.4. See docs/ddlint-design.md §3.3. +- [x] 4.1.1. Implement `unused-relation` diagnostics for declared relations with + no resolved read-like uses (rule-head writes do not count as reads). Requires + 3.3.2 and 3.3.4. See docs/ddlint-design.md §3.3. - [ ] 4.1.2. Implement `unused-variable` diagnostics for variables defined but not used within a rule, treating `_` as explicit ignore. Requires 3.3.3 and 3.3.4. See docs/ddlint-design.md §3.3. diff --git a/src/linter/mod.rs b/src/linter/mod.rs index 3493191c..7c1c92ee 100644 --- a/src/linter/mod.rs +++ b/src/linter/mod.rs @@ -6,6 +6,7 @@ mod macros; mod rule; +pub mod rules; mod runner; mod store; diff --git a/src/linter/rules/correctness/mod.rs b/src/linter/rules/correctness/mod.rs new file mode 100644 index 00000000..45e80926 --- /dev/null +++ b/src/linter/rules/correctness/mod.rs @@ -0,0 +1,5 @@ +//! Correctness lint rules that flag likely semantic mistakes. + +mod unused_relation; + +pub use unused_relation::UnusedRelationRule; diff --git a/src/linter/rules/correctness/unused_relation.rs b/src/linter/rules/correctness/unused_relation.rs new file mode 100644 index 00000000..652660cb --- /dev/null +++ b/src/linter/rules/correctness/unused_relation.rs @@ -0,0 +1,127 @@ +//! `unused-relation` warns about declared relations with no resolved read-like uses. +//! +//! A relation counts as read when it appears in a rule body, `for` iterable, or +//! `for` guard position and that use resolves to the declaration. Rule-head +//! writes and unresolved relation uses do not count as reads, so head-only +//! relations and relations referenced only in broken rules still trigger +//! warnings. +//! +//! This rule uses `SemanticModel::has_resolved_relation_read()` to check +//! whether a relation has at least one resolved read-like use. + +use crate::linter::{LintDiagnostic, Rule}; +use crate::parser::ast::rule::text_range_to_span; +use crate::{SyntaxKind, declare_lint}; + +declare_lint! { + /// Detects relations declared but with no resolved read-like uses. + /// + /// A relation is considered read when it appears in a rule body, `for` + /// iterable, or `for` guard and that use resolves to the declaration. + /// Rule-head writes and unresolved uses do not count. + pub UnusedRelationRule { + name: "unused-relation", + group: "correctness", + level: warn, + target_kinds: [SyntaxKind::N_RELATION_DECL], + fn check_node(&self, node, ctx, diagnostics) { + let declaration_range = node.text_range(); + let declaration_span = text_range_to_span(declaration_range); + let Some(symbol_id) = ctx + .semantic_model() + .relation_symbol_at_span(&declaration_span) + else { + return; + }; + let Some(symbol) = ctx.semantic_model().symbol(symbol_id) else { + return; + }; + + if ctx.semantic_model().has_resolved_relation_read(symbol_id) { + return; + } + + diagnostics.push(LintDiagnostic::new( + self.name(), + format!("relation `{}` is declared but never read from", symbol.name()), + declaration_range, + )); + } + } +} + +#[cfg(test)] +mod tests { + use rstest::rstest; + + // Import the shared test helper from the tests/support module. + // This ensures unit and behavioral tests use identical rule-running logic. + fn run_rule(source: &str) -> Vec { + // We can't directly use the tests/support module from src/ unit tests, + // so we duplicate the minimal logic here but keep it aligned. + let parsed = crate::parse(source); + assert!( + parsed.errors().is_empty(), + "unused-relation test source should parse cleanly: {:?}", + parsed.errors() + ); + let mut store = crate::linter::CstRuleStore::new(); + store.register(Box::new(super::UnusedRelationRule)); + crate::linter::Runner::new(&store, source, &parsed, crate::linter::RuleConfig::new()).run() + } + + #[rstest] + fn warns_for_declared_relation_with_no_reads() { + let diagnostics = run_rule(concat!( + "input relation Source(x: u32)\n", + "relation Sink(x: u32)\n", + "Sink(x) :- Source(x).\n", + )); + + assert_eq!(diagnostics.len(), 1); + assert_eq!( + diagnostics + .first() + .map(crate::linter::LintDiagnostic::rule_name), + Some("unused-relation") + ); + assert_eq!( + diagnostics + .first() + .map(crate::linter::LintDiagnostic::message), + Some("relation `Sink` is declared but never read from") + ); + } + + #[rstest] + fn does_not_warn_for_relation_with_resolved_read() { + let diagnostics = run_rule(concat!( + "input relation Source(x: u32)\n", + "relation Used(x: u32)\n", + "relation Sink(x: u32)\n", + "Used(x) :- Source(x).\n", + "Sink(x) :- Used(x).\n", + )); + + assert!( + diagnostics.iter().all(|diagnostic| diagnostic.message() + != "relation `Used` is declared but never read from"), + "Used should not be reported once it is read from a rule body", + ); + } + + #[rstest] + fn ignores_unresolved_relation_uses_when_checking_reads() { + let diagnostics = run_rule(concat!( + "relation Declared(x: u32)\n", + "relation Sink(x: u32)\n", + "Sink(x) :- Missing(x).\n", + )); + + let messages: Vec<_> = diagnostics + .iter() + .map(crate::linter::LintDiagnostic::message) + .collect(); + assert!(messages.contains(&"relation `Declared` is declared but never read from")); + } +} diff --git a/src/linter/rules/mod.rs b/src/linter/rules/mod.rs new file mode 100644 index 00000000..ce86c638 --- /dev/null +++ b/src/linter/rules/mod.rs @@ -0,0 +1,3 @@ +//! Production lint rules shipped by the linter. + +pub mod correctness; diff --git a/src/sema/builder.rs b/src/sema/builder.rs index 2e680d55..b2f6dd7c 100644 --- a/src/sema/builder.rs +++ b/src/sema/builder.rs @@ -1,6 +1,6 @@ //! Internal semantic-model builder state and high-level collection passes. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use rowan::SyntaxNode; @@ -10,8 +10,8 @@ use crate::parser::ast; use crate::parser::ast::SemanticRule; use crate::parser::ast::rule::text_range_to_span; use crate::sema::model::{ - DeclarationKind, Scope, ScopeId, ScopeKind, ScopeOrigin, SemanticModel, Symbol, SymbolId, - SymbolOrigin, + DeclarationKind, Resolution, Scope, ScopeId, ScopeKind, ScopeOrigin, SemanticModel, Symbol, + SymbolId, SymbolOrigin, UseKind, }; use super::resolve::collect_pattern_binding_names; @@ -58,12 +58,38 @@ impl SemanticModelBuilder { } } + fn is_relation_read_use(use_site: &crate::sema::UseSite) -> bool { + use_site.kind() == UseKind::Relation && use_site.origin().is_relation_read() + } + pub(crate) fn finish(self) -> SemanticModel { + // Precompute span-to-relation-symbol index + let span_to_relation_symbol: HashMap = self + .symbols + .iter() + .enumerate() + .filter(|(_, symbol)| symbol.kind() == DeclarationKind::Relation) + .map(|(index, symbol)| (symbol.span().clone(), SymbolId(index))) + .collect(); + + // Precompute symbols-with-reads set + let symbols_with_reads: HashSet = self + .uses + .iter() + .filter(|u| Self::is_relation_read_use(u)) + .filter_map(|u| match u.resolution() { + Resolution::Resolved(symbol_id) => Some(symbol_id), + _ => None, + }) + .collect(); + SemanticModel { program_scope: self.program_scope, scopes: self.scopes, symbols: self.symbols, uses: self.uses, + span_to_relation_symbol, + symbols_with_reads, } } diff --git a/src/sema/mod.rs b/src/sema/mod.rs index fcae8467..42dcd104 100644 --- a/src/sema/mod.rs +++ b/src/sema/mod.rs @@ -14,7 +14,7 @@ mod variables; pub use build::{build, build_from_parts, build_from_root}; pub use model::{ DeclarationKind, Resolution, Scope, ScopeId, ScopeKind, ScopeOrigin, SemanticModel, Symbol, - SymbolId, SymbolOrigin, UseKind, UseSite, + SymbolId, SymbolOrigin, UseKind, UseOrigin, UseSite, }; #[cfg(test)] diff --git a/src/sema/model.rs b/src/sema/model.rs index 2bab6291..e37f1596 100644 --- a/src/sema/model.rs +++ b/src/sema/model.rs @@ -3,6 +3,8 @@ //! The semantic model stores only owned data and opaque identifiers so it can //! be shared safely across linter worker threads. +use std::collections::{HashMap, HashSet}; + use crate::Span; use crate::parser::ast::SemanticRuleOrigin; @@ -82,6 +84,32 @@ pub enum UseKind { Variable, } +/// Provenance for one recorded use site. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UseOrigin { + /// Relation use from a rule head, which writes to the relation. + RelationHead, + /// Relation use from a normal rule-body atom or semantic-rule body atom. + RelationBody, + /// Relation use from a `for` iterable expression. + ForIterable, + /// Relation use from a `for` guard expression. + ForGuard, + /// Variable use recorded while traversing expressions. + Variable, +} + +impl UseOrigin { + /// Return `true` when this origin is a read-like relation position. + #[must_use] + pub fn is_relation_read(self) -> bool { + matches!( + self, + Self::RelationBody | Self::ForIterable | Self::ForGuard + ) + } +} + /// Final name-resolution result for one use site. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Resolution { @@ -189,6 +217,7 @@ impl Symbol { pub struct UseSite { pub(crate) name: String, pub(crate) kind: UseKind, + pub(crate) origin: UseOrigin, pub(crate) scope: ScopeId, pub(crate) span: Span, pub(crate) source_order: usize, @@ -208,6 +237,12 @@ impl UseSite { self.kind } + /// Provenance for this use site. + #[must_use] + pub fn origin(&self) -> UseOrigin { + self.origin + } + /// Scope in which the use occurred. #[must_use] pub fn scope(&self) -> ScopeId { @@ -240,6 +275,10 @@ pub struct SemanticModel { pub(crate) scopes: Vec, pub(crate) symbols: Vec, pub(crate) uses: Vec, + /// Precomputed index mapping relation declaration spans to symbol IDs. + pub(crate) span_to_relation_symbol: HashMap, + /// Precomputed set of symbol IDs that have at least one resolved read-like use. + pub(crate) symbols_with_reads: HashSet, } impl SemanticModel { @@ -261,6 +300,15 @@ impl SemanticModel { &self.symbols } + /// Return every recorded relation declaration together with its symbol id. + pub fn relation_symbols(&self) -> impl Iterator + '_ { + self.symbols + .iter() + .enumerate() + .filter(|(_, symbol)| symbol.kind() == DeclarationKind::Relation) + .map(|(index, symbol)| (SymbolId(index), symbol)) + } + /// Return every recorded use site in stable build order. #[must_use] pub fn uses(&self) -> &[UseSite] { @@ -279,6 +327,18 @@ impl SemanticModel { self.symbols.get(id.0) } + /// Return the relation symbol declared at the given span, if any. + #[must_use] + pub fn relation_symbol_at_span(&self, span: &Span) -> Option { + self.span_to_relation_symbol.get(span).copied() + } + + /// Return `true` when the relation has at least one resolved read-like use. + #[must_use] + pub fn has_resolved_relation_read(&self, symbol_id: SymbolId) -> bool { + self.symbols_with_reads.contains(&symbol_id) + } + /// Return the resolved symbol for a use site when resolution succeeded. #[must_use] pub fn resolved_symbol(&self, use_site: &UseSite) -> Option<&Symbol> { diff --git a/src/sema/tests.rs b/src/sema/tests.rs index 3a672dcf..e8f10e49 100644 --- a/src/sema/tests.rs +++ b/src/sema/tests.rs @@ -4,7 +4,8 @@ use crate::parse; use rstest::{fixture, rstest}; use super::{ - DeclarationKind, Resolution, ScopeKind, SymbolOrigin, UseKind, build, build_from_root, + DeclarationKind, Resolution, ScopeKind, SymbolOrigin, UseKind, UseOrigin, build, + build_from_root, }; fn parse_ok(source: &str) -> crate::Parsed { @@ -58,6 +59,12 @@ fn symbols_named<'a>( ) } +fn relation_symbol_id(model: &super::SemanticModel, name: &str) -> Option { + model + .relation_symbols() + .find_map(|(symbol_id, symbol)| (symbol.name() == name).then_some(symbol_id)) +} + #[fixture] fn parsed_case(#[default("")] source: &str) -> crate::Parsed { parse_ok(source) @@ -122,6 +129,83 @@ fn head_bindings_are_visible_from_rule_start( ); } +#[rstest] +fn relation_use_origins_distinguish_heads_from_reads( + #[with(concat!( + "Sink(x) :- ", + "Source(x), ", + "for (y in Items(x)) Check(y), ", + "for (z in Items(x) if Check(z)) Check(z)." + ))] + semantic_model: super::SemanticModel, +) { + let sink_uses = uses_named(&semantic_model, "Sink", UseKind::Relation); + let source_uses = uses_named(&semantic_model, "Source", UseKind::Relation); + let items_uses = uses_named(&semantic_model, "Items", UseKind::Relation); + let check_uses = uses_named(&semantic_model, "Check", UseKind::Relation); + + let sink_origins: Vec<_> = sink_uses.iter().map(|use_site| use_site.origin()).collect(); + let source_origins: Vec<_> = source_uses + .iter() + .map(|use_site| use_site.origin()) + .collect(); + let items_origins: Vec<_> = items_uses + .iter() + .map(|use_site| use_site.origin()) + .collect(); + let check_origins: Vec<_> = check_uses + .iter() + .map(|use_site| use_site.origin()) + .collect(); + + // `Sink` only appears in rule heads. + assert!(!sink_origins.is_empty(), "expected at least one `Sink` use"); + assert!( + sink_origins + .iter() + .all(|origin| matches!(origin, UseOrigin::RelationHead)), + "expected all `Sink` uses to be rule heads, got: {sink_origins:?}", + ); + + // `Source` only appears as a body read. + assert!( + !source_origins.is_empty(), + "expected at least one `Source` use" + ); + assert!( + source_origins + .iter() + .all(|origin| matches!(origin, UseOrigin::RelationBody)), + "expected all `Source` uses to be body reads, got: {source_origins:?}", + ); + + // `Items` only appears as the iterable of a `for`. + assert!( + !items_origins.is_empty(), + "expected at least one `Items` use" + ); + assert!( + items_origins + .iter() + .all(|origin| matches!(origin, UseOrigin::ForIterable)), + "expected all `Items` uses to be `for` iterables, got: {items_origins:?}", + ); + + // `Check` appears both in the `for` body and in the `for` guard. + assert!( + check_origins + .iter() + .any(|origin| matches!(origin, UseOrigin::RelationBody)), + "expected at least one body use for `Check`, got: {check_origins:?}", + ); + assert!( + check_origins + .iter() + .any(|origin| matches!(origin, UseOrigin::ForGuard)), + "expected at least one guard use for `Check`, got: {check_origins:?}", + ); +} + #[rstest] #[expect( clippy::expect_used, @@ -197,6 +281,8 @@ fn for_loop_bindings_do_not_escape_loop_scope( fn top_level_for_semantic_rules_participate_in_analysis( #[with("for (x in Source(x)) Output(0).")] semantic_model: super::SemanticModel, ) { + let output_uses = uses_named(&semantic_model, "Output", UseKind::Relation); + let source_uses = uses_named(&semantic_model, "Source", UseKind::Relation); let x_uses = uses_named(&semantic_model, "x", UseKind::Variable); assert!( @@ -207,9 +293,19 @@ fn top_level_for_semantic_rules_participate_in_analysis( "semantic rule should produce a rule scope", ); assert!( - !uses_named(&semantic_model, "Output", UseKind::Relation).is_empty(), + !output_uses.is_empty(), "semantic rule head should record relation use", ); + assert_eq!( + output_uses.first().map(|use_site| use_site.origin()), + Some(UseOrigin::RelationHead) + ); + assert_eq!( + source_uses + .first() + .map(|use_site| use_site.origin().is_relation_read()), + Some(true) + ); assert!( x_uses .iter() @@ -253,3 +349,41 @@ fn relation_use_prefers_relation_over_function_with_same_name( assert_eq!(resolved_symbol, foo_relation); } } + +#[rstest] +#[expect( + clippy::expect_used, + reason = "tests should fail with concise lookup messages" +)] +fn relation_read_helpers_ignore_head_only_and_unresolved_uses( + #[with(concat!( + "input relation Source(x: u32)\n", + "relation Sink(x: u32)\n", + "relation HeadOnly(x: u32)\n", + "relation NeverRead(x: u32)\n", + "Sink(x) :- Source(x), Missing(x).\n", + "HeadOnly(x) :- Source(x).\n", + ))] + semantic_model: super::SemanticModel, +) { + let source_id = relation_symbol_id(&semantic_model, "Source").expect("missing Source"); + let sink_id = relation_symbol_id(&semantic_model, "Sink").expect("missing Sink"); + let head_only_id = relation_symbol_id(&semantic_model, "HeadOnly").expect("missing HeadOnly"); + let never_read_id = + relation_symbol_id(&semantic_model, "NeverRead").expect("missing NeverRead"); + + assert!(semantic_model.has_resolved_relation_read(source_id)); + assert!(!semantic_model.has_resolved_relation_read(sink_id)); + assert!(!semantic_model.has_resolved_relation_read(head_only_id)); + assert!(!semantic_model.has_resolved_relation_read(never_read_id)); + + let source_span = semantic_model + .symbol(source_id) + .expect("missing Source symbol") + .span() + .clone(); + assert_eq!( + semantic_model.relation_symbol_at_span(&source_span), + Some(source_id) + ); +} diff --git a/src/sema/traverse.rs b/src/sema/traverse.rs index 3f961529..dc93dbce 100644 --- a/src/sema/traverse.rs +++ b/src/sema/traverse.rs @@ -4,7 +4,7 @@ use crate::Span; use crate::parser::ast; use crate::parser::ast::{Expr, RuleBodyTerm}; use crate::sema::model::{ - DeclarationKind, ScopeId, ScopeKind, ScopeOrigin, SymbolOrigin, UseKind, UseSite, + DeclarationKind, ScopeId, ScopeKind, ScopeOrigin, SymbolOrigin, UseKind, UseOrigin, UseSite, }; use super::builder::{ScopeSpec, SemanticModelBuilder, SymbolSpec}; @@ -36,7 +36,7 @@ impl SemanticModelBuilder { pub(crate) fn collect_head_expr(&mut self, expr: &Expr, ctx: RuleHeadContext<'_>) { self.record_top_level_relation_use( VariableUseContext::new(ctx.scope, 0, ctx.span, 0), - UseKind::Relation, + UseOrigin::RelationHead, expr, ); for binding_name in collect_head_binding_names(expr) { @@ -68,7 +68,7 @@ impl SemanticModelBuilder { } pub(crate) fn collect_expression_term(&mut self, expr: &Expr, context: VariableUseContext<'_>) { - self.record_top_level_relation_use(context, UseKind::Relation, expr); + self.record_top_level_relation_use(context, UseOrigin::RelationBody, expr); self.walk_variable_uses(expr, context); } @@ -105,10 +105,10 @@ impl SemanticModelBuilder { for_loop: &ast::RuleForLoop, context: VariableUseContext<'_>, ) { - self.record_top_level_relation_use(context, UseKind::Relation, &for_loop.iterable); + self.record_top_level_relation_use(context, UseOrigin::ForIterable, &for_loop.iterable); self.walk_variable_uses(&for_loop.iterable, context); if let Some(guard) = for_loop.guard.as_ref() { - self.record_top_level_relation_use(context, UseKind::Relation, guard); + self.record_top_level_relation_use(context, UseOrigin::ForGuard, guard); self.walk_variable_uses(guard, context); } @@ -148,9 +148,10 @@ impl SemanticModelBuilder { fn record_top_level_relation_use( &mut self, context: VariableUseContext<'_>, - use_kind: UseKind, + origin: UseOrigin, expr: &Expr, ) { + let use_kind = UseKind::Relation; let Some(name) = relation_name(expr) else { return; }; @@ -158,6 +159,7 @@ impl SemanticModelBuilder { self.uses.push(UseSite { name: name.to_string(), kind: use_kind, + origin, scope: context.current_scope(), span: context.span().clone(), source_order: context.literal_index(), diff --git a/src/sema/variables.rs b/src/sema/variables.rs index 6ea2ed6a..47843080 100644 --- a/src/sema/variables.rs +++ b/src/sema/variables.rs @@ -2,7 +2,7 @@ use crate::Span; use crate::parser::ast::Expr; -use crate::sema::model::{ScopeId, UseKind, UseSite}; +use crate::sema::model::{ScopeId, UseKind, UseOrigin, UseSite}; use super::builder::SemanticModelBuilder; @@ -202,6 +202,7 @@ impl SemanticModelBuilder { self.uses.push(UseSite { name: name.to_string(), kind: UseKind::Variable, + origin: UseOrigin::Variable, scope: context.current_scope(), span: context.span().clone(), source_order: context.literal_index(), diff --git a/tests/semantic_scope_resolution.rs b/tests/semantic_scope_resolution.rs index a9f0597a..9cb57988 100644 --- a/tests/semantic_scope_resolution.rs +++ b/tests/semantic_scope_resolution.rs @@ -1,7 +1,7 @@ //! Behavioural tests for semantic symbol tables and scope resolution. use ddlint::linter::{CstRule, CstRuleStore, LintDiagnostic, Rule, RuleConfig, Runner}; -use ddlint::sema::{self, DeclarationKind, Resolution, UseKind}; +use ddlint::sema::{self, DeclarationKind, Resolution, UseKind, UseOrigin}; use ddlint::{SyntaxKind, parse}; use rstest::{fixture, rstest}; @@ -50,6 +50,10 @@ fn semantic_model_records_declarations_and_resolved_uses_end_to_end( assert_eq!(source_declarations.len(), 1); assert_eq!(source_uses.len(), 1); + assert_eq!( + source_uses.first().map(|use_site| use_site.origin()), + Some(UseOrigin::RelationBody) + ); assert!(matches!( source_uses.first().map(|use_site| use_site.resolution()), Some(Resolution::Resolved(_)) @@ -62,6 +66,35 @@ fn semantic_model_records_declarations_and_resolved_uses_end_to_end( ); } +#[rstest] +fn semantic_model_keeps_relation_reads_distinct_from_head_writes( + #[with("for (x in Source(x)) Output(x).\nHeadOnly(x) :- Source(x).")] + semantic_model: ddlint::sema::SemanticModel, +) { + let output_uses = uses_named(&semantic_model, "Output", UseKind::Relation); + let head_only_uses = uses_named(&semantic_model, "HeadOnly", UseKind::Relation); + let source_uses = uses_named(&semantic_model, "Source", UseKind::Relation); + + assert_eq!( + output_uses.first().map(|use_site| use_site.origin()), + Some(UseOrigin::RelationHead) + ); + assert_eq!( + head_only_uses.first().map(|use_site| use_site.origin()), + Some(UseOrigin::RelationHead) + ); + assert!( + !source_uses.is_empty(), + "expected at least one Source use recorded", + ); + assert!( + source_uses + .iter() + .all(|use_site| use_site.origin().is_relation_read()), + "Source should only appear in read-like relation positions", + ); +} + #[rstest] fn semantic_model_keeps_unresolved_names_without_crashing( #[with("Output(x) :- Source(x), Missing(y).")] semantic_model: ddlint::sema::SemanticModel, diff --git a/tests/support.rs b/tests/support.rs new file mode 100644 index 00000000..d0f38a5d --- /dev/null +++ b/tests/support.rs @@ -0,0 +1,24 @@ +//! Shared test utilities for behavioural tests. + +use ddlint::linter::rules::correctness::UnusedRelationRule; +use ddlint::linter::{CstRuleStore, RuleConfig, Runner}; +use ddlint::parse; + +/// Run the `unused-relation` lint rule on the given source code. +/// +/// # Panics +/// +/// Panics if the source code fails to parse cleanly. +#[must_use] +pub fn run_unused_relation_rule(source: &str) -> Vec { + let parsed = parse(source); + assert!( + parsed.errors().is_empty(), + "unused-relation test source should parse cleanly: {:?}", + parsed.errors() + ); + + let mut store = CstRuleStore::new(); + store.register(Box::new(UnusedRelationRule)); + Runner::new(&store, source, &parsed, RuleConfig::new()).run() +} diff --git a/tests/unused_relation_rule.rs b/tests/unused_relation_rule.rs new file mode 100644 index 00000000..52eb4a2b --- /dev/null +++ b/tests/unused_relation_rule.rs @@ -0,0 +1,99 @@ +//! Behavioural tests for the shipped `unused-relation` lint rule. + +use rstest::rstest; + +mod support; +use support::run_unused_relation_rule; + +fn run_rule(source: &str) -> Vec { + run_unused_relation_rule(source) +} + +#[rstest] +#[case( + concat!( + "input relation Source(x: u32)\n", + "relation Sink(x: u32)\n", + "Sink(x) :- Source(x).\n", + ), + vec!["relation `Sink` is declared but never read from"], +)] +#[case( + concat!( + "input relation Source(x: u32)\n", + "relation Used(x: u32)\n", + "relation Sink(x: u32)\n", + "Used(x) :- Source(x).\n", + "Sink(x) :- Used(x).\n", + ), + vec!["relation `Sink` is declared but never read from"], +)] +#[case( + concat!( + "input relation Source(x: u32)\n", + "relation HeadOnly(x: u32)\n", + "HeadOnly(x) :- Source(x).\n", + ), + vec!["relation `HeadOnly` is declared but never read from"], +)] +#[case( + concat!( + "relation Declared(x: u32)\n", + "relation Sink(x: u32)\n", + "Sink(x) :- Missing(x).\n", + ), + vec![ + "relation `Declared` is declared but never read from", + "relation `Sink` is declared but never read from", + ], +)] +#[case( + concat!( + "relation Zebra(x: u32)\n", + "relation Alpha(x: u32)\n", + "relation Middle(x: u32)\n", + ), + vec![ + "relation `Zebra` is declared but never read from", + "relation `Alpha` is declared but never read from", + "relation `Middle` is declared but never read from", + ], +)] +#[case( + concat!( + "input relation Source(x: u32)\n", + "relation ForRead(x: u32)\n", + "relation Sink(x: u32)\n", + "ForRead(x) :- Source(x).\n", + "Sink(x) :- for (y in ForRead(x)) Inner(y).\n", + ), + vec!["relation `Sink` is declared but never read from"], +)] +#[case( + concat!( + "input relation Source(x: u32)\n", + "relation GuardRead(x: u32)\n", + "relation Sink(x: u32)\n", + "GuardRead(x) :- Source(x).\n", + "Sink(x) :- for (y in Source(x) if GuardRead(y)) Inner(y).\n", + ), + vec!["relation `Sink` is declared but never read from"], +)] +fn unused_relation_rule_matches_expected_messages( + #[case] source: &str, + #[case] expected_messages: Vec<&str>, +) { + let diagnostics = run_rule(source); + let actual_messages: Vec<_> = diagnostics + .iter() + .map(ddlint::linter::LintDiagnostic::message) + .collect(); + + assert_eq!(actual_messages, expected_messages); + assert!( + diagnostics + .iter() + .all(|diagnostic| diagnostic.rule_name() == "unused-relation"), + "all diagnostics should come from the shipped rule", + ); +}