Skip to content

Add facts and fact_filter to Piranha rules - #780

Merged
ketkarameya merged 5 commits into
uber:masterfrom
ketkarameya:feature/facts-and-fact-filter
Mar 12, 2026
Merged

Add facts and fact_filter to Piranha rules#780
ketkarameya merged 5 commits into
uber:masterfrom
ketkarameya:feature/facts-and-fact-filter

Conversation

@ketkarameya

Copy link
Copy Markdown
Collaborator

Introduces two new features:

Facts: Rules can record structured String->String metadata about matched nodes without modifying code. Facts are associated with the matched node's range, stored per-file in SourceCodeUnit, and included in PiranhaOutputSummary output. When a rewrite overlaps a fact's range the fact is marked voided; rewrites that precede a fact's range shift its byte offsets.

Fact filter: Rule filters gain a fact_filter field. A filter with a non-empty fact_filter only passes when at least one non-voided recorded fact in the file contains all the specified key-value pairs. This lets rewrite rules be gated on information recorded by earlier fact rules.

TOML usage:
[[rules]]
name = "record_api"
query = "..."
replace_node = "call"
fact = { type = "deprecated", method = "@name" }

[[rules.filters]]
fact_filter = { type = "deprecated" }

Also adds 11 unit tests and 2 integration tests covering: basic fact recording, @tag substitution, multi-match ordering, voiding by rewrite, byte-offset shifting, two facts on the same range, fact_filter blocking/allowing rewrites, and voided-fact semantics end-to-end.

@CLAassistant

CLAassistant commented Mar 11, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Introduces two new features:

**Facts**: Rules can record structured String->String metadata about matched
nodes without modifying code. Facts are associated with the matched node's range,
stored per-file in SourceCodeUnit, and included in PiranhaOutputSummary output.
When a rewrite overlaps a fact's range the fact is marked voided; rewrites that
precede a fact's range shift its byte offsets.

**Fact filter**: Rule filters gain a fact_filter field. A filter with a non-empty
fact_filter only passes when at least one non-voided recorded fact in the file
contains all the specified key-value pairs. This lets rewrite rules be gated on
information recorded by earlier fact rules.

TOML usage:
  [[rules]]
  name = "record_api"
  query = "..."
  replace_node = "call"
  fact = { type = "deprecated", method = "@name" }

  [[rules.filters]]
  fact_filter = { type = "deprecated" }

Also adds 11 unit tests and 2 integration tests covering: basic fact recording,
@tag substitution, multi-match ordering, voiding by rewrite, byte-offset shifting,
two facts on the same range, fact_filter blocking/allowing rewrites, and
voided-fact semantics end-to-end.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@ketkarameya
ketkarameya force-pushed the feature/facts-and-fact-filter branch from 614ceae to 5947b61 Compare March 12, 2026 06:52

/// Checks if a rule is a `fact` rule i.e. it records facts without rewriting code
pub(crate) fn is_fact_rule(&self) -> bool {
!self.fact().is_empty()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a user sets both fact and replace in TOML, this check fires first and the replace is silently ignored. should validate() reject rules that have both?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rules with both is weird. Do u mean, we want toads facts to the rewritten stuff ? In that case we can always have a self edge to add the fact

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im just thinking whether we should panic when we read a rule that;s supposed to generate facts, but then sets rewrite?

Comment thread crates/core/src/models/fact.rs Outdated

/// Shift byte offsets by `delta` after a preceding edit.
/// Point (row/col) offsets are not adjusted since we don't have source code available here.
pub(crate) fn shift_range(&mut self, delta: isize) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shifts byte offsets but leaves start_point / end_point stale. since Range exposes both, consumers could read wrong row/col from shifted facts, right?

- validate(): reject rules that set both `fact` and `replace` — they are
  mutually exclusive. Previously the fact branch silently won and `replace`
  was ignored; now a clear error is surfaced at rule construction time.

- shift_range(): was only updating start_byte/end_byte but left start_point
  and end_point stale, so consumers reading row/col from a shifted fact would
  get incorrect values. Now propagates row/col deltas from the InputEdit:
  facts on a later row shift by row_delta; facts on the same row as the edit
  end also shift their column.

Adds two new tests:
  - test_validate_rejects_fact_and_replace_together
  - test_fact_shift_updates_row_col_points

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

@danieltrt danieltrt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, cargo fmt is failing

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@ketkarameya
ketkarameya enabled auto-merge (squash) March 12, 2026 20:44
ketkarameya and others added 2 commits March 12, 2026 14:18
The hook ran `cargo clippy --fix --allow-staged` which refused to apply
fixes whenever any file had unstaged changes. Adding `--allow-dirty`
lets it proceed regardless of unstaged changes in other files.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@ketkarameya
ketkarameya merged commit 82f5d6a into uber:master Mar 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants