Skip to content

Commit feb4738

Browse files
author
Maddie Clayton
authored
Merge pull request #6192 from cormacpayne/static-analysis-rules
Add new rules to SignatureVerifier
2 parents bd03687 + 06377d3 commit feb4738

File tree

6 files changed

+3494
-1133
lines changed

6 files changed

+3494
-1133
lines changed

‎tools/AzureRM/AzureRM.psm1

8.44 KB
Binary file not shown.

‎tools/StaticAnalysis/Exceptions/SignatureIssues.csv

Lines changed: 3257 additions & 1131 deletions
Large diffs are not rendered by default.

‎tools/StaticAnalysis/ProblemIDs.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ public static class SignatureProblemId
99
public const int CmdletWithUnapprovedVerb = 8300;
1010
public const int CmdletWithPluralNoun = 8400;
1111
public const int ParameterWithPluralNoun = 8410;
12+
public const int ParameterWithOutOfRangePosition = 8420;
13+
public const int ParameterSetWithSpace = 8500;
14+
public const int MultipleParameterSetsWithNoDefault = 8510;
15+
public const int CmdletWithNoOutputType = 8600;
1216
public const int CmdletWithDestructiveVerb = 98300;
1317
public const int CmdletWithForceParameter = 98310;
1418
}

‎tools/StaticAnalysis/SignatureVerifier/SignatureVerifier.cs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,20 @@ public void Analyze(IEnumerable<string> cmdletProbingDirs,
204204
remediation: "Consider using a singular noun for the cmdlet name.");
205205
}
206206

207+
if (!cmdlet.OutputTypes.Any())
208+
{
209+
issueLogger.LogSignatureIssue(
210+
cmdlet: cmdlet,
211+
severity: 1,
212+
problemId: SignatureProblemId.CmdletWithNoOutputType,
213+
description:
214+
string.Format(
215+
"Cmdlet '{0}' has no defined output type.", cmdlet.Name),
216+
remediation: "Add an OutputType attribute that declares the type of the object(s) returned " +
217+
"by this cmdlet. If this cmdlet returns no output, please set the output " +
218+
"type to 'bool' and make sure to implement the 'PassThru' parameter.");
219+
}
220+
207221
foreach (var parameter in cmdlet.GetParametersWithPluralNoun())
208222
{
209223
issueLogger.LogSignatureIssue(
@@ -217,6 +231,51 @@ public void Analyze(IEnumerable<string> cmdletProbingDirs,
217231
parameter.Name, cmdlet.Name),
218232
remediation: "Consider using a singular noun for the parameter name.");
219233
}
234+
235+
foreach (var parameterSet in cmdlet.ParameterSets)
236+
{
237+
if (parameterSet.Name.Contains(" "))
238+
{
239+
issueLogger.LogSignatureIssue(
240+
cmdlet: cmdlet,
241+
severity: 1,
242+
problemId: SignatureProblemId.ParameterSetWithSpace,
243+
description:
244+
string.Format(
245+
"Parameter set '{0}' of cmdlet '{1}' contains a space, which " +
246+
"is discouraged for PowerShell parameter sets.",
247+
parameterSet.Name, cmdlet.Name),
248+
remediation: "Remove the space(s) in the parameter set name.");
249+
}
250+
251+
if (parameterSet.Parameters.Any(p => p.Position >= 4))
252+
{
253+
issueLogger.LogSignatureIssue(
254+
cmdlet: cmdlet,
255+
severity: 1,
256+
problemId: SignatureProblemId.ParameterWithOutOfRangePosition,
257+
description:
258+
string.Format(
259+
"Parameter set '{0}' of cmdlet '{1}' contains at least one parameter " +
260+
"with a position larger than four, which is discouraged.",
261+
parameterSet.Name, cmdlet.Name),
262+
remediation: "Limit the number of positional parameters in a single parameter set to " +
263+
"four or fewer.");
264+
}
265+
}
266+
267+
if (cmdlet.ParameterSets.Count() > 2 && cmdlet.DefaultParameterSetName == "__AllParameterSets")
268+
{
269+
issueLogger.LogSignatureIssue(
270+
cmdlet: cmdlet,
271+
severity: 1,
272+
problemId: SignatureProblemId.MultipleParameterSetsWithNoDefault,
273+
description:
274+
string.Format(
275+
"Cmdlet '{0}' has multiple parameter sets, but no defined default parameter set.",
276+
cmdlet.Name),
277+
remediation: "Define a default parameter set in the cmdlet attribute.");
278+
}
220279
}
221280

