meziantou / meziantou.analyzer

A Roslyn analyzer to enforce some good practices in C#.

Hi there 👋

My name is Gérald Barré (@meziantou). I'm an experienced software developer with more than 10 years of experience in the industry. I currently work as a Software Engineer, where I'm part of a team that is responsible for developing and maintaining tools to help developers adopt InnerSource and increase code sharing in the company.

I have a strong background in C#, .NET, and web development, as well as a deep understanding of software architecture and design patterns. I'm also proficient in many other technologies, such as SQL Server, PowerShell, Docker, and Azure. I'm an expert at writing clean code that is easy to maintain and understand. I'm passionate about building high-quality software solutions and take pride in my work. I'm always eager to learn new technologies and love to experiment with different technologies to find the best solution for each problem.

In addition to my professional work, I'm highly involved in the tech community. I'm a recognized Microsoft MVP for my contributions to the developer community, which include speaking at tech events, writing technical blog posts and contributing to open-source. I'm also a maintainer of various open-source projects, such as Meziantou.Analyzer or Meziantou.Framework. I'm passionate about sharing my knowledge and expertise with others through my blog, which can be found at https://www.meziantou.net.

📗 Recent blog posts

• Using type aliases to make code clearer with generics
• Computing code coverage for a .NET project
• Creating ico files from multiple images in .NET
• Json schema validation in .NET
• Convert DateTime to user's time zone with Blazor in .NET 8
• Generate OpenAPI specification at build time from the code in ASP.NET Core
• Set a blank page for new tabs in Microsoft Edge
• Optional parameters may appear in the middle of the parameter list
• Getting all exceptions thrown from Parallel.ForEachAsync
• Creating an HttpClient that uses DNS over Https

Checkout out my complete list of blog entries!

💹 Stats

meziantou.analyzer's Issues

sealed internal class if possible

Suggest to seal internal class when possible (add an option to seal public class)

Make class static fix produces invalid code

partial class A {}
partial static class A {} // Invalid because partial must be just before class

Generate a page that list all rules

It should include a table with the following columns:

• Rule id
• Rule name
• Severity
• Category
• Has code fix

New rule: Parameters with "[DefaultParameterValue]" attributes should also be marked "[Optional]"

New Rule: Override equals and operator equals on value types

For performance reasons, you should override Equals and GetHashCode on value types (struct).

https://blog.martindoms.com/2011/01/03/c-tip-override-equals-on-value-types-for-better-performance

New rule: The length returned from Stream.Read should be used

public void DoSomething(string fileName)
{
  using (var stream = File.Open(fileName, FileMode.Open))
  {
    var result = new byte[stream.Length];
    stream.Read(result, 0, (int)stream.Length); // Noncompliant, should be readLength = stream.Read(...) and readLength should be used
  }
}

Please fix typo in title

pratices > practices

New rule: Method overrides should not change parameter defaults

public class Foo
{
  public virtual void Write(int i = 42)
  {
    Console.WriteLine(i);
  }
}

public class Bar : Foo
{
  public override void Write(int i = 5) // Noncompliant
  {
    Console.WriteLine(i);
  }
}

Ensure the inner exception is set when throwing an exception in a catch block

This may be an improvement for the current rule: DoNotRemoveOriginalExceptionFromThrowStatement

try
{
}
catch(Exception ex)
{
    throw new Exception(); // Should be throw new Exception("", ex);
}

Analyzer 'Meziantou.Analyzer.Rules.ReturnTaskFromResultInsteadOfReturningNullAnalyzer' threw an exception of type 'System.NullReferenceException'

SyntaxNode: ref coll.FindAsync(filter) [LocalFunctionStatementSyntax]@[6688..6714) (172,12)-(172,38)

System.NullReferenceException: Object reference not set to an instance of an object.
   at Meziantou.Analyzer.Rules.ReturnTaskFromResultInsteadOfReturningNullAnalyzer.IsTaskType(Compilation compilation, ITypeSymbol typeSyntax)
   at Meziantou.Analyzer.Rules.ReturnTaskFromResultInsteadOfReturningNullAnalyzer.AnalyzeLocalFunction(SyntaxNodeAnalysisContext context)
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__44`1.<ExecuteSyntaxNodeAction>b__44_0(ValueTuple`2 data)
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)

New rule: Non-flags enums should not be marked with "FlagsAttribute"

    [FlagsAttribute] // Noncompliant, Orange is neither a power of two, nor a combination of any of the defined values
    public enum Color
    {
        None    = 0,
        Red     = 1,
        Orange  = 3, 
        Yellow  = 4,
    }

New rule: `[DefaultValue]` should not be used when `[DefaultParameterValue]` is meant

New rule: Constructors should only call non-overridable methods

Calling an overridable method from a constructor could result in failures or strange behaviors when instantiating a subclass which overrides the method.

public class Foo
{
  public Foo()
  {
    VirtualMethod();  // Noncompliant
  }

  public virtual void VirtualMethod()
  {
    ...
  }
}

Skip the validation if the class is sealed

New rule: `ConstructorArgument` parameters should exist in constructors

using System;

namespace myLibrary
{
  public class MyExtension : MarkupExtension
  {
    public MyExtension() { }

    public MyExtension(object value1)
    {
      Value1 = value1;
    }

    [ConstructorArgument("value2")] // Error, value2 is not a parameter of the constructor
    public object Value1 { get; set; }
  }
}

New rule: Use task result instead of task itself

interface Repository 
{
    Task FindProduct(string name);
}

public async Task<int> GetId(string name)
{
    await Task.Yield(); //Some stuff

    var product = _repository.FindProduct(name);

    return product.Id;
}

Should be

public async Task<int> GetId(string name)
{
    await Task.Yield(); //Some stuff

    var product = await _repository.FindProduct(name);

    return product.Id; // Or product.Result.Id
}

