Implement polymorphic dispatch error handling and add tests for derived member resolution#77
Open
koenbeuk wants to merge 1 commit into
Open
Implement polymorphic dispatch error handling and add tests for derived member resolution#77koenbeuk wants to merge 1 commit into
koenbeuk wants to merge 1 commit into
Conversation
…ed member resolution
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the runtime experience around polymorphic dispatch of virtual/override [Expressive] members by surfacing a more actionable exception when an override’s expression cannot be resolved, and it adds coverage to validate both the new error message and resolver-isolation expectations.
Changes:
- Add a dedicated polymorphic-dispatch probe (
TryGetReflectedExpressionForDispatch) that wraps resolver failures with remediation guidance. - Add a test ensuring a derived-override resolution failure produces an actionable
InvalidOperationExceptionwith an inner exception. - Add a test asserting polymorphic plan caching does not leak derived-override discovery across different resolvers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs | Adds tests for actionable polymorphic-dispatch failure messaging and for resolver-isolated polymorphic plan behavior. |
| src/ExpressiveSharp/Services/ExpressiveReplacer.cs | Introduces a dispatch-specific resolver probe that wraps failures with a more actionable exception message. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
359
to
363
| private PolymorphicPlan BuildPolymorphicPlan(Type rootType, MemberInfo baseMember) | ||
| { | ||
| var rootMember = ResolveConcreteMember(rootType, baseMember) ?? baseMember; | ||
| var rootRegistered = TryGetReflectedExpressionSafe(rootMember, out _); | ||
| var rootRegistered = TryGetReflectedExpressionForDispatch(rootMember, baseMember, out _); | ||
|
|
Comment on lines
+474
to
+475
| $"Polymorphic dispatch for '{baseMember.DeclaringType}.{baseMember.Name}' requires the generated " + | ||
| $"expression for [Expressive] member '{memberInfo.DeclaringType}.{memberInfo.Name}', which could not " + |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
small QOL improvement to show a better exception message when polymorphic dispatch fails due to missing generated expressions on derived members.