Adding analyzers for common mistakes with live renderables

examples/Console/AnalyzerTester/Program.cs

@@ -1,32 +1,13 @@
+using System.Threading;
 using Spectre.Console;
 
 namespace AnalyzerTester
 {
     class Program
     {
-        static void Main(string[] args)
+        static void Main()
         {
-            AnsiConsole.WriteLine("Hello World!");
-        }
-    }
-
-    class Dependency
-    {
-        private readonly IAnsiConsole _ansiConsole;
-
-        public Dependency(IAnsiConsole ansiConsole)
-        {
-            _ansiConsole = ansiConsole;
-        }
-
-        public void DoIt()
-        {
-            _ansiConsole.WriteLine("Hey mom!");
-        }
-
-        public void DoIt(IAnsiConsole thisConsole)
-        {
-            thisConsole.WriteLine("Hey mom!");
+            AnsiConsole.WriteLine("Project is set up with a reference to Spectre.Console.Analyzer");
         }
     }
 }

src/Spectre.Console.Analyzer/Analyzers/NoConcurrentLiveRenderablesAnalyzer.cs

@@ -0,0 +1,78 @@
+using System.Collections.Immutable;
+using System.Composition;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Spectre.Console.Analyzer
+{
+    /// <summary>
+    /// Analyzer to detect calls to live renderables within a live renderable context.
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    [Shared]
+    public class NoConcurrentLiveRenderablesAnalyzer : BaseAnalyzer
+    {
+        private static readonly DiagnosticDescriptor _diagnosticDescriptor =
+            Descriptors.S1020_AvoidConcurrentCallsToMultipleLiveRenderables;
+
+        /// <inheritdoc />
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+            ImmutableArray.Create(_diagnosticDescriptor);
+
+        /// <inheritdoc />
+        protected override void AnalyzeCompilation(CompilationStartAnalysisContext compilationStartContext)
+        {
+            compilationStartContext.RegisterOperationAction(
+                context =>
+                {
+                    var invocationOperation = (IInvocationOperation)context.Operation;
+                    var methodSymbol = invocationOperation.TargetMethod;
+
+                    const string StartMethod = "Start";
+                    if (methodSymbol.Name != StartMethod)
+                    {
+                        return;
+                    }
+
+                    var liveTypes = Constants.LiveRenderables
+                        .Select(i => context.Compilation.GetTypeByMetadataName(i))
+                        .ToImmutableArray();
+
+                    if (liveTypes.All(i => !Equals(i, methodSymbol.ContainingType)))
+                    {
+                        return;
+                    }
+
+                    var model = context.Compilation.GetSemanticModel(context.Operation.Syntax.SyntaxTree);
+                    var parentInvocations = invocationOperation
+                        .Syntax.Ancestors()
+                        .OfType<InvocationExpressionSyntax>()
+                        .Select(i => model.GetOperation(i))
+                        .OfType<IInvocationOperation>()
+                        .ToList();
+
+                    if (parentInvocations.All(parent =>
+                        parent.TargetMethod.Name != StartMethod || !liveTypes.Contains(parent.TargetMethod.ContainingType)))
+                    {
+                        return;
+                    }
+
+                    var displayString = SymbolDisplay.ToDisplayString(
+                        methodSymbol,
+                        SymbolDisplayFormat.CSharpShortErrorMessageFormat
+                            .WithParameterOptions(SymbolDisplayParameterOptions.None)
+                            .WithGenericsOptions(SymbolDisplayGenericsOptions.None));
+
+                    context.ReportDiagnostic(
+                        Diagnostic.Create(
+                            _diagnosticDescriptor,
+                            invocationOperation.Syntax.GetLocation(),
+                            displayString));
+                }, OperationKind.Invocation);
+        }
+    }
+}

src/Spectre.Console.Analyzer/Analyzers/NoPromptsDuringLiveRenderablesAnalyzer.cs

