test(parser): planeswalker inverted counter prohibition regression#3501
Conversation
Explicit regression for Planeswalkers can't have counters put on them. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds two new unit tests, inverted_counter_prohibition_planeswalkers and inverted_counter_prohibition_artifacts, to verify the parsing of counter prohibition replacement effects. Feedback indicates that these tests are redundant because their coverage is already thoroughly handled by the existing inverted_typed_counter_prohibition_covers_every_permanent_type test, so they should be removed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| #[test] | ||
| fn inverted_counter_prohibition_planeswalkers() { | ||
| let def = parse_replacement_line( | ||
| "Planeswalkers can't have counters put on them.", | ||
| "Test Card", | ||
| ) | ||
| .expect("planeswalker counter prohibition must parse"); | ||
| assert_eq!(def.event, ReplacementEvent::AddCounter); | ||
| assert_eq!( | ||
| def.quantity_modification, | ||
| Some(QuantityModification::Prevent) | ||
| ); | ||
| assert!(matches!( | ||
| def.valid_card, | ||
| Some(TargetFilter::Typed(tf)) | ||
| if tf.type_filters == vec![TypeFilter::Planeswalker] | ||
| )); | ||
| } | ||
|
|
||
| #[test] | ||
| fn inverted_counter_prohibition_artifacts() { | ||
| let def = parse_replacement_line("Artifacts can't have counters put on them.", "Test Card") | ||
| .expect("artifact counter prohibition must parse"); | ||
| assert_eq!(def.event, ReplacementEvent::AddCounter); | ||
| assert_eq!( | ||
| def.quantity_modification, | ||
| Some(QuantityModification::Prevent) | ||
| ); | ||
| assert!(matches!( | ||
| def.valid_card, | ||
| Some(TargetFilter::Typed(tf)) | ||
| if tf.type_filters == vec![TypeFilter::Artifact] | ||
| )); | ||
| } | ||
|
|
There was a problem hiding this comment.
[MEDIUM] Redundant test cases. Evidence: crates/engine/src/parser/oracle_replacement.rs:12349-12383. Why it matters: These tests duplicate coverage already provided by inverted_typed_counter_prohibition_covers_every_permanent_type (lines 11189-11223), which is even more thorough as it also asserts the InZone property and that the controller is None. Suggested fix: Remove the redundant test functions.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer review — ENQUEUE-READY (test PR)
Verdict: looks good. Both tests drive the real parser entry point and discriminate against live logic.
Test discrimination — PASS
inverted_counter_prohibition_planeswalkers and inverted_counter_prohibition_artifacts call parse_replacement_line("Planeswalkers can't have counters put on them.", ...) / "Artifacts ..." — the full parser entry — and assert:
event == ReplacementEvent::AddCounterquantity_modification == Some(Prevent)valid_cardisTyped(tf)withtf.type_filters == [Planeswalker]/[Artifact]
These pin the output of parse_inverted_typed_counter_prohibition (oracle_replacement.rs:5400), which is real, general logic — it uses separated_list1 over parse_counter_prohibition_type and explicitly admits Artifact/Creature/Enchantment/Land/Planeswalker/Battle. If the dispatch arm (:536) or that combinator were reverted, parse_replacement_line returns None, the .expect(...) panics, and both tests fail. Discriminating, not AST-shape-only.
Notes
- Good that both Planeswalker and Artifact are covered — exercises the building block across the type-list class rather than one subject. Confirms the inverted ("type is subject") surface form, distinct from the
Counters can't be put on Xform. Closes #3500. Closes #3453— #3453 is an issue, not an open PR; no duplicate-PR collision.
No production behavior change. Approving.
2 participants
Summary
inverted_counter_prohibition_planeswalkersregression testTest plan
cargo test -p engine --lib inverted_counter_prohibition_planeswalkersCloses #3500. Closes #3453.
Made with Cursor