222281
AppDomain.Unload(_appDomain);

‎tools/StaticAnalysis/StaticAnalysis.Test/SignatureVerifierTests.cs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,54 @@ public void CmdletWithPluralNoun()
208208
}
209209
#endregion
210210

211-
#region ParameterWithPluralNoun
211+
#region OutputChecks
212+
[Fact]
213+
[Trait(Category.AcceptanceType, Category.CheckIn)]
214+
public void CmdletWithNoOutput()
215+
{
216+
cmdletSignatureVerifier.Analyze(
217+
new List<string> { _testCmdletDirPath },
218+
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
219+
(cmdletName) => cmdletName.Equals("Get-CmdletWithNoOutput", StringComparison.OrdinalIgnoreCase));
220+
221+
AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
222+
Assert.Equal(1, testReport.ProblemIdList.Count);
223+
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.CmdletWithNoOutputType)).SingleOrDefault<int>().Equals(SignatureProblemId.CmdletWithNoOutputType));
224+
}
225+
226+
#endregion
227+
228+
#region ParameterSetChecks
229+
[Fact]
230+
[Trait(Category.AcceptanceType, Category.CheckIn)]
231+
public void ParameterSetNameWithSpace()
232+
{
233+
cmdletSignatureVerifier.Analyze(
234+
new List<string> { _testCmdletDirPath },
235+
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
236+
(cmdletName) => cmdletName.Equals("Get-ParameterSetNameWithSpace", StringComparison.OrdinalIgnoreCase));
237+
238+
AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
239+
Assert.Equal(1, testReport.ProblemIdList.Count);
240+
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.ParameterSetWithSpace)).SingleOrDefault<int>().Equals(SignatureProblemId.ParameterSetWithSpace));
241+
}
242+
243+
[Fact]
244+
[Trait(Category.AcceptanceType, Category.CheckIn)]
245+
public void MultipleParameterSetsWithNoDefault()
246+
{
247+
cmdletSignatureVerifier.Analyze(
248+
new List<string> { _testCmdletDirPath },
249+
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
250+
(cmdletName) => cmdletName.Equals("Get-MultipleParameterSetsWithNoDefault", StringComparison.OrdinalIgnoreCase));
251+
252+
AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
253+
Assert.Equal(1, testReport.ProblemIdList.Count);
254+
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.MultipleParameterSetsWithNoDefault)).SingleOrDefault<int>().Equals(SignatureProblemId.MultipleParameterSetsWithNoDefault));
255+
}
256+
#endregion
257+
258+
#region ParameterChecks
212259
[Fact]
213260
[Trait(Category.AcceptanceType, Category.CheckIn)]
214261
public void ParameterWithSingularNoun()
@@ -248,6 +295,20 @@ public void CmdletAndParameterWithSingularNounInList()
248295
AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
249296
Assert.Equal(0, testReport.ProblemIdList.Count);
250297
}
298+
299+
[Fact]
300+
[Trait(Category.AcceptanceType, Category.CheckIn)]
301+
public void ParameterWithOutOfRangePosition()
302+
{
303+
cmdletSignatureVerifier.Analyze(
304+
new List<string> { _testCmdletDirPath },
305+
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
306+
(cmdletName) => cmdletName.Equals("Get-ParameterWithOutOfRangePosition", StringComparison.OrdinalIgnoreCase));
307+
308+
AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
309+
Assert.Equal(1, testReport.ProblemIdList.Count);
310+
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.ParameterWithOutOfRangePosition)).SingleOrDefault<int>().Equals(SignatureProblemId.ParameterWithOutOfRangePosition));
311+
}
251312
#endregion
252313
}
253314
}

