Skip to content

Commit 097a362

Browse files
rjmurillorjmurillo
authored
Feature/ecs0001 prefer implicitly typed local variables (#21)
* Add initial version of ECS0001 * Add additional checks for conversion error * Update implementation of code fix provider * Add benchmark for ECS0001 * Fix member visibility
1 parent e7c151c commit 097a362

12 files changed

+453
-0
lines changed

Diff for: ‎docs/rules/ECS0001.md

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# ECS0001: Prefer Implicitly Typed Local Variables
2+
3+
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/).
4+
5+
## Cause
6+
7+
Explicitly typing local variables when the type can be inferred from the initialization expression.
8+
9+
## Rule description
10+
11+
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.
12+
13+
## How to fix violations
14+
15+
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.
16+
17+
## When to suppress warnings
18+
19+
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.
20+
21+
## Example of a violation
22+
23+
### Description
24+
25+
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.
26+
27+
### Code
28+
29+
```csharp
30+
var f = GetMagicNumber();
31+
var total = 100 * f / 6;
32+
Console.WriteLine($"Declared Type:{total.GetType().Name}, Value:{total}");
33+
34+
double GetMagicNumber() => 100.0;
35+
```
36+
37+
There are five outputs to the code example depending on the type returned from `GetMagicNumber()`. Here are five outputs:
38+
39+
Declared Type: Double, Value = 166.666666666667
40+
Declared Type: Single, Value = 166.6667
41+
Declared Type: Decimal, Value = 166.66666666666666666666666667
42+
Declared Type: Int32, Value = 166
43+
Declared Type: Int64, Value = 166
44+
45+
The differences in the type are caused by the way the compiler infers the type of `f`, which modifies the inferred type of `total`.
46+
47+
## Example of how to fix
48+
49+
### Description
50+
51+
Use `var` to allow the compiler to infer the type of the local variable.
52+
53+
### Code
54+
55+
```csharp
56+
double f = GetMagicNumber();
57+
double total = 100 * f / 6;
58+
Console.WriteLine($"Declared Type:{total.GetType().Name}, Value:{total}");
59+
60+
double GetMagicNumber() => 100.0;
61+
```

Diff for: ‎src/EffectiveCSharp.Analyzers/AnalyzerReleases.Unshipped.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
Rule ID | Category | Severity | Notes
66
--------|----------|----------|-------
7+
ECS0001 | Style | Info | PreferImplicitlyTypedLocalVariablesAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/e7c151c721c3039011356d6012838f46e4b60a21/docs/ECS0001.md)
78
ECS0002 | Maintainability | Info | PreferReadonlyOverConstAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/10c2d53afd688efe5a59097f76cb4edf33f6a474/docs/ECS0002.md)
89
ECS0006 | Refactoring | Info | AvoidStringlyTypedApisAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0006.md)
910
ECS0009 | Performance | Info | MinimizeBoxingUnboxingAnalyzer, [Documentation](https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/6213cba8473dac61d6132e205550884eae1c94bf/docs/ECS0009.md)

Diff for: ‎src/EffectiveCSharp.Analyzers/Build/Config/General.globalconfig

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
is_global = true
2+
global_level = -1
3+
4+
# Title : Use implicit type
5+
# Category : Style
6+
# Help Link: https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0007
7+
#
8+
# Effective C# Item #1 - Prefer implicitly typed local variables
9+
# Use var to declare local variables for better readability and efficiency, except for built-in numeric types where explicit typing prevents potential conversion issues.
10+
dotnet_diagnostic.IDE0007.severity = suggestion
11+
csharp_style_var_elsewhere = true
12+
csharp_style_var_for_built_in_types = true
13+
csharp_style_var_when_type_is_apparent = true

Diff for: ‎src/EffectiveCSharp.Analyzers/Build/EffectiveCSharp.Analyzers.props

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
2+
<PropertyGroup>
3+
<RunAnalyzers>true</RunAnalyzers>
4+
<RunAnalyzersDuringBuild>true</RunAnalyzersDuringBuild>
5+
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
6+
<SkipGlobalAnalyzerConfigForPackage>true</SkipGlobalAnalyzerConfigForPackage>
7+
</PropertyGroup>
8+
9+
<ItemGroup>
10+
<AdditionalFiles Include="$(MSBuildThisFileDirectory)Stylecop.json" Visible="false" />
11+
<EditorConfigFiles Include="$(MSBuildThisFileDirectory)config/General.globalconfig" />
12+
</ItemGroup>
13+
</Project>

Diff for: ‎src/EffectiveCSharp.Analyzers/Common/ITypeSymbolExtensions.cs

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
namespace EffectiveCSharp.Analyzers.Common;
2+
3+
internal static class ITypeSymbolExtensions
4+
{
5+
internal static bool IsNumericType(this ITypeSymbol? type)
6+
{
7+
return type?.SpecialType is SpecialType.System_Int32
8+
or SpecialType.System_Int64
9+
or SpecialType.System_Single
10+
or SpecialType.System_Double
11+
or SpecialType.System_Decimal;
12+
}
13+
}

Diff for: ‎src/EffectiveCSharp.Analyzers/DiagnosticIds.cs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
internal static class DiagnosticIds
44
{
5+
internal const string PreferImplicitlyTypedLocalVariables = "ECS0001";
56
internal const string PreferReadonlyOverConst = "ECS0002";
67
internal const string AvoidStringlyTypedApis = "ECS0006";
78
internal const string MinimizeBoxingUnboxing = "ECS0009";

Diff for: ‎src/EffectiveCSharp.Analyzers/EffectiveCSharp.Analyzers.csproj

+5
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,9 @@
4747
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
4848
</None>
4949
</ItemGroup>
50+
51+
<ItemGroup>
52+
<Content Include="build\*" PackagePath="build" />
53+
<Content Include="build\config\*" PackagePath="build\config" />
54+
</ItemGroup>
5055
</Project>

Diff for: ‎src/EffectiveCSharp.Analyzers/PreferExplicitTypesForNumbersCodeFixProvider.cs

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
namespace EffectiveCSharp.Analyzers;
2+
3+
/// <summary>
4+
/// A <see cref="CodeFixProvider"/> that provides a code fix for the <see cref="PreferExplicitTypesOnNumbersAnalyzer"/>.
5+
/// </summary>
6+
/// <seealso cref="CodeFixProvider" />
7+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferExplicitTypesForNumbersCodeFixProvider))]
8+
[Shared]
9+
public class PreferExplicitTypesForNumbersCodeFixProvider : CodeFixProvider
10+
{
11+
private const string Title = "Use explicit type";
12+
13+
/// <inheritdoc />
14+
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIds.PreferImplicitlyTypedLocalVariables);
15+
16+
/// <inheritdoc />
17+
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
18+
19+
/// <inheritdoc />
20+
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
21+
{
22+
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
23+
Diagnostic diagnostic = context.Diagnostics.First();
24+
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;
25+
26+
if (root is null)
27+
{
28+
return;
29+
}
30+
31+
LocalDeclarationStatementSyntax? declaration = root.FindToken(diagnosticSpan.Start).Parent?.AncestorsAndSelf().OfType<LocalDeclarationStatementSyntax>().First();
32+
33+
context.RegisterCodeFix(
34+
CodeAction.Create(
35+
title: Title,
36+
createChangedDocument: c => UseExplicitTypeAsync(context.Document, declaration, c),
37+
equivalenceKey: Title),
38+
diagnostic);
39+
}
40+
41+
private static async Task<Document> UseExplicitTypeAsync(Document document, LocalDeclarationStatementSyntax? localDeclaration, CancellationToken cancellationToken)
42+
{
43+
if (localDeclaration is null)
44+
{
45+
return document;
46+
}
47+
48+
SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
49+
VariableDeclaratorSyntax? variable = localDeclaration.Declaration.Variables.First();
50+
ExpressionSyntax? initializer = variable.Initializer?.Value;
51+
52+
if (initializer is null)
53+
{
54+
return document;
55+
}
56+
57+
TypeInfo typeInfo = semanticModel.GetTypeInfo(initializer, cancellationToken);
58+
ITypeSymbol? type = typeInfo.ConvertedType;
59+
60+
if (type is null)
61+
{
62+
return document;
63+
}
64+
65+
TypeSyntax explicitType = SyntaxFactory
66+
.ParseTypeName(type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat))
67+
.WithTriviaFrom(localDeclaration.Declaration.Type);
68+
69+
LocalDeclarationStatementSyntax newDeclaration = localDeclaration.WithDeclaration(
70+
localDeclaration.Declaration.WithType(explicitType)
71+
.WithVariables(SyntaxFactory.SingletonSeparatedList(variable)));
72+
73+
SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
74+
75+
if (root is null)
76+
{
77+
return document;
78+
}
79+
80+
SyntaxNode newRoot = root.ReplaceNode(localDeclaration, newDeclaration);
81+
82+
return document.WithSyntaxRoot(newRoot);
83+
}
84+
}

