Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed all occurenceas of var #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Changed all occurenceas of var #1

wants to merge 5 commits into from

Conversation

lizard-boy
Copy link

@lizard-boy lizard-boy commented Nov 1, 2024

To have a consistent code base I changed all usages of var and changed the editorconfig rule to error.

Do you want this things to be on error in the editorconfig?

Greptile Summary

This PR enforces explicit type declarations across the ImageSharp codebase by modifying code style rules and updating syntax.

  • Changed .editorconfig and added .DotSettings to enforce explicit type declarations as errors instead of warnings
  • Replaced var usage with explicit types throughout the codebase for consistency
  • Updated constructor calls to use target-typed new() expressions (C# 9.0 feature)
  • Changed array initialization syntax to use collection expressions (C# 12 feature)
  • Consider whether enforcing explicit types as errors may be too restrictive for developer productivity

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30 file(s) reviewed, 30 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +138 to +140
csharp_style_var_for_built_in_types = false:error
csharp_style_var_when_type_is_apparent = false:error
csharp_style_var_elsewhere = false:error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Changing var preferences from warning to error may be too restrictive. Consider keeping as warning to maintain flexibility where explicit types harm readability.

Comment on lines +107 to +108
dotnet_style_object_initializer = true:error
dotnet_style_collection_initializer = true:error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Making object/collection initializer rules errors instead of warnings could be overly strict for cases where alternative syntax improves clarity.

@@ -160,7 +160,7 @@ csharp_style_pattern_local_over_anonymous_function = true:warning
csharp_style_deconstructed_variable_declaration = true:warning
csharp_style_prefer_index_operator = true:warning
csharp_style_prefer_range_operator = true:warning
csharp_style_implicit_object_creation_when_type_is_apparent = true:warning
csharp_style_implicit_object_creation_when_type_is_apparent = true:error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider keeping implicit object creation as warning - error severity may force unnecessary verbosity in obvious type scenarios.

Comment on lines +2 to +4
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FBuiltInTypes/@EntryIndexedValue">ERROR</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FElsewhere/@EntryIndexedValue">ERROR</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestVarOrType_005FSimpleTypes/@EntryIndexedValue">ERROR</s:String>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Setting var-to-type suggestions as errors may be too strict and could reduce code readability in cases where types are obvious or very long. Consider using WARNING instead of ERROR to provide flexibility.

@@ -8,15 +8,15 @@ namespace SixLabors.ImageSharp;
/// </content>
public partial struct Color
{
private static readonly Lazy<Color[]> WebSafePaletteLazy = new Lazy<Color[]>(CreateWebSafePalette, true);
private static readonly Lazy<Color[]> WebSafePaletteLazy = new(CreateWebSafePalette, true);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider keeping explicit type for Lazy<Color[]> constructor since removing it doesn't improve readability here

@@ -134,7 +134,7 @@ private static void PackFromRgbPlanesScalarBatchedReduce(
ref Rgba32 rgb = ref MemoryMarshal.GetReference(destination);

nuint count = (uint)redChannel.Length / 4;
destination.Fill(new Rgba32(0, 0, 0, 255));
destination.Fill(new(0, 0, 0, 255));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: This change removes the explicit type name Rgba32 from the constructor, which seems to contradict the PR's goal of making types more explicit. Consider keeping new Rgba32(0, 0, 0, 255) for clarity.

@@ -29,7 +29,7 @@ internal static partial class SimdUtils
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static Vector4 PseudoRound(this Vector4 v)
{
Vector4 sign = Numerics.Clamp(v, new Vector4(-1), new Vector4(1));
Vector4 sign = Numerics.Clamp(v, new(-1), new(1));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The simplified constructor syntax 'new(-1)' relies on type inference from the Clamp method signature. Consider keeping the explicit 'new Vector4(-1)' for better readability and maintainability.

@@ -20,7 +20,7 @@ internal readonly struct TolerantMath
/// It is a field so it can be passed as an 'in' parameter.
/// Does not necessarily fit all use cases!
/// </summary>
public static readonly TolerantMath Default = new TolerantMath(1e-8);
public static readonly TolerantMath Default = new(1e-8);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The target-typed new expression is valid here since C# 9.0, but consider keeping the explicit type for better readability in static field initialization

@@ -131,9 +131,9 @@ public static ExifResolutionValues GetExifResolutionValues(PixelResolutionUnit u
ushort exifUnit = (ushort)(unit + 1);
if (unit == PixelResolutionUnit.AspectRatio)
{
return new ExifResolutionValues(exifUnit, null, null);
return new(exifUnit, null, null);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider keeping the explicit type ExifResolutionValues here for better readability since the type is not immediately obvious from the constructor parameters

}

return new ExifResolutionValues(exifUnit, horizontal, vertical);
return new(exifUnit, horizontal, vertical);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider keeping the explicit type ExifResolutionValues here for better readability since the type is not immediately obvious from the constructor parameters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants