UseShellExecute analyzer #707
Conversation
meziantou
left a comment
meziantou
left a comment
There was a problem hiding this comment.
Before completing the PR, you'll need to run the DocumentationGenerator project to generate the md files.
tests/Meziantou.Analyzer.Test/Rules/UseShellExecuteAnalyzerTests.cs
Outdated
Show resolved
Hide resolved
jairbubbles
force-pushed
the
useshellexecute-analyzer
branch
2 times, most recently
from
bf255a2 to
6d3a1a9
Compare
|
This pull request is stale because it has been open for 60 days with no activity. |
|
This pull request is stale because it has been open for 60 days with no activity. |
jairbubbles
force-pushed
the
useshellexecute-analyzer
branch
from
6d3a1a9 to
40008ca
Compare
|
(false manipulation) |
|
I revived the PR @meziantou, let me know what you think. |
meziantou
left a comment
meziantou
left a comment
There was a problem hiding this comment.
Just a few changes, then I'll merge it
| description: "", | ||
| helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseShellExecuteMustBeSet)); | ||
|
|
||
| private static readonly DiagnosticDescriptor UseProcessStartOverload = new( |
There was a problem hiding this comment.
You should have a different id per DiagnosticDescriptor. It gives more flexibility for the consumer of the package.
There was a problem hiding this comment.
You mean add another rule "MA0162"?
There was a problem hiding this comment.
Yep, in fact you should have 3 rule ids.
jairbubbles
force-pushed
the
useshellexecute-analyzer
branch
from
1bfb880 to
6c4e863
Compare
|
@meziantou I renamed the analyzer and created 3 different IDs and disabled 2 out of the 3. NB: The .cmd is failing on my computer (double |
|
I still think we should enforce users to explicitly set |
2 participants
It's probably the number 1 issue I've seen between .NET Framework and .NET Core so I thought that an analyzer might be helpful. Let me know what you think and I'll dig more.