Add analyzer and code fix to recommend against IHeaderDictionary.Add (#…

…44463)

* Add IHeaderDictionary.Add analyzer and code fix

* Change wording from "Disallow-" to "RecommendAgainst-"

* Change diagnostic rule ID

* Remove analyzer and code fix from AspNetCore.Analyzers project

* Add analyzer to Framework/AspNetCoreAnalyzers

* Add code fix to Framework/AspNetCoreAnalyzers

* Fix nullable reference type

* Fix diagnostics in the project caused by new analyzer

* Fix using directive insertion logic

* Fix StringComparison warning

* Update src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj

Co-authored-by: Safia Abdalla <[email protected]>

* Update src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs

Co-authored-by: Safia Abdalla <[email protected]>

* Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs

Co-authored-by: Safia Abdalla <[email protected]>

* Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs

Co-authored-by: Youssef Victor <[email protected]>

* Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs

Co-authored-by: Youssef Victor <[email protected]>

* Update code fix equivalence key referenced in tests

* Remove redundant test

* Merge analyzer and code fix tests into single test file

* Use top-level statements

* Add test cases for multiple diagnostics

* Add comment about IDictionary.Add to diagnostic message

* Update diagnostic message

* Move checks before code fix registration

* Pass true for getInnermostNodeForTie

* Perform symbol comparison for IHeaderDictionary

* Add using directive via syntax annotation

* Add editorconfig

* Revert "Add editorconfig"

This reverts commit 620275d.

* Skip test on Linux, macOS

Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: Youssef Victor <[email protected]>

src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
@@ -8,7 +8,6 @@
 
   <ItemGroup>
     <ProjectReference Include="$(RepoRoot)src\Analyzers\Analyzers\src\Microsoft.AspNetCore.Analyzers.csproj" />
-
     <Reference Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.XUnit" />
     <Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
     <Reference Include="Microsoft.Extensions.DependencyModel" />

src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs

@@ -151,4 +151,13 @@ internal static class DiagnosticDescriptors
         DiagnosticSeverity.Info,
         isEnabledByDefault: true,
         helpLinkUri: "https://aka.ms/aspnet/analyzers");
+
+    internal static readonly DiagnosticDescriptor DoNotUseIHeaderDictionaryAdd = new(
+        "ASP0019",
+        new LocalizableResourceString(nameof(Resources.Analyzer_HeaderDictionaryAdd_Title), Resources.ResourceManager, typeof(Resources)),
+        new LocalizableResourceString(nameof(Resources.Analyzer_HeaderDictionaryAdd_Message), Resources.ResourceManager, typeof(Resources)),
+        "Usage",
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        helpLinkUri: "https://aka.ms/aspnet/analyzers");
 }

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs

@@ -0,0 +1,92 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.AspNetCore.Analyzers.Http;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class HeaderDictionaryAddAnalyzer : DiagnosticAnalyzer
+{
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterCompilationStartAction(OnCompilationStart);
+    }
+
+    private static void OnCompilationStart(CompilationStartAnalysisContext context)
+    {
+        var symbols = new HeaderDictionarySymbols(context.Compilation);
+
+        if (!symbols.HasRequiredSymbols)
+        {
+            return;
+        }
+
+        context.RegisterOperationAction(context =>
+        {
+            var invocation = (IInvocationOperation)context.Operation;
+
+            if (SymbolEqualityComparer.Default.Equals(symbols.IHeaderDictionary, invocation.Instance?.Type)
+                && IsAddMethod(invocation.TargetMethod)
+                && invocation.TargetMethod.Parameters.Length == 2)
+            {
+                AddDiagnosticWarning(context, invocation.Syntax.GetLocation());
+            }
+
+        }, OperationKind.Invocation);
+    }
+
+    private static bool IsAddMethod(IMethodSymbol method)
+    {
+        return method is
+        {
+            Name: "Add",
+            ContainingType:
+            {
+                Name: "IDictionary",
+                ContainingNamespace:
+                {
+                    Name: "Generic",
+                    ContainingNamespace:
+                    {
+                        Name: "Collections",
+                        ContainingNamespace:
+                        {
+                            Name: "System",
+                            ContainingNamespace:
+                            {
+                                IsGlobalNamespace: true
+                            }
+                        }
+                    }
+                }
+            }
+        };
+    }
+
+    private static void AddDiagnosticWarning(OperationAnalysisContext context, Location location)
+    {
+        context.ReportDiagnostic(Diagnostic.Create(
+            DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd,
+            location));
+    }
+
+    private sealed class HeaderDictionarySymbols
+    {
+        public HeaderDictionarySymbols(Compilation compilation)
+        {
+            IHeaderDictionary = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary");
+        }
+
+        public bool HasRequiredSymbols => IHeaderDictionary is not null;
+
+        public INamedTypeSymbol IHeaderDictionary { get; }
+    }
+}

