You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fix stackoverflow on nullable readonly existing target
Description
This PR fixes the stack overflow exception for nullable, readonly existing target mappings.
I’d also like to get your thoughts on an idea about the tests: These tests practically test the same output on different cases. Is it enough to generate a snapshot only for a single one, and just assert for the same output on the rest? I think there are too much snapsots and hard to manage. This approach can limit it.
Another thought about the bug report itself: when one of the parameters was the Task, the generator explored it deeply, which is a waste of resources. It doesn't fix the root problem, but somehow this should be limited. Since limiting this type or that couldn't really help. My idea is to limit exploring the read-only targets themselves. So if the target is read-only, then it is explored only if it is explicitly asked in some manner. WDYT?
I’d also like to get your thoughts on an idea about the tests: These tests practically test the same output on different cases. Is it enough to generate a snapshot only for a single one, and just assert for the same output on the rest? I think there are too much snapsots and hard to manage. This approach can limit it.
IMO we should decide on a case by case basis whether a snapshot test makes sense. In this case, since it is only about diagnostics, I prefer the fluent TestHelper assertions. They are more focused and make these tests easier to maintain. However, whenever asserting generated mapping code, I think it is simpler to use snapshot tests.
Another thought about the bug report itself: when one of the parameters was the Task, the generator explored it deeply, which is a waste of resources. It doesn't fix the root problem, but somehow this should be limited. Since limiting this type or that couldn't really help. My idea is to limit exploring the read-only targets themselves. So if the target is read-only, then it is explored only if it is explicitly asked in some manner. WDYT?
Yeah I think it would make sense to only try to map direct properties of the root but not explore further. Does that make sense?
latonz
changed the title
Fix stackoverflow on nullable readonly existing target
fix: stackoverflow on nullable readonly existing target
Dec 26, 2025
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
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.
latonz
changed the title
Fix stackoverflow on nullable readonly existing target
Description
This PR fixes the stack overflow exception for nullable, readonly existing target mappings.
I’d also like to get your thoughts on an idea about the tests: These tests practically test the same output on different cases. Is it enough to generate a snapshot only for a single one, and just assert for the same output on the rest? I think there are too much snapsots and hard to manage. This approach can limit it.
Another thought about the bug report itself: when one of the parameters was the Task, the generator explored it deeply, which is a waste of resources. It doesn't fix the root problem, but somehow this should be limited. Since limiting this type or that couldn't really help. My idea is to limit exploring the read-only targets themselves. So if the target is read-only, then it is explored only if it is explicitly asked in some manner. WDYT?
Fixes #2078
Checklist