Skip to content

Feature/ecs0001 prefer implicitly typed local variables #21

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions docs/rules/ECS0001.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# ECS0001: Prefer Implicitly Typed Local Variables

This rule is described in detail in [Effective C#: 50 Specific Ways to Improve your C#](https://www.oreilly.com/library/view/effective-c-50/9780134579290/).

## Cause

Explicitly typing local variables when the type can be inferred from the initialization expression.

## Rule description

Using implicitly typed local variables (declared with `var`) can enhance code readability and maintainability by focusing on the variable's semantic meaning rather than its type. It also allows the compiler to choose the best type, reducing potential type conversion issues and errors. However, for built-in numeric types (int, float, double, etc.), it is recommended to use explicit typing to prevent unintended type conversions and maintain precision.

## How to fix violations

Replace explicitly typed local variable declarations with `var` when the type can be inferred from the initialization expression. For built-in numeric types, retain explicit type declarations.

## When to suppress warnings

Suppress warnings if explicit typing is necessary for clarity, especially when the type is not easily inferred from the initialization expression, or when dealing with built-in numeric types where precision and conversion issues are not a concern.

## Example of a violation

### Description

Problems caused by implicitly typed locals when you delcare variables of built-in numeric types can occur. There are numerous conversions between the built-in numeric types. Widening conversions, such as from float to double, are always safe. There are also narrowing conversions, such as from long to int, that involve a loss of precision. By explicitly declaring the types of all numeric variables, you retain some control over the types used, and you help the compiler warn you about possible dangerious conversions.

### Code

```csharp
var f = GetMagicNumber();
var total = 100 * f / 6;
Console.WriteLine($"Declared Type:{total.GetType().Name}, Value:{total}");

double GetMagicNumber() => 100.0;
```

There are five outputs to the code example depending on the type returned from `GetMagicNumber()`. Here are five outputs:

Declared Type: Double, Value = 166.666666666667
Declared Type: Single, Value = 166.6667
Declared Type: Decimal, Value = 166.66666666666666666666666667
Declared Type: Int32, Value = 166
Declared Type: Int64, Value = 166

The differences in the type are caused by the way the compiler infers the type of `f`, which modifies the inferred type of `total`.

## Example of how to fix

### Description

Use `var` to allow the compiler to infer the type of the local variable.

### Code

```csharp
double f = GetMagicNumber();
double total = 100 * f / 6;
Console.WriteLine($"Declared Type:{total.GetType().Name}, Value:{total}");

double GetMagicNumber() => 100.0;
```
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
ECS0001 | Style | Info | PreferImplicitlyTypedLocalVariablesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/e7c151c721c3039011356d6012838f46e4b60a21/docs/ECS0001.md)
ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/ECS0002.md)
ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0006.md)
ECS0009 | Performance | Info | MinimizeBoxingUnboxingAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0009.md)
Expand Down
13 changes: 13 additions & 0 deletions src/EffectiveCSharp.Analyzers/Build/Config/General.globalconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
is_global = true
global_level = -1

# Title : Use implicit type
# Category : Style
# Help Link: https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0007
#
# Effective C# Item #1 - Prefer implicitly typed local variables
# Use var to declare local variables for better readability and efficiency, except for built-in numeric types where explicit typing prevents potential conversion issues.
dotnet_diagnostic.IDE0007.severity = suggestion
csharp_style_var_elsewhere = true
csharp_style_var_for_built_in_types = true
csharp_style_var_when_type_is_apparent = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<RunAnalyzers>true</RunAnalyzers>
<RunAnalyzersDuringBuild>true</RunAnalyzersDuringBuild>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<SkipGlobalAnalyzerConfigForPackage>true</SkipGlobalAnalyzerConfigForPackage>
</PropertyGroup>

<ItemGroup>
<AdditionalFiles Include="$(MSBuildThisFileDirectory)Stylecop.json" Visible="false" />
<EditorConfigFiles Include="$(MSBuildThisFileDirectory)config/General.globalconfig" />
</ItemGroup>
</Project>
13 changes: 13 additions & 0 deletions src/EffectiveCSharp.Analyzers/Common/ITypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
namespace EffectiveCSharp.Analyzers.Common;

internal static class ITypeSymbolExtensions
{
internal static bool IsNumericType(this ITypeSymbol? type)
{
return type?.SpecialType is SpecialType.System_Int32
or SpecialType.System_Int64
or SpecialType.System_Single
or SpecialType.System_Double
or SpecialType.System_Decimal;
}
}
1 change: 1 addition & 0 deletions src/EffectiveCSharp.Analyzers/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

