Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add infrastructure for creating diagnostics for general API usage #685

Merged
merged 15 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
## Current

### Added
- Added analyzers for identifying common namespaces, types, and members that require manual fixup and will produce diagnostics with links to relevant docs. The list of APIs identified by the analyzer can be expanded by adding to DefaultApiAlerts.json or by adding a .apitargets file to a project's additional files. [#685](https://github.com/dotnet/upgrade-assistant/pull/685)
- Link to survey [#735](https://github.com/dotnet/upgrade-assistant/pull/735)

## Version 0.2.236301 - 2021-07-15 ([Link](https://www.nuget.org/packages/upgrade-assistant/0.2.236301))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ private static void AddAnalyzersAndCodeFixProviders(IServiceCollection services)
{
// Add source analyzers and code fix providers (note that order doesn't matter as they're run alphabetically)
// Analyzers
services.AddTransient<DiagnosticAnalyzer, ApiAlertAnalyzer>();
services.AddTransient<DiagnosticAnalyzer, AttributeUpgradeAnalyzer>();
services.AddTransient<DiagnosticAnalyzer, BinaryFormatterUnsafeDeserializeAnalyzer>();
services.AddTransient<DiagnosticAnalyzer, HtmlHelperAnalyzer>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static StringComparer GetStringComparer(this ISymbol? symbol)
/// <summary>
/// Finds all members including those on the base classes.
/// </summary>
/// <param name="symbol">Symbol to search</param>
/// <param name="symbol">Symbol to search.</param>
/// <returns>A collection of base members.</returns>
public static IEnumerable<ISymbol> GetAllMembers(this INamedTypeSymbol? symbol)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,12 @@ public static SyntaxNode GetQualifiedName(this SyntaxNode node)
throw new ArgumentNullException(nameof(node));
}

// If the node is part of a qualified name, we want to get the full qualified name
while (node.Parent is CSSyntax.NameSyntax || node.Parent is VBSyntax.NameSyntax)
{
node = node.Parent;
}

// If the node is part of a member access expression (a static member access, for example), then the
// qualified name will be a member access expression rather than a name syntax.
if ((node.Parent is CSSyntax.MemberAccessExpressionSyntax csMAE && csMAE.Name.IsEquivalentTo(node))
// If the node is part of a qualified name or a member access expression, we want to get the full qualified name
// which will be the parent of that node. It's not necessary to recurse to the parent's ancestors since qualified
// names and member access expressions are composed of simple names on the right and the entire qualifier on the left.
if ((node.Parent is CSSyntax.QualifiedNameSyntax csName && csName.Right.IsEquivalentTo(node))
|| (node.Parent is VBSyntax.QualifiedNameSyntax vbName && vbName.Right.IsEquivalentTo(node))
|| (node.Parent is CSSyntax.MemberAccessExpressionSyntax csMAE && csMAE.Name.IsEquivalentTo(node))
|| (node.Parent is VBSyntax.MemberAccessExpressionSyntax vbMAE && vbMAE.Name.IsEquivalentTo(node)))
{
node = node.Parent;
Expand Down Expand Up @@ -278,6 +275,19 @@ private static bool RootIncludesImport(this SyntaxNode documentRoot, string name
throw new NotImplementedException(Resources.UnknownLanguage);
}

/// <summary>
/// Gets the ExpressionSyntax (the left part of the member access) from a C# or VB MemberAccessExpressionSyntax.
/// </summary>
/// <param name="memberAccessSyntax">The syntax whose expression propety should be returned. Note that this syntax must be of type Microsoft.CodeAnalysis.CSharp.Syntax.MemberAccessExpressionSyntax or Microsoft.CodeAnalysis.VisualBasic.Syntax.MemberAccessExpressionSyntax.</param>
/// <returns>The member access expression's expression (the left part of the member access).</returns>
public static SyntaxNode? GetChildExpressionSyntax(this SyntaxNode memberAccessSyntax) =>
memberAccessSyntax switch
{
CSSyntax.MemberAccessExpressionSyntax csMAE => csMAE.Expression,
VBSyntax.MemberAccessExpressionSyntax vbMAE => vbMAE.Expression,
_ => null
};

public static StringComparison GetStringComparison(this SyntaxNode? node)
=> node?.Language switch
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ UA0008 | Upgrade | Warning | UrlHelperAnalyzer
UA0010 | Upgrade | Warning | AllowHtmlAttributeAnalyzer
UA0011 | Upgrade | Warning | SystemDeploymentAnalyzer
UA0012 | Upgrade | Warning | BinaryFormatterUnsafeDeserializeAnalyzer
UA0013 | Upgrade | Warning | ApiAlert
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
// 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.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using CS = Microsoft.CodeAnalysis.CSharp;
using CSSyntax = Microsoft.CodeAnalysis.CSharp.Syntax;
using VB = Microsoft.CodeAnalysis.VisualBasic;
using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;

namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Default.Analyzers
{
/// <summary>
/// Analyzer for identifying usage of APIs that should be reported to the
/// user along with messaging about how the API should be (manually) replaced
/// to complete the upgrade.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public class ApiAlertAnalyzer : DiagnosticAnalyzer
{
/// <summary>
/// The base diagnostic ID that diagnostics produced by this analyzer will use as a prefix.
/// </summary>
public const string BaseDiagnosticId = "UA0013";

/// <summary>
/// The diagnsotic category for diagnostics produced by this analyzer.
/// </summary>
private const string Category = "Upgrade";
private const string AttributeSuffix = "Attribute";
private const string DefaultApiAlertsResourceName = "Microsoft.DotNet.UpgradeAssistant.Extensions.Default.Analyzers.DefaultApiAlerts.apitargets";

private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ApiAlertGenericTitle), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.ApiAlertGenericMessageFormat), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableString Description = new LocalizableResourceString(nameof(Resources.ApiAlertGenericDescription), Resources.ResourceManager, typeof(Resources));
private static readonly DiagnosticDescriptor GenericRule = new(BaseDiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description);

private readonly Lazy<IEnumerable<TargetSyntaxMessage>> _targetSyntaxes = new(() =>
{
using var resourceStream = new StreamReader(typeof(ApiAlertAnalyzer).Assembly.GetManifestResourceStream(DefaultApiAlertsResourceName));
return TargetSyntaxMessageLoader.LoadMappings(resourceStream.ReadToEnd())
?? throw new InvalidOperationException($"Could not read target syntax messages from resource {DefaultApiAlertsResourceName}");
});

// Supported diagnostics include all of the specific diagnostics read from DefaultApiAlerts.json and the generic diagnostic used for additional target syntax messages loaded at runtime.
// For some reason, Roslyn's analyzer scanning analyzer (that compares diagnostic IDs against AnalyzerReleases.* files) only identifies
// the generic UA0013 diagnostic here, so that's the only one added to AnalyzerReleases.Unshipped.md.
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.CreateRange(_targetSyntaxes.Value
.Select(t => new DiagnosticDescriptor($"{BaseDiagnosticId}_{t.Id}", $"Replace usage of {string.Join(", ", t.TargetSyntaxes.Select(a => a.FullName))}", t.Message, Category, DiagnosticSeverity.Warning, true, t.Message))
.Concat(new[] { GenericRule }));

/// <summary>
/// Initializes the analyzer by registering analysis callback methods.
/// </summary>
/// <param name="context">The context to use for initialization.</param>
public override void Initialize(AnalysisContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}

context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);

context.RegisterCompilationStartAction(context =>
{
// Load analyzer configuration defining the types that should be mapped.
var additionalTargetSyntaxes =
TargetSyntaxMessageLoader.LoadMappings(context.Options.AdditionalFiles);

var combinedTargetSyntaxes = _targetSyntaxes.Value.Concat(additionalTargetSyntaxes);

// Register actions for handling both C# and VB identifiers
context.RegisterSyntaxNodeAction(context => AnalyzeIdentifier(context, combinedTargetSyntaxes), CS.SyntaxKind.IdentifierName, CS.SyntaxKind.GenericName);
context.RegisterSyntaxNodeAction(context => AnalyzeIdentifier(context, combinedTargetSyntaxes), VB.SyntaxKind.IdentifierName, VB.SyntaxKind.GenericName);
});
}

private void AnalyzeIdentifier(SyntaxNodeAnalysisContext context, IEnumerable<TargetSyntaxMessage> targetSyntaxMessages)
{
var simpleName = context.Node switch
{
CSSyntax.SimpleNameSyntax csIdentifier => csIdentifier.Identifier.ValueText,
VBSyntax.SimpleNameSyntax vbIdentifier => vbIdentifier.Identifier.ValueText,
_ => throw new InvalidOperationException($"Unsupported syntax kind (expected C# or VB identifier name): {context.Node.GetType()}")
};

// Find target syntax/message mappings that include this node's simple name
var fullyQualifiedName = UnbindGenericName(context.Node.GetQualifiedName().ToString());

var stringComparison = context.Node.GetStringComparison();
var partialMatches = targetSyntaxMessages.SelectMany(m => m.TargetSyntaxes.Select(s => (TargetSyntax: s, Mapping: m)))
.Where(t => t.TargetSyntax.SyntaxType is TargetSyntaxType.Member

// For members, check that the syntax is a method access expression and only match the simple name since
// the expression portion of a member access expression may be a local name
? (context.Node.Parent?.IsMemberAccessExpression() ?? false) && t.TargetSyntax.SimpleName.Equals(simpleName, stringComparison)

// For types and namespaces, the syntax's entire name needs to match the target
: t.TargetSyntax.NameMatcher.MatchesPartiallyQualifiedType(fullyQualifiedName, stringComparison)
|| t.TargetSyntax.NameMatcher.MatchesPartiallyQualifiedType($"{fullyQualifiedName}{AttributeSuffix}", stringComparison));

if (!partialMatches.Any())
{
return;
}

foreach (var match in partialMatches)
{
if (match.TargetSyntax.SyntaxType switch
{
TargetSyntaxType.Member => AnalyzeMember(context, fullyQualifiedName, match.TargetSyntax),
TargetSyntaxType.Type => AnalyzeType(context, fullyQualifiedName, match.TargetSyntax),
TargetSyntaxType.Namespace => AnalyzeNamespace(context, fullyQualifiedName, match.TargetSyntax),
_ => false,
})
{
// Get the diagnostic descriptor correspdoning to the target API message map or, if a specific descriptor doesn't exist for it,
// get the default (first) one.
var id = $"{BaseDiagnosticId}_{match.Mapping.Id}";
var diagnosticDescriptor = SupportedDiagnostics.FirstOrDefault(d => d.Id.Equals(id, StringComparison.Ordinal)) ?? SupportedDiagnostics.First();

// Create and report the diagnostic. Note that the fully qualified name's location is used so
// that any future code fix provider can directly replace the node without needing to consider its parents.
context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, context.Node.GetQualifiedName().GetLocation(), match.Mapping.Message));
}
}
}