Diff for: ‎src/EffectiveCSharp.Analyzers/PreferExplicitTypesOnNumbersAnalyzer.cs

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
namespace EffectiveCSharp.Analyzers;
2+
3+
/// <summary>
4+
/// A <see cref="DiagnosticAnalyzer"/> for Effective C# Item #1 - Prefer implicit types except on numbers.
5+
/// </summary>
6+
/// <seealso cref="Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer" />
7+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
8+
public class PreferExplicitTypesOnNumbersAnalyzer : DiagnosticAnalyzer
9+
{
10+
private const string Id = DiagnosticIds.PreferImplicitlyTypedLocalVariables;
11+
12+
private static readonly DiagnosticDescriptor Rule = new(
13+
id: Id,
14+
title: "Prefer implicitly typed local variables",
15+
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.",
16+
messageFormat: "Use explicit type instead of 'var' for numeric variables",
17+
category: "Style",
18+
defaultSeverity: DiagnosticSeverity.Info,
19+
isEnabledByDefault: true,
20+
helpLinkUri: $"https://github.com/rjmurillo/EffectiveCSharp.Analyzers/blob/{ThisAssembly.GitCommitId}/docs/{Id}.md");
21+
22+
/// <inheritdoc />
23+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
24+
25+
/// <inheritdoc />
26+
public override void Initialize(AnalysisContext context)
27+
{
28+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
29+
context.EnableConcurrentExecution();
30+
context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.LocalDeclarationStatement);
31+
}
32+
33+
private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
34+
{
35+
LocalDeclarationStatementSyntax localDeclaration = (LocalDeclarationStatementSyntax)context.Node;
36+
37+
foreach (VariableDeclaratorSyntax variable in localDeclaration.Declaration.Variables)
38+
{
39+
ExpressionSyntax? initializer = variable.Initializer?.Value;
40+
41+
if (initializer is null)
42+
{
43+
continue;
44+
}
45+
46+
TypeInfo typeInfo = context.SemanticModel.GetTypeInfo(initializer, context.CancellationToken);
47+
ITypeSymbol? type = typeInfo.ConvertedType;
48+
49+
if (type?.IsNumericType() != true)
50+
{
51+
continue;
52+
}
53+
54+
if (localDeclaration.Declaration.Type.IsVar)
55+
{
56+
Diagnostic diagnostic = localDeclaration.GetLocation().CreateDiagnostic(Rule);
57+
context.ReportDiagnostic(diagnostic);
58+
}
59+
else if (HasPotentialConversionIssues(type, initializer, context.SemanticModel, context.CancellationToken))
60+
{
61+
Diagnostic diagnostic = initializer.GetLocation().CreateDiagnostic(Rule);
62+
context.ReportDiagnostic(diagnostic);
63+
}
64+
}
65+
}
66+
67+
private static bool HasPotentialConversionIssues(ITypeSymbol type, ExpressionSyntax initializer, SemanticModel semanticModel, CancellationToken cancellationToken)
68+
{
69+
TypeInfo typeInfo = semanticModel.GetTypeInfo(initializer, cancellationToken);
70+
return !SymbolEqualityComparer.IncludeNullability.Equals(typeInfo.Type, type);
71+
}
72+
}