@@ -0,0 +1,84 @@
+using System.Collections.Immutable;
+using System.Composition;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Spectre.Console.Analyzer
+{
+    /// <summary>
+    /// Analyzer to detect calls to live renderables within a live renderable context.
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    [Shared]
+    public class NoPromptsDuringLiveRenderablesAnalyzer : BaseAnalyzer
+    {
+        private static readonly DiagnosticDescriptor _diagnosticDescriptor =
+            Descriptors.S1021_AvoidPromptCallsDuringLiveRenderables;
+
+        /// <inheritdoc />
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+            ImmutableArray.Create(_diagnosticDescriptor);
+
+        /// <inheritdoc />
+        protected override void AnalyzeCompilation(CompilationStartAnalysisContext compilationStartContext)
+        {
+            compilationStartContext.RegisterOperationAction(
+                context =>
+                {
+                    // if this operation isn't an invocation against one of the System.Console methods
+                    // defined in _methods then we can safely stop analyzing and return;
+                    var invocationOperation = (IInvocationOperation)context.Operation;
+                    var methodSymbol = invocationOperation.TargetMethod;
+
+                    var promptMethods = ImmutableArray.Create("Ask", "Confirm", "Prompt");
+                    if (!promptMethods.Contains(methodSymbol.Name))
+                    {
+                        return;
+                    }
+
+                    var ansiConsoleType = context.Compilation.GetTypeByMetadataName("Spectre.Console.AnsiConsole");
+                    var ansiConsoleExtensionsType = context.Compilation.GetTypeByMetadataName("Spectre.Console.AnsiConsoleExtensions");
+
+                    if (!Equals(methodSymbol.ContainingType, ansiConsoleType) && !Equals(methodSymbol.ContainingType, ansiConsoleExtensionsType))
+                    {
+                        return;
+                    }
+
+                    var model = context.Compilation.GetSemanticModel(context.Operation.Syntax.SyntaxTree);
+                    var parentInvocations = invocationOperation
+                        .Syntax.Ancestors()
+                        .OfType<InvocationExpressionSyntax>()
+                        .Select(i => model.GetOperation(i))
+                        .OfType<IInvocationOperation>()
+                        .ToList();
+
+                    var liveTypes = Constants.LiveRenderables
+                        .Select(i => context.Compilation.GetTypeByMetadataName(i))
+                        .ToImmutableArray();
+
+                    if (parentInvocations.All(parent =>
+                        parent.TargetMethod.Name != "Start" ||
+                        !liveTypes.Contains(parent.TargetMethod.ContainingType)))
+                    {
+                        return;
+                    }
+
+                    var displayString = SymbolDisplay.ToDisplayString(
+                        methodSymbol,
+                        SymbolDisplayFormat.CSharpShortErrorMessageFormat
+                            .WithParameterOptions(SymbolDisplayParameterOptions.None)
+                            .WithGenericsOptions(SymbolDisplayGenericsOptions.None));
+
+                    context.ReportDiagnostic(
+                        Diagnostic.Create(
+                            _diagnosticDescriptor,
+                            invocationOperation.Syntax.GetLocation(),
+                            displayString));
+                }, OperationKind.Invocation);
+        }
+    }
+}

src/Spectre.Console.Analyzer/Analyzers/UseSpectreInsteadOfSystemConsoleAnalyzer.cs

@@ -1,5 +1,4 @@
 using System.Collections.Immutable;