private static bool AnalyzeNamespace(SyntaxNodeAnalysisContext context, string qualifiedName, TargetSyntax targetSyntax)
{
// If the node is a fully qualified name from the specified namespace, return true
// This should cover both import statements and fully qualified type names, so no addtional checks
// for import statements are needed.
if (qualifiedName.Equals(targetSyntax.FullName, context.Node.GetStringComparison()))
{
return true;
}

// This method intentionally doesn't check type symbols to see if they're part of the
// targeted namespace as that diagnostic would likely be too noisy. This will just flag
// the import statement or fully-qualified use of the namespace name, instead.
return false;
}

private static bool AnalyzeType(SyntaxNodeAnalysisContext context, string qualifiedName, TargetSyntax targetSyntax)
{
// If the node matches the target's fully qualified name, return true.
if (qualifiedName.Equals(targetSyntax.FullName, context.Node.GetStringComparison()))
{
return true;
}

// Attempt to get the type symbol (either by getting the type symbol directly,
// or by getting general symbol info, as required to get the type symbol a ctor
// corresponds to)
var symbol = context.SemanticModel.GetTypeInfo(context.Node).Type
?? context.SemanticModel.GetSymbolInfo(context.Node).Symbol;

// If the node's type can be resolved, return true only if it matches
// the expected full name. If the node resolves to a non-type symbol,
// return false as this could indicate a local variable or something similar
// with a name that matches the target.
if (symbol is not null)
{
if (symbol is ITypeSymbol typeSymbol)
{
// This conditional can't be combined with the previous one because
// failing this condition should not lead to the else clause.
// In other words, if the symbol is a type symbol but is an error type
// symbol then don't return here (rather than returning false as would
// happen for non-type symbols).
if (typeSymbol is not IErrorTypeSymbol)
{
return targetSyntax.NameMatcher.Matches(typeSymbol);
}
}
else
{
return false;
}
}

// If the node's full type can't be determined (either by symbol or fully qualified syntax),
// return true for the partial match only if ambiguous matching is enabled
return targetSyntax.AlertOnAmbiguousMatch;
}

