AlwaysListMatcher::match_value always returns false, contradicting its own doc comment

Open Beginner friendly
#190 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
1/5
Estimated time
1-3 hours
Newbie friendliness
90/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
rust
Domain
backend

Research direction

Open engine/src/list_matcher.rs and inspect AlwaysListMatcher alongside NeverList; the issue identifies the incorrect behavior and the expected result. Run the relevant Rust tests, then add a regression case for the provided scheme and filter example, verifying that the AlwaysList match succeeds without breaking existing matcher or visitor tests.

Written by the indexing model from the issue text.

Description

`AlwaysList` is documented as "List that always matches." (`engine/src/list_matcher.rs:80`). Its
matcher, however, is byte-identical to `NeverList`'s — "List that never matches."
(`engine/src/list_matcher.rs:111`):

```rust
// engine/src/list_matcher.rs:103-109
impl ListMatcher for AlwaysListMatcher {
fn match_value(&self, _: &str, : &LhsValue<'>) -> bool {
false
}
fn clear(&mut self) {}
}
```

Per the doc comment this should return `true`.

Reachability: any scheme built with `builder.add_list(ty, AlwaysList {})` (Rust) or
`wirefilter_add_always_list_to_scheme` (`ffi/src/lib.rs:303-306`, the FFI equivalent), then a filter
using `in $listname` against a field of that type.

Impact: a field typed against `AlwaysList` never matches — the opposite of its documented
behavior. No crash, no error, no signal anything is wrong.

Minimal repro:
```rust
let mut builder = SchemeBuilder::new();
builder.add_field("num", Type::Int).unwrap();
builder.add_list(Type::Int, AlwaysList {}).unwrap();
let scheme = builder.build();

let ast = scheme.parse("num in $numbers").unwrap();
let filter = ast.compile();
let mut ctx = ExecutionContext::new(&scheme);
ctx.set_field_value_from_name("num", 42).unwrap();
assert_eq!(filter.execute(&ctx).unwrap(), true); // fails: actually false
```

Suggested fix: change the return value from `false` to `true`. Verified this doesn't break either
existing consumer — `list_matcher.rs`'s own test compares matcher identity, not match results, and
`ast/visitor.rs`'s tests only use `AlwaysList` to make a list name resolvable, never assert on match
outcomes. Happy to open this as a one-line PR with a regression test instead of an issue, if preferred.

Found with the rust-in-peace pipeline.

Dominant language
Rust
Stars
1.2k
Forks
123
Avg merge
5h 20m
Merged PRs (30d)
4

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cloudflare/wirefilter

All issues in cloudflare/wirefilter

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.