internal static class DiagnosticIds
{
internal const string PreferImplicitlyTypedLocalVariables = "ECS0001";
internal const string PreferReadonlyOverConst = "ECS0002";
internal const string AvoidStringlyTypedApis = "ECS0006";
internal const string MinimizeBoxingUnboxing = "ECS0009";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,9 @@
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
</ItemGroup>

<ItemGroup>
<Content Include="build\*" PackagePath="build" />
<Content Include="build\config\*" PackagePath="build\config" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
namespace EffectiveCSharp.Analyzers;

Check failure on line 1 in src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs#L1

Add or update the header of this file.

Check warning on line 1 in src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs#L1

Provide a 'CLSCompliant' attribute for assembly 'srcassembly.dll'.

Check warning on line 1 in src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs#L1

Provide a 'ComVisible' attribute for assembly 'srcassembly.dll'.

Check failure on line 1 in src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

/// <summary>
/// A <see cref="CodeFixProvider"/> that provides a code fix for the <see cref="PreferExplicitTypesOnNumbersAnalyzer"/>.
/// </summary>
/// <seealso cref="CodeFixProvider" />
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferExplicitTypesForNumbersCodeFixProvider))]
[Shared]
public class PreferExplicitTypesForNumbersCodeFixProvider : CodeFixProvider
{
private const string Title = "Use explicit type";

/// <inheritdoc />
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.PreferImplicitlyTypedLocalVariables);

/// <inheritdoc />
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

/// <inheritdoc />
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
Diagnostic diagnostic = context.Diagnostics.First();
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;

if (root is null)
{
return;
}

LocalDeclarationStatementSyntax? declaration = root.FindToken(diagnosticSpan.Start).Parent?.AncestorsAndSelf().OfType<LocalDeclarationStatementSyntax>().First();

context.RegisterCodeFix(
CodeAction.Create(
title: Title,
createChangedDocument: c => UseExplicitTypeAsync(context.Document, declaration, c),
equivalenceKey: Title),
diagnostic);
}

private static async Task<Document> UseExplicitTypeAsync(Document document, LocalDeclarationStatementSyntax? localDeclaration, CancellationToken cancellationToken)
{
if (localDeclaration is null)
{
return document;
}

SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
VariableDeclaratorSyntax? variable = localDeclaration.Declaration.Variables.First();
ExpressionSyntax? initializer = variable.Initializer?.Value;

if (initializer is null)
{
return document;
}

TypeInfo typeInfo = semanticModel.GetTypeInfo(initializer, cancellationToken);
ITypeSymbol? type = typeInfo.ConvertedType;

if (type is null)
{
return document;
}

TypeSyntax explicitType = SyntaxFactory
.ParseTypeName(type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat))
.WithTriviaFrom(localDeclaration.Declaration.Type);

LocalDeclarationStatementSyntax newDeclaration = localDeclaration.WithDeclaration(
localDeclaration.Declaration.WithType(explicitType)
.WithVariables(SyntaxFactory.SingletonSeparatedList(variable)));

SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

if (root is null)
{
return document;
}

SyntaxNode newRoot = root.ReplaceNode(localDeclaration, newDeclaration);

return document.WithSyntaxRoot(newRoot);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
namespace EffectiveCSharp.Analyzers;

/// <summary>
/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #1 - Prefer implicit types except on numbers.
/// </summary>
/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" />
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class PreferExplicitTypesOnNumbersAnalyzer : DiagnosticAnalyzer
{
private const string Id = DiagnosticIds.PreferImplicitlyTypedLocalVariables;

private static readonly DiagnosticDescriptor Rule = new(
id: Id,
title: "Prefer implicitly typed local variables",
description: "Use var to declare local variables for better readability and efficiency, except for built-in numeric types where explicit typing prevents potential conversion issues.",
messageFormat: "Use explicit type instead of 'var' for numeric variables",
category: "Style",
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true,
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md");

/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

/// <inheritdoc />
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.LocalDeclarationStatement);
}