-using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Diagnostics;
@@ -27,37 +26,38 @@ namespace Spectre.Console.Analyzer
         {
             compilationStartContext.RegisterOperationAction(
                 context =>
-            {
-                // if this operation isn't an invocation against one of the System.Console methods
-                // defined in _methods then we can safely stop analyzing and return;
-                var invocationOperation = (IInvocationOperation)context.Operation;
-                var systemConsoleType = context.Compilation.GetTypeByMetadataName("System.Console");
-
-                if (!Equals(invocationOperation.TargetMethod.ContainingType, systemConsoleType))
                 {
-                    return;
-                }
+                    // if this operation isn't an invocation against one of the System.Console methods
+                    // defined in _methods then we can safely stop analyzing and return;
+                    var invocationOperation = (IInvocationOperation)context.Operation;
 
-                var methodName = System.Array.Find(_methods, i => i.Equals(invocationOperation.TargetMethod.Name));
-                if (methodName == null)
-                {
-                    return;
-                }
+                    var methodName = System.Array.Find(_methods, i => i.Equals(invocationOperation.TargetMethod.Name));
+                    if (methodName == null)
+                    {
+                        return;
+                    }
 
-                var methodSymbol = invocationOperation.TargetMethod;
+                    var systemConsoleType = context.Compilation.GetTypeByMetadataName("System.Console");
 
-                var displayString = SymbolDisplay.ToDisplayString(
-                    methodSymbol,
-                    SymbolDisplayFormat.CSharpShortErrorMessageFormat
-                        .WithParameterOptions(SymbolDisplayParameterOptions.None)
-                        .WithGenericsOptions(SymbolDisplayGenericsOptions.None));
+                    if (!Equals(invocationOperation.TargetMethod.ContainingType, systemConsoleType))
+                    {
+                        return;
+                    }
 
-                context.ReportDiagnostic(
-                    Diagnostic.Create(
-                        _diagnosticDescriptor,
-                        invocationOperation.Syntax.GetLocation(),
-                        displayString));
-            }, OperationKind.Invocation);
+                    var methodSymbol = invocationOperation.TargetMethod;
+
+                    var displayString = SymbolDisplay.ToDisplayString(
+                        methodSymbol,
+                        SymbolDisplayFormat.CSharpShortErrorMessageFormat
+                            .WithParameterOptions(SymbolDisplayParameterOptions.None)
+                            .WithGenericsOptions(SymbolDisplayGenericsOptions.None));
+
+                    context.ReportDiagnostic(
+                        Diagnostic.Create(
+                            _diagnosticDescriptor,
+                            invocationOperation.Syntax.GetLocation(),
+                            displayString));
+                }, OperationKind.Invocation);
         }
     }
 }

src/Spectre.Console.Analyzer/Constants.cs

@@ -4,5 +4,12 @@ namespace Spectre.Console.Analyzer
     {
         internal const string StaticInstance = "AnsiConsole";
         internal const string SpectreConsole = "Spectre.Console";
+
+        internal static readonly string[] LiveRenderables =
+        {
+            "Spectre.Console.LiveDisplay",
+            "Spectre.Console.Progress",
+            "Spectre.Console.Status",
+        };
     }
 }

src/Spectre.Console.Analyzer/Descriptors.cs

@@ -45,5 +45,27 @@ namespace Spectre.Console.Analyzer
                 Usage,
                 Info,
                 "Favor the use of the instance of AnsiConsole over the static helper.");
+
+        /// <summary>
+        /// Gets definitions of diagnostics Spectre1020.
+        /// </summary>
+        public static DiagnosticDescriptor S1020_AvoidConcurrentCallsToMultipleLiveRenderables { get; } =
+            Rule(
+                "Spectre1020",
+                "Avoid calling other live renderables while a current renderable is running.",
+                Usage,
+                Warning,
+                "Avoid calling other live renderables while a current renderable is running.");
+
+        /// <summary>
+        /// Gets definitions of diagnostics Spectre1020.
+        /// </summary>
+        public static DiagnosticDescriptor S1021_AvoidPromptCallsDuringLiveRenderables { get; } =
+            Rule(
+                "Spectre1021",
+                "Avoid prompting for input while a current renderable is running.",
+                Usage,
+                Warning,
+                "Avoid prompting for input while a current renderable is running.");
     }
 }

src/Spectre.Console.Tests/CodeAnalyzer/Analyzers/NoConcurrentLiveRenderablesTests.cs

