diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 771088cab..6067c1864 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -100,6 +100,10 @@ internal set /// private Dictionary VariableAnalysisDictionary; + private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:" }; + + private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":" }; + #endregion /// @@ -269,6 +273,60 @@ item is TypeDefinitionAst return false; } + private string NameWithoutScope(string name, string[] scopes) + { + if (String.IsNullOrWhiteSpace(name) || scopes == null) + { + return name; + } + + // checks whether function name starts with scope + foreach (string scope in scopes) + { + // trim the scope part + if (name.IndexOf(scope) == 0) + { + return name.Substring(scope.Length); + } + } + + // no scope + return name; + } + + /// + /// Given a function name, strip the scope of the name + /// + /// + /// + public string FunctionNameWithoutScope(string functionName) + { + return NameWithoutScope(functionName, functionScopes); + } + + /// + /// Given a variable name, strip the scope + /// + /// + /// + public string VariableNameWithoutScope(VariablePath variablePath) + { + if (variablePath == null || variablePath.UserPath == null) + { + return null; + } + + // strip out the drive if there is one + if (!string.IsNullOrWhiteSpace(variablePath.DriveName) + // checks that variable starts with drivename: + && variablePath.UserPath.IndexOf(string.Concat(variablePath.DriveName, ":")) == 0) + { + return variablePath.UserPath.Substring(variablePath.DriveName.Length + 1); + } + + return NameWithoutScope(variablePath.UserPath, variableScopes); + } + /// /// Given a commandast, checks whether it uses splatted variable /// diff --git a/Engine/VariableAnalysis.cs b/Engine/VariableAnalysis.cs index 201b1f9d0..2c988a9c2 100644 --- a/Engine/VariableAnalysis.cs +++ b/Engine/VariableAnalysis.cs @@ -92,10 +92,13 @@ private void ProcessParameters(IEnumerable parameters) { foreach (NamedAttributeArgumentAst arg in args) { - if (String.Equals(arg.ArgumentName, "mandatory", StringComparison.OrdinalIgnoreCase) - && String.Equals(arg.Argument.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase)) - { - isSwitchOrMandatory = true; + if (String.Equals(arg.ArgumentName, "mandatory", StringComparison.OrdinalIgnoreCase)) + { + // check for the case mandatory=$true and just mandatory + if (arg.ExpressionOmitted || (!arg.ExpressionOmitted && String.Equals(arg.Argument.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))) + { + isSwitchOrMandatory = true; + } } } } diff --git a/Rules/AvoidReservedCharInCmdlet.cs b/Rules/AvoidReservedCharInCmdlet.cs index 2dddfbe3c..cbf4a248a 100644 --- a/Rules/AvoidReservedCharInCmdlet.cs +++ b/Rules/AvoidReservedCharInCmdlet.cs @@ -15,6 +15,7 @@ using System.Linq; using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using Microsoft.Windows.PowerShell.ScriptAnalyzer; using System.ComponentModel.Composition; using System.Globalization; @@ -43,7 +44,9 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) foreach (FunctionDefinitionAst funcAst in funcAsts) { - if (funcAst.Name != null && funcAst.Name.Intersect(reservedChars).Count() > 0) + string funcName = Helper.Instance.FunctionNameWithoutScope(funcAst.Name); + + if (funcName != null && funcName.Intersect(reservedChars).Count() > 0) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ReservedCmdletCharError, funcAst.Name), funcAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); diff --git a/Rules/UseApprovedVerbs.cs b/Rules/UseApprovedVerbs.cs index 4a5ae083b..e210d7b8a 100644 --- a/Rules/UseApprovedVerbs.cs +++ b/Rules/UseApprovedVerbs.cs @@ -54,7 +54,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { foreach (FunctionDefinitionAst funcAst in funcAsts) { - funcName = funcAst.Name; + funcName = Helper.Instance.FunctionNameWithoutScope(funcAst.Name); if (funcName != null && funcName.Contains('-')) { diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index e00538fd3..eec0e84b5 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -53,13 +53,17 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (assignmentVarAst != null) { - //Ignore if variable is global or environment variable + //Ignore if variable is global or environment variable or scope is function if (!Helper.Instance.IsVariableGlobalOrEnvironment(assignmentVarAst, ast) - && !assignmentVarAst.VariablePath.IsScript) + && !assignmentVarAst.VariablePath.IsScript + && !string.Equals(assignmentVarAst.VariablePath.DriveName, "function", StringComparison.OrdinalIgnoreCase)) { - if (!assignments.ContainsKey(assignmentVarAst.VariablePath.UserPath)) + + string variableName = Helper.Instance.VariableNameWithoutScope(assignmentVarAst.VariablePath); + + if (!assignments.ContainsKey(variableName)) { - assignments.Add(assignmentVarAst.VariablePath.UserPath, assignmentAst); + assignments.Add(variableName, assignmentAst); } } } @@ -70,7 +74,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { foreach (VariableExpressionAst varAst in varAsts) { - varKey = varAst.VariablePath.UserPath; + varKey = Helper.Instance.VariableNameWithoutScope(varAst.VariablePath); inAssignment = false; if (assignments.ContainsKey(varKey)) diff --git a/Rules/UseShouldProcessCorrectly.cs b/Rules/UseShouldProcessCorrectly.cs index fef838fd0..3100fc67f 100644 --- a/Rules/UseShouldProcessCorrectly.cs +++ b/Rules/UseShouldProcessCorrectly.cs @@ -54,7 +54,9 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) attributeAsts = funcDefAst.FindAll(testAst => testAst is NamedAttributeArgumentAst, true); foreach (NamedAttributeArgumentAst attributeAst in attributeAsts) { - hasShouldProcessAttribute |= attributeAst.ArgumentName.Equals(supportsShouldProcess, StringComparison.OrdinalIgnoreCase) && attributeAst.Argument.Extent.Text.Equals(trueString, StringComparison.OrdinalIgnoreCase); + hasShouldProcessAttribute |= ((attributeAst.ArgumentName.Equals(supportsShouldProcess, StringComparison.OrdinalIgnoreCase) && attributeAst.Argument.Extent.Text.Equals(trueString, StringComparison.OrdinalIgnoreCase)) + // checks for the case if the user just use the attribute without setting it to true + || (attributeAst.ArgumentName.Equals(supportsShouldProcess, StringComparison.OrdinalIgnoreCase) && attributeAst.ExpressionOmitted)); } memberAsts = funcDefAst.FindAll(testAst => testAst is MemberExpressionAst, true); diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1 index 93958ebf7..d5311440b 100644 --- a/Tests/Rules/GoodCmdlet.ps1 +++ b/Tests/Rules/GoodCmdlet.ps1 @@ -107,8 +107,117 @@ function Get-File } } +<# +.Synopsis + Short description +.DESCRIPTION + Long description +.EXAMPLE + Example of how to use this cmdlet +.EXAMPLE + Another example of how to use this cmdlet +.INPUTS + Inputs to this cmdlet (if any) +.OUTPUTS + Output from this cmdlet (if any) +.NOTES + General notes +.COMPONENT + The component this cmdlet belongs to +.ROLE + The role this cmdlet belongs to +.FUNCTIONALITY + The functionality that best describes this cmdlet +#> +function Get-Folder +{ + [CmdletBinding(DefaultParameterSetName='Parameter Set 1', + SupportsShouldProcess, + PositionalBinding=$false, + HelpUri = 'http://www.microsoft.com/', + ConfirmImpact='Medium')] + [Alias()] + [OutputType([String], [System.Double], [Hashtable], "MyCustom.OutputType")] + [OutputType("System.Int32", ParameterSetName="ID")] + + Param + ( + # Param1 help description + [Parameter(Mandatory=$true, + ValueFromPipeline=$true, + ValueFromPipelineByPropertyName=$true, + ValueFromRemainingArguments=$false, + Position=0, + ParameterSetName='Parameter Set 1')] + [ValidateNotNull()] + [ValidateNotNullOrEmpty()] + [ValidateCount(0,5)] + [ValidateSet("sun", "moon", "earth")] + [Alias("p1")] + $Param1, + + # Param2 help description + [Parameter(ParameterSetName='Parameter Set 1')] + [AllowNull()] + [AllowEmptyCollection()] + [AllowEmptyString()] + [ValidateScript({$true})] + [ValidateRange(0,5)] + [int] + $Param2, + + # Param3 help description + [Parameter(ParameterSetName='Another Parameter Set')] + [ValidatePattern("[a-z]*")] + [ValidateLength(0,15)] + [String] + $Param3, + [bool] + $Force + ) + + Begin + { + } + Process + { + if ($pscmdlet.ShouldProcess("Target", "Operation")) + { + Write-Verbose "Write Verbose" + Get-Process + } + + $a = 4.5 + + if ($true) + { + $a + } + + $a | Write-Warning + + $b = @{"hash"="table"} + + Write-Debug @b + + [pscustomobject]@{ + PSTypeName = 'MyCustom.OutputType' + Prop1 = 'SomeValue' + Prop2 = 'OtherValue' + } + + return @{"hash"="true"} + } + End + { + if ($pscmdlet.ShouldContinue("Yes", "No")) { + } + [System.Void] $Param3 + } +} + #Write-Verbose should not be required because this is not an advanced function -function Get-SimpleFunc +function global:Get-SimpleFunc { } \ No newline at end of file diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignmentsNoViolations.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignmentsNoViolations.ps1 index ee0749333..ee634d2d7 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignmentsNoViolations.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignmentsNoViolations.ps1 @@ -5,4 +5,14 @@ $foo.property = "This also should not raise errors" # Output field separator builtin special variable $OFS = ', ' -[string]('apple', 'banana', 'orange') \ No newline at end of file +[string]('apple', 'banana', 'orange') + +# Using scope +$private:x = 42 +$x + +$variable:a = 52 +$a + +#function +$function:prompt = [ScriptBlock]::Create($newPrompt) \ No newline at end of file