private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
{
LocalDeclarationStatementSyntax localDeclaration = (LocalDeclarationStatementSyntax)context.Node;

foreach (VariableDeclaratorSyntax variable in localDeclaration.Declaration.Variables)
{
ExpressionSyntax? initializer = variable.Initializer?.Value;

if (initializer is null)
{
continue;
}

TypeInfo typeInfo = context.SemanticModel.GetTypeInfo(initializer, context.CancellationToken);
ITypeSymbol? type = typeInfo.ConvertedType;

if (type?.IsNumericType() != true)
{
continue;
}

if (localDeclaration.Declaration.Type.IsVar)
{
Diagnostic diagnostic = localDeclaration.GetLocation().CreateDiagnostic(Rule);
context.ReportDiagnostic(diagnostic);
}
else if (HasPotentialConversionIssues(type, initializer, context.SemanticModel, context.CancellationToken))

Check failure on line 59 in src/EffectiveCSharp.Analyzers/PreferExplicitTypesOnNumbersAnalyzer.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/EffectiveCSharp.Analyzers/PreferExplicitTypesOnNumbersAnalyzer.cs#L59

Add the missing 'else' clause with either the appropriate action or a suitable comment as to why no action is taken.
{
Diagnostic diagnostic = initializer.GetLocation().CreateDiagnostic(Rule);
context.ReportDiagnostic(diagnostic);
}
}
}

private static bool HasPotentialConversionIssues(ITypeSymbol type, ExpressionSyntax initializer, SemanticModel semanticModel, CancellationToken cancellationToken)
{
TypeInfo typeInfo = semanticModel.GetTypeInfo(initializer, cancellationToken);
return !SymbolEqualityComparer.IncludeNullability.Equals(typeInfo.Type, type);
}
}
74 changes: 74 additions & 0 deletions tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
namespace EffectiveCSharp.Analyzers.Benchmarks;

Check failure on line 1 in tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs#L1

Add or update the header of this file.

Check warning on line 1 in tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs#L1

Provide a 'CLSCompliant' attribute for assembly 'srcassembly.dll'.

Check warning on line 1 in tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs#L1

Provide a 'ComVisible' attribute for assembly 'srcassembly.dll'.

Check failure on line 1 in tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.

[InProcess]
[MemoryDiagnoser]
public class Ecs0001Benchmarks
{
private static CompilationWithAnalyzers? BaselineCompilation { get; set; }

private static CompilationWithAnalyzers? TestCompilation { get; set; }

[IterationSetup]
[SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "Async setup not supported in BenchmarkDotNet.See https://github.com/dotnet/BenchmarkDotNet/issues/2442.")]
public static void SetupCompilation()
{
List<(string Name, string Content)> sources = [];
for (int index = 0; index < Constants.NumberOfCodeFiles; index++)
{
string name = $"TypeName{index}";
sources.Add((name, @$"
using System;

public class {name}
{{
public void MyMethod()
{{
var f = GetMagicNumber();
var total = 100 * f / 6;
Console.WriteLine($""Declared Type:{{total.GetType().Name}}, Value:{{total}}"");
}}

decimal GetMagicNumber() => 100.0M;
}}
"));
}

(BaselineCompilation, TestCompilation) =
BenchmarkCSharpCompilationFactory
.CreateAsync<PreferExplicitTypesOnNumbersAnalyzer>(sources.ToArray())
.GetAwaiter()
.GetResult();
}

[Benchmark]
public async Task Ecs0001WithDiagnostics()
{
ImmutableArray<Diagnostic> diagnostics =
(await TestCompilation!
.GetAnalysisResultAsync(CancellationToken.None)
.ConfigureAwait(false))
.AssertValidAnalysisResult()
.GetAllDiagnostics();

if (diagnostics.Length != Constants.NumberOfCodeFiles * 2)
{
throw new InvalidOperationException($"Expected '{Constants.NumberOfCodeFiles:N0}' analyzer diagnostics but found '{diagnostics.Length:N0}'");
}
}

[Benchmark(Baseline = true)]
public async Task Ecs0001Baseline()
{
ImmutableArray<Diagnostic> diagnostics =
(await BaselineCompilation!
.GetAnalysisResultAsync(CancellationToken.None)
.ConfigureAwait(false))
.AssertValidAnalysisResult()
.GetAllDiagnostics();

if (diagnostics.Length != 0)
{
throw new InvalidOperationException($"Expected no analyzer diagnostics but found '{diagnostics.Length}'");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ internal static class ReferenceAssemblyCatalog

public static string Net90 => nameof(ReferenceAssemblies.Net.Net90);

public static string Latest => nameof(ReferenceAssemblies.Net.Net80);

public static IReadOnlyDictionary<string, ReferenceAssemblies> Catalog { get; } = new Dictionary<string, ReferenceAssemblies>(StringComparer.Ordinal)
{
{ Net48, ReferenceAssemblies.NetFramework.Net48.Default },
Expand Down
Loading
Loading