@@ -0,0 +1,86 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Testing;
+using Spectre.Console.Analyzer;
+using Xunit;
+using AnalyzerVerify =
+    Spectre.Console.Tests.CodeAnalyzers.SpectreAnalyzerVerifier<
+        Spectre.Console.Analyzer.NoConcurrentLiveRenderablesAnalyzer>;
+
+namespace Spectre.Console.Tests.CodeAnalyzers.Analyzers
+{
+    public class NoCurrentLiveRenderablesTests
+    {
+        private static readonly DiagnosticResult _expectedDiagnostics = new(
+            Descriptors.S1020_AvoidConcurrentCallsToMultipleLiveRenderables.Id,
+            DiagnosticSeverity.Warning);
+
+        [Fact]
+        public async void Status_call_within_live_call_warns()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    void Go()
+    {
+        AnsiConsole.Live(new Table()).Start(ctx =>
+        {
+            AnsiConsole.Status().Start(""go"", innerCtx => {});
+        });
+    }
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source, _expectedDiagnostics.WithLocation(10, 13))
+                .ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async void Status_call_within_live_call_warns_with_instance()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class Child
+{
+    public readonly IAnsiConsole _console = AnsiConsole.Console;
+
+    public void Go()
+    {
+        _console.Status().Start(""starting"", context =>
+        {
+            _console.Progress().Start(progressContext => { });
+        });
+    }
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source, _expectedDiagnostics.WithLocation(12, 13))
+                .ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async void Calling_start_on_non_live_renderable_has_no_warning()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class Program
+{
+    static void Main()
+    {
+        Start();
+    }
+
+    static void Start() =>  AnsiConsole.WriteLine(""Starting..."");
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source)
+                .ConfigureAwait(false);
+        }
+    }
+    
+    
+}

src/Spectre.Console.Tests/CodeAnalyzer/Analyzers/NoPromptsDuringLiveRenderablesTests.cs

@@ -0,0 +1,108 @@
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Testing;
+using Spectre.Console.Analyzer;
+using Xunit;
+using AnalyzerVerify =
+    Spectre.Console.Tests.CodeAnalyzers.SpectreAnalyzerVerifier<
+        Spectre.Console.Analyzer.NoPromptsDuringLiveRenderablesAnalyzer>;
+
+namespace Spectre.Console.Tests.CodeAnalyzers.Analyzers
+{
+    public class NoPromptsDuringLiveRenderablesTests
+    {
+        private static readonly DiagnosticResult _expectedDiagnostics = new(
+            Descriptors.S1021_AvoidPromptCallsDuringLiveRenderables.Id,
+            DiagnosticSeverity.Warning);
+
+        [Fact]
+        public async Task Prompt_out_of_progress_does_not_warn()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    void Go()
+    {
+        var s = AnsiConsole.Ask<string>(""How are you?"");
+    }
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source)
+                .ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async Task Instance_variables_warn()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    public IAnsiConsole _console = AnsiConsole.Console;
+
+    public void Go()
+    {
+        _console.Status().Start(""starting"", context =>
+        {
+            var result = _console.Confirm(""we ok?"");
+        });
+    }
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source, _expectedDiagnostics.WithLocation(12, 26))
+                .ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async Task Prompt_in_progress_warns()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    void Go()
+    {
+        AnsiConsole.Progress().Start(_ =>
+        {
+            AnsiConsole.Ask<string>(""How are you?"");
+        });
+    }
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source, _expectedDiagnostics.WithLocation(10, 13))
+                .ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async Task Can_call_other_methods_from_within_renderables()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class Program
+{
+    static void Main()
+    {
+        AnsiConsole.Status().Start(""here we go"", context =>
+        {
+            var result = Confirm();
+
+        });
+    }
+
+    static string Confirm() => string.Empty;
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source)
+                .ConfigureAwait(false);
+        }
+    }
+}

src/Spectre.Console.Tests/CodeAnalyzer/Analyzers/UseInstanceAnsiConsoleTests.cs

@@ -14,6 +14,46 @@ namespace Spectre.Console.Tests.CodeAnalyzers.Analyzers
             Descriptors.S1010_FavorInstanceAnsiConsoleOverStatic.Id,
             DiagnosticSeverity.Info);
 
