Unitialized analyzer#938
Open
JoschkaBC wants to merge 6 commits into
Open
Conversation
Owner
|
Many thanks! I just merge mine, but I'll take a look at this one too to see if there's any difference in functionality. The fixers will be very handy! |
Contributor
|
The very important main difference is, that your version does not cover assignments in constructors and ignores fields all together. Thats the main complexity in my implementation. (2 different accounts, because the work was mainly done during working hours). Not sure about all the edge cases. |
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.
fixex #934
closes #936
Adds a new analyzer
VOG038that tries to detect the pattern described in ##934.Also adds two code fixes to make these properties either nullable or required.
Concerns
Use cases in which uninitialized members are expected
In some cases it would be inconvenient to always suppress this warning, because the member is set dynamically and some point in time. Like in an EFCore Interceptor.
The analyzer warning is correct in those cases, but one may want to suppress the warning easily across their code base.
For this
[MayBeUninitialized]was added. This attribute suppresses the analyzer for a given member.It will also flow down to interface implementations or overwrites.
Example
``` c# interface IAuditable { [MayBeUninitialized] DateModifiedVo DateModified {get;} } public DbEntity : IAuditable { // This is set by an EFCore Interceptor and should not be modified or supplied by the user // It will be unitialized between the instantiation and a SaveChanges call // But the warning will be suppressed by thanks to the attribute on the interface public DateModifiedVo DateModified { get; private init; } } ```
Complex constructor assignments
Sometimes a constructor sets values through helper methods.
And we cannot detect this, because traversing down the entire call tree would be very complex, error prone and expensive.
.NET also recognizes this as a problem with
requiredmembers and introduces the constructor attribute[SetsRequiredMembers].This PR adds an equivalent
[SetsUninitializedMembers]which will mark a constructor as "this constructor always initialized all value objects".Yielding the full responsibility back to the user.
AI Summary of the changes
1. New Analyzer:
DoNotUseUninitializedMembersAnalyzer(VOG038)required.2. New Attributes for Control
[MayBeUninitialized]: Can be applied to a property or field to explicitly suppress the VOG038 warning for that member.[SetsUninitializedMembers]: Can be applied to a constructor to indicate that it handles the initialization of all members, similar to how[SetsRequiredMembers]works in C#.3. Code Fixers
Provided two automated fixes for the VOG038 diagnostic:
requiredmodifier to the property or field.MyVotoMyVo?).