src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx

@@ -201,4 +201,10 @@
   <data name="Analyzer_UnusedParameter_Title" xml:space="preserve">
     <value>Unused route parameter</value>
   </data>
+  <data name="Analyzer_HeaderDictionaryAdd_Message" xml:space="preserve">
+    <value>Use IHeaderDictionary.Append or the indexer to append or set headers. IDictionary.Add will throw an ArgumentException when attempting to add a duplicate key.</value>
+  </data>
+  <data name="Analyzer_HeaderDictionaryAdd_Title" xml:space="preserve">
+    <value>Suggest using IHeaderDictionary.Append or the indexer</value>
+  </data>
 </root>

src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs

@@ -0,0 +1,129 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Immutable;
+using System.Composition;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Simplification;
+
+namespace Microsoft.AspNetCore.Analyzers.Http.Fixers;
+
+[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+public sealed class HeaderDictionaryAddFixer : CodeFixProvider
+{
+    public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd.Id);
+
+    public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+    public sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        foreach (var diagnostic in context.Diagnostics)
+        {
+            context.Document.TryGetSyntaxRoot(out var root);
+
+            if (CanReplaceWithAppend(diagnostic, root, out var invocation))
+            {
+                var appendTitle = "Use 'IHeaderDictionary.Append'";
+                context.RegisterCodeFix(
+                    CodeAction.Create(appendTitle,
+                        cancellationToken => ReplaceWithAppend(diagnostic, context.Document, invocation, cancellationToken),
+                        equivalenceKey: appendTitle),
+                    diagnostic);
+            }
+
+            if (CanReplaceWithIndexer(diagnostic, root, out var assignment))
+            {
+                var indexerTitle = "Use indexer";
+                context.RegisterCodeFix(
+                    CodeAction.Create(indexerTitle,
+                        cancellationToken => ReplaceWithIndexer(diagnostic, context.Document, assignment, cancellationToken),
+                        equivalenceKey: indexerTitle),
+                    diagnostic);
+            }
+        }
+
+        return Task.CompletedTask;
+    }
+
+    private static async Task<Document> ReplaceWithAppend(Diagnostic diagnostic, Document document, InvocationExpressionSyntax invocation, CancellationToken cancellationToken)
+    {
+        var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+
+        var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
+
+        var model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+        var headerDictionaryExtensionsSymbol = model.Compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HeaderDictionaryExtensions");
+        var annotation = new SyntaxAnnotation("SymbolId", DocumentationCommentId.CreateReferenceId(headerDictionaryExtensionsSymbol));
+
+        return document.WithSyntaxRoot(
+            root.ReplaceNode(diagnosticTarget, invocation.WithAdditionalAnnotations(Simplifier.AddImportsAnnotation, annotation)));
+    }
+
+    private static bool CanReplaceWithAppend(Diagnostic diagnostic, SyntaxNode root, out InvocationExpressionSyntax invocation)
+    {
+        invocation = null;
+
+        if (root is not CompilationUnitSyntax)
+        {
+            return false;
+        }
+
+        var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
+
+        if (diagnosticTarget is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name.Identifier: { } identifierToken } } invocationExpression)
+        {
+            invocation = invocationExpression.ReplaceToken(identifierToken, SyntaxFactory.Identifier("Append"));
+
+            return true;
+        }
+
+        return false;
+    }
+
+    private static async Task<Document> ReplaceWithIndexer(Diagnostic diagnostic, Document document, AssignmentExpressionSyntax assignment, CancellationToken cancellationToken)
+    {
+        var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+
+        var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
+
+        return document.WithSyntaxRoot(root.ReplaceNode(diagnosticTarget, assignment));
+    }
+
+    private static bool CanReplaceWithIndexer(Diagnostic diagnostic, SyntaxNode root, out AssignmentExpressionSyntax assignment)
+    {
+        assignment = null;
+
+        if (root is null)
+        {
+            return false;
+        }
+
+        var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
+
+        if (diagnosticTarget is InvocationExpressionSyntax
+            {
+                Expression: MemberAccessExpressionSyntax memberAccessExpression,
+                ArgumentList.Arguments: { Count: 2 } arguments
+            })
+        {
+            assignment =
+                SyntaxFactory.AssignmentExpression(
+                    SyntaxKind.SimpleAssignmentExpression,
+                    SyntaxFactory.ElementAccessExpression(
+                        memberAccessExpression.Expression,
+                        SyntaxFactory.BracketedArgumentList(
+                            SyntaxFactory.SeparatedList(new[] { arguments[0] }))),
+                    arguments[1].Expression);
+
+            return true;
+        }
+
+        return false;
+    }
+}