From edc99e4297d5a26b31ec67ee1ccbd24bcb1d361b Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Thu, 7 Apr 2016 12:15:06 -0700 Subject: [PATCH] Move to using an IOperation analyzer for PopulateSwitch. --- src/Features/Core/Portable/Features.csproj | 2 +- .../AbstractPopulateSwitchCodeFixProvider.cs | 21 ++-- ...cs => PopulateSwitchDiagnosticAnalyzer.cs} | 51 ++++---- .../PopulateSwitch/PopulateSwitchHelpers.cs | 118 +++++++++++++++--- 4 files changed, 131 insertions(+), 61 deletions(-) rename src/Features/Core/Portable/PopulateSwitch/{AbstractPopulateSwitchDiagnosticAnalyzer.cs => PopulateSwitchDiagnosticAnalyzer.cs} (54%) diff --git a/src/Features/Core/Portable/Features.csproj b/src/Features/Core/Portable/Features.csproj index 243de09a5fb..4201a37e5d1 100644 --- a/src/Features/Core/Portable/Features.csproj +++ b/src/Features/Core/Portable/Features.csproj @@ -294,7 +294,7 @@ - + diff --git a/src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchCodeFixProvider.cs b/src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchCodeFixProvider.cs index 7140a470666..49b40fe8cf4 100644 --- a/src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchCodeFixProvider.cs +++ b/src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchCodeFixProvider.cs @@ -10,15 +10,16 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Semantics; using Microsoft.CodeAnalysis.Simplification; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.PopulateSwitch { internal abstract class AbstractPopulateSwitchCodeFixProvider : CodeFixProvider - where TSwitchBlockSyntax : SyntaxNode - where TSwitchSectionSyntax : SyntaxNode - where TExpressionSyntax : SyntaxNode + where TSwitchBlockSyntax : SyntaxNode + where TSwitchSectionSyntax : SyntaxNode + where TExpressionSyntax : SyntaxNode { public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(IDEDiagnosticIds.PopulateSwitchDiagnosticId); @@ -63,16 +64,12 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) return SpecializedTasks.EmptyTask; } - protected abstract TExpressionSyntax GetSwitchExpression(TSwitchBlockSyntax switchBlock); - protected abstract int InsertPosition(SyntaxList sections); protected abstract SyntaxList GetSwitchSections(TSwitchBlockSyntax switchBlock); protected abstract TSwitchBlockSyntax NewSwitchNode(TSwitchBlockSyntax switchBlock, SyntaxList sections); - protected abstract List GetCaseLabels(TSwitchBlockSyntax switchBlock, out bool containsDefaultLabel); - private async Task AddMissingSwitchCasesAsync( CodeFixContext context, bool includeMissingCases, bool includeDefaultCase) { @@ -82,13 +79,13 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var switchNode = (TSwitchBlockSyntax) root.FindNode(span); - var enumType = (INamedTypeSymbol)model.GetTypeInfo(GetSwitchExpression(switchNode)).Type; + var switchNode = (TSwitchBlockSyntax)root.FindNode(span); + var switchStatement = (ISwitchStatement)model.GetOperation(switchNode, cancellationToken); + var enumType = switchStatement.Value.Type; - bool containsDefaultCase; - var caseLabels = GetCaseLabels(switchNode, out containsDefaultCase); - var missingLabels = PopulateSwitchHelpers.GetMissingSwitchCases(model, enumType, caseLabels); + var containsDefaultCase = PopulateSwitchHelpers.HasDefaultCase(switchStatement); + var missingLabels = PopulateSwitchHelpers.GetMissingEnumMembers(switchStatement, enumType); var generator = SyntaxGenerator.GetGenerator(document); diff --git a/src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchDiagnosticAnalyzer.cs b/src/Features/Core/Portable/PopulateSwitch/PopulateSwitchDiagnosticAnalyzer.cs similarity index 54% rename from src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchDiagnosticAnalyzer.cs rename to src/Features/Core/Portable/PopulateSwitch/PopulateSwitchDiagnosticAnalyzer.cs index d65067af360..eea3ef58945 100644 --- a/src/Features/Core/Portable/PopulateSwitch/AbstractPopulateSwitchDiagnosticAnalyzer.cs +++ b/src/Features/Core/Portable/PopulateSwitch/PopulateSwitchDiagnosticAnalyzer.cs @@ -3,13 +3,14 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis.Diagnostics; using System.Diagnostics; +using Microsoft.CodeAnalysis.Semantics; +using System.Linq; +using System; namespace Microsoft.CodeAnalysis.PopulateSwitch { - internal abstract class AbstractPopulateSwitchDiagnosticAnalyzerBase : DiagnosticAnalyzer, IBuiltInAnalyzer - where TLanguageKindEnum : struct - where TSwitchBlockSyntax : SyntaxNode - where TExpressionSyntax : SyntaxNode + [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] + internal class PopulateSwitchDiagnosticAnalyzer : DiagnosticAnalyzer, IBuiltInAnalyzer { private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(FeaturesResources.Add_missing_switch_cases), FeaturesResources.ResourceManager, typeof(FeaturesResources)); private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(WorkspacesResources.PopulateSwitch), WorkspacesResources.ResourceManager, typeof(WorkspacesResources)); @@ -23,26 +24,22 @@ internal abstract class AbstractPopulateSwitchDiagnosticAnalyzerBase SyntaxKindsOfInterest { get; } - protected abstract TExpressionSyntax GetExpression(TSwitchBlockSyntax node); - protected abstract List GetCaseLabels(TSwitchBlockSyntax switchBlock, out bool hasDefaultCase); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(s_descriptor); public override void Initialize(AnalysisContext context) { - context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKindsOfInterest); + context.RegisterOperationAction(AnalyzeOperation, OperationKind.SwitchStatement); } - private void AnalyzeNode(SyntaxNodeAnalysisContext context) + private void AnalyzeOperation(OperationAnalysisContext context) { - var model = context.SemanticModel; - var tree = model.SyntaxTree; - var switchBlock = (TSwitchBlockSyntax)context.Node; + var switchOperation = (ISwitchStatement)context.Operation; + var switchBlock = switchOperation.Syntax; + var tree = switchBlock.SyntaxTree; bool missingCases; bool missingDefaultCase; - if (SwitchIsIncomplete(model, switchBlock, out missingCases, out missingDefaultCase) && + if (SwitchIsIncomplete(switchOperation, out missingCases, out missingDefaultCase) && !tree.OverlapsHiddenPosition(switchBlock.Span, context.CancellationToken)) { Debug.Assert(missingCases || missingDefaultCase); @@ -59,32 +56,28 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context) #endregion private bool SwitchIsIncomplete( - SemanticModel model, TSwitchBlockSyntax node, + ISwitchStatement switchStatement, out bool missingCases, out bool missingDefaultCase) { - bool hasDefaultCase; - var caseLabels = GetCaseLabels(node, out hasDefaultCase); - - missingDefaultCase = !hasDefaultCase; + missingDefaultCase = !PopulateSwitchHelpers.HasDefaultCase(switchStatement); missingCases = false; - // We know the switch has a 'default' label. Now we need to determine if there are - // any missing labels so that we can offer to generate them for the user. + var switchExpression = switchStatement.Value; + var switchExpressionType = switchExpression?.Type; - // If we can't determine the type of this switch, or we're switching over someting - // that sin't an enum, just consider this switch complete. We can't add any cases - // here. - var enumType = model.GetTypeInfo(GetExpression(node)).Type as INamedTypeSymbol; - if (enumType != null && enumType.TypeKind == TypeKind.Enum) + if (switchExpressionType?.TypeKind == TypeKind.Enum) { - var missingSwitchCases = PopulateSwitchHelpers.GetMissingSwitchCases(model, enumType, caseLabels); - missingCases = missingSwitchCases.Count > 0; + var missingEnumMembers = PopulateSwitchHelpers.GetMissingEnumMembers( + switchStatement, switchExpressionType); + missingCases = missingEnumMembers.Count > 0; } // The switch is incomplete if we're missing any cases or we're missing a default case. return missingDefaultCase || missingCases; } - public DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SyntaxAnalysis; + + + public DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SemanticSpanAnalysis; } } \ No newline at end of file diff --git a/src/Features/Core/Portable/PopulateSwitch/PopulateSwitchHelpers.cs b/src/Features/Core/Portable/PopulateSwitch/PopulateSwitchHelpers.cs index 7180f92ecb2..5938964ea0b 100644 --- a/src/Features/Core/Portable/PopulateSwitch/PopulateSwitchHelpers.cs +++ b/src/Features/Core/Portable/PopulateSwitch/PopulateSwitchHelpers.cs @@ -1,4 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using Microsoft.CodeAnalysis.Semantics; +using Microsoft.CodeAnalysis.Shared.Utilities; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.PopulateSwitch @@ -8,12 +11,89 @@ internal static class PopulateSwitchHelpers public const string MissingCases = nameof(MissingCases); public const string MissingDefaultCase = nameof(MissingDefaultCase); - public static IReadOnlyList GetMissingSwitchCases( - SemanticModel model, - INamedTypeSymbol enumType, - IReadOnlyList labelNames) where TExpressionSyntax : SyntaxNode + public static bool HasDefaultCase(ISwitchStatement switchStatement) + { + foreach (var switchCase in switchStatement.Cases) + { + if (HasDefaultCase(switchCase)) + { + return true; + } + } + + return false; + } + + private static bool HasDefaultCase(ISwitchCase switchCase) + { + foreach (var clause in switchCase.Clauses) + { + if (clause.CaseKind == CaseKind.Default) + { + return true; + } + } + + return false; + } + + public static ICollection GetMissingEnumMembers( + ISwitchStatement switchStatement, ITypeSymbol enumType) + { + var enumMembers = new Dictionary(); + if (!TryGetAllEnumMembers(enumType, enumMembers) || + !TryRemoveExistingEnumMembers(switchStatement, enumMembers)) + { + return SpecializedCollections.EmptyCollection(); + } + + return enumMembers.Values; + } + + private static bool TryRemoveExistingEnumMembers(ISwitchStatement switchStatement, Dictionary enumValues) + { + foreach (var switchCase in switchStatement.Cases) + { + foreach (var clause in switchCase.Clauses) + { + switch (clause.CaseKind) + { + default: + case CaseKind.None: + case CaseKind.Relational: + case CaseKind.Range: + // This was some sort of complex switch. For now just ignore + // these and assume that they're complete. + return false; + + case CaseKind.Default: + // ignore the 'default/else' clause. + continue; + + case CaseKind.SingleValue: + var value = ((ISingleValueCaseClause)clause).Value; + if (!value.ConstantValue.HasValue) + { + // We had a case which didn't resolve properly. + // Assume the switch is complete. + return false; + } + + var caseValue = IntegerUtilities.ToInt64(value.ConstantValue.Value); + enumValues.Remove(caseValue); + + break; + } + } + } + + return true; + } + + private static bool TryGetAllEnumMembers( + ITypeSymbol enumType, + Dictionary enumValues) { - var missingSwitchCases = new List(); foreach (var member in enumType.GetMembers()) { // skip `.ctor` and `__value` @@ -23,24 +103,24 @@ internal static class PopulateSwitchHelpers continue; } - missingSwitchCases.Add(member); - } - - foreach (var label in labelNames) - { - var symbol = model.GetSymbolInfo(label).Symbol; - if (symbol == null) + if (fieldSymbol.ConstantValue == null) { - // something is wrong with the label and the SemanticModel was unable to - // determine its symbol. Abort the analyzer by considering this switch - // statement as complete. - return SpecializedCollections.EmptyReadOnlyList(); + // We have an enum that has problems with it (i.e. non-const members). We won't + // be able to determine properly if the switch is complete. Assume it is so we + // don't offer to do anything. + return false; } - missingSwitchCases.Remove(symbol); + // Multiple enum members may have the same value. Only consider the first one + // we run int. + var enumValue = IntegerUtilities.ToInt64(fieldSymbol.ConstantValue); + if (!enumValues.ContainsKey(enumValue)) + { + enumValues.Add(enumValue, fieldSymbol); + } } - return missingSwitchCases; + return true; } } } -- GitLab