Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1373fbe
Add IHeaderDictionary.Add analyzer and code fix
david-acker Oct 11, 2022
aabfef9
Change wording from "Disallow-" to "RecommendAgainst-"
david-acker Oct 11, 2022
eb91cd8
Change diagnostic rule ID
david-acker Oct 11, 2022
16332ba
Merge branch 'main' into add-header-dictionary-analyzer
david-acker Oct 13, 2022
2bf902d
Remove analyzer and code fix from AspNetCore.Analyzers project
david-acker Oct 13, 2022
88625b2
Add analyzer to Framework/AspNetCoreAnalyzers
david-acker Oct 13, 2022
dd36127
Add code fix to Framework/AspNetCoreAnalyzers
david-acker Oct 13, 2022
e6190b7
Fix nullable reference type
david-acker Oct 13, 2022
049bde0
Fix diagnostics in the project caused by new analyzer
david-acker Oct 13, 2022
1577df7
Fix using directive insertion logic
david-acker Oct 14, 2022
a2331a8
Fix StringComparison warning
david-acker Oct 14, 2022
d9aff0a
Update src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Te…
david-acker Oct 17, 2022
a7d5124
Update src/Framework/AspNetCoreAnalyzers/src/Analyzers/Http/HeaderDic…
david-acker Oct 17, 2022
ed6ddcf
Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDic…
david-acker Oct 17, 2022
faee1d0
Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDic…
david-acker Oct 17, 2022
81f6715
Update src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Http/HeaderDic…
david-acker Oct 17, 2022
c2d8519
Update code fix equivalence key referenced in tests
david-acker Oct 17, 2022
8a0bc4b
Remove redundant test
david-acker Oct 17, 2022
362d172
Merge analyzer and code fix tests into single test file
david-acker Oct 17, 2022
e3caef0
Use top-level statements
david-acker Oct 19, 2022
debb846
Add test cases for multiple diagnostics
david-acker Oct 19, 2022
40bd514
Add comment about IDictionary.Add to diagnostic message
david-acker Oct 19, 2022
e2f0a12
Update diagnostic message
david-acker Oct 19, 2022
a216f87
Move checks before code fix registration
david-acker Oct 21, 2022
1bad1a3
Pass true for getInnermostNodeForTie
david-acker Oct 25, 2022
289260a
Perform symbol comparison for IHeaderDictionary
david-acker Oct 25, 2022
bc3ef4d
Add using directive via syntax annotation
david-acker Oct 27, 2022
620275d
Add editorconfig
david-acker Nov 19, 2022
117ab9e
Revert "Add editorconfig"
david-acker Nov 24, 2022
a57af9b
Skip test on Linux, macOS
david-acker Nov 24, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
Expand All @@ -8,7 +8,6 @@

<ItemGroup>
<ProjectReference Include="$(RepoRoot)src\Analyzers\Analyzers\src\Microsoft.AspNetCore.Analyzers.csproj" />

<Reference Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.XUnit" />
<Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
<Reference Include="Microsoft.Extensions.DependencyModel" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an issue tracking documentation for analyzers?

I see ASP0015, ASP0016, ASP0017, and ASP0018 with the same help link are not yet documented.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@david-acker You can file a docs issue in the docs repo here https://github.com/dotnet/AspNetCore.Docs/issues.

If you're so inclined, you can also submit the doc for this by updating this page.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.AspNetCore.Analyzers.Http;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class HeaderDictionaryAddAnalyzer : DiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterCompilationStartAction(OnCompilationStart);
}

private static void OnCompilationStart(CompilationStartAnalysisContext context)
{
var symbols = new HeaderDictionarySymbols(context.Compilation);

if (!symbols.HasRequiredSymbols)
{
return;
}

context.RegisterOperationAction(context =>
{
var invocation = (IInvocationOperation)context.Operation;

if (SymbolEqualityComparer.Default.Equals(symbols.IHeaderDictionary, invocation.Instance?.Type)
&& IsAddMethod(invocation.TargetMethod)
&& invocation.TargetMethod.Parameters.Length == 2)
{
AddDiagnosticWarning(context, invocation.Syntax.GetLocation());
}

}, OperationKind.Invocation);
}

private static bool IsAddMethod(IMethodSymbol method)
{
return method is
{
Name: "Add",
ContainingType:
{
Name: "IDictionary",
ContainingNamespace:
{
Name: "Generic",
ContainingNamespace:
{
Name: "Collections",
ContainingNamespace:
{
Name: "System",
ContainingNamespace:
{
IsGlobalNamespace: true
}
}
}
}
}
};
}