+        [Fact]
+        public async void Instance_console_has_no_warnings()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    IAnsiConsole _ansiConsole = AnsiConsole.Console;    
+
+    void TestMethod() 
+    {
+        _ansiConsole.Write(""this is fine"");
+    } 
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source)
+                .ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async void Static_console_with_no_instance_variables_has_no_warnings()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    void TestMethod() 
+    {
+        AnsiConsole.Write(""this is fine"");
+    } 
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source)
+                .ConfigureAwait(false);
+        }
+
         [Fact]
         public async void Console_Write_Has_Warning()
         {

src/Spectre.Console.Tests/CodeAnalyzer/Analyzers/UseSpectreInsteadOfSystemConsoleAnalyzerTests.cs

@@ -14,6 +14,24 @@ namespace Spectre.Console.Tests.CodeAnalyzers.Analyzers
             Descriptors.S1000_UseAnsiConsoleOverSystemConsole.Id,
             DiagnosticSeverity.Warning);
 
+        [Fact]
+        public async void Non_configured_SystemConsole_methods_report_no_warnings()
+        {
+            const string Source = @"
+using System;
+
+class TestClass {
+    void TestMethod() 
+    {
+        var s = Console.ReadLine();
+    } 
+}";
+
+            await AnalyzerVerify
+                .VerifyAnalyzerAsync(Source)
+                .ConfigureAwait(false);
+        }
+        
         [Fact]
         public async void Console_Write_Has_Warning()
         {

src/Spectre.Console.Tests/CodeAnalyzer/CodeFixProviderDiscovery.cs

@@ -6,7 +6,6 @@ using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.VisualStudio.Composition;
-using Spectre.Console.Analyzer;
 using Spectre.Console.Analyzer.FixProviders;
 
 namespace Spectre.Console.Tests.CodeAnalyzers

src/Spectre.Console.Tests/CodeAnalyzer/Fixes/UseInstanceOfStaticAnsiConsoleTests.cs

@@ -16,7 +16,7 @@ namespace Spectre.Console.Tests.CodeAnalyzers.Fixes
             DiagnosticSeverity.Info);
 
         [Fact]
-        public async Task SystemConsole_replaced_with_AnsiConsole()
+        public async Task Static_call_replaced_with_field_call()
         {
             const string Source = @"
 using Spectre.Console;
@@ -50,5 +50,71 @@ class TestClass
                 .VerifyCodeFixAsync(Source, _expectedDiagnostic.WithLocation(11, 9), FixedSource)
                 .ConfigureAwait(false);
         }
+
+        [Fact]
+        public async Task Static_call_replaced_with_parameter_call()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    void TestMethod(IAnsiConsole ansiConsole) 
+    {
+        AnsiConsole.Write(""Hello, World"");
+    } 
+}";
+
+            const string FixedSource = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    void TestMethod(IAnsiConsole ansiConsole) 
+    {
+        ansiConsole.Write(""Hello, World"");
+    } 
+}";
+
+            await AnalyzerVerify
+                .VerifyCodeFixAsync(Source, _expectedDiagnostic.WithLocation(8, 9), FixedSource)
+                .ConfigureAwait(false);
+        }
+        
+        [Fact]
+        public async Task Static_call_replaced_with_static_field_if_valid()
+        {
+            const string Source = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    static IAnsiConsole staticConsole;
+    IAnsiConsole instanceConsole;
+
+    static void TestMethod() 
+    {
+        AnsiConsole.Write(""Hello, World"");
+    } 
+}";
+
+            const string FixedSource = @"
+using Spectre.Console;
+
+class TestClass 
+{
+    static IAnsiConsole staticConsole;
+    IAnsiConsole instanceConsole;
+
+    static void TestMethod() 
+    {
+        staticConsole.Write(""Hello, World"");
+    } 
+}";
+
+            await AnalyzerVerify
+                .VerifyCodeFixAsync(Source, _expectedDiagnostic.WithLocation(11, 9), FixedSource)
+                .ConfigureAwait(false);
+        }
     }
 }

src/Spectre.Console.Tests/CodeAnalyzer/SpectreAnalyzerVerifier.cs

@@ -43,8 +43,6 @@ namespace Spectre.Console.Tests.CodeAnalyzers
             {
                 TestCode = source,
                 CompilerDiagnostics = CompilerDiagnostics.All,
-                ReferenceAssemblies = CodeAnalyzerHelper.CurrentSpectre,
-                TestBehaviors = TestBehaviors.SkipGeneratedCodeCheck,
             };
 
             test.ExpectedDiagnostics.AddRange(expected);