Skip to content

Commit 9c95e25

Browse files
jairbubblesjairbubbles
authored
UseShellExecute analyzer (#707)
1 parent 40008ca commit 9c95e25

9 files changed

Lines changed: 562 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ If you are already using other analyzers, you can check [which rules are duplica
176176
|[MA0158](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0158.md)|Performance|Use System.Threading.Lock|⚠️|✔️|❌|
177177
|[MA0159](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0159.md)|Performance|Use 'Order' instead of 'OrderBy'|ℹ️|✔️|✔️|
178178
|[MA0160](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0160.md)|Performance|Use ContainsKey instead of TryGetValue|ℹ️|✔️|❌|
179+
|[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|ℹ️|❌|❌|
180+
|[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|ℹ️|❌|❌|
181+
|[MA0163](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0163.md)|Usage|UseShellExecute must be false when redirecting standard input or output|⚠️|✔️|❌|
179182

180183
<!-- rules -->
181184

‎docs/README.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@
160160
|[MA0158](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0158.md)|Performance|Use System.Threading.Lock|<span title='Warning'>⚠️</span>|✔️|❌|
161161
|[MA0159](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0159.md)|Performance|Use 'Order' instead of 'OrderBy'|<span title='Info'>ℹ️</span>|✔️|✔️|
162162
|[MA0160](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0160.md)|Performance|Use ContainsKey instead of TryGetValue|<span title='Info'>ℹ️</span>|✔️|❌|
163+
|[MA0161](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0161.md)|Usage|UseShellExecute must be explicitly set|<span title='Info'>ℹ️</span>|❌|❌|
164+
|[MA0162](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0162.md)|Usage|Use Process.Start overload with ProcessStartInfo|<span title='Info'>ℹ️</span>|❌|❌|
165+
|[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>|✔️|❌|
163166

164167
|Id|Suppressed rule|Justification|
165168
|--|---------------|-------------|
@@ -646,6 +649,15 @@ dotnet_diagnostic.MA0159.severity = suggestion
646649
647650
# MA0160: Use ContainsKey instead of TryGetValue
648651
dotnet_diagnostic.MA0160.severity = suggestion
652+
653+
# MA0161: UseShellExecute must be explicitly set
654+
dotnet_diagnostic.MA0161.severity = none
655+
656+
# MA0162: Use Process.Start overload with ProcessStartInfo
657+
dotnet_diagnostic.MA0162.severity = none
658+
659+
# MA0163: UseShellExecute must be false when redirecting standard input or output
660+
dotnet_diagnostic.MA0163.severity = warning
649661
```
650662

651663
# .editorconfig - all rules disabled
@@ -1127,4 +1139,13 @@ dotnet_diagnostic.MA0159.severity = none
11271139
11281140
# MA0160: Use ContainsKey instead of TryGetValue
11291141
dotnet_diagnostic.MA0160.severity = none
1142+
1143+
# MA0161: UseShellExecute must be explicitly set
1144+
dotnet_diagnostic.MA0161.severity = none
1145+
1146+
# MA0162: Use Process.Start overload with ProcessStartInfo
1147+
dotnet_diagnostic.MA0162.severity = none
1148+
1149+
# MA0163: UseShellExecute must be false when redirecting standard input or output
1150+
dotnet_diagnostic.MA0163.severity = none
11301151
```

‎docs/Rules/MA0161.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# MA0161 - UseShellExecute must be explicitly set
2+
3+
Detects when `Process.Start` is called without specifying the value of `UseShellExecute`.
4+
5+
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.
6+
7+
8+
````c#
9+
using System.Diasgnostics;
10+
11+
// Non compliant
12+
13+
Process.Start(new ProcessStartInfo("cmd")); // Intent is not clear if you want to use ShellExecute or not
14+
15+
Process.Start(new ProcessStartInfo("https://www.meziantou.net/")); // Will fail on .NET Core apps
16+
17+
// Compliant
18+
19+
Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
20+
{
21+
UseShellExecute = true,
22+
});
23+
24+
````

‎docs/Rules/MA0162.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# MA0162 - Use Process.Start overload with ProcessStartInfo
2+
3+
Detects when `Process.Start` is called without the `ProcessStartInfo` parameter.
4+
5+
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.
6+
7+
````c#
8+
using System.Diasgnostics;
9+
10+
// Non compliant
11+
12+
Process.Start("cmd"); // Intent is not clear if you want to use ShellExecute or not
13+
14+
Process.Start("https://www.meziantou.net/"); // Will fail on .NET Core apps
15+
16+
// Compliant
17+
18+
Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
19+
{
20+
UseShellExecute = true,
21+
});
22+
23+
````

‎docs/Rules/MA0163.md‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# MA0163 - UseShellExecute must be false when redirecting standard input or output
2+
3+
Detects when `Process.Start` is called without specifying the value of `UseShellExecute`.
4+
5+
Specifying the value is important because:
6+
- 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.
7+
- It must be set to to `false` when redirecting I/O. Otherwise you'll get an issue at runtime.
8+
9+
10+
````c#
11+
using System.Diasgnostics;
12+
13+
// Non compliant
14+
15+
Process.Start("cmd"); // Intent is not clear if you want to use ShellExecute or not
16+
17+
Process.Start("https://www.meziantou.net/"); // Will fail on .NET Core apps
18+
19+
Process.Start(new ProcessStartInfo("cmd")
20+
{
21+
RedirectStandardOutput = true,
22+
UseShellExecute = true,
23+
}); // It will throw with error "UseShellExecute must be set to false when redirecting I/O"
24+
25+
Process.Start(new ProcessStartInfo("cmd")
26+
{
27+
RedirectStandardOutput = true,
28+
}); // It will throw with error "UseShellExecute must be set to false when redirecting I/O" on .NET Framework apps
29+
30+
// Compliant
31+
32+
Process.Start(new ProcessStartInfo("https://www.meziantou.net/")
33+
{
34+
UseShellExecute = true,
35+
});
36+
37+
Process.Start(new ProcessStartInfo("cmd")
38+
{
39+
RedirectStandardOutput = true,
40+
UseShellExecute = false,
41+
});
42+
43+
````

‎src/Meziantou.Analyzer/RuleIdentifiers.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,9 @@ internal static class RuleIdentifiers
163163
public const string UseSystemThreadingLockInsteadOfObject = "MA0158";
164164
public const string OptimizeEnumerable_UseOrder = "MA0159";
165165
public const string UseContainsKeyInsteadOfTryGetValue = "MA0160";
166+
public const string UseShellExecuteMustBeSet = "MA0161";
167+
public const string UseProcessStartOverload = "MA0162";
168+
public const string UseShellExecuteMustBeFalse = "MA0163";
166169

167170
public static string GetHelpUri(string identifier)
168171
{

‎src/Meziantou.Analyzer/Rules/ProcessStartAnalyzer.cs‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
using System.Collections.Immutable;
2+
using System.Linq;
3+
using Microsoft.CodeAnalysis;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
using Microsoft.CodeAnalysis.Operations;
7+
8+
namespace Meziantou.Analyzer.Rules;
9+
10+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
11+
public sealed class ProcessStartAnalyzer : DiagnosticAnalyzer
12+
{
13+
private static readonly DiagnosticDescriptor UseShellExecuteMustBeExplicitlySet = new(
14+
RuleIdentifiers.UseShellExecuteMustBeSet,
15+
title: "UseShellExecute must be explicitly set",
16+
messageFormat: "UseShellExecute must be explicitly set when initializing a ProcessStartInfo",
17+
RuleCategories.Usage,
18+
DiagnosticSeverity.Info,
19+
isEnabledByDefault: false,
20+
description: "",
21+
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseShellExecuteMustBeSet));
22+
23+
private static readonly DiagnosticDescriptor UseProcessStartOverload = new(
24+
RuleIdentifiers.UseProcessStartOverload,
25+
title: "Use Process.Start overload with ProcessStartInfo",
26+
messageFormat: "Use an overload of Process.Start that has a ProcessStartInfo parameter",
27+
RuleCategories.Usage,
28+
DiagnosticSeverity.Info,
29+
isEnabledByDefault: false,
30+
description: "",
31+
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseProcessStartOverload));
32+
33+
private static readonly DiagnosticDescriptor SetToFalseWhenRedirectingOutput = new(
34+
RuleIdentifiers.UseShellExecuteMustBeFalse,
35+
title: "UseShellExecute must be false when redirecting standard input or output",
36+
messageFormat: "Set UseShellExecute to false when redirecting standard input or output",
37+
RuleCategories.Usage,
38+
DiagnosticSeverity.Warning,
39+
isEnabledByDefault: true,
40+
description: "",
41+
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseShellExecuteMustBeFalse));
42+
43+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
44+
ImmutableArray.Create(UseShellExecuteMustBeExplicitlySet, SetToFalseWhenRedirectingOutput, UseProcessStartOverload);
45+
46+
public override void Initialize(AnalysisContext context)
47+
{
48+
context.EnableConcurrentExecution();
49+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
50+
51+
context.RegisterCompilationStartAction(ctx =>
52+
{
53+
var analyzerContext = new AnalyzerContext(ctx.Compilation);
54+
if (!analyzerContext.IsValid)
55+
return;
56+
57+
ctx.RegisterOperationAction(analyzerContext.AnalyzeInvocation, OperationKind.Invocation);
58+
ctx.RegisterOperationAction(analyzerContext.AnalyzeObjectCreation, OperationKind.ObjectCreation);
59+
});
60+
61+
}
62+
63+
private sealed class AnalyzerContext(Compilation compilation)
64+
{
65+
private readonly INamedTypeSymbol? _processStartInfoSymbol = compilation.GetBestTypeByMetadataName("System.Diagnostics.ProcessStartInfo");
66+
67+
private readonly INamedTypeSymbol? _processSymbol = compilation.GetBestTypeByMetadataName("System.Diagnostics.Process");
68+
69+
public bool IsValid => _processStartInfoSymbol is not null;
70+
71+
public void AnalyzeInvocation(OperationAnalysisContext context)
72+
{
73+
var operation = (IInvocationOperation)context.Operation;
74+
if (IsProcessStartInvocation(operation))
75+
{
76+
if (!operation.Arguments.Any(IsProcessStartInfo))
77+
{
78+
// Calling Process.Start without ProcessStartInfo
79+
context.ReportDiagnostic(UseProcessStartOverload, operation);
80+
}
81+
}
82+
}
83+
84+
public void AnalyzeObjectCreation(OperationAnalysisContext context)
85+
{
86+
var operation = (IObjectCreationOperation)context.Operation;
87+
if (IsProcessStartInfoCreation(operation))
88+
{
89+
if (operation is { Initializer: {} initializer } )
90+
{
91+
var useShellExecuteInitializer = initializer.Initializers.OfType<ISimpleAssignmentOperation>()
92+
.FirstOrDefault(x => x.Target.Syntax is IdentifierNameSyntax { Identifier.Text: "UseShellExecute" });
93+
94+
if (useShellExecuteInitializer is null)
95+
{
96+
if (IsRedirectingInputOrOutput(operation.SemanticModel!, initializer))
97+
{
98+
// Redirecting standard input or output while UseShellExecute is not explicitly set
99+
context.ReportDiagnostic(SetToFalseWhenRedirectingOutput, operation);
100+
}
101+
else
102+
{
103+
// Constructing ProcessStartInfo without setting UseShellExecute in the initializer
104+
context.ReportDiagnostic(UseShellExecuteMustBeExplicitlySet, operation);
105+
}
106+
}
107+
else if (IsInitializedToTrue(operation.SemanticModel!, useShellExecuteInitializer))
108+
{
109+
if (IsRedirectingInputOrOutput(operation.SemanticModel!, initializer))
110+
{
111+
// Redirecting standard input or output while UseShellExecute is set to true
112+
context.ReportDiagnostic(SetToFalseWhenRedirectingOutput, operation);
113+
}
114+
}
115+
}
116+
else
117+
{
118+
// Constructing ProcessStartInfo with not initializer at all
119+
context.ReportDiagnostic(UseShellExecuteMustBeExplicitlySet, operation);
120+
}
121+
}
122+
}
123+
124+
private static bool IsInitializedToTrue(SemanticModel semanticModel, ISimpleAssignmentOperation simpleAssignmentOperation)
125+
=> semanticModel.GetConstantValue(simpleAssignmentOperation.Value.Syntax) is { HasValue: true, Value: true };
126+
127+
private static bool IsRedirectingInputOrOutput(SemanticModel semanticModel,
128+
IObjectOrCollectionInitializerOperation initializer) =>
129+
initializer.Initializers.OfType<ISimpleAssignmentOperation>()
130+
.Any(x => x.Target.Syntax is IdentifierNameSyntax { Identifier.Text: "RedirectStandardError" or "RedirectStandardInput" or "RedirectStandardOutput" }
131+
&& IsInitializedToTrue(semanticModel, x));
132+
133+
private bool IsProcessStartInfo(IArgumentOperation operation)
134+
=> operation.Value.Type.IsEqualTo(_processStartInfoSymbol);
135+
136+
private bool IsProcessStartInfoCreation(IObjectCreationOperation operation)
137+
=> operation.Type.IsEqualTo(_processStartInfoSymbol);
138+
139+
private bool IsProcessStartInvocation(IInvocationOperation operation)
140+
=> operation.TargetMethod.Name == "Start"
141+
&& operation.TargetMethod.ContainingType.IsEqualTo(_processSymbol)
142+
&& operation.TargetMethod.IsStatic;
143+
}
144+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,9 @@ public ProjectBuilder ShouldReportDiagnostic(params DiagnosticResult[] expectedD
339339

340340
public ProjectBuilder ShouldReportDiagnosticWithMessage(string message)
341341
{
342+
if (_diagnosticMessageIndex >= ExpectedDiagnosticResults.Count)
343+
throw new InvalidOperationException("Did you forget to annotate the code with [||]?");
344+
342345
ExpectedDiagnosticResults[_diagnosticMessageIndex].Message = message;
343346
_diagnosticMessageIndex++;
344347
return this;

0 commit comments

Comments
 (0)