UseShellExecute analyzer (#707)

README.md

@@ -176,6 +176,9 @@ If you are already using other analyzers, you can check [which rules are duplica
 |[MA0158](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0158.md)|Performance|Use System.Threading.Lock|⚠️|✔️|❌|
 |[MA0159](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0159.md)|Performance|Use 'Order' instead of 'OrderBy'|ℹ️|✔️|✔️|
 |[MA0160](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0160.md)|Performance|Use ContainsKey instead of TryGetValue|ℹ️|✔️|❌|
+|[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|ℹ️|❌|❌|
+|[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|ℹ️|❌|❌|
+|[MA0163](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0163.md)|Usage|UseShellExecute must be false when redirecting standard input or output|⚠️|✔️|❌|
 
 <!-- rules -->
 

docs/README.md

@@ -160,6 +160,9 @@
 |[MA0158](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0158.md)|Performance|Use System.Threading.Lock|<span title='Warning'>⚠️</span>|✔️|❌|
 |[MA0159](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0159.md)|Performance|Use 'Order' instead of 'OrderBy'|<span title='Info'>ℹ️</span>|✔️|✔️|
 |[MA0160](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0160.md)|Performance|Use ContainsKey instead of TryGetValue|<span title='Info'>ℹ️</span>|✔️|❌|
+|[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|<span title='Info'>ℹ️</span>|❌|❌|
+|[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|<span title='Info'>ℹ️</span>|❌|❌|
+|[MA0163](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0163.md)|Usage|UseShellExecute must be false when redirecting standard input or output|<span title='Warning'>⚠️</span>|✔️|❌|
 
 |Id|Suppressed rule|Justification|
 |--|---------------|-------------|
@@ -646,6 +649,15 @@ dotnet_diagnostic.MA0159.severity = suggestion
 
 # MA0160: Use ContainsKey instead of TryGetValue
 dotnet_diagnostic.MA0160.severity = suggestion
+
+# MA0161: UseShellExecute must be explicitly set
+dotnet_diagnostic.MA0161.severity = none
+
+# MA0162: Use Process.Start overload with ProcessStartInfo
+dotnet_diagnostic.MA0162.severity = none
+
+# MA0163: UseShellExecute must be false when redirecting standard input or output
+dotnet_diagnostic.MA0163.severity = warning
 ```
 
 # .editorconfig - all rules disabled
@@ -1127,4 +1139,13 @@ dotnet_diagnostic.MA0159.severity = none
 
 # MA0160: Use ContainsKey instead of TryGetValue
 dotnet_diagnostic.MA0160.severity = none
+
+# MA0161: UseShellExecute must be explicitly set
+dotnet_diagnostic.MA0161.severity = none
+
+# MA0162: Use Process.Start overload with ProcessStartInfo
+dotnet_diagnostic.MA0162.severity = none
+
+# MA0163: UseShellExecute must be false when redirecting standard input or output
+dotnet_diagnostic.MA0163.severity = none
 ```

docs/Rules/MA0161.md

@@ -0,0 +1,24 @@
+# MA0161 - UseShellExecute must be explicitly set
+
+Detects when `Process.Start` is called without specifying the value of `UseShellExecute`.
+
+Specifying the value is important because the default value for this property is `true` on .NET Framework apps and `false` on .NET Core apps. It's a common issue when migrating a desktop app from .NET Framework to .NET Core.
+
+
+````c#
+using System.Diasgnostics;
+
+// Non compliant
+
+Process.Start(new ProcessStartInfo("cmd")); // Intent is not clear if you want to use ShellExecute or not
+
+Process.Start(new ProcessStartInfo("https://www.meziantou.net/")); // Will fail on .NET Core apps
+
+// Compliant
+
+Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
+{
+    UseShellExecute = true,
+});
+
+````

docs/Rules/MA0162.md

@@ -0,0 +1,23 @@
+# MA0162 - Use Process.Start overload with ProcessStartInfo
+
+Detects when `Process.Start` is called without the `ProcessStartInfo` parameter.
+
+Specifying a `ProcessStartInfo` allows to specify the `UseShellExecute` property. This value is important because the default value for this property is `true` on .NET Framework apps and `false` on .NET Core apps. It's a common issue when migrating a desktop app from .NET Framework to .NET Core.
+
+````c#
+using System.Diasgnostics;
+
+// Non compliant
+
+Process.Start("cmd"); // Intent is not clear if you want to use ShellExecute or not
+
+Process.Start("https://www.meziantou.net/"); // Will fail on .NET Core apps
+
+// Compliant
+
+Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
+{
+    UseShellExecute = true,
+});
+
+````

docs/Rules/MA0163.md

@@ -0,0 +1,43 @@
+# MA0163 - UseShellExecute must be false when redirecting standard input or output
+
+Detects when `Process.Start` is called without specifying the value of `UseShellExecute`.
+
+Specifying the value is important because:
+- The default value for this property is `true` on .NET Framework apps and `false` on .NET Core apps. It's a common issue when migrating a desktop app from .NET Framework to .NET Core.
+- It must be set to to `false` when redirecting I/O. Otherwise you'll get an issue at runtime.
+
+
+````c#
+using System.Diasgnostics;
+
+// Non compliant
+
+Process.Start("cmd"); // Intent is not clear if you want to use ShellExecute or not
+
+Process.Start("https://www.meziantou.net/"); // Will fail on .NET Core apps
+
+Process.Start(new ProcessStartInfo("cmd")
+{
+    RedirectStandardOutput = true,
+    UseShellExecute = true,
+});  // It will throw with error "UseShellExecute must be set to false when redirecting I/O"
+
+Process.Start(new ProcessStartInfo("cmd")
+{
+    RedirectStandardOutput = true,
+});  // It will throw with error "UseShellExecute must be set to false when redirecting I/O" on .NET Framework apps
+
+// Compliant
+
+Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
+{
+    UseShellExecute = true,
+});
+
+Process.Start(new ProcessStartInfo("cmd")
+{
+    RedirectStandardOutput = true,
+    UseShellExecute = false,
+});
+
+````

src/Meziantou.Analyzer/RuleIdentifiers.cs

@@ -163,6 +163,9 @@ internal static class RuleIdentifiers
     public const string UseSystemThreadingLockInsteadOfObject = "MA0158";
     public const string OptimizeEnumerable_UseOrder = "MA0159";
     public const string UseContainsKeyInsteadOfTryGetValue = "MA0160";
+    public const string UseShellExecuteMustBeSet = "MA0161";
+    public const string UseProcessStartOverload = "MA0162";
+    public const string UseShellExecuteMustBeFalse = "MA0163";
 
     public static string GetHelpUri(string identifier)
     {

src/Meziantou.Analyzer/Rules/ProcessStartAnalyzer.cs

@@ -0,0 +1,144 @@
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Meziantou.Analyzer.Rules;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class ProcessStartAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly DiagnosticDescriptor UseShellExecuteMustBeExplicitlySet = new(
+        RuleIdentifiers.UseShellExecuteMustBeSet,
+        title: "UseShellExecute must be explicitly set",
+        messageFormat: "UseShellExecute must be explicitly set when initializing a ProcessStartInfo",
+        RuleCategories.Usage,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: false,
+        description: "",
+        helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseShellExecuteMustBeSet));
+
+    private static readonly DiagnosticDescriptor UseProcessStartOverload = new(
+        RuleIdentifiers.UseProcessStartOverload,
+        title: "Use Process.Start overload with ProcessStartInfo",
+        messageFormat: "Use an overload of Process.Start that has a ProcessStartInfo parameter",
+        RuleCategories.Usage,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: false,
+        description: "",
+        helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseProcessStartOverload));
+
+    private static readonly DiagnosticDescriptor SetToFalseWhenRedirectingOutput = new(
+        RuleIdentifiers.UseShellExecuteMustBeFalse,
+        title: "UseShellExecute must be false when redirecting standard input or output",
+        messageFormat: "Set UseShellExecute to false when redirecting standard input or output",
+        RuleCategories.Usage,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "",
+        helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseShellExecuteMustBeFalse));
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+        ImmutableArray.Create(UseShellExecuteMustBeExplicitlySet, SetToFalseWhenRedirectingOutput, UseProcessStartOverload);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+        context.RegisterCompilationStartAction(ctx =>
+        {
+            var analyzerContext = new AnalyzerContext(ctx.Compilation);
+            if (!analyzerContext.IsValid)
+                return;
+
+            ctx.RegisterOperationAction(analyzerContext.AnalyzeInvocation, OperationKind.Invocation);
+            ctx.RegisterOperationAction(analyzerContext.AnalyzeObjectCreation, OperationKind.ObjectCreation);
+        });
+
+    }
+
+    private sealed class AnalyzerContext(Compilation compilation)
+    {
+        private readonly INamedTypeSymbol? _processStartInfoSymbol = compilation.GetBestTypeByMetadataName("System.Diagnostics.ProcessStartInfo");
+
+        private readonly INamedTypeSymbol? _processSymbol = compilation.GetBestTypeByMetadataName("System.Diagnostics.Process");
+
+        public bool IsValid => _processStartInfoSymbol is not null;
+
+        public void AnalyzeInvocation(OperationAnalysisContext context)
+        {
+            var operation = (IInvocationOperation)context.Operation;
+            if (IsProcessStartInvocation(operation))
+            {
+                if (!operation.Arguments.Any(IsProcessStartInfo))
+                {
+                    // Calling Process.Start without ProcessStartInfo
+                    context.ReportDiagnostic(UseProcessStartOverload, operation);
+                }
+            }
+        }
+
+        public void AnalyzeObjectCreation(OperationAnalysisContext context)
+        {
+            var operation = (IObjectCreationOperation)context.Operation;
+            if (IsProcessStartInfoCreation(operation))
+            {
+                if (operation is { Initializer: {} initializer } )
+                {
+                    var useShellExecuteInitializer = initializer.Initializers.OfType<ISimpleAssignmentOperation>()
+                        .FirstOrDefault(x => x.Target.Syntax is IdentifierNameSyntax { Identifier.Text: "UseShellExecute" });
+
+                    if (useShellExecuteInitializer is null)
+                    {
+                        if (IsRedirectingInputOrOutput(operation.SemanticModel!, initializer))
+                        {
+                            // Redirecting standard input or output while UseShellExecute is not explicitly set
+                            context.ReportDiagnostic(SetToFalseWhenRedirectingOutput, operation);
+                        }
+                        else
+                        {
+                            // Constructing ProcessStartInfo without setting UseShellExecute in the initializer
+                            context.ReportDiagnostic(UseShellExecuteMustBeExplicitlySet, operation);
+                        }
+                    }
+                    else if (IsInitializedToTrue(operation.SemanticModel!, useShellExecuteInitializer))
+                    {
+                        if (IsRedirectingInputOrOutput(operation.SemanticModel!, initializer))
+                        {
+                            // Redirecting standard input or output while UseShellExecute is set to true
+                            context.ReportDiagnostic(SetToFalseWhenRedirectingOutput, operation);
+                        }
+                    }
+                }
+                else
+                {
+                    // Constructing ProcessStartInfo with not initializer at all
+                    context.ReportDiagnostic(UseShellExecuteMustBeExplicitlySet, operation);
+                }
+            }
+        }
+
+        private static bool IsInitializedToTrue(SemanticModel semanticModel, ISimpleAssignmentOperation simpleAssignmentOperation)
+            => semanticModel.GetConstantValue(simpleAssignmentOperation.Value.Syntax) is { HasValue: true, Value: true };
+
+        private static bool IsRedirectingInputOrOutput(SemanticModel semanticModel,
+            IObjectOrCollectionInitializerOperation initializer) =>
+            initializer.Initializers.OfType<ISimpleAssignmentOperation>()
+                .Any(x => x.Target.Syntax is IdentifierNameSyntax { Identifier.Text: "RedirectStandardError" or "RedirectStandardInput" or "RedirectStandardOutput" }
+                                             && IsInitializedToTrue(semanticModel, x));
+
+        private bool IsProcessStartInfo(IArgumentOperation operation)
+            => operation.Value.Type.IsEqualTo(_processStartInfoSymbol);
+
+        private bool IsProcessStartInfoCreation(IObjectCreationOperation operation)
+            => operation.Type.IsEqualTo(_processStartInfoSymbol);
+
+        private bool IsProcessStartInvocation(IInvocationOperation operation)
+            => operation.TargetMethod.Name == "Start"
+            && operation.TargetMethod.ContainingType.IsEqualTo(_processSymbol)
+            && operation.TargetMethod.IsStatic;
+    }
+}

tests/Meziantou.Analyzer.Test/Helpers/ProjectBuilder.cs

@@ -339,6 +339,9 @@ public ProjectBuilder ShouldReportDiagnostic(params DiagnosticResult[] expectedD
 
     public ProjectBuilder ShouldReportDiagnosticWithMessage(string message)
     {
+        if (_diagnosticMessageIndex >= ExpectedDiagnosticResults.Count)
+            throw new InvalidOperationException("Did you forget to annotate the code with [||]?");
+
         ExpectedDiagnosticResults[_diagnosticMessageIndex].Message = message;
         _diagnosticMessageIndex++;
         return this;