Skip to content

Add new rules to SignatureVerifier #6192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 21, 2018
Merged

Add new rules to SignatureVerifier #6192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
566bfff
Add StaticAnalysis checks for
cormacpayne May 10, 2018
55ca43f
Add tests for new static analysis checks
cormacpayne May 10, 2018
39ca73d
Merge branch 'preview' of github.com:Azure/azure-powershell into stat…
cormacpayne May 11, 2018
59f7dc2
Update SignatureIssues.csv
cormacpayne May 15, 2018
06377d3
Merge branch 'preview' of github.com:Azure/azure-powershell into stat…
cormacpayne May 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified tools/AzureRM/AzureRM.psm1
View file
Open in desktop
Binary file not shown.
4,388 changes: 3,257 additions & 1,131 deletions tools/StaticAnalysis/Exceptions/SignatureIssues.csv
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions tools/StaticAnalysis/ProblemIDs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ public static class SignatureProblemId
public const int CmdletWithUnapprovedVerb = 8300;
public const int CmdletWithPluralNoun = 8400;
public const int ParameterWithPluralNoun = 8410;
public const int ParameterWithOutOfRangePosition = 8420;
public const int ParameterSetWithSpace = 8500;
public const int MultipleParameterSetsWithNoDefault = 8510;
public const int CmdletWithNoOutputType = 8600;
public const int CmdletWithDestructiveVerb = 98300;
public const int CmdletWithForceParameter = 98310;
}
Expand Down
59 changes: 59 additions & 0 deletions tools/StaticAnalysis/SignatureVerifier/SignatureVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,20 @@ public void Analyze(IEnumerable<string> cmdletProbingDirs,
remediation: "Consider using a singular noun for the cmdlet name.");
}

if (!cmdlet.OutputTypes.Any())
{
issueLogger.LogSignatureIssue(
cmdlet: cmdlet,
severity: 1,
problemId: SignatureProblemId.CmdletWithNoOutputType,
description:
string.Format(
"Cmdlet '{0}' has no defined output type.", cmdlet.Name),
remediation: "Add an OutputType attribute that declares the type of the object(s) returned " +
"by this cmdlet. If this cmdlet returns no output, please set the output " +
"type to 'bool' and make sure to implement the 'PassThru' parameter.");
}

foreach (var parameter in cmdlet.GetParametersWithPluralNoun())
{
issueLogger.LogSignatureIssue(
Expand All @@ -217,6 +231,51 @@ public void Analyze(IEnumerable<string> cmdletProbingDirs,
parameter.Name, cmdlet.Name),
remediation: "Consider using a singular noun for the parameter name.");
}

foreach (var parameterSet in cmdlet.ParameterSets)
{
if (parameterSet.Name.Contains(" "))
{
issueLogger.LogSignatureIssue(
cmdlet: cmdlet,
severity: 1,
problemId: SignatureProblemId.ParameterSetWithSpace,
description:
string.Format(
"Parameter set '{0}' of cmdlet '{1}' contains a space, which " +
"is discouraged for PowerShell parameter sets.",
parameterSet.Name, cmdlet.Name),
remediation: "Remove the space(s) in the parameter set name.");
}

if (parameterSet.Parameters.Any(p => p.Position >= 4))
{
issueLogger.LogSignatureIssue(
cmdlet: cmdlet,
severity: 1,
problemId: SignatureProblemId.ParameterWithOutOfRangePosition,
description:
string.Format(
"Parameter set '{0}' of cmdlet '{1}' contains at least one parameter " +
"with a position larger than four, which is discouraged.",
parameterSet.Name, cmdlet.Name),
remediation: "Limit the number of positional parameters in a single parameter set to " +
"four or fewer.");
}
}

if (cmdlet.ParameterSets.Count() > 2 && cmdlet.DefaultParameterSetName == "__AllParameterSets")
{
issueLogger.LogSignatureIssue(
cmdlet: cmdlet,
severity: 1,
problemId: SignatureProblemId.MultipleParameterSetsWithNoDefault,
description:
string.Format(
"Cmdlet '{0}' has multiple parameter sets, but no defined default parameter set.",
cmdlet.Name),
remediation: "Define a default parameter set in the cmdlet attribute.");
}
}

AppDomain.Unload(_appDomain);
Expand Down
63 changes: 62 additions & 1 deletion tools/StaticAnalysis/StaticAnalysis.Test/SignatureVerifierTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,54 @@ public void CmdletWithPluralNoun()
}
#endregion

#region ParameterWithPluralNoun
#region OutputChecks
[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void CmdletWithNoOutput()
{
cmdletSignatureVerifier.Analyze(
new List<string> { _testCmdletDirPath },
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
(cmdletName) => cmdletName.Equals("Get-CmdletWithNoOutput", StringComparison.OrdinalIgnoreCase));

AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
Assert.Equal(1, testReport.ProblemIdList.Count);
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.CmdletWithNoOutputType)).SingleOrDefault<int>().Equals(SignatureProblemId.CmdletWithNoOutputType));
}

#endregion

#region ParameterSetChecks
[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void ParameterSetNameWithSpace()
{
cmdletSignatureVerifier.Analyze(
new List<string> { _testCmdletDirPath },
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
(cmdletName) => cmdletName.Equals("Get-ParameterSetNameWithSpace", StringComparison.OrdinalIgnoreCase));

AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
Assert.Equal(1, testReport.ProblemIdList.Count);
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.ParameterSetWithSpace)).SingleOrDefault<int>().Equals(SignatureProblemId.ParameterSetWithSpace));
}

[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void MultipleParameterSetsWithNoDefault()
{
cmdletSignatureVerifier.Analyze(
new List<string> { _testCmdletDirPath },
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
(cmdletName) => cmdletName.Equals("Get-MultipleParameterSetsWithNoDefault", StringComparison.OrdinalIgnoreCase));

AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
Assert.Equal(1, testReport.ProblemIdList.Count);
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.MultipleParameterSetsWithNoDefault)).SingleOrDefault<int>().Equals(SignatureProblemId.MultipleParameterSetsWithNoDefault));
}
#endregion

#region ParameterChecks
[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void ParameterWithSingularNoun()
Expand Down Expand Up @@ -248,6 +295,20 @@ public void CmdletAndParameterWithSingularNounInList()
AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
Assert.Equal(0, testReport.ProblemIdList.Count);
}

[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]
public void ParameterWithOutOfRangePosition()
{
cmdletSignatureVerifier.Analyze(
new List<string> { _testCmdletDirPath },
((dirList) => { return new List<string> { _testCmdletDirPath }; }),
(cmdletName) => cmdletName.Equals("Get-ParameterWithOutOfRangePosition", StringComparison.OrdinalIgnoreCase));

AnalysisReport testReport = cmdletSignatureVerifier.GetAnalysisReport();
Assert.Equal(1, testReport.ProblemIdList.Count);
Assert.True(testReport.ProblemIdList.Where<int>((problemId) => problemId.Equals(SignatureProblemId.ParameterWithOutOfRangePosition)).SingleOrDefault<int>().Equals(SignatureProblemId.ParameterWithOutOfRangePosition));
}
#endregion
}
}
113 changes: 112 additions & 1 deletion tools/StaticAnalysis/StaticAnalysis.Test/TestCmdlets/SignatureVerifier_Cmdlet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.AddVerbWithoutSupportsShouldP
/// Check if a cmdlet that has Verbs that require ShouldProcess has ShouldProcess parameter
/// </summary>
[Cmdlet(VerbsCommon.Add, "AddVerbWithoutSupportsShouldProcessParameter")]
[OutputType(typeof(bool))]
public class AddVerbWithoutSupportsShouldProcessParameter : Cmdlet
{
/// <summary>
Expand Down Expand Up @@ -42,6 +43,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.AddVerbWithSupportsShouldProc
/// Check if a cmdlet that has Verbs that require ShouldProcess has ShouldProcess parameter
/// </summary>
[Cmdlet(VerbsCommon.Add, "AddVerbWithSupportsShouldProcessParameter", SupportsShouldProcess = true)]
[OutputType(typeof(bool))]
public class AddVerbWithSupportsShouldProcessParameter : Cmdlet
{
/// <summary>
Expand Down Expand Up @@ -73,6 +75,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ForceParameterWithoutSupports
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
/// </summary>
[Cmdlet(VerbsDiagnostic.Test, "ForceParameterWithoutSupportsShouldProcess")]
[OutputType(typeof(bool))]
public class ForceParameterWithoutSupportsShouldProcess : Cmdlet
{
/// <summary>
Expand Down Expand Up @@ -108,6 +111,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ForceParameterWithSupportsSho
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
/// </summary>
[Cmdlet(VerbsDiagnostic.Test, "ForceParameterWithSupportsShouldProcess", SupportsShouldProcess = true)]
[OutputType(typeof(bool))]
public class ForceParameterWithSupportsShouldProcess : Cmdlet
{
/// <summary>
Expand Down Expand Up @@ -137,6 +141,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ConfirmImpactWithoutSupportsS
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
/// </summary>
[Cmdlet(VerbsDiagnostic.Test, "ConfirmImpactWithoutSupportsShouldProcess", ConfirmImpact = ConfirmImpact.High)]
[OutputType(typeof(bool))]
public class ConfirmImpactWithoutSupportsShouldProcess : Cmdlet
{
/// <summary>
Expand All @@ -158,6 +163,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ConfirmImpactWithSupportsShou
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
/// </summary>
[Cmdlet(VerbsDiagnostic.Test, "ConfirmImpactWithSupportsShouldProcess", ConfirmImpact = ConfirmImpact.Medium, SupportsShouldProcess = true)]
[OutputType(typeof(bool))]
public class ConfirmImpactWithSupportsShouldProcess : Cmdlet
{
/// <summary>
Expand Down Expand Up @@ -187,6 +193,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ShouldContinueVerbWithForceSw
/// Verify if cmdlet that has Force parameter should also define SupportsShouldProcess parameter
/// </summary>
[Cmdlet(VerbsCommon.Copy, "ShouldContinueVerbWithForceSwitch", SupportsShouldProcess = true)]
[OutputType(typeof(bool))]
public class ShouldContinueVerbWithForceSwitch : Cmdlet
{
/// <summary>
Expand Down Expand Up @@ -216,6 +223,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithApprovedVerb
/// Verify if a cmdlet has an approved verb in its name.
/// </summary>
[Cmdlet(VerbsCommon.Get, "SampleCmdlet")]
[OutputType(typeof(bool))]
public class CmdletWithApprovedVerb : Cmdlet
{
/// <summary>
Expand All @@ -237,6 +245,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithUnapprovedVerb
/// Verify if a cmdlet has an approved verb in its name.
/// </summary>
[Cmdlet("Prepare", "SampleCmdlet")]
[OutputType(typeof(bool))]
public class CmdletWithUnapprovedVerb : Cmdlet
{
/// <summary>
Expand All @@ -260,6 +269,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithSingularNoun
/// Verify if a cmdlet has a singular noun in its name.
/// </summary>
[Cmdlet(VerbsCommon.Get, "SampleKey")]
[OutputType(typeof(bool))]
public class CmdletGetSampleKey : Cmdlet
{
/// <summary>
Expand All @@ -281,6 +291,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithPluralNoun
/// Verify if a cmdlet has a plural noun in its name.
/// </summary>
[Cmdlet("Get", "SampleKeys")]
[OutputType(typeof(bool))]
public class CmdletGetSampleKeys : Cmdlet
{
/// <summary>
Expand All @@ -295,7 +306,72 @@ protected override void BeginProcessing()
}
#endregion

#region ParameterWithPluralNoun
#region OutputChecks
namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletWithNoOutput
{
using System.Management.Automation;

[Cmdlet(VerbsCommon.Get, "CmdletWithNoOutput")]
public class CmdletGetCmdletWithNoOutput : Cmdlet
{
protected override void BeginProcessing()
{
WriteObject("Get-CmdletWithNoOutput BeginProcessing()");
WriteInformation("Info", null);
}
}
}

#endregion

#region ParameterSetChecks
namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterSetNameWithSpace
{
using System.Management.Automation;

[Cmdlet(VerbsCommon.Get, "ParameterSetNameWithSpace", DefaultParameterSetName = "GetFoo")]
[OutputType(typeof(bool))]
public class CmdletGetParameterSetNameWithSpace : Cmdlet
{
[Parameter(ParameterSetName = "GetFoo")]
public string Foo { get; set; }

[Parameter(ParameterSetName = "Get Bar")]
public string Bar { get; set; }

protected override void BeginProcessing()
{
WriteObject("Get-ParameterSetNameWithSpace BeginProcessing()");
WriteInformation("Info", null);
}
}
}

namespace StaticAnalysis.Test.CmdletTest.Signature.MultipleParameterSetsWithNoDefault
{
using System.Management.Automation;

[Cmdlet(VerbsCommon.Get, "MultipleParameterSetsWithNoDefault")]
[OutputType(typeof(bool))]
public class CmdletGetMultipleParameterSetsWithNoDefault : Cmdlet
{
[Parameter(ParameterSetName = "GetFoo")]
public string Foo { get; set; }

[Parameter(ParameterSetName = "GetBar")]
public string Bar { get; set; }

protected override void BeginProcessing()
{
WriteObject("Get-MultipleParameterSetsWithNoDefault BeginProcessing()");
WriteInformation("Info", null);
}
}
}

#endregion

#region ParameterChecks
namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithSingularNoun
{
using System.Management.Automation;
Expand All @@ -304,6 +380,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithSingularNoun
/// Verify if a parameter has a singular noun in its name.
/// </summary>
[Cmdlet(VerbsCommon.Get, "SampleFoo")]
[OutputType(typeof(bool))]
public class CmdletGetSampleFoo : Cmdlet
{
[Parameter(Mandatory = false)]
Expand All @@ -328,6 +405,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithPluralNoun
/// Verify if a parameter has a plural noun in its name.
/// </summary>
[Cmdlet("Get", "SampleBar")]
[OutputType(typeof(bool))]
public class CmdletGetSampleBar : Cmdlet
{
[Parameter(Mandatory = false)]
Expand All @@ -353,6 +431,7 @@ namespace StaticAnalysis.Test.CmdletTest.Signature.CmdletAndParameterWithSingula
/// accepted nouns ending with "s".
/// </summary>
[Cmdlet("Get", "SampleAddress")]
[OutputType(typeof(bool))]
public class CmdletGetSampleAddress : Cmdlet
{
[Parameter(Mandatory = false)]
Expand All @@ -368,4 +447,36 @@ protected override void BeginProcessing()
}
}
}

namespace StaticAnalysis.Test.CmdletTest.Signature.ParameterWithOutOfRangePosition
{
using System.Management.Automation;

[Cmdlet(VerbsCommon.Get, "ParameterWithOutOfRangePosition")]
[OutputType(typeof(bool))]
public class CmdletGetParameterWithOutOfRangePosition : Cmdlet
{
[Parameter(Position = 0)]
public string FirstParameter { get; set; }

[Parameter(Position = 1)]
public string SecondParameter { get; set; }

[Parameter(Position = 2)]
public string ThirdParameter { get; set; }

[Parameter(Position = 3)]
public string FourthParameter { get; set; }

[Parameter(Position = 4)]
public string FifthParameter { get; set; }

protected override void BeginProcessing()
{
WriteObject("Get-ParameterWithOutOfRangePosition BeginProcessing()");
WriteInformation("Info", null);
}
}
}

#endregion