private static void AddDiagnosticWarning(OperationAnalysisContext context, Location location)
{
context.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd,
location));
}

private sealed class HeaderDictionarySymbols
{
public HeaderDictionarySymbols(Compilation compilation)
{
IHeaderDictionary = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IHeaderDictionary");
}

public bool HasRequiredSymbols => IHeaderDictionary is not null;

public INamedTypeSymbol IHeaderDictionary { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,10 @@
<data name="Analyzer_UnusedParameter_Title" xml:space="preserve">
<value>Unused route parameter</value>
</data>
<data name="Analyzer_HeaderDictionaryAdd_Message" xml:space="preserve">
<value>Use IHeaderDictionary.Append or the indexer to append or set headers. IDictionary.Add will throw an ArgumentException when attempting to add a duplicate key.</value>
</data>
<data name="Analyzer_HeaderDictionaryAdd_Title" xml:space="preserve">
<value>Suggest using IHeaderDictionary.Append or the indexer</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Simplification;

using Microsoft.CodeAnalysis.Simplification;

namespace Microsoft.AspNetCore.Analyzers.Http.Fixers;

[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class HeaderDictionaryAddFixer : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.DoNotUseIHeaderDictionaryAdd.Id);

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

public sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
{
foreach (var diagnostic in context.Diagnostics)
{
context.Document.TryGetSyntaxRoot(out var root);

if (CanReplaceWithAppend(diagnostic, root, out var invocation))
{
var appendTitle = "Use 'IHeaderDictionary.Append'";
context.RegisterCodeFix(
CodeAction.Create(appendTitle,
cancellationToken => ReplaceWithAppend(diagnostic, context.Document, invocation, cancellationToken),
equivalenceKey: appendTitle),
diagnostic);
}

if (CanReplaceWithIndexer(diagnostic, root, out var assignment))
{
var indexerTitle = "Use indexer";
context.RegisterCodeFix(
CodeAction.Create(indexerTitle,
cancellationToken => ReplaceWithIndexer(diagnostic, context.Document, assignment, cancellationToken),
equivalenceKey: indexerTitle),
diagnostic);
}
}

return Task.CompletedTask;
}

private static async Task<Document> ReplaceWithAppend(Diagnostic diagnostic, Document document, InvocationExpressionSyntax invocation, CancellationToken cancellationToken)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);

var model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var headerDictionaryExtensionsSymbol = model.Compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HeaderDictionaryExtensions");
var annotation = new SyntaxAnnotation("SymbolId", DocumentationCommentId.CreateReferenceId(headerDictionaryExtensionsSymbol));

return document.WithSyntaxRoot(
root.ReplaceNode(diagnosticTarget, invocation.WithAdditionalAnnotations(Simplifier.AddImportsAnnotation, annotation)));
}

private static bool CanReplaceWithAppend(Diagnostic diagnostic, SyntaxNode root, out InvocationExpressionSyntax invocation)
{
invocation = null;

if (root is not CompilationUnitSyntax)
{
return false;
}

var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);

if (diagnosticTarget is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name.Identifier: { } identifierToken } } invocationExpression)
{
invocation = invocationExpression.ReplaceToken(identifierToken, SyntaxFactory.Identifier("Append"));

return true;
}

return false;
}

private static async Task<Document> ReplaceWithIndexer(Diagnostic diagnostic, Document document, AssignmentExpressionSyntax assignment, CancellationToken cancellationToken)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);

return document.WithSyntaxRoot(root.ReplaceNode(diagnosticTarget, assignment));
}

private static bool CanReplaceWithIndexer(Diagnostic diagnostic, SyntaxNode root, out AssignmentExpressionSyntax assignment)
{
assignment = null;

if (root is null)
{
return false;
}

var diagnosticTarget = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);

if (diagnosticTarget is InvocationExpressionSyntax
{
Expression: MemberAccessExpressionSyntax memberAccessExpression,
ArgumentList.Arguments: { Count: 2 } arguments
})
{
assignment =
SyntaxFactory.AssignmentExpression(
SyntaxKind.SimpleAssignmentExpression,
SyntaxFactory.ElementAccessExpression(
memberAccessExpression.Expression,
SyntaxFactory.BracketedArgumentList(
SyntaxFactory.SeparatedList(new[] { arguments[0] }))),
arguments[1].Expression);

return true;
}

return false;
}
}
Loading