‎tools/StaticAnalysis/StaticAnalysis.Test/TestCmdlets/SignatureVerifier_Cmdlet.cs

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.AddVerbWithoutSupportsShouldP
1313
/// Check if a cmdlet that has Verbs that require ShouldProcess has ShouldProcess parameter
1414
/// </summary>
1515
[Cmdlet(VerbsCommon.Add, "AddVerbWithoutSupportsShouldProcessParameter")]
16+
[OutputType(typeof(bool))]
1617
public class AddVerbWithoutSupportsShouldProcessParameter : Cmdlet
1718
{
1819
/// <summary>
@@ -42,6 +43,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.AddVerbWithSupportsShouldProc
4243
/// Check if a cmdlet that has Verbs that require ShouldProcess has ShouldProcess parameter
4344
/// </summary>
4445
[Cmdlet(VerbsCommon.Add, "AddVerbWithSupportsShouldProcessParameter", SupportsShouldProcess = true)]
46+
[OutputType(typeof(bool))]
4547
public class AddVerbWithSupportsShouldProcessParameter : Cmdlet
4648
{
4749
/// <summary>
@@ -73,6 +75,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ForceParameterWithoutSupports
7375
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
7476
/// </summary>
7577
[Cmdlet(VerbsDiagnostic.Test, "ForceParameterWithoutSupportsShouldProcess")]
78+
[OutputType(typeof(bool))]
7679
public class ForceParameterWithoutSupportsShouldProcess : Cmdlet
7780
{
7881
/// <summary>
@@ -108,6 +111,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ForceParameterWithSupportsSho
108111
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
109112
/// </summary>
110113
[Cmdlet(VerbsDiagnostic.Test, "ForceParameterWithSupportsShouldProcess", SupportsShouldProcess = true)]
114+
[OutputType(typeof(bool))]
111115
public class ForceParameterWithSupportsShouldProcess : Cmdlet
112116
{
113117
/// <summary>
@@ -137,6 +141,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ConfirmImpactWithoutSupportsS
137141
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
138142
/// </summary>
139143
[Cmdlet(VerbsDiagnostic.Test, "ConfirmImpactWithoutSupportsShouldProcess", ConfirmImpact = ConfirmImpact.High)]
144+
[OutputType(typeof(bool))]
140145
public class ConfirmImpactWithoutSupportsShouldProcess : Cmdlet
141146
{
142147
/// <summary>
@@ -158,6 +163,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ConfirmImpactWithSupportsShou
158163
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
159164
/// </summary>
160165
[Cmdlet(VerbsDiagnostic.Test, "ConfirmImpactWithSupportsShouldProcess", ConfirmImpact = ConfirmImpact.Medium, SupportsShouldProcess = true)]
166+
[OutputType(typeof(bool))]
161167
public class ConfirmImpactWithSupportsShouldProcess : Cmdlet
162168
{
163169
/// <summary>
@@ -187,6 +193,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ShouldContinueVerbWithForceSw
187193
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
188194
/// </summary>
189195
[Cmdlet(VerbsCommon.Copy, "ShouldContinueVerbWithForceSwitch", SupportsShouldProcess = true)]
196+
[OutputType(typeof(bool))]
190197
public class ShouldContinueVerbWithForceSwitch : Cmdlet
191198
{
192199
/// <summary>
@@ -216,6 +223,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithApprovedVerb
216223
/// Verify if a cmdlet has an approved verb in its name.
217224
/// </summary>
218225
[Cmdlet(VerbsCommon.Get, "SampleCmdlet")]
226+
[OutputType(typeof(bool))]
219227
public class CmdletWithApprovedVerb : Cmdlet
220228
{
221229
/// <summary>
@@ -237,6 +245,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithUnapprovedVerb
237245
/// Verify if a cmdlet has an approved verb in its name.
238246
/// </summary>
239247
[Cmdlet("Prepare", "SampleCmdlet")]
248+
[OutputType(typeof(bool))]
240249
public class CmdletWithUnapprovedVerb : Cmdlet
241250
{
242251
/// <summary>
@@ -260,6 +269,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithSingularNoun
260269
/// Verify if a cmdlet has a singular noun in its name.
261270
/// </summary>
262271
[Cmdlet(VerbsCommon.Get, "SampleKey")]
272+
[OutputType(typeof(bool))]
263273
public class CmdletGetSampleKey : Cmdlet
264274
{
265275
/// <summary>
@@ -281,6 +291,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithPluralNoun
281291
/// Verify if a cmdlet has a plural noun in its name.
282292
/// </summary>
283293
[Cmdlet("Get", "SampleKeys")]
294+
[OutputType(typeof(bool))]
284295
public class CmdletGetSampleKeys : Cmdlet
285296
{
286297
/// <summary>
@@ -295,7 +306,72 @@ protected override void BeginProcessing()
295306
}
296307
#endregion
297308

298-
#region ParameterWithPluralNoun
309+
#region OutputChecks
310+
namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithNoOutput
311+
{
312+
using System.Management.Automation;
313+
314+
[Cmdlet(VerbsCommon.Get, "CmdletWithNoOutput")]
315+
public class CmdletGetCmdletWithNoOutput : Cmdlet
316+
{
317+
protected override void BeginProcessing()
318+
{
319+
WriteObject("Get-CmdletWithNoOutput BeginProcessing()");
320+
WriteInformation("Info", null);
321+
}
322+
}
323+
}
324+
325+
#endregion
326+
327+
#region ParameterSetChecks
328+
namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterSetNameWithSpace
329+
{
330+
using System.Management.Automation;
331+
332+
[Cmdlet(VerbsCommon.Get, "ParameterSetNameWithSpace", DefaultParameterSetName = "GetFoo")]
333+
[OutputType(typeof(bool))]
334+
public class CmdletGetParameterSetNameWithSpace : Cmdlet
335+
{
336+
[Parameter(ParameterSetName = "GetFoo")]
337+
public string Foo { get; set; }
338+
339+
[Parameter(ParameterSetName = "Get Bar")]
340+
public string Bar { get; set; }
341+
342+
protected override void BeginProcessing()
343+
{
344+
WriteObject("Get-ParameterSetNameWithSpace BeginProcessing()");
345+
WriteInformation("Info", null);
346+
}
347+
}
348+
}
349+
350+
namespace StaticAnalysis.Test.CmdletTest.Signature.MultipleParameterSetsWithNoDefault
351+
{
352+
using System.Management.Automation;
353+
354+
[Cmdlet(VerbsCommon.Get, "MultipleParameterSetsWithNoDefault")]
355+
[OutputType(typeof(bool))]
356+
public class CmdletGetMultipleParameterSetsWithNoDefault : Cmdlet
357+
{
358+
[Parameter(ParameterSetName = "GetFoo")]
359+
public string Foo { get; set; }
360+
361+
[Parameter(ParameterSetName = "GetBar")]
362+
public string Bar { get; set; }
363+
364+
protected override void BeginProcessing()
365+
{
366+
WriteObject("Get-MultipleParameterSetsWithNoDefault BeginProcessing()");
367+
WriteInformation("Info", null);
368+
}
369+
}
370+
}
371+
372+
#endregion
373+
374+
#region ParameterChecks
299375
namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithSingularNoun
300376
{
301377
using System.Management.Automation;
@@ -304,6 +380,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithSingularNoun
304380
/// Verify if a parameter has a singular noun in its name.
305381
/// </summary>
306382
[Cmdlet(VerbsCommon.Get, "SampleFoo")]
383+
[OutputType(typeof(bool))]
307384
public class CmdletGetSampleFoo : Cmdlet
308385
{
309386
[Parameter(Mandatory = false)]
@@ -328,6 +405,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithPluralNoun
328405
/// Verify if a parameter has a plural noun in its name.
329406
/// </summary>
330407
[Cmdlet("Get", "SampleBar")]
408+
[OutputType(typeof(bool))]
331409
public class CmdletGetSampleBar : Cmdlet
332410
{
333411
[Parameter(Mandatory = false)]
@@ -353,6 +431,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletAndParameterWithSingula
353431
/// accepted nouns ending with "s".
354432
/// </summary>
355433
[Cmdlet("Get", "SampleAddress")]
434+
[OutputType(typeof(bool))]
356435
public class CmdletGetSampleAddress : Cmdlet
357436
{
358437
[Parameter(Mandatory = false)]
@@ -368,4 +447,36 @@ protected override void BeginProcessing()
368447
}
369448
}
370449
}
450+
451+
namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithOutOfRangePosition
452+
{
453+
using System.Management.Automation;
454+
455+
[Cmdlet(VerbsCommon.Get, "ParameterWithOutOfRangePosition")]
456+
[OutputType(typeof(bool))]
457+
public class CmdletGetParameterWithOutOfRangePosition : Cmdlet
458+
{
459+
[Parameter(Position = 0)]
460+
public string FirstParameter { get; set; }
461+
462+
[Parameter(Position = 1)]
463+
public string SecondParameter { get; set; }
464+
465+
[Parameter(Position = 2)]
466+
public string ThirdParameter { get; set; }
467+
468+
[Parameter(Position = 3)]
469+
public string FourthParameter { get; set; }
470+
471+
[Parameter(Position = 4)]
472+
public string FifthParameter { get; set; }
473+
474+
protected override void BeginProcessing()
475+
{
476+
WriteObject("Get-ParameterWithOutOfRangePosition BeginProcessing()");
477+
WriteInformation("Info", null);
478+
}
479+
}
480+
}
481+
371482
#endregion

0 commit comments

Comments
 (0)