Simplify resolution of dependency injection candidates#5548
Merged
peppy merged 8 commits intoppy:masterfrom Nov 29, 2022
Merged
Simplify resolution of dependency injection candidates#5548peppy merged 8 commits intoppy:masterfrom
peppy merged 8 commits intoppy:masterfrom
Conversation
This method internally checks for a `null` value and either throws an exception or allows it to be cached for value types.
Contributor
Author
|
Will need a source generator nupkg update with the changes in this PR, for this PR >_> |
1 task
peppy
approved these changes
Nov 29, 2022
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.
A common theme I observed is that finding candidate classes for dependency injection has considerable overheads, both in terms of correctness and likely also in performance (since more symbols have to be analysed otherwise).
The latter is the reason why the source generator didn't do as aggressive of a job as the analyser in generating the dependency injection classes, but this resulted in a disparity between the two that couldn't exist. Either both of them should miss inspections, or both of them should not.
This PR takes the second route of not missing inspections (greatest correctness) by requiring dependency injection classes to implement the
IDependencyInjectionCandidateinterface. For the most part this is a negligible overhead - the only such class outside of tests that I'm aware of is the osu!-sideRoom.As a result, not only has the analyser been simplified to a quarter of its original size, but the SG perfectly follows the inspections provided by the analyser. The future incremental source generator implementation will also benefit from this.
The downside is that this is a breaking change, but as I mentioned above it's not a big one.
vNext
Classes used in dependency injection need to implement the
IDependencyInjectionCandidateinterfaceThis does not affect
Drawablesubclasses.