CSharpExtensions icon indicating copy to clipboard operation
CSharpExtensions copied to clipboard

InitRequired and InitRequiredForNotNull - Redeasign proposition

Open cezarypiatek opened this issue 2 years ago • 3 comments

I recently reviewed the current implementation and I found a few things that might be misleading, non-obvious, or behave in a slightly unexpected way.

[InitRequired]

Current behavior: Defined on the type level enforces mandatory initialization in the init blocks for non-nullable members (reference and value types. Defined on the member level enforces mandatory initialization in the init blocks whenever what member type is. There is no option to use it on the assembly level.

My expectations: Defined on the type level should enforce mandatory initialization in the init blocks for all members no matter what kind of nullability is defined. This will be a breaking change but it could reveal a potential issue with non-initialized members that were accidentally omitted.

Defined on the member level should behave in the same way as it is now. There should be an option to use it on the assembly level. Should discard CSE004: InitOnlyOptional requires default value

[InitRequiredForNotNull]

Current behavior: Can be defined only on the assembly level. Enforces mandatory init via init block for all non-nullable (references and value type) members defined in that assembly. Discards all occurrences of CSE004: InitOnlyOptional requires default value

My expectations: There should be an option to use it on the type as well as the assembly level. Should discard of CSE004: based on the application level (for every type in the assembly or only for the decorated one).


Those are my notes for further reference but everybody is welcome to give here their own feedback on this subject.

cezarypiatek avatar Sep 30 '21 17:09 cezarypiatek

I support this suggestion. Consistency is important

jamshally avatar Apr 28 '22 17:04 jamshally

I have spent a little more time using these attributes, in particular [InitRequiredForNotNull], and think it is fantastic. My only question now is, why is everyone not using this?! It is a game-changer, and makes enforcement of Null Reference Types a viable option for the first time.

I want to add a +1 to the suggestion of having [InitRequiredForNotNull] to be a Type level attribute, not just assembly level. I think that when changing default Static Analysis behavior (as this does) it is better to have a visible attribute on each class so that this behavior is obvious and understood when viewing the source of the relevant Type. This also gives flexibility to more selectively apply this behavior

jamshally avatar Apr 29 '22 09:04 jamshally

@jamshally +1, game changer.

Consider adding this to the entire solution with Directory.Build.props:

  <ItemGroup>
    <AssemblyAttribute Include="SmartAnalyzers.CSharpExtensions.Annotations.InitRequiredForNotNullAttribute" />
  </ItemGroup>

rafalschmidt97 avatar May 11 '22 09:05 rafalschmidt97