This one is hard to see/debug 😖😭😆

Analyzer 'Meziantou.Analyzer.Rules.DoNotCallVirtualMethodInConstructorAnalyzer' threw an exception of type 'System.InvalidOperationException' with message 'Sequence contains more than one element'.

Analyzer 'Meziantou.Analyzer.Rules.DoNotCallVirtualMethodInConstructorAnalyzer' threw the following exception:'
Exception occurred with following context:
Compilation: xxx
SyntaxTree: D:\xxx\QueryResultSchema.cs
SyntaxNode: public QueryResultSchema(string ... [ConstructorDeclarationSyntax]@[480..866) (19,8)-(29,9)
System.InvalidOperationException: Sequence contains more than one element
   at System.Linq.Enumerable.SingleOrDefault[TSource](IEnumerable`1 source)
   at Meziantou.Analyzer.Rules.DoNotCallVirtualMethodInConstructorAnalyzer.AnalyzeConstructorSyntax(SyntaxNodeAnalysisContext context)
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__44`1.<ExecuteSyntaxNodeAction>b__44_0(ValueTuple`2 data)
   at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)-----'.
--

New rule: Anonymous delegates should not be used to unsubscribe from Events

sample.SampleEvent -= (obj, args) => { }; // error

New rule: Destructors should not throw exceptions

New rule: ToString should not return null

New rule: StringBuilder.AppendJoin

sb.Append(string.Join(',', new [] { "a", "b" }));

Should be:

sb.AppendJoin(',', new [] { "a", "b" });

New rule: Avoid locking on publicly accessible instance

public void Foo()
{
    lock (this) // Error
    {
    }
}

public void Foo()
{
    lock (typeof(Bar)) // Error
    {
    }
}

class Bar
{
    public object public_field;

    public void Foo()
    {
        lock (public_field) // Error
        {
        }
    }
}

Detect usage of System.Net.Http.HttpClientHandler.ServerCertificateCustomValidationCallback

New rule: Local variables should not shadow class fields

It avoid problems such as

public class Sample
{
    private string field;
    public Sample(string field)
    {
        field = field; // Assign parameter instead of field
    }
}

BTW, it should also detect assignment of the same variable

var a = 10;
a = a; // Error

`new System.EventArgs()` => `EventArgs.Empty`

New rule: Replace new Guid() by Guid.Empty

New rule: Exceptions should not be thrown in finally blocks

MA0032 should not report when there is no overload with a CancellationToken

class Test
{
    public static void A()
    {
        Sample(""); // reported here
    }

    public static void Sample(string a) { }
    public static void Sample(string a, string b) { }
}

New rule: destructor should not be used

Destructors allow to release native resources. You use wrap all native handles in SafeHandle. Thus, you don't need to use a destructor.

MA0046 should not report when the delegate match the event signature

public delegate void DataReceivedEventHandler(object sender, System.Diagnostics.DataReceivedEventArgs e);
public event System.Diagnostics.DataReceivedEventHandler ErrorDataReceived { add { } remove { } }

New rule: Obsolete attributes should include explanations

New rule: Remove useless binary comparisons with bool

bool condition;
if (condition == true) // should be if (condition)
if (condition == false) // should be if (!condition)

New rule: Replace string.StartsWith("a") with string.StartsWith('a')

• StartsWith and EndsWith, Contains
• The overload with char is not available in .NET Framework

New rule: Classes that provide "Equals(<T>)" should implement "IEquatable<T>"

Replace Enum.Value.ToString with nameof(Enum.Value)

EqualityComparer<string>.Default => StringComparer.Ordinal

Suggest using StringComparison in JObject.Property

New rule: "Equals" and the comparison operators should be overridden when implementing "IComparable"

New rule: Replace Select(o => (Sample)o) by Cast<Sample>()

details.Select(o => (Sample)o);
details.Cast<Sample>();

New rule: Events should have proper arguments

• The sender must not be null if it's an instance event
• The sender must be null if it's a static event
• Args must not be null (use EventArgs.Empty)

New rule: finally/else block should not be empty

MA0016 should not report for override or interface implementation

Suggest replacing extension methods by existing methods for `List<T>`

• Enumerable.FirstOrDefault => Find
• Enumerable.IndexOf => FindIndexOf
• Enumerable.LastIndexOf => FindLastIndexOf

New rule: "params" should be used on overrides

class Base
{
  public virtual void Method(params int[] numbers) => { };
}

class Derived : Base
{
  public override void Method(int[] numbers) // Error "params" is missing
  {
  }
}

MA0001 should detect more methods

• Enumerable.Contains<string>
• Enumerable.Distinct<string>
• Enumerable.Except<string>
• Enumerable.GroupBy<string>
• Enumerable.GroupJoin<string>
• Enumerable.Intersect<string>
• Enumerable.Join<string>
• Enumerable.OrderBy<string>
• Enumerable.OrderByDescending<string>
• Enumerable.SequenceEqual<string>
• Enumerable.ToDictionary<string>
• Enumerable.ToLookup<string>
• Enumerable.Union<string>

New rule: Non-constant static fields should not be visible

New rule: Attribute, EventArgs, and Exception type names should end with the type being extended

class SampleAttr : Attribute  // Noncompliant
{
}

class Foo : Exception // Noncompliant
{
}

Type that contains IDisposable members should implement IDisposable

Suggest replacing string.GetHashCode with StringComparer.Ordinal.GetHashCode

New rule: "NaN" should not be used in comparisons

if (a == double.NaN) // always return false, should be double.IsNaN(a)

New Rule: Call Where before OrderBy

enumerable.Where(...).OrderBy(...) // compliant
enumerable.OrderBy(...).Where(...) // non-compliant