Merge pull request #17262 from CyrusNajmabadi/alwaysOfferToConvertToFromBlocks

Always offer to convert to/from expression/block bodies.

src/EditorFeatures/CSharpTest/UseExpressionBody/UseExpressionBodyForAccessorsTests.cs

@@ -24,23 +24,23 @@ internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProvider
         private static readonly Dictionary<OptionKey, object> UseExpressionBody =
             new Dictionary<OptionKey, object>
             {
-                { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CodeStyleOptions.TrueWithNoneEnforcement },
-                { CSharpCodeStyleOptions.PreferExpressionBodiedProperties, CodeStyleOptions.FalseWithNoneEnforcement },
-                { CSharpCodeStyleOptions.PreferExpressionBodiedIndexers, CodeStyleOptions.FalseWithNoneEnforcement }
+                { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CodeStyleOptions.TrueWithSuggestionEnforcement },
+                { CSharpCodeStyleOptions.PreferExpressionBodiedProperties, CodeStyleOptions.FalseWithSuggestionEnforcement },
+                { CSharpCodeStyleOptions.PreferExpressionBodiedIndexers, CodeStyleOptions.FalseWithSuggestionEnforcement }
             };
 
         private static readonly Dictionary<OptionKey, object> UseExpressionBodyIncludingPropertiesAndIndexers =
             new Dictionary<OptionKey, object>
             {
-                { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CodeStyleOptions.TrueWithNoneEnforcement },
-                { CSharpCodeStyleOptions.PreferExpressionBodiedProperties, CodeStyleOptions.TrueWithNoneEnforcement },
-                { CSharpCodeStyleOptions.PreferExpressionBodiedIndexers, CodeStyleOptions.TrueWithNoneEnforcement }
+                { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CodeStyleOptions.TrueWithSuggestionEnforcement },
+                { CSharpCodeStyleOptions.PreferExpressionBodiedProperties, CodeStyleOptions.TrueWithSuggestionEnforcement },
+                { CSharpCodeStyleOptions.PreferExpressionBodiedIndexers, CodeStyleOptions.TrueWithSuggestionEnforcement }
             };
 
         private static readonly Dictionary<OptionKey, object> UseBlockBody =
             new Dictionary<OptionKey, object>
             {
-                { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CodeStyleOptions.FalseWithNoneEnforcement }
+                { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CodeStyleOptions.FalseWithSuggestionEnforcement }
             };
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExpressionBody)]
@@ -62,8 +62,8 @@ int Foo
     int Foo
     {
         get => Bar();
-        }
-    }", options: UseExpressionBody);
+    }
+}", options: UseExpressionBody);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExpressionBody)]
@@ -147,14 +147,31 @@ int Foo
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExpressionBody)]
         public async Task TestMissingWithOnlySetter()
         {
-            await TestMissingAsync(
+            await TestActionCountAsync(
 @"class C
 {
     int Foo
     {
         set => [|Bar|]();
-        }
-    }", options: UseExpressionBody);
+    }
+}", count: 1, featureOptions: UseExpressionBody);
+
+            // There is a hidden diagnostic that still offers to convert expression-body to block-body.
+            await TestAsync(
+@"class C
+{
+    int Foo
+    {
+        set => [|Bar|]();
+    }
+}",
+@"class C
+{
+    int Foo
+    {
+        set { Bar(); }
+    }
+}", options: UseExpressionBody);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExpressionBody)]

src/Features/CSharp/Portable/UseExpressionBody/AbstractUseExpressionBodyCodeFixProvider.cs

@@ -41,16 +41,20 @@ internal abstract partial class AbstractUseExpressionBodyCodeFixProvider<TDeclar
             _useBlockBodyTitle = useBlockBodyTitle;
         }
 
+        protected override bool IncludeDiagnosticDuringFixAll(Diagnostic diagnostic)
+            => diagnostic.Severity != DiagnosticSeverity.Hidden;
+
         public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
         {
             var diagnostic = context.Diagnostics.First();
             var documentOptionSet = await context.Document.GetOptionsAsync(context.CancellationToken).ConfigureAwait(false);
-            var title = documentOptionSet.GetOption(_option).Value
-                ? _useExpressionBodyTitle
-                : _useBlockBodyTitle;
+
+            var priority = diagnostic.Severity == DiagnosticSeverity.Hidden
+                ? CodeActionPriority.Low
+                : CodeActionPriority.Medium;
 
             context.RegisterCodeFix(
-                new MyCodeAction(title, c => FixAsync(context.Document, diagnostic, c)),
+                new MyCodeAction(diagnostic.GetMessage(), priority, c => FixAsync(context.Document, diagnostic, c)),
                 diagnostic);
         }
 
@@ -59,31 +63,30 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
             SyntaxEditor editor, CancellationToken cancellationToken)
         {
             var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false);
