Enforce Explicit Types and Target Type new #2951
Conversation
|
Collection Expression is still a problem.. Do we wanna enforce Collection Expressions? I think we have this options |
stefannikolei
changed the title
I think, in this case, moving the comment above the declaration would be the correct solution. As to whether we should enforce collection expressions.... I don't know, I like them very much but sometimes they can reduce readabilty. |
|
OK Then let's go this way. This PR is complete from my side. When this one is merged I will open a draft for the Collection Expressions so we have a clear view how it is gonna be for this repo. Sounds good? |
| @@ -24,7 +24,7 @@ namespace SixLabors.ImageSharp.ColorProfiles; | |||
| /// <param name="l">The l value (lightness) component.</param> | |||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | |||
| public Hsl(float h, float s, float l) | |||
| : this(new Vector3(h, s, l)) | |||
| : this(new(h, s, l)) | |||
There was a problem hiding this comment.
@JimBobSquarePants I'm late to the party again, but I dislike this particular group of the changes. Now a code reader needs to navigate to the other method definition to see what type is being instantiated here, which makes it harder/slower to read the code. Same for return new(...) and similar expressions which remove type information completely from a given line.
There was a problem hiding this comment.
Yeah that's a drawback about this. I will create a PR which tackles this. Rider/Resharper has more settings for this and was probably a bit more aggressive. Could you check that branch out and look what VS is doing? #2958
There was a problem hiding this comment.
Thanks! #2958 looks good based on the diff, but I can't tell if it's missing something without pulling the code down and review offline.
There was a problem hiding this comment.
Apologies, I missed this. According to IDE0090 the analyzer should only be flagging when the type is apparent. Is Rider/Resharper buggy?
There was a problem hiding this comment.
Rider generates a warning at that rule stating that this one is unknown/unsupported.
It has more granular editorconfig settings.
Prerequisites
Description
Reworkes #2855 after too Manz conflicts