Diff for: ‎tests/EffectiveCSharp.Analyzers.Benchmarks/Ecs0001Benchmarks.cs

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
namespace EffectiveCSharp.Analyzers.Benchmarks;
2+
3+
[InProcess]
4+
[MemoryDiagnoser]
5+
public class Ecs0001Benchmarks
6+
{
7+
private static CompilationWithAnalyzers? BaselineCompilation { get; set; }
8+
9+
private static CompilationWithAnalyzers? TestCompilation { get; set; }
10+
11+
[IterationSetup]
12+
[SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "Async setup not supported in BenchmarkDotNet.See https://github.com/dotnet/BenchmarkDotNet/issues/2442.")]
13+
public static void SetupCompilation()
14+
{
15+
List<(string Name, string Content)> sources = [];
16+
for (int index = 0; index < Constants.NumberOfCodeFiles; index++)
17+
{
18+
string name = $"TypeName{index}";
19+
sources.Add((name, @$"
20+
using System;
21+
22+
public class {name}
23+
{{
24+
public void MyMethod()
25+
{{
26+
var f = GetMagicNumber();
27+
var total = 100 * f / 6;
28+
Console.WriteLine($""Declared Type:{{total.GetType().Name}}, Value:{{total}}"");
29+
}}
30+
31+
decimal GetMagicNumber() => 100.0M;
32+
}}
33+
"));
34+
}
35+
36+
(BaselineCompilation, TestCompilation) =
37+
BenchmarkCSharpCompilationFactory
38+
.CreateAsync<PreferExplicitTypesOnNumbersAnalyzer>(sources.ToArray())
39+
.GetAwaiter()
40+
.GetResult();
41+
}
42+
43+
[Benchmark]
44+
public async Task Ecs0001WithDiagnostics()
45+
{
46+
ImmutableArray<Diagnostic> diagnostics =
47+
(await TestCompilation!
48+
.GetAnalysisResultAsync(CancellationToken.None)
49+
.ConfigureAwait(false))
50+
.AssertValidAnalysisResult()
51+
.GetAllDiagnostics();
52+
53+
if (diagnostics.Length != Constants.NumberOfCodeFiles * 2)
54+
{
55+
throw new InvalidOperationException($"Expected '{Constants.NumberOfCodeFiles:N0}' analyzer diagnostics but found '{diagnostics.Length:N0}'");
56+
}
57+
}
58+
59+
[Benchmark(Baseline = true)]
60+
public async Task Ecs0001Baseline()
61+
{
62+
ImmutableArray<Diagnostic> diagnostics =
63+
(await BaselineCompilation!
64+
.GetAnalysisResultAsync(CancellationToken.None)
65+
.ConfigureAwait(false))
66+
.AssertValidAnalysisResult()
67+
.GetAllDiagnostics();
68+
69+
if (diagnostics.Length != 0)
70+
{
71+
throw new InvalidOperationException($"Expected no analyzer diagnostics but found '{diagnostics.Length}'");
72+
}
73+
}
74+
}

Diff for: ‎tests/EffectiveCSharp.Analyzers.Tests/Helpers/ReferenceAssemblyCatalog.cs

+2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ internal static class ReferenceAssemblyCatalog
4747

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

50+
public static string Latest => nameof(ReferenceAssemblies.Net.Net80);
51+
5052
public static IReadOnlyDictionary<string, ReferenceAssemblies> Catalog { get; } = new Dictionary<string, ReferenceAssemblies>(StringComparer.Ordinal)
5153
{
5254
{ Net48, ReferenceAssemblies.NetFramework.Net48.Default },

0 commit comments

Comments
 (0)