Repository navigation
Conversation
|
Thanks! I'm just thinking about this a bit. The issue title sort of buries the fact that there's an intended ripple effect, so that
Tensions I'm mulling over
|
… match Make the structural matching of Error.Is and its ripple into List.Is explicit via doc comments, and pin the List structural-match case with a test, per review discussion.
|
Good catch on the ripple effect. I kept the structural List.Is behavior and made it explicit in d2e6bd9: doc comments on both Is methods spelling out the semantics, plus a List test covering the structural match case, so the change is deliberate and pinned by tests. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The tests do not yet protect the documented behavior that matching errors may have different wrapped causes.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR adds structural matching for gqlerror.Error under errors.Is, addressing #249.
Changes:
- Compare the error’s identifying fields while leaving wrapped causes to
Unwrap. - Add equality and list-matching tests.
| File | Description |
|---|---|
gqlerror/error.go |
Adds Error.Is and clarifies List.Is. |
gqlerror/error_test.go |
Tests structural matching and wrapped-error behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| { | ||
| name: "wrapped non gqlerror target", |

Summary
*gqlerror.Errornow implementsIs(error) bool, so two structurally-equal gqlerrors compare as equal undererrors.Is(andcmpopts.EquateErrors, which is built on it). Previouslyerrors.Isfell back to pointer identity, so two errors carrying identical fields but distinct instances never matched.Background
The gqlerror package already ships a
List.Ismethod, soError.Iscompletes that pattern; #249 requested it and a maintainer replied "Excellent idea! PR welcome!". The method declines non-*Errortargets (lettingerrors.Iskeep walking the receiver'sUnwrapchain) and, for a*Errortarget, compares the identifying fields the issue means by "the same fields":Message,Rule,Path,Locations, andExtensions. The private wrapped cause (Err) is intentionally left to the existingUnwrapchain rather than folded intoIs, and the comparison is against the direct target only (not the target's unwrap chain), matching Go'serrors.Issemantics. A nil receiver is guarded.I have:
List.Is/List.Asstyle)Closes #249