diff --git a/README.md b/README.md index b84c2381..680800e2 100755 --- a/README.md +++ b/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|⚠️|✔️|❌| diff --git a/docs/README.md b/docs/README.md index 23b4a1da..bbe1cb81 100755 --- a/docs/README.md +++ b/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|⚠️|✔️|❌| |[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|⚠️|✔️|❌| |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 ``` diff --git a/docs/Rules/MA0161.md b/docs/Rules/MA0161.md new file mode 100644 index 00000000..139a5b89 --- /dev/null +++ b/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, +}); + +```` diff --git a/docs/Rules/MA0162.md b/docs/Rules/MA0162.md new file mode 100644 index 00000000..4205057f --- /dev/null +++ b/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, +}); + +```` \ No newline at end of file diff --git a/docs/Rules/MA0163.md b/docs/Rules/MA0163.md new file mode 100644 index 00000000..8ce3b2ce --- /dev/null +++ b/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, +}); + +```` \ No newline at end of file diff --git a/src/Meziantou.Analyzer/RuleIdentifiers.cs b/src/Meziantou.Analyzer/RuleIdentifiers.cs index 2a70c6e5..5a64213f 100755 --- a/src/Meziantou.Analyzer/RuleIdentifiers.cs +++ b/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) { diff --git a/src/Meziantou.Analyzer/Rules/ProcessStartAnalyzer.cs b/src/Meziantou.Analyzer/Rules/ProcessStartAnalyzer.cs new file mode 100644 index 00000000..ad3b8518 --- /dev/null +++ b/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 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() + .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() + .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; + } +} diff --git a/tests/Meziantou.Analyzer.Test/Helpers/ProjectBuilder.cs b/tests/Meziantou.Analyzer.Test/Helpers/ProjectBuilder.cs index 646cb169..a2946d97 100755 --- a/tests/Meziantou.Analyzer.Test/Helpers/ProjectBuilder.cs +++ b/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; diff --git a/tests/Meziantou.Analyzer.Test/Rules/ProcessStartAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/ProcessStartAnalyzerTests.cs new file mode 100644 index 00000000..2963fbc5 --- /dev/null +++ b/tests/Meziantou.Analyzer.Test/Rules/ProcessStartAnalyzerTests.cs @@ -0,0 +1,298 @@ +using System.Threading.Tasks; +using Meziantou.Analyzer.Rules; +using TestHelper; +using Xunit; + +namespace Meziantou.Analyzer.Test.Rules; + +public sealed class ProcessStartAnalyzerTests +{ + [Fact] + public async Task Process_start_should_not_report_when_use_shell_execute_is_set_to_false() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + Process.Start(new ProcessStartInfo { UseShellExecute = false }); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_not_report_when_use_shell_execute_is_set_to_true() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + Process.Start(new ProcessStartInfo { UseShellExecute = true }); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo(); + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("UseShellExecute must be explicitly set when initializing a ProcessStartInfo") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_set_to_true_and_output_redirected() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + const bool useShellExecute = true; + var processStartInfo = [||]new ProcessStartInfo() + { + RedirectStandardOutput = true, + UseShellExecute = useShellExecute, + }; + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0163") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Set UseShellExecute to false when redirecting standard input or output") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set_and_output_redirected() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo() + { + RedirectStandardOutput = true, + }; + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0163") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Set UseShellExecute to false when redirecting standard input or output") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set_and_error_redirected() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo() + { + RedirectStandardError = true, + }; + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0163") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Set UseShellExecute to false when redirecting standard input or output") + .ValidateAsync(); + } + + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set_and_input_redirected() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo() + { + RedirectStandardInput = true, + UseShellExecute = true, + }; + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0163") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Set UseShellExecute to false when redirecting standard input or output") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_false_positives() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo(); + processStartInfo.UseShellExecute = false; + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("UseShellExecute must be explicitly set when initializing a ProcessStartInfo") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set_2() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo() + { + FileName = "notepad", + }; + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("UseShellExecute must be explicitly set when initializing a ProcessStartInfo") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set_3() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo("notepad"); + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("UseShellExecute must be explicitly set when initializing a ProcessStartInfo") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_use_shell_execute_is_not_set_4() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + var processStartInfo = [||]new ProcessStartInfo("notepad", string.Empty); + Process.Start(processStartInfo); + } + } + """; + await CreateProjectBuilder("MA0161") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("UseShellExecute must be explicitly set when initializing a ProcessStartInfo") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_using_overload_with_no_process_start_info() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + [||]Process.Start("notepad"); + } + } + """; + await CreateProjectBuilder("MA0162") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Use an overload of Process.Start that has a ProcessStartInfo parameter") + .ValidateAsync(); + } + + [Fact] + public async Task Process_start_should_report_when_using_overload_with_no_process_start_info_2() + { + const string SourceCode = """ + using System.Diagnostics; + + class TypeName + { + public void Test() + { + [||]Process.Start("notepad", "file.txt"); + } + } + """; + await CreateProjectBuilder("MA0162") + .WithSourceCode(SourceCode) + .ShouldReportDiagnosticWithMessage("Use an overload of Process.Start that has a ProcessStartInfo parameter") + .ValidateAsync(); + } + + private static ProjectBuilder CreateProjectBuilder(string id) => new ProjectBuilder().WithAnalyzer(id); +}