From 1373fbe45d9da3451722059d118882bdd67bc785 Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 10 Oct 2022 21:04:00 -0400 Subject: [PATCH 01/29] Add IHeaderDictionary.Add analyzer and code fix --- ...HeaderDictionaryAddAnalyzer.Diagnostics.cs | 28 +++ .../DisallowIHeaderDictionaryAddAnalyzer.cs | 69 +++++++ ...llowIHeaderDictionaryAddCodeFixProvider.cs | 170 ++++++++++++++++++ .../IHeaderDictionaryFacts.cs | 46 +++++ .../IHeaderDictionarySymbols.cs | 18 ++ .../src/IHeaderDictionary/SymbolNames.cs | 16 ++ .../src/Microsoft.AspNetCore.Analyzers.csproj | 1 + ...isallowIHeaderDictionaryAddAnalyzerTest.cs | 112 ++++++++++++ ...IHeaderDictionaryAddCodeFixProviderTest.cs | 103 +++++++++++ .../IHeaderDictionaryFactsTest.cs | 96 ++++++++++ .../IHeaderDictionary/TestAnalyzerRunner.cs | 43 +++++ ...Microsoft.AspNetCore.Analyzers.Test.csproj | 5 +- 12 files changed, 705 insertions(+), 2 deletions(-) create mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs create mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs create mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs create mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs create mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs create mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs create mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs create mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs create mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs create mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs new file mode 100644 index 000000000000..beb3885adf92 --- /dev/null +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs @@ -0,0 +1,28 @@ +// 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.Diagnostics; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +public partial class DisallowIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer +{ + internal static class Diagnostics + { + internal static readonly DiagnosticDescriptor DisallowIHeaderDictionaryAdd = new( + "ASP0008", + "Suggest using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", + "Suggest using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", + "Usage", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: "https://aka.ms/aspnet/analyzers"); + + public static readonly ImmutableArray SupportedDiagnostics = ImmutableArray.Create(new[] + { + DisallowIHeaderDictionaryAdd + }); + } +} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs new file mode 100644 index 000000000000..b613da0b3563 --- /dev/null +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs @@ -0,0 +1,69 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis; +using System.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public partial class DisallowIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics => Diagnostics.SupportedDiagnostics; + + public override void Initialize(AnalysisContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private void OnCompilationStart(CompilationStartAnalysisContext context) + { + var symbols = new IHeaderDictionarySymbols(context.Compilation); + + // Don't run analyzer if ASP.NET Core types cannot be found + if (!symbols.HasRequiredSymbols) + { + Debug.Fail("One or more types could not be found."); + return; + } + + var entryPoint = context.Compilation.GetEntryPoint(context.CancellationToken); + + context.RegisterOperationAction(context => + { + var invocation = (IInvocationOperation)context.Operation; + + if (IsAddInvocation(symbols, invocation)) + { + context.ReportDiagnostic( + Diagnostic.Create( + Diagnostics.DisallowIHeaderDictionaryAdd, + invocation.Syntax.GetLocation(), + invocation.Syntax.ToString())); + } + + }, OperationKind.Invocation); + } + + private static bool IsAddInvocation(IHeaderDictionarySymbols symbols, IInvocationOperation invocation) + { + if (invocation.Instance?.Type is not INamedTypeSymbol instanceTypeSymbol) + { + return false; + } + + return IHeaderDictionaryFacts.IsIHeaderDictionary(symbols, instanceTypeSymbol) + && IHeaderDictionaryFacts.IsAdd(symbols, invocation.TargetMethod); + } +} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs new file mode 100644 index 000000000000..b3bb2d5569cf --- /dev/null +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs @@ -0,0 +1,170 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +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.Editing; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public sealed class DisallowIHeaderDictionaryAddCodeFixProvider : CodeFixProvider +{ + public sealed override ImmutableArray FixableDiagnosticIds + => ImmutableArray.Create(DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (context.Diagnostics.Length != 1) + { + return Task.CompletedTask; + } + + var diagnostic = context.Diagnostics[0]; + if (diagnostic.Id != DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id) + { + return Task.CompletedTask; + } + + context.RegisterCodeFix(new UseAppendCodeAction(context.Document, context.Span), diagnostic); + context.RegisterCodeFix(new UseIndexerCodeAction(context.Document, context.Span), diagnostic); + + return Task.CompletedTask; + } + + private sealed class UseAppendCodeAction : CodeAction + { + private readonly Document _document; + private readonly TextSpan _invocationSpan; + + public UseAppendCodeAction(Document document, TextSpan invocationSpan) + { + _document = document; + _invocationSpan = invocationSpan; + } + + public override string EquivalenceKey => $"{DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id}.Append"; + + public override string Title => "Use Append"; + + protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) + { + var rootNode = await _document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false); + + var syntaxTree = await _document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); + var compilationUnitRoot = syntaxTree.GetCompilationUnitRoot(cancellationToken); + AddRequiredUsingDirectives(editor, compilationUnitRoot); + + var invocationExpressionSyntax = (InvocationExpressionSyntax)rootNode!.FindNode(_invocationSpan); + var addMethodNode = GetAddMethodIdentiferNode(invocationExpressionSyntax); + + var appendMethodNode = addMethodNode.ReplaceToken( + addMethodNode.Identifier, + SyntaxFactory.Identifier( + SymbolNames.IHeaderDictionary.AppendMethodName)); + + editor.ReplaceNode(addMethodNode, appendMethodNode); + + return editor.GetChangedDocument(); + } + + private static IdentifierNameSyntax GetAddMethodIdentiferNode(InvocationExpressionSyntax invocationExpressionSyntax) + { + return invocationExpressionSyntax.DescendantNodes() + .OfType() + .First() + .DescendantNodes() + .OfType() + .First(x => string.Equals(x.Identifier.ValueText, SymbolNames.IHeaderDictionary.AddMethodName, StringComparison.Ordinal)); + } + + private static void AddRequiredUsingDirectives(DocumentEditor editor, CompilationUnitSyntax compilationUnitSyntax) + { + if (!compilationUnitSyntax.Usings.Any(u => string.Equals(u.Name.ToString(), "Microsoft.AspNetCore.Http", StringComparison.Ordinal))) + { + // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. + var usingDirectiveSyntax = SyntaxFactory.UsingDirective(SyntaxFactory.IdentifierName("Microsoft.AspNetCore.Http")); + + var newCompilationUnitSyntax = + compilationUnitSyntax.WithUsings(new SyntaxList(usingDirectiveSyntax)); + + editor.ReplaceNode(compilationUnitSyntax, newCompilationUnitSyntax); + } + } + } + + private sealed class UseIndexerCodeAction : CodeAction + { + private readonly Document _document; + private readonly TextSpan _invocationSpan; + + public UseIndexerCodeAction(Document document, TextSpan invocationSpan) + { + _document = document; + _invocationSpan = invocationSpan; + } + + public override string EquivalenceKey => $"{DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id}.Indexer"; + + public override string Title => "Use Indexer"; + + protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) + { + var rootNode = await _document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false); + + var invocationExpressionSyntax = (InvocationExpressionSyntax)rootNode!.FindNode(_invocationSpan); + + var headerDictionaryNode = GetHeaderDictionaryNodes(invocationExpressionSyntax); + var (keyArgumentNode, valueArgumentNode) = GetArgumentNodes(invocationExpressionSyntax); + + var targetNode = + SyntaxFactory.ElementAccessExpression( + headerDictionaryNode, + SyntaxFactory.BracketedArgumentList( + SyntaxFactory.SeparatedList(new[] { keyArgumentNode }))); + + var indexerAssignmentExpression = + SyntaxFactory.AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + targetNode, + valueArgumentNode.Expression); ; + + editor.ReplaceNode(invocationExpressionSyntax, indexerAssignmentExpression); + + return editor.GetChangedDocument(); + } + + private static (ArgumentSyntax keyArgument, ArgumentSyntax valueArgument) GetArgumentNodes( + InvocationExpressionSyntax invocationExpressionSyntax) + { + var arguments = invocationExpressionSyntax.DescendantNodes() + .OfType() + .First() + .Arguments; + + return (arguments[0], arguments[1]); + } + + private static ExpressionSyntax GetHeaderDictionaryNodes(InvocationExpressionSyntax invocationExpressionSyntax) + { + return invocationExpressionSyntax.DescendantNodes() + .OfType() + .First() + .Expression; + } + } +} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs new file mode 100644 index 000000000000..a2f133278c7a --- /dev/null +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs @@ -0,0 +1,46 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +internal static class IHeaderDictionaryFacts +{ + public static bool IsIHeaderDictionary(IHeaderDictionarySymbols symbols, INamedTypeSymbol symbol) + { + if (symbols is null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + return SymbolEqualityComparer.Default.Equals(symbol, symbols.IHeaderDictionary); + } + + public static bool IsAdd(IHeaderDictionarySymbols symbols, IMethodSymbol symbol) + { + if (symbols is null) + { + throw new ArgumentNullException(nameof(symbols)); + } + + if (symbol.DeclaredAccessibility != Accessibility.Public) + { + return false; + } + + if (symbol.Name == null || + !string.Equals(symbol.Name, SymbolNames.IHeaderDictionary.AddMethodName, StringComparison.Ordinal)) + { + return false; + } + + if (symbol.Parameters.Length != 2) + { + return false; + } + + return true; + } +} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs new file mode 100644 index 000000000000..89c990180a52 --- /dev/null +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +internal sealed class IHeaderDictionarySymbols +{ + public IHeaderDictionarySymbols(Compilation compilation) + { + IHeaderDictionary = compilation.GetTypeByMetadataName(SymbolNames.IHeaderDictionary.MetadataName); + } + + public bool HasRequiredSymbols => IHeaderDictionary != null; + + public INamedTypeSymbol IHeaderDictionary { get; } +} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs new file mode 100644 index 000000000000..c405583ceac9 --- /dev/null +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +internal static class SymbolNames +{ + public static class IHeaderDictionary + { + public const string MetadataName = "Microsoft.AspNetCore.Http.IHeaderDictionary"; + + public const string AddMethodName = "Add"; + + public const string AppendMethodName = "Append"; + } +} diff --git a/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj b/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj index 01f8879aff83..46ea698109fc 100644 --- a/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj +++ b/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj @@ -20,6 +20,7 @@ + diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs new file mode 100644 index 000000000000..4c2c2cb2c1a7 --- /dev/null +++ b/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs @@ -0,0 +1,112 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Testing.Verifiers; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +public class DisallowIHeaderDictionaryAddAnalyzerTest +{ + [Fact] + public async Task Analyze_WithAdd_ReportsDiagnostics() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddAnalyzerTest; + +public class Test +{ + public static void Main() + { + var context = new DefaultHttpContext(); + {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; + } +} +"; + + var diagnosticResult = new DiagnosticResult(DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd) + .WithLocation(0); + + // Act + Assert + await VerifyAnalyzerAsync(source, diagnosticResult); + } + + [Fact] + public async Task Analyze_WithAppend_ReportsNoDiagnostics() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddAnalyzerTest; + +public class Test +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Append(""Accept"", ""text/html""); + } +} +"; + + // Act + Assert + await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); + } + + [Fact] + public async Task Analyze_WithIndexer_ReportsNoDiagnostics() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddAnalyzerTest; + +public class Test +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers[""Accept""] = ""text/html""; + } +} +"; + + // Act + Assert + await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); + } + + private static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new DisallowIHeaderDictionaryAddCSharpAnalyzerTest(new DisallowIHeaderDictionaryAddAnalyzer(), TestReferences.MetadataReferences) + { + TestCode = source, + ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies + }; + + test.ExpectedDiagnostics.AddRange(expected); + return test.RunAsync(); + } + + internal sealed class DisallowIHeaderDictionaryAddCSharpAnalyzerTest : CSharpAnalyzerTest + { + public DisallowIHeaderDictionaryAddCSharpAnalyzerTest(DisallowIHeaderDictionaryAddAnalyzer analyzer, ImmutableArray metadataReferences) + { + DisallowIHeaderDictionaryAddAnalyzer = analyzer; + TestState.OutputKind = OutputKind.WindowsApplication; + TestState.AdditionalReferences.AddRange(metadataReferences); + } + + public DisallowIHeaderDictionaryAddAnalyzer DisallowIHeaderDictionaryAddAnalyzer { get; } + + protected override IEnumerable GetDiagnosticAnalyzers() => new[] { DisallowIHeaderDictionaryAddAnalyzer }; + } +} diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs new file mode 100644 index 000000000000..c41e5bd6a10f --- /dev/null +++ b/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs @@ -0,0 +1,103 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Analyzer.Testing; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +public class DisallowIHeaderDictionaryAddCodeFixProviderTest +{ + [Fact] + public async Task CodeFix_ReplacesAddWithAppend() + { + var source = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; + +public class Test +{ + public void Method() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Add(""Accept"", ""text/html""); + } +} +"; + + var fixedSource = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; + +public class Test +{ + public void Method() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Append(""Accept"", ""text/html""); + } +} +"; + + await RunTest(source, 0, fixedSource); + } + + [Fact] + public async Task CodeFix_ReplacesAddWithIndexer() + { + var source = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; + +public class Test +{ + public void Method() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Add(""Accept"", ""text/html""); + } +} +"; + + var fixedSource = @" +using Microsoft.AspNetCore.Http; + +namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; + +public class Test +{ + public void Method() + { + var context = new DefaultHttpContext(); + context.Request.Headers[""Accept""] = ""text/html""; + } +} +"; + + await RunTest(source, 1, fixedSource); + } + + private async Task RunTest(string source, int codeFixIndex, string fixedSource) + { + // Arrange + var analyzerRunner = new TestAnalyzerRunner(new DisallowIHeaderDictionaryAddAnalyzer()); + var codeFixRunner = new CodeFixRunner(); + + var project = TestAnalyzerRunner.CreateProjectWithReferencesInBinDir(GetType().Assembly, new[] { source }); + var documentId = project.DocumentIds[0]; + + // Act + Assert + var diagnostics = await analyzerRunner.GetDiagnosticsAsync(project); + var diagnostic = Assert.Single(diagnostics); + + var actual = await codeFixRunner.ApplyCodeFixAsync( + new DisallowIHeaderDictionaryAddCodeFixProvider(), + project.GetDocument(documentId), + diagnostic, + codeFixIndex: codeFixIndex); + + Assert.Equal(fixedSource, actual, ignoreLineEndingDifferences: true); + } +} diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs new file mode 100644 index 000000000000..442a15ed31bd --- /dev/null +++ b/src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs @@ -0,0 +1,96 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +public class IHeaderDictionaryFactsTest +{ + private const string IHeaderDictionaryWithAddSource = @" +using Microsoft.AspNetCore.Http; + +namespace IHeaderDictionaryFactsTest; + +public class Test +{ + public void Method() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Add(""Accept"", ""text/html""); + } +} +"; + + [Fact] + public void IsIHeaderDictionary_FindsIHeaderDictionaryType() + { + // Arrange + var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); + + var symbols = new IHeaderDictionarySymbols(compilation); + var type = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary"); + + // Act + var result = IHeaderDictionaryFacts.IsIHeaderDictionary(symbols, type); + + // Arrange + Assert.True(result); + } + + [Fact] + public void IsIHeaderDictionary_RejectsNonIHeaderDictionaryType() + { + // Arrange + var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); + + var symbols = new IHeaderDictionarySymbols(compilation); + var type = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpContext.DefaultHttpContext"); + + // Act + var result = IHeaderDictionaryFacts.IsIHeaderDictionary(symbols, type); + + // Arrange + Assert.False(result); + } + + [Fact] + public void IsAdd_FindsAddMethod() + { + // Arrange + var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); + + var symbols = new IHeaderDictionarySymbols(compilation); + var method = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary").Interfaces + .Single(i => i.Name == "IDictionary") + .GetMembers("Add") + .Cast() + .Single(); + + // Act + var result = IHeaderDictionaryFacts.IsAdd(symbols, method); + + // Arrange + Assert.True(result); + } + + [Fact] + public void IsAdd_RejectsNonAddMethod() + { + // Arrange + var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); + + var symbols = new IHeaderDictionarySymbols(compilation); + var method = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary").Interfaces + .Single(i => i.Name == "IDictionary") + .GetMembers("Remove") + .Cast() + .Single(); + + // Act + var result = IHeaderDictionaryFacts.IsAdd(symbols, method); + + // Arrange + Assert.False(result); + } +} diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs new file mode 100644 index 000000000000..9fbe51c994a7 --- /dev/null +++ b/src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; +using Microsoft.AspNetCore.Analyzer.Testing; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; + +internal sealed class TestAnalyzerRunner : DiagnosticAnalyzerRunner +{ + public TestAnalyzerRunner(DiagnosticAnalyzer analyzer) + { + Analyzer = analyzer; + } + + public DiagnosticAnalyzer Analyzer { get; } + + public static Project CreateProjectWithReferencesInBinDir(Assembly testAssembly, params string[] source) + { + // The deps file in the project is incorrect and does not contain "compile" nodes for some references. + // However these binaries are always present in the bin output. As a "temporary" workaround, we'll add + // every dll file that's present in the test's build output as a metadatareference. + + var project = DiagnosticProject.Create(testAssembly, source); + + foreach (var assembly in Directory.EnumerateFiles(AppContext.BaseDirectory, "*.dll")) + { + if (!project.MetadataReferences.Any(c => string.Equals(Path.GetFileNameWithoutExtension(c.Display), Path.GetFileNameWithoutExtension(assembly), StringComparison.OrdinalIgnoreCase))) + { + project = project.AddMetadataReference(MetadataReference.CreateFromFile(assembly)); + } + } + + return project; + } + + public Task GetDiagnosticsAsync(Project project) + { + return GetDiagnosticsAsync(new[] { project }, Analyzer, Array.Empty()); + } +} diff --git a/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj b/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj index 9ef68dad7e29..112b738ec923 100644 --- a/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj +++ b/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj @@ -1,4 +1,4 @@ - + $(DefaultNetCoreTargetFramework) @@ -8,7 +8,8 @@ - + + From aabfef9c555cbea708ed6974ecae2fbb851459ed Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 10 Oct 2022 21:13:06 -0400 Subject: [PATCH 02/29] Change wording from "Disallow-" to "RecommendAgainst-" --- ...eaderDictionaryAddAnalyzer.Diagnostics.cs} | 10 ++++----- ...endAgainstIHeaderDictionaryAddAnalyzer.cs} | 4 ++-- ...nstIHeaderDictionaryAddCodeFixProvider.cs} | 10 ++++----- ...gainstIHeaderDictionaryAddAnalyzerTest.cs} | 22 +++++++++---------- ...HeaderDictionaryAddCodeFixProviderTest.cs} | 14 ++++++------ 5 files changed, 30 insertions(+), 30 deletions(-) rename src/Analyzers/Analyzers/src/IHeaderDictionary/{DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs => RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs} (61%) rename src/Analyzers/Analyzers/src/IHeaderDictionary/{DisallowIHeaderDictionaryAddAnalyzer.cs => RecommendAgainstIHeaderDictionaryAddAnalyzer.cs} (93%) rename src/Analyzers/Analyzers/src/IHeaderDictionary/{DisallowIHeaderDictionaryAddCodeFixProvider.cs => RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs} (90%) rename src/Analyzers/Analyzers/test/IHeaderDictionary/{DisallowIHeaderDictionaryAddAnalyzerTest.cs => RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs} (67%) rename src/Analyzers/Analyzers/test/IHeaderDictionary/{DisallowIHeaderDictionaryAddCodeFixProviderTest.cs => RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs} (80%) diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs similarity index 61% rename from src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs rename to src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs index beb3885adf92..d9029c7e4c41 100644 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.cs +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs @@ -7,14 +7,14 @@ namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; -public partial class DisallowIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer +public partial class RecommendAgainstIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer { internal static class Diagnostics { - internal static readonly DiagnosticDescriptor DisallowIHeaderDictionaryAdd = new( + internal static readonly DiagnosticDescriptor RecommendAgainstIHeaderDictionaryAdd = new( "ASP0008", - "Suggest using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", - "Suggest using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", + "Recommend using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", + "Recommend using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, @@ -22,7 +22,7 @@ internal static class Diagnostics public static readonly ImmutableArray SupportedDiagnostics = ImmutableArray.Create(new[] { - DisallowIHeaderDictionaryAdd + RecommendAgainstIHeaderDictionaryAdd }); } } diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs similarity index 93% rename from src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs rename to src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs index b613da0b3563..a5c18b18d109 100644 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzer.cs +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; [DiagnosticAnalyzer(LanguageNames.CSharp)] -public partial class DisallowIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer +public partial class RecommendAgainstIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer { public override ImmutableArray SupportedDiagnostics => Diagnostics.SupportedDiagnostics; @@ -48,7 +48,7 @@ private void OnCompilationStart(CompilationStartAnalysisContext context) { context.ReportDiagnostic( Diagnostic.Create( - Diagnostics.DisallowIHeaderDictionaryAdd, + Diagnostics.RecommendAgainstIHeaderDictionaryAdd, invocation.Syntax.GetLocation(), invocation.Syntax.ToString())); } diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs similarity index 90% rename from src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs rename to src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs index b3bb2d5569cf..690b97add3aa 100644 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProvider.cs +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs @@ -18,10 +18,10 @@ namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; [ExportCodeFixProvider(LanguageNames.CSharp), Shared] -public sealed class DisallowIHeaderDictionaryAddCodeFixProvider : CodeFixProvider +public sealed class RecommendAgainstIHeaderDictionaryAddCodeFixProvider : CodeFixProvider { public sealed override ImmutableArray FixableDiagnosticIds - => ImmutableArray.Create(DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id); + => ImmutableArray.Create(RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id); public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; @@ -33,7 +33,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) } var diagnostic = context.Diagnostics[0]; - if (diagnostic.Id != DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id) + if (diagnostic.Id != RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id) { return Task.CompletedTask; } @@ -55,7 +55,7 @@ public UseAppendCodeAction(Document document, TextSpan invocationSpan) _invocationSpan = invocationSpan; } - public override string EquivalenceKey => $"{DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id}.Append"; + public override string EquivalenceKey => $"{RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id}.Append"; public override string Title => "Use Append"; @@ -117,7 +117,7 @@ public UseIndexerCodeAction(Document document, TextSpan invocationSpan) _invocationSpan = invocationSpan; } - public override string EquivalenceKey => $"{DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd.Id}.Indexer"; + public override string EquivalenceKey => $"{RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id}.Indexer"; public override string Title => "Use Indexer"; diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs similarity index 67% rename from src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs rename to src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs index 4c2c2cb2c1a7..978eb4b9ac42 100644 --- a/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddAnalyzerTest.cs +++ b/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; -public class DisallowIHeaderDictionaryAddAnalyzerTest +public class RecommendAgainstIHeaderDictionaryAddAnalyzerTest { [Fact] public async Task Analyze_WithAdd_ReportsDiagnostics() @@ -19,7 +19,7 @@ public async Task Analyze_WithAdd_ReportsDiagnostics() var source = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddAnalyzerTest; +namespace RecommendAgainstIHeaderDictionaryAddAnalyzerTest; public class Test { @@ -31,7 +31,7 @@ public static void Main() } "; - var diagnosticResult = new DiagnosticResult(DisallowIHeaderDictionaryAddAnalyzer.Diagnostics.DisallowIHeaderDictionaryAdd) + var diagnosticResult = new DiagnosticResult(RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd) .WithLocation(0); // Act + Assert @@ -45,7 +45,7 @@ public async Task Analyze_WithAppend_ReportsNoDiagnostics() var source = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddAnalyzerTest; +namespace RecommendAgainstIHeaderDictionaryAddAnalyzerTest; public class Test { @@ -68,7 +68,7 @@ public async Task Analyze_WithIndexer_ReportsNoDiagnostics() var source = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddAnalyzerTest; +namespace RecommendAgainstIHeaderDictionaryAddAnalyzerTest; public class Test { @@ -86,7 +86,7 @@ public static void Main() private static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) { - var test = new DisallowIHeaderDictionaryAddCSharpAnalyzerTest(new DisallowIHeaderDictionaryAddAnalyzer(), TestReferences.MetadataReferences) + var test = new RecommendAgainstIHeaderDictionaryAddCSharpAnalyzerTest(new RecommendAgainstIHeaderDictionaryAddAnalyzer(), TestReferences.MetadataReferences) { TestCode = source, ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies @@ -96,17 +96,17 @@ private static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] return test.RunAsync(); } - internal sealed class DisallowIHeaderDictionaryAddCSharpAnalyzerTest : CSharpAnalyzerTest + internal sealed class RecommendAgainstIHeaderDictionaryAddCSharpAnalyzerTest : CSharpAnalyzerTest { - public DisallowIHeaderDictionaryAddCSharpAnalyzerTest(DisallowIHeaderDictionaryAddAnalyzer analyzer, ImmutableArray metadataReferences) + public RecommendAgainstIHeaderDictionaryAddCSharpAnalyzerTest(RecommendAgainstIHeaderDictionaryAddAnalyzer analyzer, ImmutableArray metadataReferences) { - DisallowIHeaderDictionaryAddAnalyzer = analyzer; + RecommendAgainstIHeaderDictionaryAddAnalyzer = analyzer; TestState.OutputKind = OutputKind.WindowsApplication; TestState.AdditionalReferences.AddRange(metadataReferences); } - public DisallowIHeaderDictionaryAddAnalyzer DisallowIHeaderDictionaryAddAnalyzer { get; } + public RecommendAgainstIHeaderDictionaryAddAnalyzer RecommendAgainstIHeaderDictionaryAddAnalyzer { get; } - protected override IEnumerable GetDiagnosticAnalyzers() => new[] { DisallowIHeaderDictionaryAddAnalyzer }; + protected override IEnumerable GetDiagnosticAnalyzers() => new[] { RecommendAgainstIHeaderDictionaryAddAnalyzer }; } } diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs similarity index 80% rename from src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs rename to src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs index c41e5bd6a10f..7ab42d26a5a4 100644 --- a/src/Analyzers/Analyzers/test/IHeaderDictionary/DisallowIHeaderDictionaryAddCodeFixProviderTest.cs +++ b/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs @@ -5,7 +5,7 @@ namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; -public class DisallowIHeaderDictionaryAddCodeFixProviderTest +public class RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest { [Fact] public async Task CodeFix_ReplacesAddWithAppend() @@ -13,7 +13,7 @@ public async Task CodeFix_ReplacesAddWithAppend() var source = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; +namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; public class Test { @@ -28,7 +28,7 @@ public void Method() var fixedSource = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; +namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; public class Test { @@ -49,7 +49,7 @@ public async Task CodeFix_ReplacesAddWithIndexer() var source = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; +namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; public class Test { @@ -64,7 +64,7 @@ public void Method() var fixedSource = @" using Microsoft.AspNetCore.Http; -namespace DisallowIHeaderDictionaryAddCodeFixProviderTest; +namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; public class Test { @@ -82,7 +82,7 @@ public void Method() private async Task RunTest(string source, int codeFixIndex, string fixedSource) { // Arrange - var analyzerRunner = new TestAnalyzerRunner(new DisallowIHeaderDictionaryAddAnalyzer()); + var analyzerRunner = new TestAnalyzerRunner(new RecommendAgainstIHeaderDictionaryAddAnalyzer()); var codeFixRunner = new CodeFixRunner(); var project = TestAnalyzerRunner.CreateProjectWithReferencesInBinDir(GetType().Assembly, new[] { source }); @@ -93,7 +93,7 @@ private async Task RunTest(string source, int codeFixIndex, string fixedSource) var diagnostic = Assert.Single(diagnostics); var actual = await codeFixRunner.ApplyCodeFixAsync( - new DisallowIHeaderDictionaryAddCodeFixProvider(), + new RecommendAgainstIHeaderDictionaryAddCodeFixProvider(), project.GetDocument(documentId), diagnostic, codeFixIndex: codeFixIndex); From eb91cd840190a6f7cc330b99591c46dd3a77be8e Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 10 Oct 2022 21:26:46 -0400 Subject: [PATCH 03/29] Change diagnostic rule ID --- .../RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs index d9029c7e4c41..3e9e0d95cbd2 100644 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs +++ b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs @@ -12,7 +12,7 @@ public partial class RecommendAgainstIHeaderDictionaryAddAnalyzer : DiagnosticAn internal static class Diagnostics { internal static readonly DiagnosticDescriptor RecommendAgainstIHeaderDictionaryAdd = new( - "ASP0008", + "ASP0015", "Recommend using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", "Recommend using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", "Usage", From 2bf902dda6c63846161467a510457d1485cfc101 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 17:40:46 -0400 Subject: [PATCH 04/29] Remove analyzer and code fix from AspNetCore.Analyzers project --- .../IHeaderDictionaryFacts.cs | 46 ----- .../IHeaderDictionarySymbols.cs | 18 -- ...HeaderDictionaryAddAnalyzer.Diagnostics.cs | 28 --- ...mendAgainstIHeaderDictionaryAddAnalyzer.cs | 69 ------- ...instIHeaderDictionaryAddCodeFixProvider.cs | 170 ------------------ .../src/IHeaderDictionary/SymbolNames.cs | 16 -- .../src/Microsoft.AspNetCore.Analyzers.csproj | 1 - .../IHeaderDictionaryFactsTest.cs | 96 ---------- ...AgainstIHeaderDictionaryAddAnalyzerTest.cs | 112 ------------ ...IHeaderDictionaryAddCodeFixProviderTest.cs | 103 ----------- .../IHeaderDictionary/TestAnalyzerRunner.cs | 43 ----- ...Microsoft.AspNetCore.Analyzers.Test.csproj | 1 - 12 files changed, 703 deletions(-) delete mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs delete mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs delete mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs delete mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs delete mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs delete mode 100644 src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs delete mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs delete mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs delete mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs delete mode 100644 src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs deleted file mode 100644 index a2f133278c7a..000000000000 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionaryFacts.cs +++ /dev/null @@ -1,46 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using Microsoft.CodeAnalysis; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -internal static class IHeaderDictionaryFacts -{ - public static bool IsIHeaderDictionary(IHeaderDictionarySymbols symbols, INamedTypeSymbol symbol) - { - if (symbols is null) - { - throw new ArgumentNullException(nameof(symbols)); - } - - return SymbolEqualityComparer.Default.Equals(symbol, symbols.IHeaderDictionary); - } - - public static bool IsAdd(IHeaderDictionarySymbols symbols, IMethodSymbol symbol) - { - if (symbols is null) - { - throw new ArgumentNullException(nameof(symbols)); - } - - if (symbol.DeclaredAccessibility != Accessibility.Public) - { - return false; - } - - if (symbol.Name == null || - !string.Equals(symbol.Name, SymbolNames.IHeaderDictionary.AddMethodName, StringComparison.Ordinal)) - { - return false; - } - - if (symbol.Parameters.Length != 2) - { - return false; - } - - return true; - } -} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs deleted file mode 100644 index 89c990180a52..000000000000 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/IHeaderDictionarySymbols.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.CodeAnalysis; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -internal sealed class IHeaderDictionarySymbols -{ - public IHeaderDictionarySymbols(Compilation compilation) - { - IHeaderDictionary = compilation.GetTypeByMetadataName(SymbolNames.IHeaderDictionary.MetadataName); - } - - public bool HasRequiredSymbols => IHeaderDictionary != null; - - public INamedTypeSymbol IHeaderDictionary { get; } -} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs deleted file mode 100644 index 3e9e0d95cbd2..000000000000 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.cs +++ /dev/null @@ -1,28 +0,0 @@ -// 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.Diagnostics; -using Microsoft.CodeAnalysis; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -public partial class RecommendAgainstIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer -{ - internal static class Diagnostics - { - internal static readonly DiagnosticDescriptor RecommendAgainstIHeaderDictionaryAdd = new( - "ASP0015", - "Recommend using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", - "Recommend using IHeaderDictionary.Append or the indexer over IHeaderDictionary.Add", - "Usage", - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - helpLinkUri: "https://aka.ms/aspnet/analyzers"); - - public static readonly ImmutableArray SupportedDiagnostics = ImmutableArray.Create(new[] - { - RecommendAgainstIHeaderDictionaryAdd - }); - } -} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs deleted file mode 100644 index a5c18b18d109..000000000000 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzer.cs +++ /dev/null @@ -1,69 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Immutable; -using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis; -using System.Diagnostics; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -[DiagnosticAnalyzer(LanguageNames.CSharp)] -public partial class RecommendAgainstIHeaderDictionaryAddAnalyzer : DiagnosticAnalyzer -{ - public override ImmutableArray SupportedDiagnostics => Diagnostics.SupportedDiagnostics; - - public override void Initialize(AnalysisContext context) - { - if (context is null) - { - throw new ArgumentNullException(nameof(context)); - } - - context.EnableConcurrentExecution(); - context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - context.RegisterCompilationStartAction(OnCompilationStart); - } - - private void OnCompilationStart(CompilationStartAnalysisContext context) - { - var symbols = new IHeaderDictionarySymbols(context.Compilation); - - // Don't run analyzer if ASP.NET Core types cannot be found - if (!symbols.HasRequiredSymbols) - { - Debug.Fail("One or more types could not be found."); - return; - } - - var entryPoint = context.Compilation.GetEntryPoint(context.CancellationToken); - - context.RegisterOperationAction(context => - { - var invocation = (IInvocationOperation)context.Operation; - - if (IsAddInvocation(symbols, invocation)) - { - context.ReportDiagnostic( - Diagnostic.Create( - Diagnostics.RecommendAgainstIHeaderDictionaryAdd, - invocation.Syntax.GetLocation(), - invocation.Syntax.ToString())); - } - - }, OperationKind.Invocation); - } - - private static bool IsAddInvocation(IHeaderDictionarySymbols symbols, IInvocationOperation invocation) - { - if (invocation.Instance?.Type is not INamedTypeSymbol instanceTypeSymbol) - { - return false; - } - - return IHeaderDictionaryFacts.IsIHeaderDictionary(symbols, instanceTypeSymbol) - && IHeaderDictionaryFacts.IsAdd(symbols, invocation.TargetMethod); - } -} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs deleted file mode 100644 index 690b97add3aa..000000000000 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProvider.cs +++ /dev/null @@ -1,170 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Immutable; -using System.Composition; -using System.Linq; -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.Editing; -using Microsoft.CodeAnalysis.Text; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -[ExportCodeFixProvider(LanguageNames.CSharp), Shared] -public sealed class RecommendAgainstIHeaderDictionaryAddCodeFixProvider : CodeFixProvider -{ - public sealed override ImmutableArray FixableDiagnosticIds - => ImmutableArray.Create(RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id); - - public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - - public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) - { - if (context.Diagnostics.Length != 1) - { - return Task.CompletedTask; - } - - var diagnostic = context.Diagnostics[0]; - if (diagnostic.Id != RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id) - { - return Task.CompletedTask; - } - - context.RegisterCodeFix(new UseAppendCodeAction(context.Document, context.Span), diagnostic); - context.RegisterCodeFix(new UseIndexerCodeAction(context.Document, context.Span), diagnostic); - - return Task.CompletedTask; - } - - private sealed class UseAppendCodeAction : CodeAction - { - private readonly Document _document; - private readonly TextSpan _invocationSpan; - - public UseAppendCodeAction(Document document, TextSpan invocationSpan) - { - _document = document; - _invocationSpan = invocationSpan; - } - - public override string EquivalenceKey => $"{RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id}.Append"; - - public override string Title => "Use Append"; - - protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) - { - var rootNode = await _document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false); - - var syntaxTree = await _document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); - var compilationUnitRoot = syntaxTree.GetCompilationUnitRoot(cancellationToken); - AddRequiredUsingDirectives(editor, compilationUnitRoot); - - var invocationExpressionSyntax = (InvocationExpressionSyntax)rootNode!.FindNode(_invocationSpan); - var addMethodNode = GetAddMethodIdentiferNode(invocationExpressionSyntax); - - var appendMethodNode = addMethodNode.ReplaceToken( - addMethodNode.Identifier, - SyntaxFactory.Identifier( - SymbolNames.IHeaderDictionary.AppendMethodName)); - - editor.ReplaceNode(addMethodNode, appendMethodNode); - - return editor.GetChangedDocument(); - } - - private static IdentifierNameSyntax GetAddMethodIdentiferNode(InvocationExpressionSyntax invocationExpressionSyntax) - { - return invocationExpressionSyntax.DescendantNodes() - .OfType() - .First() - .DescendantNodes() - .OfType() - .First(x => string.Equals(x.Identifier.ValueText, SymbolNames.IHeaderDictionary.AddMethodName, StringComparison.Ordinal)); - } - - private static void AddRequiredUsingDirectives(DocumentEditor editor, CompilationUnitSyntax compilationUnitSyntax) - { - if (!compilationUnitSyntax.Usings.Any(u => string.Equals(u.Name.ToString(), "Microsoft.AspNetCore.Http", StringComparison.Ordinal))) - { - // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. - var usingDirectiveSyntax = SyntaxFactory.UsingDirective(SyntaxFactory.IdentifierName("Microsoft.AspNetCore.Http")); - - var newCompilationUnitSyntax = - compilationUnitSyntax.WithUsings(new SyntaxList(usingDirectiveSyntax)); - - editor.ReplaceNode(compilationUnitSyntax, newCompilationUnitSyntax); - } - } - } - - private sealed class UseIndexerCodeAction : CodeAction - { - private readonly Document _document; - private readonly TextSpan _invocationSpan; - - public UseIndexerCodeAction(Document document, TextSpan invocationSpan) - { - _document = document; - _invocationSpan = invocationSpan; - } - - public override string EquivalenceKey => $"{RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd.Id}.Indexer"; - - public override string Title => "Use Indexer"; - - protected override async Task GetChangedDocumentAsync(CancellationToken cancellationToken) - { - var rootNode = await _document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false); - - var invocationExpressionSyntax = (InvocationExpressionSyntax)rootNode!.FindNode(_invocationSpan); - - var headerDictionaryNode = GetHeaderDictionaryNodes(invocationExpressionSyntax); - var (keyArgumentNode, valueArgumentNode) = GetArgumentNodes(invocationExpressionSyntax); - - var targetNode = - SyntaxFactory.ElementAccessExpression( - headerDictionaryNode, - SyntaxFactory.BracketedArgumentList( - SyntaxFactory.SeparatedList(new[] { keyArgumentNode }))); - - var indexerAssignmentExpression = - SyntaxFactory.AssignmentExpression( - SyntaxKind.SimpleAssignmentExpression, - targetNode, - valueArgumentNode.Expression); ; - - editor.ReplaceNode(invocationExpressionSyntax, indexerAssignmentExpression); - - return editor.GetChangedDocument(); - } - - private static (ArgumentSyntax keyArgument, ArgumentSyntax valueArgument) GetArgumentNodes( - InvocationExpressionSyntax invocationExpressionSyntax) - { - var arguments = invocationExpressionSyntax.DescendantNodes() - .OfType() - .First() - .Arguments; - - return (arguments[0], arguments[1]); - } - - private static ExpressionSyntax GetHeaderDictionaryNodes(InvocationExpressionSyntax invocationExpressionSyntax) - { - return invocationExpressionSyntax.DescendantNodes() - .OfType() - .First() - .Expression; - } - } -} diff --git a/src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs b/src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs deleted file mode 100644 index c405583ceac9..000000000000 --- a/src/Analyzers/Analyzers/src/IHeaderDictionary/SymbolNames.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -internal static class SymbolNames -{ - public static class IHeaderDictionary - { - public const string MetadataName = "Microsoft.AspNetCore.Http.IHeaderDictionary"; - - public const string AddMethodName = "Add"; - - public const string AppendMethodName = "Append"; - } -} diff --git a/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj b/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj index 46ea698109fc..01f8879aff83 100644 --- a/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj +++ b/src/Analyzers/Analyzers/src/Microsoft.AspNetCore.Analyzers.csproj @@ -20,7 +20,6 @@ - diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs deleted file mode 100644 index 442a15ed31bd..000000000000 --- a/src/Analyzers/Analyzers/test/IHeaderDictionary/IHeaderDictionaryFactsTest.cs +++ /dev/null @@ -1,96 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.CodeAnalysis; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -public class IHeaderDictionaryFactsTest -{ - private const string IHeaderDictionaryWithAddSource = @" -using Microsoft.AspNetCore.Http; - -namespace IHeaderDictionaryFactsTest; - -public class Test -{ - public void Method() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Add(""Accept"", ""text/html""); - } -} -"; - - [Fact] - public void IsIHeaderDictionary_FindsIHeaderDictionaryType() - { - // Arrange - var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); - - var symbols = new IHeaderDictionarySymbols(compilation); - var type = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary"); - - // Act - var result = IHeaderDictionaryFacts.IsIHeaderDictionary(symbols, type); - - // Arrange - Assert.True(result); - } - - [Fact] - public void IsIHeaderDictionary_RejectsNonIHeaderDictionaryType() - { - // Arrange - var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); - - var symbols = new IHeaderDictionarySymbols(compilation); - var type = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpContext.DefaultHttpContext"); - - // Act - var result = IHeaderDictionaryFacts.IsIHeaderDictionary(symbols, type); - - // Arrange - Assert.False(result); - } - - [Fact] - public void IsAdd_FindsAddMethod() - { - // Arrange - var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); - - var symbols = new IHeaderDictionarySymbols(compilation); - var method = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary").Interfaces - .Single(i => i.Name == "IDictionary") - .GetMembers("Add") - .Cast() - .Single(); - - // Act - var result = IHeaderDictionaryFacts.IsAdd(symbols, method); - - // Arrange - Assert.True(result); - } - - [Fact] - public void IsAdd_RejectsNonAddMethod() - { - // Arrange - var compilation = TestCompilation.Create(IHeaderDictionaryWithAddSource); - - var symbols = new IHeaderDictionarySymbols(compilation); - var method = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary").Interfaces - .Single(i => i.Name == "IDictionary") - .GetMembers("Remove") - .Cast() - .Single(); - - // Act - var result = IHeaderDictionaryFacts.IsAdd(symbols, method); - - // Arrange - Assert.False(result); - } -} diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs deleted file mode 100644 index 978eb4b9ac42..000000000000 --- a/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddAnalyzerTest.cs +++ /dev/null @@ -1,112 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.CodeAnalysis.CSharp.Testing; -using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis; -using System.Collections.Immutable; -using Microsoft.CodeAnalysis.Testing; -using Microsoft.CodeAnalysis.Testing.Verifiers; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -public class RecommendAgainstIHeaderDictionaryAddAnalyzerTest -{ - [Fact] - public async Task Analyze_WithAdd_ReportsDiagnostics() - { - // Arrange - var source = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddAnalyzerTest; - -public class Test -{ - public static void Main() - { - var context = new DefaultHttpContext(); - {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; - } -} -"; - - var diagnosticResult = new DiagnosticResult(RecommendAgainstIHeaderDictionaryAddAnalyzer.Diagnostics.RecommendAgainstIHeaderDictionaryAdd) - .WithLocation(0); - - // Act + Assert - await VerifyAnalyzerAsync(source, diagnosticResult); - } - - [Fact] - public async Task Analyze_WithAppend_ReportsNoDiagnostics() - { - // Arrange - var source = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddAnalyzerTest; - -public class Test -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Append(""Accept"", ""text/html""); - } -} -"; - - // Act + Assert - await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); - } - - [Fact] - public async Task Analyze_WithIndexer_ReportsNoDiagnostics() - { - // Arrange - var source = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddAnalyzerTest; - -public class Test -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers[""Accept""] = ""text/html""; - } -} -"; - - // Act + Assert - await VerifyAnalyzerAsync(source, DiagnosticResult.EmptyDiagnosticResults); - } - - private static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) - { - var test = new RecommendAgainstIHeaderDictionaryAddCSharpAnalyzerTest(new RecommendAgainstIHeaderDictionaryAddAnalyzer(), TestReferences.MetadataReferences) - { - TestCode = source, - ReferenceAssemblies = TestReferences.EmptyReferenceAssemblies - }; - - test.ExpectedDiagnostics.AddRange(expected); - return test.RunAsync(); - } - - internal sealed class RecommendAgainstIHeaderDictionaryAddCSharpAnalyzerTest : CSharpAnalyzerTest - { - public RecommendAgainstIHeaderDictionaryAddCSharpAnalyzerTest(RecommendAgainstIHeaderDictionaryAddAnalyzer analyzer, ImmutableArray metadataReferences) - { - RecommendAgainstIHeaderDictionaryAddAnalyzer = analyzer; - TestState.OutputKind = OutputKind.WindowsApplication; - TestState.AdditionalReferences.AddRange(metadataReferences); - } - - public RecommendAgainstIHeaderDictionaryAddAnalyzer RecommendAgainstIHeaderDictionaryAddAnalyzer { get; } - - protected override IEnumerable GetDiagnosticAnalyzers() => new[] { RecommendAgainstIHeaderDictionaryAddAnalyzer }; - } -} diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs deleted file mode 100644 index 7ab42d26a5a4..000000000000 --- a/src/Analyzers/Analyzers/test/IHeaderDictionary/RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest.cs +++ /dev/null @@ -1,103 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.AspNetCore.Analyzer.Testing; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -public class RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest -{ - [Fact] - public async Task CodeFix_ReplacesAddWithAppend() - { - var source = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; - -public class Test -{ - public void Method() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Add(""Accept"", ""text/html""); - } -} -"; - - var fixedSource = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; - -public class Test -{ - public void Method() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Append(""Accept"", ""text/html""); - } -} -"; - - await RunTest(source, 0, fixedSource); - } - - [Fact] - public async Task CodeFix_ReplacesAddWithIndexer() - { - var source = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; - -public class Test -{ - public void Method() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Add(""Accept"", ""text/html""); - } -} -"; - - var fixedSource = @" -using Microsoft.AspNetCore.Http; - -namespace RecommendAgainstIHeaderDictionaryAddCodeFixProviderTest; - -public class Test -{ - public void Method() - { - var context = new DefaultHttpContext(); - context.Request.Headers[""Accept""] = ""text/html""; - } -} -"; - - await RunTest(source, 1, fixedSource); - } - - private async Task RunTest(string source, int codeFixIndex, string fixedSource) - { - // Arrange - var analyzerRunner = new TestAnalyzerRunner(new RecommendAgainstIHeaderDictionaryAddAnalyzer()); - var codeFixRunner = new CodeFixRunner(); - - var project = TestAnalyzerRunner.CreateProjectWithReferencesInBinDir(GetType().Assembly, new[] { source }); - var documentId = project.DocumentIds[0]; - - // Act + Assert - var diagnostics = await analyzerRunner.GetDiagnosticsAsync(project); - var diagnostic = Assert.Single(diagnostics); - - var actual = await codeFixRunner.ApplyCodeFixAsync( - new RecommendAgainstIHeaderDictionaryAddCodeFixProvider(), - project.GetDocument(documentId), - diagnostic, - codeFixIndex: codeFixIndex); - - Assert.Equal(fixedSource, actual, ignoreLineEndingDifferences: true); - } -} diff --git a/src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs b/src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs deleted file mode 100644 index 9fbe51c994a7..000000000000 --- a/src/Analyzers/Analyzers/test/IHeaderDictionary/TestAnalyzerRunner.cs +++ /dev/null @@ -1,43 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Reflection; -using Microsoft.AspNetCore.Analyzer.Testing; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace Microsoft.AspNetCore.Analyzers.IHeaderDictionary; - -internal sealed class TestAnalyzerRunner : DiagnosticAnalyzerRunner -{ - public TestAnalyzerRunner(DiagnosticAnalyzer analyzer) - { - Analyzer = analyzer; - } - - public DiagnosticAnalyzer Analyzer { get; } - - public static Project CreateProjectWithReferencesInBinDir(Assembly testAssembly, params string[] source) - { - // The deps file in the project is incorrect and does not contain "compile" nodes for some references. - // However these binaries are always present in the bin output. As a "temporary" workaround, we'll add - // every dll file that's present in the test's build output as a metadatareference. - - var project = DiagnosticProject.Create(testAssembly, source); - - foreach (var assembly in Directory.EnumerateFiles(AppContext.BaseDirectory, "*.dll")) - { - if (!project.MetadataReferences.Any(c => string.Equals(Path.GetFileNameWithoutExtension(c.Display), Path.GetFileNameWithoutExtension(assembly), StringComparison.OrdinalIgnoreCase))) - { - project = project.AddMetadataReference(MetadataReference.CreateFromFile(assembly)); - } - } - - return project; - } - - public Task GetDiagnosticsAsync(Project project) - { - return GetDiagnosticsAsync(new[] { project }, Analyzer, Array.Empty()); - } -} diff --git a/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj b/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj index 112b738ec923..9e78bc9f6c53 100644 --- a/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj +++ b/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj @@ -8,7 +8,6 @@ - From 88625b2a3575145a4aaa466ab64e4a4366738e88 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 17:46:58 -0400 Subject: [PATCH 05/29] Add analyzer to Framework/AspNetCoreAnalyzers --- .../src/Analyzers/DiagnosticDescriptors.cs | 9 ++ .../Http/HeaderDictionaryAddAnalyzer.cs | 96 +++++++++++++++++++ .../src/Analyzers/Resources.resx | 6 ++ .../Http/HeaderDictionaryAddAnalyzerTests.cs | 65 +++++++++++++ 4 files changed, 176 insertions(+) create mode 100644 src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs index d9b830b01e3b..bb01a4919baa 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs +++ b/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"); } diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs new file mode 100644 index 000000000000..c577bff8b72e --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs @@ -0,0 +1,96 @@ +// 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 partial class HeaderDictionaryAddAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterOperationAction(context => + { + var invocation = (IInvocationOperation)context.Operation; + + if (invocation.Instance?.Type is INamedTypeSymbol type && + IsIHeadersDictionaryType(type)) + { + if (invocation.TargetMethod.Parameters.Length == 2 && + IsAddMethod(invocation.TargetMethod)) + { + AddDiagnosticWarning(context, invocation.Syntax.GetLocation()); + } + } + }, OperationKind.Invocation); + } + + private static bool IsIHeadersDictionaryType(INamedTypeSymbol type) + { + // Only IHeaderDictionary is valid. Types like HeaderDictionary, which implement IHeaderDictionary, + // can't access header properties unless cast as IHeaderDictionary. + return type is + { + Name: "IHeaderDictionary", + ContainingNamespace: + { + Name: "Http", + ContainingNamespace: + { + Name: "AspNetCore", + ContainingNamespace: + { + Name: "Microsoft", + ContainingNamespace: + { + IsGlobalNamespace: true + } + } + } + } + }; + } + + 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)); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 09b7e852e242..155d95572878 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -201,4 +201,10 @@ Unused route parameter + + Suggest using IHeaderDictionary.Append or the indexer instead of Add + + + Suggest using IHeaderDictionary.Append or the indexer + \ No newline at end of file diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs new file mode 100644 index 000000000000..9f1f58f65634 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs @@ -0,0 +1,65 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis.Testing; +using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpAnalyzerVerifier< + Microsoft.AspNetCore.Analyzers.Http.HeaderDictionaryAddAnalyzer>; + +namespace Microsoft.AspNetCore.Analyzers.Http; + +public class HeaderDictionaryAddAnalyzerTests +{ + [Fact] + public async Task IHeaderDictionary_WithAdd_ReportsDiagnostics() + { + // Arrange & Act & Assert + await VerifyCS.VerifyAnalyzerAsync(@" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddAnalyzerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; + } +}", + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message)); + } + + [Fact] + public async Task IHeaderDictionary_WithAppend_NoDiagnostics() + { + // Arrange & Act & Assert + await VerifyCS.VerifyAnalyzerAsync(@" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddAnalyzerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Append(""Accept"", ""text/html""); + } +}"); + } + + [Fact] + public async Task IHeaderDictionary_WithIndexer_NoDiagnostics() + { + // Arrange & Act & Assert + await VerifyCS.VerifyAnalyzerAsync(@" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddAnalyzerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers[""Accept""] = ""text/html""; + } +}"); + } +} From dd3612712e27d27f997a8caec40be7f9c34b5e37 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 18:37:59 -0400 Subject: [PATCH 06/29] Add code fix to Framework/AspNetCoreAnalyzers --- .../Http/HeaderDictionaryAddFixer.cs | 143 +++++++++++ .../Http/HeaderDictionaryAddFixerTests.cs | 229 ++++++++++++++++++ .../test/Verifiers/CSharpCodeFixVerifier.cs | 3 +- 3 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs new file mode 100644 index 000000000000..cf4899caf95c --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -0,0 +1,143 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +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; + +namespace Microsoft.AspNetCore.Analyzers.Http.Fixers; + +[ExportCodeFixProvider(LanguageNames.CSharp), Shared] +public class HeaderDictionaryAddFixer : CodeFixProvider +{ + public override ImmutableArray 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) + { + var appendTitle = "Use IHeaderDictionary.Append"; + context.RegisterCodeFix( + CodeAction.Create(appendTitle, + cancellationToken => ReplaceAddWithAppendAsync(diagnostic, context.Document, cancellationToken), + equivalenceKey: appendTitle), + diagnostic); + + var indexerTitle = "Use the indexer"; + context.RegisterCodeFix( + CodeAction.Create(indexerTitle, + cancellationToken => ReplaceAddWithIndexerAsync(diagnostic, context.Document, cancellationToken), + equivalenceKey: indexerTitle), + diagnostic); + } + + return Task.CompletedTask; + } + + private static async Task ReplaceAddWithAppendAsync(Diagnostic diagnostic, Document document, CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is not CompilationUnitSyntax compilationUnitSyntax) + { + return document; + } + + var invocation = compilationUnitSyntax.FindNode(diagnostic.Location.SourceSpan); + + if (invocation is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name.Identifier: { } identifierToken } }) + { + compilationUnitSyntax = compilationUnitSyntax.ReplaceToken(identifierToken, SyntaxFactory.Identifier("Append")); + + // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. + // We'll need to add the required using directive, when not already present. + compilationUnitSyntax = AddRequiredUsingDirectiveForAppend(compilationUnitSyntax); + + return document.WithSyntaxRoot(compilationUnitSyntax); + } + + return document; + } + + private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(CompilationUnitSyntax compilationUnitSyntax) + { + var usingDirectives = compilationUnitSyntax.Usings; + + var includesRequiredUsingDirective = false; + var insertionIndex = 0; + + for (var i = 0; i < usingDirectives.Count; i++) + { + var result = string.Compare( + "Microsoft.AspNetCore.Http", + usingDirectives[i].Name.ToString(), + StringComparison.Ordinal); + + if (result == 0) + { + includesRequiredUsingDirective = true; + break; + } + + if (result < 0) + { + insertionIndex = i; + } + } + + if (includesRequiredUsingDirective) + { + return compilationUnitSyntax; + } + + var requiredUsingDirective = + SyntaxFactory.UsingDirective( + SyntaxFactory.QualifiedName( + SyntaxFactory.QualifiedName( + SyntaxFactory.IdentifierName("Microsoft"), + SyntaxFactory.IdentifierName("AspNetCore")), + SyntaxFactory.IdentifierName("Http"))); + + return compilationUnitSyntax.WithUsings( + usingDirectives.Insert(insertionIndex, requiredUsingDirective)); + } + + private static async Task ReplaceAddWithIndexerAsync(Diagnostic diagnostic, Document document, CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root == null) + { + return document; + } + + var invocation = root.FindNode(diagnostic.Location.SourceSpan); + + if (invocation is InvocationExpressionSyntax + { + Expression: MemberAccessExpressionSyntax memberAccessExpression, + ArgumentList.Arguments: { Count: 2 } arguments + }) + { + var assignmentExpression = + SyntaxFactory.AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + SyntaxFactory.ElementAccessExpression( + memberAccessExpression.Expression, + SyntaxFactory.BracketedArgumentList( + SyntaxFactory.SeparatedList(new[] { arguments[0] }))), + arguments[1].Expression); + + return document.WithSyntaxRoot(root.ReplaceNode(invocation, assignmentExpression)); + } + + return document; + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs new file mode 100644 index 000000000000..9486ce363e1b --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs @@ -0,0 +1,229 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.CodeAnalysis.Testing; +using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpCodeFixVerifier< + Microsoft.AspNetCore.Analyzers.Http.HeaderDictionaryAddAnalyzer, + Microsoft.AspNetCore.Analyzers.Http.Fixers.HeaderDictionaryAddFixer>; + +namespace Microsoft.AspNetCore.Analyzers.Http; + +public class HeaderDictionaryAddFixerTests +{ + private const string AppendCodeActionEquivalenceKey = "Use IHeaderDictionary.Append"; + private const string IndexerCodeActionEquivalenceKey = "Use the indexer"; + + [Fact] + public async Task IHeaderDictionary_WithAdd_FixedWithAppend() + { + // Arrange & Act & Assert + await VerifyCS.VerifyCodeFixAsync(@" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddFixerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; + } +}", + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddFixerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Append(""Accept"", ""text/html""); + } +}", + codeActionEquivalenceKey: AppendCodeActionEquivalenceKey); + } + + public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() + { + yield return new[] + { + @" +using Microsoft.AspNetCore.Mvc; +namespace HeaderDictionaryAddFixerTests; +public class TestController : ControllerBase +{ + public IActionResult Endpoint() + { + {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; + return Ok(); + } +} + +public class Program +{ + public static void Main() + { + } +}", + @" +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +namespace HeaderDictionaryAddFixerTests; +public class TestController : ControllerBase +{ + public IActionResult Endpoint() + { + Response.Headers.Append(""Content-Type"", ""text/html""); + return Ok(); + } +} + +public class Program +{ + public static void Main() + { + } +}" + }; + + yield return new[] + { + @" +using System.Collections.Generic; +using Microsoft.AspNetCore.Mvc; +namespace HeaderDictionaryAddFixerTests; +public class TestController : ControllerBase +{ + public IActionResult Endpoint() + { + {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; + return Ok(new List()); + } +} + +public class Program +{ + public static void Main() + { + } +}", + @" +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +namespace HeaderDictionaryAddFixerTests; +public class TestController : ControllerBase +{ + public IActionResult Endpoint() + { + Response.Headers.Append(""Content-Type"", ""text/html""); + return Ok(new List()); + } +} + +public class Program +{ + public static void Main() + { + } +}" + }; + + yield return new[] + { + @" +namespace HeaderDictionaryAddFixerTests; +public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase +{ + public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() + { + {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; + return Ok(); + } +} + +public class Program +{ + public static void Main() + { + } +}", + @" +using Microsoft.AspNetCore.Http; + +namespace HeaderDictionaryAddFixerTests; +public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase +{ + public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() + { + Response.Headers.Append(""Content-Type"", ""text/html""); + return Ok(); + } +} + +public class Program +{ + public static void Main() + { + } +}" + }; + } + + [Theory] + [MemberData(nameof(FixedWithAppendAddsUsingDirectiveTestData))] + public async Task IHeaderDictionary_WithAdd_FixedWithAppend_AddsUsingDirective(string source, string fixedSource) + { + // Arrange & Act & Assert + await VerifyCS.VerifyCodeFixAsync( + source.TrimStart(), + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + fixedSource.TrimStart(), + codeActionEquivalenceKey: AppendCodeActionEquivalenceKey); + } + + [Fact] + public async Task IHeaderDictionary_WithAdd_FixedWithIndexer() + { + // Arrange & Act & Assert + await VerifyCS.VerifyCodeFixAsync(@" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddFixerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; + } +}", + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddFixerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers[""Accept""] = ""text/html""; + } +}", + codeActionEquivalenceKey: IndexerCodeActionEquivalenceKey); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs index 1a0bd0874dbb..a5a7c4ebddcd 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs @@ -52,7 +52,7 @@ public static async Task VerifyCodeFixAsync(string source, DiagnosticResult expe => await VerifyCodeFixAsync(source, new[] { expected }, fixedSource); /// - public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource, int? expectedIterations = null, string usageSource = null) + public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource, int? expectedIterations = null, string usageSource = null, string? codeActionEquivalenceKey = null) { var test = new CSharpCodeFixTest { @@ -63,6 +63,7 @@ public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] ex FixedState = { Sources = { fixedSource } }, ReferenceAssemblies = CSharpAnalyzerVerifier.GetReferenceAssemblies(), NumberOfFixAllIterations = expectedIterations, + CodeActionEquivalenceKey = codeActionEquivalenceKey }; if (usageSource != null) From e6190b77b6912c3d3f8e99386c6250529b1fd434 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 19:11:00 -0400 Subject: [PATCH 07/29] Fix nullable reference type --- .../AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs index a5a7c4ebddcd..124b40083885 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Verifiers/CSharpCodeFixVerifier.cs @@ -52,7 +52,7 @@ public static async Task VerifyCodeFixAsync(string source, DiagnosticResult expe => await VerifyCodeFixAsync(source, new[] { expected }, fixedSource); /// - public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource, int? expectedIterations = null, string usageSource = null, string? codeActionEquivalenceKey = null) + public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource, int? expectedIterations = null, string usageSource = null, string codeActionEquivalenceKey = null) { var test = new CSharpCodeFixTest { From 049bde0addc463e9a8a402fdedf790621cbef5b3 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 19:16:50 -0400 Subject: [PATCH 08/29] Fix diagnostics in the project caused by new analyzer --- .../JsonTranscodingServerCallContextTests.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/JsonTranscodingServerCallContextTests.cs b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/JsonTranscodingServerCallContextTests.cs index 57e541206348..c2ae97a78a1e 100644 --- a/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/JsonTranscodingServerCallContextTests.cs +++ b/src/Grpc/JsonTranscoding/test/Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests/JsonTranscodingServerCallContextTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Net; -using Google.Protobuf.Reflection; using Grpc.AspNetCore.Server; using Grpc.Core; using Grpc.Shared; @@ -38,11 +37,11 @@ public void RequestHeaders_Get_PopulatedFromHttpContext() { // Arrange var httpContext = CreateHttpContext(); - httpContext.Request.Headers.Add("TestName", "TestValue"); - httpContext.Request.Headers.Add(":method", "GET"); - httpContext.Request.Headers.Add("grpc-encoding", "identity"); - httpContext.Request.Headers.Add("grpc-timeout", "1S"); - httpContext.Request.Headers.Add("hello-bin", Convert.ToBase64String(new byte[] { 1, 2, 3 })); + httpContext.Request.Headers.Append("TestName", "TestValue"); + httpContext.Request.Headers.Append(":method", "GET"); + httpContext.Request.Headers.Append("grpc-encoding", "identity"); + httpContext.Request.Headers.Append("grpc-timeout", "1S"); + httpContext.Request.Headers.Append("hello-bin", Convert.ToBase64String(new byte[] { 1, 2, 3 })); var serverCallContext = CreateServerCallContext(httpContext); // Act From 1577df71c8ed75e426797e88b8d0e3c17e36a8d8 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 21:46:54 -0400 Subject: [PATCH 09/29] Fix using directive insertion logic --- .../Http/HeaderDictionaryAddFixer.cs | 15 ++++-- .../Http/HeaderDictionaryAddFixerTests.cs | 47 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index cf4899caf95c..80af14576be7 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -76,10 +76,16 @@ private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(Compilat for (var i = 0; i < usingDirectives.Count; i++) { - var result = string.Compare( - "Microsoft.AspNetCore.Http", - usingDirectives[i].Name.ToString(), - StringComparison.Ordinal); + var namespaceName = usingDirectives[i].Name.ToString(); + + // Always insert the new using directive after any 'System' using directives. + if (namespaceName.StartsWith("System")) + { + insertionIndex = i + 1; + continue; + } + + var result = string.Compare("Microsoft.AspNetCore.Http", namespaceName, StringComparison.Ordinal); if (result == 0) { @@ -90,6 +96,7 @@ private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(Compilat if (result < 0) { insertionIndex = i; + break; } } diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs index 9486ce363e1b..27b8ee465f83 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs @@ -54,6 +54,7 @@ public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() { @" using Microsoft.AspNetCore.Mvc; + namespace HeaderDictionaryAddFixerTests; public class TestController : ControllerBase { @@ -73,6 +74,7 @@ public static void Main() @" using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; + namespace HeaderDictionaryAddFixerTests; public class TestController : ControllerBase { @@ -96,6 +98,7 @@ public static void Main() @" using System.Collections.Generic; using Microsoft.AspNetCore.Mvc; + namespace HeaderDictionaryAddFixerTests; public class TestController : ControllerBase { @@ -116,6 +119,7 @@ public static void Main() using System.Collections.Generic; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; + namespace HeaderDictionaryAddFixerTests; public class TestController : ControllerBase { @@ -166,6 +170,49 @@ public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() } } +public class Program +{ + public static void Main() + { + } +}" + }; + + yield return new[] + { + @" +using System.Collections.Generic; + +namespace HeaderDictionaryAddFixerTests; +public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase +{ + public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() + { + {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; + return Ok(new List()); + } +} + +public class Program +{ + public static void Main() + { + } +}", + @" +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; + +namespace HeaderDictionaryAddFixerTests; +public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase +{ + public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() + { + Response.Headers.Append(""Content-Type"", ""text/html""); + return Ok(new List()); + } +} + public class Program { public static void Main() From a2331a878e15473484c0108f8ed118677c6a1152 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 13 Oct 2022 21:58:00 -0400 Subject: [PATCH 10/29] Fix StringComparison warning --- .../src/CodeFixes/Http/HeaderDictionaryAddFixer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index 80af14576be7..34b6a196601f 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -79,7 +79,7 @@ private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(Compilat var namespaceName = usingDirectives[i].Name.ToString(); // Always insert the new using directive after any 'System' using directives. - if (namespaceName.StartsWith("System")) + if (namespaceName.StartsWith("System", StringComparison.Ordinal)) { insertionIndex = i + 1; continue; From d9aff0a199b319c4d1c908dc2b41a4fdde333046 Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:27:25 -0400 Subject: [PATCH 11/29] Update src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj Co-authored-by: Safia Abdalla --- .../Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj b/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj index 9e78bc9f6c53..5c89ff96a301 100644 --- a/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj +++ b/src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj @@ -8,7 +8,6 @@ - From a7d512498107db30ce3d088cc8d2eecd6f57442b Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:32:58 -0400 Subject: [PATCH 12/29] Update src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs Co-authored-by: Safia Abdalla --- .../src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs index c577bff8b72e..cd5f420aafa8 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Analyzers.Http; [DiagnosticAnalyzer(LanguageNames.CSharp)] -public partial class HeaderDictionaryAddAnalyzer : DiagnosticAnalyzer +public sealed class HeaderDictionaryAddAnalyzer : DiagnosticAnalyzer { public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd); From ed6ddcf238df444a0ad31cec310595e202ccc89c Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:35:41 -0400 Subject: [PATCH 13/29] Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs Co-authored-by: Safia Abdalla --- .../src/CodeFixes/Http/HeaderDictionaryAddFixer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index 34b6a196601f..457530a72d1b 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -32,7 +32,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) equivalenceKey: appendTitle), diagnostic); - var indexerTitle = "Use the indexer"; + var indexerTitle = "Use indexer"; context.RegisterCodeFix( CodeAction.Create(indexerTitle, cancellationToken => ReplaceAddWithIndexerAsync(diagnostic, context.Document, cancellationToken), From faee1d031c469733a997a79186c25054a1feff51 Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:35:55 -0400 Subject: [PATCH 14/29] Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs Co-authored-by: Youssef Victor --- .../src/CodeFixes/Http/HeaderDictionaryAddFixer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index 457530a72d1b..5bcf4fced80b 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Analyzers.Http.Fixers; [ExportCodeFixProvider(LanguageNames.CSharp), Shared] -public class HeaderDictionaryAddFixer : CodeFixProvider +public sealed class HeaderDictionaryAddFixer : CodeFixProvider { public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd.Id); From 81f6715ec590423aa64db14adab8613df767fc9c Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:36:05 -0400 Subject: [PATCH 15/29] Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs Co-authored-by: Youssef Victor --- .../src/CodeFixes/Http/HeaderDictionaryAddFixer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index 5bcf4fced80b..7d9762ee8685 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -25,7 +25,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) { foreach (var diagnostic in context.Diagnostics) { - var appendTitle = "Use IHeaderDictionary.Append"; + var appendTitle = "Use 'IHeaderDictionary.Append'"; context.RegisterCodeFix( CodeAction.Create(appendTitle, cancellationToken => ReplaceAddWithAppendAsync(diagnostic, context.Document, cancellationToken), From c2d8519010de3f47fbea2d11eaa1e04dbc619eeb Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:45:00 -0400 Subject: [PATCH 16/29] Update code fix equivalence key referenced in tests --- .../test/Http/HeaderDictionaryAddFixerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs index 27b8ee465f83..a2fafe95b54d 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs @@ -10,8 +10,8 @@ namespace Microsoft.AspNetCore.Analyzers.Http; public class HeaderDictionaryAddFixerTests { - private const string AppendCodeActionEquivalenceKey = "Use IHeaderDictionary.Append"; - private const string IndexerCodeActionEquivalenceKey = "Use the indexer"; + private const string AppendCodeActionEquivalenceKey = "Use 'IHeaderDictionary.Append'"; + private const string IndexerCodeActionEquivalenceKey = "Use indexer"; [Fact] public async Task IHeaderDictionary_WithAdd_FixedWithAppend() From 8a0bc4b0d30fd8f4da7b740e3b2382c5eb79757e Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:47:27 -0400 Subject: [PATCH 17/29] Remove redundant test --- .../Http/HeaderDictionaryAddAnalyzerTests.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs index 9f1f58f65634..fe240bf4fe42 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs @@ -9,26 +9,6 @@ namespace Microsoft.AspNetCore.Analyzers.Http; public class HeaderDictionaryAddAnalyzerTests { - [Fact] - public async Task IHeaderDictionary_WithAdd_ReportsDiagnostics() - { - // Arrange & Act & Assert - await VerifyCS.VerifyAnalyzerAsync(@" -using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddAnalyzerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; - } -}", - new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) - .WithLocation(0) - .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message)); - } - [Fact] public async Task IHeaderDictionary_WithAppend_NoDiagnostics() { From 362d1720da39e2c438382129de017eb48f3cb068 Mon Sep 17 00:00:00 2001 From: David Acker Date: Mon, 17 Oct 2022 19:52:52 -0400 Subject: [PATCH 18/29] Merge analyzer and code fix tests into single test file --- .../Http/HeaderDictionaryAddAnalyzerTests.cs | 45 ------------------- ...xerTests.cs => HeaderDictionaryAddTest.cs} | 42 ++++++++++++++++- 2 files changed, 41 insertions(+), 46 deletions(-) delete mode 100644 src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs rename src/Framework/AspNetCoreAnalyzers/test/Http/{HeaderDictionaryAddFixerTests.cs => HeaderDictionaryAddTest.cs} (86%) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs deleted file mode 100644 index fe240bf4fe42..000000000000 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddAnalyzerTests.cs +++ /dev/null @@ -1,45 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.CodeAnalysis.Testing; -using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpAnalyzerVerifier< - Microsoft.AspNetCore.Analyzers.Http.HeaderDictionaryAddAnalyzer>; - -namespace Microsoft.AspNetCore.Analyzers.Http; - -public class HeaderDictionaryAddAnalyzerTests -{ - [Fact] - public async Task IHeaderDictionary_WithAppend_NoDiagnostics() - { - // Arrange & Act & Assert - await VerifyCS.VerifyAnalyzerAsync(@" -using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddAnalyzerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Append(""Accept"", ""text/html""); - } -}"); - } - - [Fact] - public async Task IHeaderDictionary_WithIndexer_NoDiagnostics() - { - // Arrange & Act & Assert - await VerifyCS.VerifyAnalyzerAsync(@" -using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddAnalyzerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers[""Accept""] = ""text/html""; - } -}"); - } -} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs similarity index 86% rename from src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs rename to src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs index a2fafe95b54d..55fd250eecae 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddFixerTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Analyzers.Http; -public class HeaderDictionaryAddFixerTests +public class HeaderDictionaryAddTest { private const string AppendCodeActionEquivalenceKey = "Use 'IHeaderDictionary.Append'"; private const string IndexerCodeActionEquivalenceKey = "Use indexer"; @@ -273,4 +273,44 @@ public static void Main() }", codeActionEquivalenceKey: IndexerCodeActionEquivalenceKey); } + + [Fact] + public async Task IHeaderDictionary_WithAppend_DoesNotProduceDiagnostics() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddAnalyzerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers.Append(""Accept"", ""text/html""); + } +}"; + + // Act & Assert + await VerifyCS.VerifyCodeFixAsync(source, source); + } + + [Fact] + public async Task IHeaderDictionary_WithIndexer_DoesNotProduceDiagnostics() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Http; +namespace HeaderDictionaryAddAnalyzerTests; +public class Program +{ + public static void Main() + { + var context = new DefaultHttpContext(); + context.Request.Headers[""Accept""] = ""text/html""; + } +}"; + + // Act & Assert + await VerifyCS.VerifyCodeFixAsync(source, source); + } } From e3caef09789ece9ae99a18640d9832d217ce116d Mon Sep 17 00:00:00 2001 From: David Acker Date: Tue, 18 Oct 2022 20:40:36 -0400 Subject: [PATCH 19/29] Use top-level statements --- .../test/Http/HeaderDictionaryAddTest.cs | 248 ++++-------------- 1 file changed, 52 insertions(+), 196 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs index 55fd250eecae..1bc89e2fe183 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs @@ -19,15 +19,10 @@ public async Task IHeaderDictionary_WithAdd_FixedWithAppend() // Arrange & Act & Assert await VerifyCS.VerifyCodeFixAsync(@" using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddFixerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; - } -}", + +var context = new DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +", new[] { new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) @@ -36,189 +31,70 @@ public static void Main() }, @" using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddFixerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Append(""Accept"", ""text/html""); - } -}", + +var context = new DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""); +", codeActionEquivalenceKey: AppendCodeActionEquivalenceKey); } public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() { + // No existing using directives yield return new[] - { - @" -using Microsoft.AspNetCore.Mvc; - -namespace HeaderDictionaryAddFixerTests; -public class TestController : ControllerBase { - public IActionResult Endpoint() - { - {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; - return Ok(); - } -} - -public class Program -{ - public static void Main() - { - } -}", + @" +var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +", @" using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -namespace HeaderDictionaryAddFixerTests; -public class TestController : ControllerBase -{ - public IActionResult Endpoint() - { - Response.Headers.Append(""Content-Type"", ""text/html""); - return Ok(); - } -} - -public class Program -{ - public static void Main() - { - } -}" +var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""); +" }; + // Inserted alphabetically based on existing using directives yield return new[] { @" -using System.Collections.Generic; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc; -namespace HeaderDictionaryAddFixerTests; -public class TestController : ControllerBase -{ - public IActionResult Endpoint() - { - {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; - return Ok(new List()); - } -} - -public class Program -{ - public static void Main() - { - } -}", +var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +", @" -using System.Collections.Generic; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; -namespace HeaderDictionaryAddFixerTests; -public class TestController : ControllerBase -{ - public IActionResult Endpoint() - { - Response.Headers.Append(""Content-Type"", ""text/html""); - return Ok(new List()); - } -} - -public class Program -{ - public static void Main() - { - } -}" - }; - - yield return new[] - { - @" -namespace HeaderDictionaryAddFixerTests; -public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase -{ - public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() - { - {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; - return Ok(); - } -} - -public class Program -{ - public static void Main() - { - } -}", - @" -using Microsoft.AspNetCore.Http; - -namespace HeaderDictionaryAddFixerTests; -public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase -{ - public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() - { - Response.Headers.Append(""Content-Type"", ""text/html""); - return Ok(); - } -} - -public class Program -{ - public static void Main() - { - } -}" +var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""); +" }; + // Inserted after 'System' using directives yield return new[] { @" using System.Collections.Generic; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc; -namespace HeaderDictionaryAddFixerTests; -public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase -{ - public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() - { - {|#0:Response.Headers.Add(""Content-Type"", ""text/html"")|}; - return Ok(new List()); - } -} - -public class Program -{ - public static void Main() - { - } -}", +var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +", @" using System.Collections.Generic; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; -namespace HeaderDictionaryAddFixerTests; -public class TestController : Microsoft.AspNetCore.Mvc.ControllerBase -{ - public Microsoft.AspNetCore.Mvc.IActionResult Endpoint() - { - Response.Headers.Append(""Content-Type"", ""text/html""); - return Ok(new List()); - } -} - -public class Program -{ - public static void Main() - { - } -}" +var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""); +" }; } @@ -245,15 +121,10 @@ public async Task IHeaderDictionary_WithAdd_FixedWithIndexer() // Arrange & Act & Assert await VerifyCS.VerifyCodeFixAsync(@" using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddFixerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; - } -}", + +var context = new DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +", new[] { new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) @@ -262,15 +133,10 @@ public static void Main() }, @" using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddFixerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers[""Accept""] = ""text/html""; - } -}", + +var context = new DefaultHttpContext(); +context.Request.Headers[""Accept""] = ""text/html""; +", codeActionEquivalenceKey: IndexerCodeActionEquivalenceKey); } @@ -280,15 +146,10 @@ public async Task IHeaderDictionary_WithAppend_DoesNotProduceDiagnostics() // Arrange var source = @" using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddAnalyzerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers.Append(""Accept"", ""text/html""); - } -}"; + +var context = new DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""); +"; // Act & Assert await VerifyCS.VerifyCodeFixAsync(source, source); @@ -300,15 +161,10 @@ public async Task IHeaderDictionary_WithIndexer_DoesNotProduceDiagnostics() // Arrange var source = @" using Microsoft.AspNetCore.Http; -namespace HeaderDictionaryAddAnalyzerTests; -public class Program -{ - public static void Main() - { - var context = new DefaultHttpContext(); - context.Request.Headers[""Accept""] = ""text/html""; - } -}"; + +var context = new DefaultHttpContext(); +context.Request.Headers[""Accept""] = ""text/html""; +"; // Act & Assert await VerifyCS.VerifyCodeFixAsync(source, source); From debb846e440e330f91f14f15c97d9921d1ba0d93 Mon Sep 17 00:00:00 2001 From: David Acker Date: Tue, 18 Oct 2022 21:07:23 -0400 Subject: [PATCH 20/29] Add test cases for multiple diagnostics --- .../test/Http/HeaderDictionaryAddTest.cs | 118 ++++++++++++++---- 1 file changed, 94 insertions(+), 24 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs index 1bc89e2fe183..f643b5cd64bf 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs @@ -13,29 +13,64 @@ public class HeaderDictionaryAddTest private const string AppendCodeActionEquivalenceKey = "Use 'IHeaderDictionary.Append'"; private const string IndexerCodeActionEquivalenceKey = "Use indexer"; - [Fact] - public async Task IHeaderDictionary_WithAdd_FixedWithAppend() + public static TheoryData FixedWithAppendTestData => new() { - // Arrange & Act & Assert - await VerifyCS.VerifyCodeFixAsync(@" + // Single diagnostic + { + @" using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; ", - new[] - { - new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) - .WithLocation(0) - .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) - }, - @" + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); context.Request.Headers.Append(""Accept"", ""text/html""); +" + }, + + // Multiple diagnostics + { + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +{|#1:context.Request.Headers.Add(""Accept"", ""text/html"")|}; ", - codeActionEquivalenceKey: AppendCodeActionEquivalenceKey); + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message), + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(1) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""); +context.Request.Headers.Append(""Accept"", ""text/html""); +" + } + }; + + [Theory] + [MemberData(nameof(FixedWithAppendTestData))] + public async Task IHeaderDictionary_WithAdd_FixedWithAppend(string source, DiagnosticResult[] expectedDiagnostics, string fixedSource) + { + // Arrange & Act & Assert + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, codeActionEquivalenceKey: AppendCodeActionEquivalenceKey); } public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() @@ -115,29 +150,64 @@ await VerifyCS.VerifyCodeFixAsync( codeActionEquivalenceKey: AppendCodeActionEquivalenceKey); } - [Fact] - public async Task IHeaderDictionary_WithAdd_FixedWithIndexer() + public static TheoryData FixedWithIndexerTestData => new() { - // Arrange & Act & Assert - await VerifyCS.VerifyCodeFixAsync(@" + // Single diagnostic + { + @" using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; ", - new[] - { - new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) - .WithLocation(0) - .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) - }, - @" + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); context.Request.Headers[""Accept""] = ""text/html""; +" + }, + + // Multiple diagnostics + { + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; +{|#1:context.Request.Headers.Add(""Accept"", ""text/html"")|}; ", - codeActionEquivalenceKey: IndexerCodeActionEquivalenceKey); + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message), + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(1) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +context.Request.Headers[""Accept""] = ""text/html""; +context.Request.Headers[""Accept""] = ""text/html""; +" + } + }; + + [Theory] + [MemberData(nameof(FixedWithIndexerTestData))] + public async Task IHeaderDictionary_WithAdd_FixedWithIndexer(string source, DiagnosticResult[] expectedDiagnostics, string fixedSource) + { + // Arrange & Act & Assert + await VerifyCS.VerifyCodeFixAsync(source, expectedDiagnostics, fixedSource, codeActionEquivalenceKey: IndexerCodeActionEquivalenceKey); } [Fact] From 40bd5143530e8b5b9ab66a1dda3c3b5a137791c5 Mon Sep 17 00:00:00 2001 From: David Acker Date: Tue, 18 Oct 2022 21:20:50 -0400 Subject: [PATCH 21/29] Add comment about IDictionary.Add to diagnostic message --- src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index 155d95572878..c43dd65e211a 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -202,7 +202,7 @@ Unused route parameter - Suggest using IHeaderDictionary.Append or the indexer instead of Add + Use IHeaderDictionary.Append or the indexer to append or set headers. IDictionary.Add will throw an ArgumentException when attempting to add duplicate keys. Suggest using IHeaderDictionary.Append or the indexer From e2f0a12b5e9401c27cd044e743af2a532586c886 Mon Sep 17 00:00:00 2001 From: David Acker Date: Tue, 18 Oct 2022 21:22:45 -0400 Subject: [PATCH 22/29] Update diagnostic message --- src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx index c43dd65e211a..7b858c8e70df 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Resources.resx @@ -202,7 +202,7 @@ Unused route parameter - Use IHeaderDictionary.Append or the indexer to append or set headers. IDictionary.Add will throw an ArgumentException when attempting to add duplicate keys. + Use IHeaderDictionary.Append or the indexer to append or set headers. IDictionary.Add will throw an ArgumentException when attempting to add a duplicate key. Suggest using IHeaderDictionary.Append or the indexer From a216f87773c8d16edec92c306c274eacb513d605 Mon Sep 17 00:00:00 2001 From: David Acker Date: Fri, 21 Oct 2022 12:16:20 -0400 Subject: [PATCH 23/29] Move checks before code fix registration --- .../Http/HeaderDictionaryAddFixer.cs | 95 ++++++++++++------- 1 file changed, 61 insertions(+), 34 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index 7d9762ee8685..cdccc3408864 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -25,46 +25,63 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) { foreach (var diagnostic in context.Diagnostics) { - var appendTitle = "Use 'IHeaderDictionary.Append'"; - context.RegisterCodeFix( - CodeAction.Create(appendTitle, - cancellationToken => ReplaceAddWithAppendAsync(diagnostic, context.Document, cancellationToken), - equivalenceKey: appendTitle), - diagnostic); - - var indexerTitle = "Use indexer"; - context.RegisterCodeFix( - CodeAction.Create(indexerTitle, - cancellationToken => ReplaceAddWithIndexerAsync(diagnostic, context.Document, cancellationToken), - equivalenceKey: indexerTitle), - diagnostic); + 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 ReplaceAddWithAppendAsync(Diagnostic diagnostic, Document document, CancellationToken cancellationToken) + private static async Task ReplaceWithAppend(Diagnostic diagnostic, Document document, InvocationExpressionSyntax invocation, CancellationToken cancellationToken) { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - if (root is not CompilationUnitSyntax compilationUnitSyntax) + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) as CompilationUnitSyntax; + + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); + + // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. + // We'll need to add the required using directive, when not already present. + return document.WithSyntaxRoot( + AddRequiredUsingDirectiveForAppend(root.ReplaceNode(diagnosticTarget, invocation))); + } + + private static bool CanReplaceWithAppend(Diagnostic diagnostic, SyntaxNode root, out InvocationExpressionSyntax invocation) + { + invocation = null; + + if (root is not CompilationUnitSyntax) { - return document; + return false; } - var invocation = compilationUnitSyntax.FindNode(diagnostic.Location.SourceSpan); + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); - if (invocation is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name.Identifier: { } identifierToken } }) + if (diagnosticTarget is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name.Identifier: { } identifierToken } } invocationExpression) { - compilationUnitSyntax = compilationUnitSyntax.ReplaceToken(identifierToken, SyntaxFactory.Identifier("Append")); + invocation = invocationExpression.ReplaceToken(identifierToken, SyntaxFactory.Identifier("Append")); - // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. - // We'll need to add the required using directive, when not already present. - compilationUnitSyntax = AddRequiredUsingDirectiveForAppend(compilationUnitSyntax); - - return document.WithSyntaxRoot(compilationUnitSyntax); + return true; } - return document; + return false; } private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(CompilationUnitSyntax compilationUnitSyntax) @@ -117,23 +134,33 @@ private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(Compilat usingDirectives.Insert(insertionIndex, requiredUsingDirective)); } - private static async Task ReplaceAddWithIndexerAsync(Diagnostic diagnostic, Document document, CancellationToken cancellationToken) + private static async Task ReplaceWithIndexer(Diagnostic diagnostic, Document document, AssignmentExpressionSyntax assignment, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - if (root == null) + + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); + + 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 document; + return false; } - var invocation = root.FindNode(diagnostic.Location.SourceSpan); + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); - if (invocation is InvocationExpressionSyntax + if (diagnosticTarget is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax memberAccessExpression, ArgumentList.Arguments: { Count: 2 } arguments }) { - var assignmentExpression = + assignment = SyntaxFactory.AssignmentExpression( SyntaxKind.SimpleAssignmentExpression, SyntaxFactory.ElementAccessExpression( @@ -142,9 +169,9 @@ private static async Task ReplaceAddWithIndexerAsync(Diagnostic diagno SyntaxFactory.SeparatedList(new[] { arguments[0] }))), arguments[1].Expression); - return document.WithSyntaxRoot(root.ReplaceNode(invocation, assignmentExpression)); + return true; } - return document; + return false; } } From 1bad1a34324449b366eb8427cc9abe5054ab519d Mon Sep 17 00:00:00 2001 From: David Acker Date: Tue, 25 Oct 2022 19:37:21 -0400 Subject: [PATCH 24/29] Pass true for getInnermostNodeForTie --- .../Http/HeaderDictionaryAddFixer.cs | 8 +- .../test/Http/HeaderDictionaryAddTest.cs | 91 ++++++++++++------- 2 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index cdccc3408864..2fc93145f67c 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -55,7 +55,7 @@ private static async Task ReplaceWithAppend(Diagnostic diagnostic, Doc { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) as CompilationUnitSyntax; - var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. // We'll need to add the required using directive, when not already present. @@ -72,7 +72,7 @@ private static bool CanReplaceWithAppend(Diagnostic diagnostic, SyntaxNode root, return false; } - var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (diagnosticTarget is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name.Identifier: { } identifierToken } } invocationExpression) { @@ -138,7 +138,7 @@ private static async Task ReplaceWithIndexer(Diagnostic diagnostic, Do { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); return document.WithSyntaxRoot(root.ReplaceNode(diagnosticTarget, assignment)); } @@ -152,7 +152,7 @@ private static bool CanReplaceWithIndexer(Diagnostic diagnostic, SyntaxNode root return false; } - var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan); + var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (diagnosticTarget is InvocationExpressionSyntax { diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs index f643b5cd64bf..d8e0b21a22d0 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs @@ -21,8 +21,7 @@ public class HeaderDictionaryAddTest using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); -{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|};", new[] { new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) @@ -33,8 +32,7 @@ public class HeaderDictionaryAddTest using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); -context.Request.Headers.Append(""Accept"", ""text/html""); -" +context.Request.Headers.Append(""Accept"", ""text/html"");" }, // Multiple diagnostics @@ -44,8 +42,7 @@ public class HeaderDictionaryAddTest var context = new DefaultHttpContext(); {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -{|#1:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#1:context.Request.Headers.Add(""Accept"", ""text/html"")|};", new[] { new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) @@ -60,8 +57,27 @@ public class HeaderDictionaryAddTest var context = new DefaultHttpContext(); context.Request.Headers.Append(""Accept"", ""text/html""); -context.Request.Headers.Append(""Accept"", ""text/html""); -" +context.Request.Headers.Append(""Accept"", ""text/html"");" + }, + + // Missing semicolon + { + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}{|CS1002:|}", + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +context.Request.Headers.Append(""Accept"", ""text/html""){|CS1002:|}" } }; @@ -80,14 +96,13 @@ public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() { @" var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); -{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|};", + @" using Microsoft.AspNetCore.Http; var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); -context.Request.Headers.Append(""Accept"", ""text/html""); -" +context.Request.Headers.Append(""Accept"", ""text/html"");" }; // Inserted alphabetically based on existing using directives @@ -98,16 +113,15 @@ public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() using Microsoft.AspNetCore.Mvc; var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); -{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|};", + @" using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); -context.Request.Headers.Append(""Accept"", ""text/html""); -" +context.Request.Headers.Append(""Accept"", ""text/html"");" }; // Inserted after 'System' using directives @@ -119,8 +133,8 @@ public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() using Microsoft.AspNetCore.Mvc; var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); -{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|};", + @" using System.Collections.Generic; using Microsoft.AspNetCore.Hosting; @@ -128,8 +142,7 @@ public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() using Microsoft.AspNetCore.Mvc; var context = new Microsoft.AspNetCore.Http.DefaultHttpContext(); -context.Request.Headers.Append(""Accept"", ""text/html""); -" +context.Request.Headers.Append(""Accept"", ""text/html"");" }; } @@ -158,8 +171,7 @@ await VerifyCS.VerifyCodeFixAsync( using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); -{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|};", new[] { new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) @@ -170,8 +182,7 @@ await VerifyCS.VerifyCodeFixAsync( using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); -context.Request.Headers[""Accept""] = ""text/html""; -" +context.Request.Headers[""Accept""] = ""text/html"";" }, // Multiple diagnostics @@ -181,8 +192,7 @@ await VerifyCS.VerifyCodeFixAsync( var context = new DefaultHttpContext(); {|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -{|#1:context.Request.Headers.Add(""Accept"", ""text/html"")|}; -", +{|#1:context.Request.Headers.Add(""Accept"", ""text/html"")|};", new[] { new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) @@ -197,8 +207,27 @@ await VerifyCS.VerifyCodeFixAsync( var context = new DefaultHttpContext(); context.Request.Headers[""Accept""] = ""text/html""; -context.Request.Headers[""Accept""] = ""text/html""; -" +context.Request.Headers[""Accept""] = ""text/html"";" + }, + + // Missing semicolon + { + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +{|#0:context.Request.Headers.Add(""Accept"", ""text/html"")|}{|CS1002:|}", + new[] + { + new DiagnosticResult(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd) + .WithLocation(0) + .WithMessage(Resources.Analyzer_HeaderDictionaryAdd_Message) + }, + @" +using Microsoft.AspNetCore.Http; + +var context = new DefaultHttpContext(); +context.Request.Headers[""Accept""] = ""text/html""{|CS1002:|}" } }; @@ -218,8 +247,7 @@ public async Task IHeaderDictionary_WithAppend_DoesNotProduceDiagnostics() using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); -context.Request.Headers.Append(""Accept"", ""text/html""); -"; +context.Request.Headers.Append(""Accept"", ""text/html"");"; // Act & Assert await VerifyCS.VerifyCodeFixAsync(source, source); @@ -233,8 +261,7 @@ public async Task IHeaderDictionary_WithIndexer_DoesNotProduceDiagnostics() using Microsoft.AspNetCore.Http; var context = new DefaultHttpContext(); -context.Request.Headers[""Accept""] = ""text/html""; -"; +context.Request.Headers[""Accept""] = ""text/html"";"; // Act & Assert await VerifyCS.VerifyCodeFixAsync(source, source); From 289260a05f36d08cce2b1d5e6f75ac3d33cb9838 Mon Sep 17 00:00:00 2001 From: David Acker Date: Tue, 25 Oct 2022 19:45:17 -0400 Subject: [PATCH 25/29] Perform symbol comparison for IHeaderDictionary --- .../Http/HeaderDictionaryAddAnalyzer.cs | 64 +++++++++---------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs index cd5f420aafa8..8a18f48dbba0 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs @@ -1,7 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Immutable; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; @@ -17,46 +19,30 @@ 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 (invocation.Instance?.Type is INamedTypeSymbol type && - IsIHeadersDictionaryType(type)) + if (SymbolEqualityComparer.Default.Equals(symbols.IHeaderDictionary, invocation.Instance?.Type) + && IsAddMethod(invocation.TargetMethod) + && invocation.TargetMethod.Parameters.Length == 2) { - if (invocation.TargetMethod.Parameters.Length == 2 && - IsAddMethod(invocation.TargetMethod)) - { - AddDiagnosticWarning(context, invocation.Syntax.GetLocation()); - } + AddDiagnosticWarning(context, invocation.Syntax.GetLocation()); } - }, OperationKind.Invocation); - } - private static bool IsIHeadersDictionaryType(INamedTypeSymbol type) - { - // Only IHeaderDictionary is valid. Types like HeaderDictionary, which implement IHeaderDictionary, - // can't access header properties unless cast as IHeaderDictionary. - return type is - { - Name: "IHeaderDictionary", - ContainingNamespace: - { - Name: "Http", - ContainingNamespace: - { - Name: "AspNetCore", - ContainingNamespace: - { - Name: "Microsoft", - ContainingNamespace: - { - IsGlobalNamespace: true - } - } - } - } - }; + }, OperationKind.Invocation); } private static bool IsAddMethod(IMethodSymbol method) @@ -93,4 +79,16 @@ private static void AddDiagnosticWarning(OperationAnalysisContext context, Locat 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; } + } } From bc3ef4dab03fc9ba8558b7a65542b013099699a9 Mon Sep 17 00:00:00 2001 From: David Acker Date: Thu, 27 Oct 2022 11:20:57 -0400 Subject: [PATCH 26/29] Add using directive via syntax annotation --- .../Http/HeaderDictionaryAddAnalyzer.cs | 2 - .../Http/HeaderDictionaryAddFixer.cs | 62 +++---------------- 2 files changed, 7 insertions(+), 57 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs index 8a18f48dbba0..989bb538ade8 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDictionaryAddAnalyzer.cs @@ -1,9 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Collections.Immutable; -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs index 2fc93145f67c..08826af2b0e7 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDictionaryAddFixer.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Collections.Immutable; using System.Composition; using System.Threading; @@ -11,6 +10,7 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Simplification; namespace Microsoft.AspNetCore.Analyzers.Http.Fixers; @@ -53,14 +53,16 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task ReplaceWithAppend(Diagnostic diagnostic, Document document, InvocationExpressionSyntax invocation, CancellationToken cancellationToken) { - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) as CompilationUnitSyntax; + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); - // IHeaderDictionary.Append is defined as an extension method on Microsoft.AspNetCore.Http.HeaderDictionaryExtensions. - // We'll need to add the required using directive, when not already present. + 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( - AddRequiredUsingDirectiveForAppend(root.ReplaceNode(diagnosticTarget, invocation))); + root.ReplaceNode(diagnosticTarget, invocation.WithAdditionalAnnotations(Simplifier.AddImportsAnnotation, annotation))); } private static bool CanReplaceWithAppend(Diagnostic diagnostic, SyntaxNode root, out InvocationExpressionSyntax invocation) @@ -84,56 +86,6 @@ private static bool CanReplaceWithAppend(Diagnostic diagnostic, SyntaxNode root, return false; } - private static CompilationUnitSyntax AddRequiredUsingDirectiveForAppend(CompilationUnitSyntax compilationUnitSyntax) - { - var usingDirectives = compilationUnitSyntax.Usings; - - var includesRequiredUsingDirective = false; - var insertionIndex = 0; - - for (var i = 0; i < usingDirectives.Count; i++) - { - var namespaceName = usingDirectives[i].Name.ToString(); - - // Always insert the new using directive after any 'System' using directives. - if (namespaceName.StartsWith("System", StringComparison.Ordinal)) - { - insertionIndex = i + 1; - continue; - } - - var result = string.Compare("Microsoft.AspNetCore.Http", namespaceName, StringComparison.Ordinal); - - if (result == 0) - { - includesRequiredUsingDirective = true; - break; - } - - if (result < 0) - { - insertionIndex = i; - break; - } - } - - if (includesRequiredUsingDirective) - { - return compilationUnitSyntax; - } - - var requiredUsingDirective = - SyntaxFactory.UsingDirective( - SyntaxFactory.QualifiedName( - SyntaxFactory.QualifiedName( - SyntaxFactory.IdentifierName("Microsoft"), - SyntaxFactory.IdentifierName("AspNetCore")), - SyntaxFactory.IdentifierName("Http"))); - - return compilationUnitSyntax.WithUsings( - usingDirectives.Insert(insertionIndex, requiredUsingDirective)); - } - private static async Task ReplaceWithIndexer(Diagnostic diagnostic, Document document, AssignmentExpressionSyntax assignment, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); From 620275d91c02992af0f0dc214cb17c123a00a311 Mon Sep 17 00:00:00 2001 From: David Acker Date: Sat, 19 Nov 2022 17:23:21 -0500 Subject: [PATCH 27/29] Add editorconfig --- src/Framework/AspNetCoreAnalyzers/test/.editorconfig | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 src/Framework/AspNetCoreAnalyzers/test/.editorconfig diff --git a/src/Framework/AspNetCoreAnalyzers/test/.editorconfig b/src/Framework/AspNetCoreAnalyzers/test/.editorconfig new file mode 100644 index 000000000000..d420ca44ce3c --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/.editorconfig @@ -0,0 +1,2 @@ +[*.cs] +end_of_line = crlf; From 117ab9e39378fcb598f9dd49819c5a051e4f5cf3 Mon Sep 17 00:00:00 2001 From: David Acker Date: Wed, 23 Nov 2022 20:29:56 -0500 Subject: [PATCH 28/29] Revert "Add editorconfig" This reverts commit 620275d91c02992af0f0dc214cb17c123a00a311. --- src/Framework/AspNetCoreAnalyzers/test/.editorconfig | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 src/Framework/AspNetCoreAnalyzers/test/.editorconfig diff --git a/src/Framework/AspNetCoreAnalyzers/test/.editorconfig b/src/Framework/AspNetCoreAnalyzers/test/.editorconfig deleted file mode 100644 index d420ca44ce3c..000000000000 --- a/src/Framework/AspNetCoreAnalyzers/test/.editorconfig +++ /dev/null @@ -1,2 +0,0 @@ -[*.cs] -end_of_line = crlf; From a57af9b4d71a20705f926fa4bbaa9b99a3a2929b Mon Sep 17 00:00:00 2001 From: David Acker Date: Wed, 23 Nov 2022 20:37:00 -0500 Subject: [PATCH 29/29] Skip test on Linux, macOS --- .../AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs index d8e0b21a22d0..5eaa129e66fe 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Http/HeaderDictionaryAddTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.AspNetCore.Testing; using Microsoft.CodeAnalysis.Testing; using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpCodeFixVerifier< Microsoft.AspNetCore.Analyzers.Http.HeaderDictionaryAddAnalyzer, @@ -146,7 +147,9 @@ public static IEnumerable FixedWithAppendAddsUsingDirectiveTestData() }; } - [Theory] + [ConditionalTheory] + [OSSkipCondition(OperatingSystems.Linux)] + [OSSkipCondition(OperatingSystems.MacOSX)] [MemberData(nameof(FixedWithAppendAddsUsingDirectiveTestData))] public async Task IHeaderDictionary_WithAdd_FixedWithAppend_AddsUsingDirective(string source, string fixedSource) {