private static bool AnalyzeMember(SyntaxNodeAnalysisContext context, string qualifiedName, TargetSyntax targetSyntax)
{
// If the node matches the target's fully qualified name, return true.
if (qualifiedName.Equals(targetSyntax.FullName, context.Node.GetStringComparison()))
{
return true;
}

// If the parent type's symbol is resolvable, return true if
// it corresponds to the target type
var typeSyntax = context.Node.Parent?.GetChildExpressionSyntax();
if (typeSyntax is not null)
{
var symbol = context.SemanticModel.GetTypeInfo(typeSyntax).Type;
if (symbol is not null && symbol is not IErrorTypeSymbol)
{
return targetSyntax.NameMatcher.Matches(symbol);
}
}

// If the node's full type can't be determined (either by symbol or fully qualified syntax),
// return true for the partial match only if ambiguous matching is enabled
return targetSyntax.AlertOnAmbiguousMatch;
}

private static readonly Regex GenericParameterMatcher = new(@"[<(].*[>)]", RegexOptions.Compiled);

private static string UnbindGenericName(string name)
{
if (name is null)
{
throw new ArgumentNullException(nameof(name));
}

var match = GenericParameterMatcher.Match(name);

// If the name contains generic parameters, replace them with `x where x is the
// number of parameters. Otherwise, return the name as is.
return match.Success
? $"{name.Substring(0, match.Index)}`{match.Value.Count(c => c == ',') + 1}"
: name;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ private static void AnalyzeAttribute(SyntaxNodeAnalysisContext context, IEnumera
}

// If the attribute name isn't one of the mapped names, bail out
var stringComparison = context.Node.GetStringComparison();
var mapping = mappings.FirstOrDefault(m =>
{
var matcher = NameMatcher.MatchType(m.OldName);
return matcher.MatchesPartiallyQualifiedType(attributeName) || matcher.MatchesPartiallyQualifiedType($"{attributeName}{AttributeSuffix}");
return matcher.MatchesPartiallyQualifiedType(attributeName, stringComparison) || matcher.MatchesPartiallyQualifiedType($"{attributeName}{AttributeSuffix}", stringComparison);
});
if (mapping is null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public sealed class BinaryFormatterUnsafeDeserializeAnalyzer : DiagnosticAnalyze

private static readonly DiagnosticDescriptor Rule = new(DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description);

public static NameMatcher BinaryFormatterUnsafeDeserialize { get; } = NameMatcher.MatchPropertyAccess(QualifiedTargetSymbolName, TargetMember);
public static NameMatcher BinaryFormatterUnsafeDeserialize { get; } = NameMatcher.MatchMemberAccess(QualifiedTargetSymbolName, TargetMember);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

Expand Down
Loading