-            var preferExpressionBody = options.GetOption(_option).Value;
 
             foreach (var diagnostic in diagnostics)
             {
                 cancellationToken.ThrowIfCancellationRequested();
-                AddEdits(editor, diagnostic, options, preferExpressionBody, cancellationToken);
+                AddEdits(editor, diagnostic, options, cancellationToken);
             }
         }
 
         private void AddEdits(
             SyntaxEditor editor, Diagnostic diagnostic,
-            OptionSet options, bool preferExpressionBody,
-            CancellationToken cancellationToken)
+            OptionSet options, CancellationToken cancellationToken)
         {
             var declarationLocation = diagnostic.AdditionalLocations[0];
             var declaration = (TDeclaration)declarationLocation.FindNode(cancellationToken);
 
-            var updatedDeclaration = this.Update(declaration, preferExpressionBody, options)
+            var updatedDeclaration = this.Update(declaration, options)
                                          .WithAdditionalAnnotations(Formatter.Annotation);
 
             editor.ReplaceNode(declaration, updatedDeclaration);
         }
 
-        private TDeclaration Update(TDeclaration declaration, bool preferExpressionBody, OptionSet options)
+        private TDeclaration Update(TDeclaration declaration, OptionSet options)
         {
+            var preferExpressionBody = GetBody(declaration) != null;
             if (preferExpressionBody)
             {
                 GetBody(declaration).TryConvertToExpressionBody(declaration.SyntaxTree.Options,
@@ -166,9 +169,12 @@ protected virtual TDeclaration WithAccessorList(TDeclaration declaration, Access
 
         private class MyCodeAction : CodeAction.DocumentChangeAction
         {
-            public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
+            internal override CodeActionPriority Priority { get; }
+
+            public MyCodeAction(string title, CodeActionPriority priority, Func<CancellationToken, Task<Document>> createChangedDocument)
                 : base(title, createChangedDocument)
             {
+                this.Priority = priority;
             }
         }
     }

src/Features/CSharp/Portable/UseExpressionBody/AbstractUseExpressionBodyDiagnosticAnalyzer.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Immutable;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
@@ -59,44 +60,76 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
 
         internal virtual Diagnostic AnalyzeSyntax(OptionSet optionSet, TDeclaration declaration)
         {
+            // Note: we will always offer to convert a block to an expression-body (and vice versa)
+            // if possible.  All the user preference does is determine if we show them anything in
+            // the UI (i.e. suggestion dots, or a squiggle) to let them know that they can make the
+            // change.  
+            //
+            // This way, users can turn off the option so they don't get notified, but they can still
+            // make the transformation on a case by case basis.
+            //
+            // Note: if we decide to hide any adornments, then we also broaden the location where the
+            // fix is available.  That way the user can go to any place in the member and choose to
+            // convert it.  Otherwise, they'd have no idea what the 'right' location was to invoke
+            // the conversion.
+            //
+            // Also, if the diagnostic is hidden, we'll lower the priority of the code action.  We
+            // always want it to be available.  But we don't want it to override issues that are
+            // actually being reported in the UI.
+
             var preferExpressionBodiedOption = optionSet.GetOption(_option);
 
             var expressionBody = GetExpressionBody(declaration);
 
-            if (preferExpressionBodiedOption.Value)
+            if (expressionBody == null)
             {
-                if (expressionBody == null)
+                // They don't have an expression body.  See if we can convert into one.
+                // If so, offer the conversion (with the proper severity depending on their options).
+                if (!GetBody(declaration).TryConvertToExpressionBody(declaration.SyntaxTree.Options,
+                        out expressionBody, out var semicolonToken))
                 {
-                    // They want expression bodies and they don't have one.  See if we can
-                    // convert this to have an expression body.
-                    if (GetBody(declaration).TryConvertToExpressionBody(declaration.SyntaxTree.Options,
-                            out expressionBody, out var semicolonToken))
-                    {
-                        var additionalLocations = ImmutableArray.Create(declaration.GetLocation());
-                        return Diagnostic.Create(
-                            CreateDescriptorWithTitle(_expressionBodyTitle, preferExpressionBodiedOption.Notification.Value),
-                            GetBody(declaration).Statements[0].GetLocation(),
-                            additionalLocations: additionalLocations);
-                    }
+                    // Not a block that can be converted to an expression.
+                    return null;
                 }
+
+                var severity = preferExpressionBodiedOption.Value
+                    ? preferExpressionBodiedOption.Notification.Value
+                    : DiagnosticSeverity.Hidden;
+
+                var location = severity == DiagnosticSeverity.Hidden
+                    ? declaration.GetLocation()
+                    : GetBody(declaration).Statements[0].GetLocation();
+
+                var additionalLocations = ImmutableArray.Create(declaration.GetLocation());
+                return Diagnostic.Create(
+                    CreateDescriptorWithTitle(_expressionBodyTitle, severity, GetCustomTags(severity)),
+                    location, additionalLocations: additionalLocations);
             }
             else
             {
-                // They don't want expression bodies but they have one.  Offer to convert this to 
-                // a normal block
-                if (expressionBody != null)
-                {
-                    var additionalLocations = ImmutableArray.Create(declaration.GetLocation());
-                    return Diagnostic.Create(
-                        CreateDescriptorWithTitle(_blockBodyTitle, preferExpressionBodiedOption.Notification.Value),
-                        expressionBody.GetLocation(),
-                        additionalLocations: additionalLocations);
-                }
-            }
+                // They have an expression body.  These can always be converted into blocks.
+                // Offer to convert this to a block, with the appropriate severity based
+                // on their options.
+                var severity = preferExpressionBodiedOption.Value
+                    ? DiagnosticSeverity.Hidden
+                    : preferExpressionBodiedOption.Notification.Value;
 
-            return null;
+                var location = severity == DiagnosticSeverity.Hidden
+                    ? declaration.GetLocation()
+                    : expressionBody.GetLocation();
+
+                var additionalLocations = ImmutableArray.Create(declaration.GetLocation());
+                return Diagnostic.Create(
+                    CreateDescriptorWithTitle(_blockBodyTitle, severity, GetCustomTags(severity)),
+                    location, additionalLocations: additionalLocations);
+            }
         }
 
+        private static string[] GetCustomTags(DiagnosticSeverity severity)
+            => severity == DiagnosticSeverity.Hidden
+                ? new[] { WellKnownDiagnosticTags.NotConfigurable }
+                : Array.Empty<string>();
+
         protected static BlockSyntax GetBodyFromSingleGetAccessor(AccessorListSyntax accessorList)
         {
             if (accessorList != null &&

src/Features/CSharp/Portable/UseExpressionBody/Accessors/UseExpressionBodyForAccessorsDiagnosticAnalyzer.cs

@@ -40,7 +40,7 @@ internal override Diagnostic AnalyzeSyntax(OptionSet optionSet, AccessorDeclarat
             {
                 var propertyDeclaration = (PropertyDeclarationSyntax)grandParent;
                 var diagnostic = propertyAnalyzer.AnalyzeSyntax(optionSet, propertyDeclaration);
-                if (diagnostic != null)
+                if (diagnostic != null && diagnostic.Severity != DiagnosticSeverity.Hidden)
                 {
                     return null;
                 }
@@ -49,7 +49,7 @@ internal override Diagnostic AnalyzeSyntax(OptionSet optionSet, AccessorDeclarat
             {
                 var indexerDeclaration = (IndexerDeclarationSyntax)grandParent;
                 var diagnostic = indexerAnalyzer.AnalyzeSyntax(optionSet, indexerDeclaration);
-                if (diagnostic != null)
+                if (diagnostic != null && diagnostic.Severity != DiagnosticSeverity.Hidden)
                 {
                     return null;
                 }

src/Features/CSharp/Portable/UseExpressionBody/Indexers/UseExpressionBodyForIndexersCodeFixProvider.cs

@@ -28,7 +28,7 @@ protected override ArrowExpressionClauseSyntax GetExpressionBody(IndexerDeclarat
             => declaration.ExpressionBody;
 
         protected override BlockSyntax GetBody(IndexerDeclarationSyntax declaration)
-            => declaration.AccessorList.Accessors[0].Body;
+            => declaration.AccessorList?.Accessors[0].Body;
 
         protected override IndexerDeclarationSyntax WithSemicolonToken(IndexerDeclarationSyntax declaration, SyntaxToken token)
             => declaration.WithSemicolonToken(token);

src/Features/CSharp/Portable/UseExpressionBody/Properties/UseExpressionBodyForPropertiesCodeFixProvider.cs

@@ -30,7 +30,7 @@ protected override ArrowExpressionClauseSyntax GetExpressionBody(PropertyDeclara
             => declaration.ExpressionBody;
 
         protected override BlockSyntax GetBody(PropertyDeclarationSyntax declaration)
-            => declaration.AccessorList.Accessors[0].Body;
+            => declaration.AccessorList?.Accessors[0].Body;
 
         protected override PropertyDeclarationSyntax WithSemicolonToken(PropertyDeclarationSyntax declaration, SyntaxToken token)
             => declaration.WithSemicolonToken(token);

src/Features/Core/Portable/CodeStyle/AbstractCodeStyleDiagnosticAnalyzer.cs

@@ -28,7 +28,7 @@ internal abstract class AbstractCodeStyleDiagnosticAnalyzer : DiagnosticAnalyzer
         /// Diagnostic descriptor for code you want to fade out and do *not* want to have a smart-tag
         /// appear for.  This is uncommon but useful in some cases.  For example, if you are fading
         /// out pieces of code before/after another piece of code *on the same line*, then you will
-        /// only want one usafe of <see cref="UnnecessaryWithSuggestionDescriptor"/> and multiple
+        /// only want one usage of <see cref="UnnecessaryWithSuggestionDescriptor"/> and multiple
         /// usages of <see cref="UnnecessaryWithoutSuggestionDescriptor"/>.
         /// 
         /// That's because if you use <see cref="UnnecessaryWithSuggestionDescriptor"/> for all the

src/Workspaces/Core/Portable/CodeFixes/SyntaxEditorBasedCodeFixProvider.FixAllProvider.cs

@@ -46,7 +46,7 @@ public sealed override async Task<CodeAction> GetFixAsync(FixAllContext fixAllCo
                 var currentSolution = fixAllState.Solution;
                 foreach (var task in updatedDocumentTasks)
                 {
-                    // 'await' the tasks so that if any completed in a cancelled manner then we'll
+                    // 'await' the tasks so that if any completed in a canceled manner then we'll
                     // throw the right exception here.  Calling .Result on the tasks might end up
                     // with AggregateExceptions being thrown instead.
                     var updatedDocument = await task.ConfigureAwait(false);