Merge pull request #164 from PowerShell/BugFixes

yutingc · yutingc · commit 2282d5322435 · 2015-05-14T14:50:53.000-07:00
Bug fixes
diff --git a/Engine/Commands/GetScriptAnalyzerRuleCommand.cs b/Engine/Commands/GetScriptAnalyzerRuleCommand.cs
@@ -142,7 +142,7 @@ protected override void ProcessRecord()
             {
                 if (severity != null)
                 {
-                    var ruleSeverity = severity.Select(item => Enum.Parse(typeof (RuleSeverity), item));
+                    var ruleSeverity = severity.Select(item => Enum.Parse(typeof (RuleSeverity), item, true));
                     rules = rules.Where(item => ruleSeverity.Contains(item.GetSeverity())).ToList();
                 }
 
diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs
@@ -581,7 +581,7 @@ private void AnalyzeFile(string filePath)
 
             if (severity != null)
             {
-                var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item));
+                var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true));
                 diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList();
             }
 
diff --git a/Engine/SpecialVars.cs b/Engine/SpecialVars.cs
@@ -91,6 +91,7 @@ static SpecialVars()
         internal const string WhatIfPreference = "WhatIfPreference";
         internal const string WarningPreference = "WarningPreference";
         internal const string ConfirmPreference = "ConfirmPreference";
+        internal const string ProgressPreference = "ProgressPreference";
 
         internal static readonly string[] PreferenceVariables = new string[]  
                                                                 {  
@@ -99,7 +100,8 @@ static SpecialVars()
                                                                     ErrorActionPreference,  
                                                                     WhatIfPreference,  
                                                                     WarningPreference,  
-                                                                    ConfirmPreference,  
+                                                                    ConfirmPreference,
+                                                                    ProgressPreference
                                                                 };
 
         internal static readonly Type[] PreferenceVariableTypes = new Type[]  
@@ -109,7 +111,8 @@ static SpecialVars()
                                                                     /* ErrorPreference */   typeof(ActionPreference),  
                                                                     /* WhatIfPreference */  typeof(SwitchParameter),  
                                                                     /* WarningPreference */ typeof(ActionPreference),  
-                                                                    /* ConfirmPreference */ typeof(ConfirmImpact),  
+                                                                    /* ConfirmPreference */ typeof(ConfirmImpact), 
+                                                                    /* ProgressPreference */ typeof(Enum),
                                                                 };
 
         internal enum AutomaticVariable
diff --git a/Engine/VariableAnalysis.cs b/Engine/VariableAnalysis.cs
@@ -339,7 +339,8 @@ public bool IsUninitialized(VariableExpressionAst varTarget)
             }
 
             return analysis.DefinedBlock == null
-                && !SpecialVars.InitializedVariables.Contains(analysis.Name, StringComparer.OrdinalIgnoreCase)
+                && !(SpecialVars.InitializedVariables.Contains(analysis.Name, StringComparer.OrdinalIgnoreCase) || 
+                SpecialVars.InitializedVariables.Contains(analysis.RealName, StringComparer.OrdinalIgnoreCase)) 
                 && !IsGlobalOrEnvironment(varTarget);
         }
 
diff --git a/Rules/AvoidDefaultTrueValueSwitchParameter.cs b/Rules/AvoidDefaultTrueValueSwitchParameter.cs
@@ -39,7 +39,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
             // Iterrates all ParamAsts and check if any are switch.
             foreach (ParameterAst paramAst in paramAsts)
             {
-                if (paramAst.Attributes.Any(attr => String.Equals(attr.TypeName.FullName, "switch", StringComparison.OrdinalIgnoreCase))
+                if (paramAst.Attributes.Any(attr => string.Equals(attr.TypeName.GetReflectionType().FullName, "system.management.automation.switchparameter", StringComparison.OrdinalIgnoreCase))
                     && paramAst.DefaultValue != null && String.Equals(paramAst.DefaultValue.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))
                 {
                     yield return new DiagnosticRecord(
diff --git a/Rules/MissingModuleManifestField.cs b/Rules/MissingModuleManifestField.cs
@@ -12,6 +12,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Management.Automation.Language;
 using System.Management.Automation;
 using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
@@ -44,6 +45,10 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 {
                     ps.AddCommand("Test-ModuleManifest");
                     ps.AddParameter("Path", fileName);
+                    
+                    // Suppress warnings emitted during the execution of Test-ModuleManifest
+                    // ModuleManifest rule must catch any violations (warnings/errors) and generate DiagnosticRecord(s)
+                    ps.AddParameter("WarningAction", ActionPreference.SilentlyContinue);                    
                     ps.Invoke();
 
                 } catch { }
diff --git a/Rules/UseOutputTypeCorrectly.cs b/Rules/UseOutputTypeCorrectly.cs
@@ -110,6 +110,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
                     || String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase)
                     || String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase)
                     || String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase)
+                    || String.Equals(typeof(void).FullName, typeName, StringComparison.OrdinalIgnoreCase)
                     || outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase))
                 {
                     continue;
diff --git a/Rules/UseSingularNouns.cs b/Rules/UseSingularNouns.cs
@@ -49,7 +49,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName) {
                     String noun = funcNamePieces[1];
                     var ps = System.Data.Entity.Design.PluralizationServices.PluralizationService.CreateService(CultureInfo.GetCultureInfo("en-us"));
 
-                    if (ps.IsPlural(noun))
+                    if (!ps.IsSingular(noun))
                     {
                         yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsError, funcAst.Name),
                             funcAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1
@@ -113,23 +113,28 @@ Describe "Test RuleExtension" {
 Describe "TestSeverity" {
     It "filters rules based on the specified rule severity" {
         $rules = Get-ScriptAnalyzerRule -Severity Error
-        $rules.Count | Should be 4
+        $rules.Count | Should be 6
     }
 
     It "filters rules based on multiple severity inputs"{
         $rules = Get-ScriptAnalyzerRule -Severity Error,Information
-        $rules.Count | Should be 8
+        $rules.Count | Should be 12
+    }
+
+        It "takes lower case inputs" {
+        $rules = Get-ScriptAnalyzerRule -Severity error
+        $rules.Count | Should be 6
     }
 }
 
 Describe "TestWildCard" {
     It "filters rules based on the -Name wild card input" {
         $rules = Get-ScriptAnalyzerRule -Name PSDSC*
-        $rules.Count | Should be 4
+        $rules.Count | Should be 6
     }
 
     It "filters rules based on wild card input and severity"{
         $rules = Get-ScriptAnalyzerRule -Name PSDSC*　-Severity Information
-        $rules.Count | Should be 2
+        $rules.Count | Should be 3
     }
 }
diff --git a/Tests/Engine/InvokeScriptAnalyzer.tests.ps1 b/Tests/Engine/InvokeScriptAnalyzer.tests.ps1
@@ -141,7 +141,7 @@ Describe "Test IncludeRule" {
 
         it "includes 2 wildcardrules" {
             $includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules, $useRules 
-            $includeWildcard.Count | Should be 7
+            $includeWildcard.Count | Should be 9
         }
     }
 }
@@ -169,6 +169,11 @@ Describe "Test Severity" {
             $errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity Information, Warning
             $errors.Count | Should Be 2
         }
+
+        It "works with lowercase argument"{
+             $errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity information, warning
+            $errors.Count | Should Be 2
+        }
     }
 
     Context "When used incorrectly" {
diff --git a/Tests/Rules/AvoidDefaultTrueValueSwitchParameter.ps1 b/Tests/Rules/AvoidDefaultTrueValueSwitchParameter.ps1
@@ -13,7 +13,11 @@
 
         # Param2 help description
         [switch]
-        $switch=$true
+        $switch=$true,
+        
+        # Param3 help description
+        [System.Management.Automation.SwitchParameter]
+        $switch2 = $true
     )
 
     Begin
diff --git a/Tests/Rules/AvoidDefaultTrueValueSwitchParameter.tests.ps1 b/Tests/Rules/AvoidDefaultTrueValueSwitchParameter.tests.ps1
@@ -7,8 +7,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\AvoidDefaultTrueValueSwitchPara
 
 Describe "AvoidDefaultTrueValueSwitchParameter" {
     Context "When there are violations" {
-        It "has 1 avoid using switch parameter default to true violation" {
-            $violations.Count | Should Be 1
+        It "has 2 avoid using switch parameter default to true violation" {
+            $violations.Count | Should Be 2
         }
 
         It "has the correct description message" {
diff --git a/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 b/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1
@@ -1,30 +1,41 @@
-﻿function Test {
-    $initialized = "Initialized"
-    $noglobal = "local"
-    $env:ShouldNotRaiseError
-}
-
-$a = 3;
-
-if ($true) {
-    $a = 4;
-    $c = 3;
-} else {
-    $b = 5;
-    $c = 4;
-}
-
-$b = 6;
-$a;
-$b;
-
-stop-process 12,23 -ErrorVariable ev -ErrorAction SilentlyContinue
-if($null -ne $ev)
-{
-    Write-host $ev[0]
-    # no error should be raised here
-    Invoke-Command {$b}
-}
-
-get-process notepad | tee-object -variable proc 
-$proc[0]
+﻿function Test {
+    $initialized = "Initialized"
+    $noglobal = "local"
+    $env:ShouldNotRaiseError
+}
+
+$a = 3;
+
+if ($true) {
+    $a = 4;
+    $c = 3;
+} else {
+    $b = 5;
+    $c = 4;
+}
+
+$b = 6;
+$a;
+$b;
+
+stop-process 12,23 -ErrorVariable ev -ErrorAction SilentlyContinue
+if($null -ne $ev)
+{
+    Write-host $ev[0]
+    # no error should be raised here
+    Invoke-Command {$b}
+}
+
+get-process notepad | tee-object -variable proc 
+$proc[0]
+
+function Test-PreferenceVariable
+{
+
+   if (-not $PSBoundParameters.ContainsKey('Verbose')) {
+       $VerbosePreference = $PSCmdlet.GetVariableValue('VerbosePreference') -as 
+        [System.Management.Automation.ActionPreference]
+        }
+
+        $VerbosePreference
+    }
diff --git a/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 b/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1
@@ -0,0 +1,24 @@
+﻿Import-Module PSScriptAnalyzer
+$AvoidUninitializedVariable = "PSAvoidUninitializedVariable"
+$violationMessage = "Variable 'MyProgressPreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
+$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
+$violations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariable.ps1 -IncludeRule $AvoidUninitializedVariable
+$noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariableNoViolations.ps1 -IncludeRule $AvoidUninitializedVariable
+
+Describe "AvoidUsingUninitializedVariable" {
+    Context "Script uses uninitialized variables - Violation" {
+        It "Have 3 rule violations" {
+            $violations.Count | Should Be 3
+        }
+
+        It "has the correct description message for UninitializedVariable rule violation" {
+            $violations[0].Message | Should Be $violationMessage            
+        }
+    }
+
+    Context "Script uses initialized variables - No violation" {
+        It "results in no rule violations" {
+            $noViolations.Count | Should Be 0
+        }
+    }
+}
diff --git a/Tests/Rules/AvoidUsingUninitializedVariable.ps1 b/Tests/Rules/AvoidUsingUninitializedVariable.ps1
@@ -0,0 +1,10 @@
+﻿# Script has uninitialized variables
+# Must result in AvoidUsingUninitializedVariablerule violations along with other violations
+
+function Test-MyPreference
+{
+    Write-Verbose $MyProgressPreference
+    Write-Verbose $MyVerbosePreference
+}
+
+Write-Verbose $MyProgressPreference
diff --git a/Tests/Rules/AvoidUsingUninitializedVariableNoViolations.ps1 b/Tests/Rules/AvoidUsingUninitializedVariableNoViolations.ps1
@@ -0,0 +1,11 @@
+﻿# Script uses built-in preference variables
+# Must not result in AvoidUsingUninitializedVariablerule violations
+# However there are other violations in this script - Write-Verbose is not using positional parameters
+
+function Test-Preference
+{
+    Write-Verbose $ProgressPreference
+    Write-Verbose $VerbosePreference
+}
+
+Write-Verbose $ProgressPreference
diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1
@@ -91,6 +91,7 @@ function Get-File
     {
         if ($pscmdlet.ShouldContinue("Yes", "No")) {
         }
+        [System.Void] $Param3
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ protected override void ProcessRecord()`
`142`	`142`	`{`
`143`	`143`	`if (severity != null)`
`144`	`144`	`{`
`145`		`- var ruleSeverity = severity.Select(item => Enum.Parse(typeof (RuleSeverity), item));`
	`145`	`+ var ruleSeverity = severity.Select(item => Enum.Parse(typeof (RuleSeverity), item, true));`
`146`	`146`	`rules = rules.Where(item => ruleSeverity.Contains(item.GetSeverity())).ToList();`
`147`	`147`	`}`
`148`	`148`
Original file line number	Diff line number	Diff line change
`@@ -581,7 +581,7 @@ private void AnalyzeFile(string filePath)`
`581`	`581`
`582`	`582`	`if (severity != null)`
`583`	`583`	`{`
`584`		`- var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item));`
	`584`	`+ var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true));`
`585`	`585`	`diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList();`
`586`	`586`	`}`
`587`	`587`
Original file line number	Diff line number	Diff line change
`@@ -339,7 +339,8 @@ public bool IsUninitialized(VariableExpressionAst varTarget)`
`339`	`339`	`}`
`340`	`340`
`341`	`341`	`return analysis.DefinedBlock == null`
`342`		`- && !SpecialVars.InitializedVariables.Contains(analysis.Name, StringComparer.OrdinalIgnoreCase)`
	`342`	`+ && !(SpecialVars.InitializedVariables.Contains(analysis.Name, StringComparer.OrdinalIgnoreCase) \|\|`
	`343`	`+ SpecialVars.InitializedVariables.Contains(analysis.RealName, StringComparer.OrdinalIgnoreCase))`
`343`	`344`	`&& !IsGlobalOrEnvironment(varTarget);`
`344`	`345`	`}`
`345`	`346`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)`
`39`	`39`	`// Iterrates all ParamAsts and check if any are switch.`
`40`	`40`	`foreach (ParameterAst paramAst in paramAsts)`
`41`	`41`	`{`
`42`		`- if (paramAst.Attributes.Any(attr => String.Equals(attr.TypeName.FullName, "switch", StringComparison.OrdinalIgnoreCase))`
	`42`	`+ if (paramAst.Attributes.Any(attr => string.Equals(attr.TypeName.GetReflectionType().FullName, "system.management.automation.switchparameter", StringComparison.OrdinalIgnoreCase))`
`43`	`43`	`&& paramAst.DefaultValue != null && String.Equals(paramAst.DefaultValue.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))`
`44`	`44`	`{`
`45`	`45`	`yield return new DiagnosticRecord(`
Original file line number	Diff line number	Diff line change
`@@ -110,6 +110,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun`
`110`	`110`	`\|\| String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase)`
`111`	`111`	`\|\| String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase)`
`112`	`112`	`\|\| String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase)`
	`113`	`+ \|\| String.Equals(typeof(void).FullName, typeName, StringComparison.OrdinalIgnoreCase)`
`113`	`114`	`\|\| outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase))`
`114`	`115`	`{`
`115`	`116`	`continue;`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName) {`
`49`	`49`	`String noun = funcNamePieces[1];`
`50`	`50`	`var ps = System.Data.Entity.Design.PluralizationServices.PluralizationService.CreateService(CultureInfo.GetCultureInfo("en-us"));`
`51`	`51`
`52`		`- if (ps.IsPlural(noun))`
	`52`	`+ if (!ps.IsSingular(noun))`
`53`	`53`	`{`
`54`	`54`	`yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseSingularNounsError, funcAst.Name),`
`55`	`55`	`funcAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);`
Original file line number	Diff line number	Diff line change
`@@ -113,23 +113,28 @@ Describe "Test RuleExtension" {`
`113`	`113`	`Describe "TestSeverity" {`
`114`	`114`	`It "filters rules based on the specified rule severity" {`
`115`	`115`	`$rules = Get-ScriptAnalyzerRule -Severity Error`
`116`		`- $rules.Count \| Should be 4`
	`116`	`+ $rules.Count \| Should be 6`
`117`	`117`	`}`
`118`	`118`
`119`	`119`	`It "filters rules based on multiple severity inputs"{`
`120`	`120`	`$rules = Get-ScriptAnalyzerRule -Severity Error,Information`
`121`		`- $rules.Count \| Should be 8`
	`121`	`+ $rules.Count \| Should be 12`
	`122`	`+ }`
	`123`	`+`
	`124`	`+ It "takes lower case inputs" {`
	`125`	`+ $rules = Get-ScriptAnalyzerRule -Severity error`
	`126`	`+ $rules.Count \| Should be 6`
`122`	`127`	`}`
`123`	`128`	`}`
`124`	`129`
`125`	`130`	`Describe "TestWildCard" {`
`126`	`131`	`It "filters rules based on the -Name wild card input" {`
`127`	`132`	`$rules = Get-ScriptAnalyzerRule -Name PSDSC*`
`128`		`- $rules.Count \| Should be 4`
	`133`	`+ $rules.Count \| Should be 6`
`129`	`134`	`}`
`130`	`135`
`131`	`136`	`It "filters rules based on wild card input and severity"{`
`132`	`137`	`$rules = Get-ScriptAnalyzerRule -Name PSDSC*　-Severity Information`
`133`		`- $rules.Count \| Should be 2`
	`138`	`+ $rules.Count \| Should be 3`
`134`	`139`	`}`
`135`	`140`	`}`
Original file line number	Diff line number	Diff line change
`@@ -141,7 +141,7 @@ Describe "Test IncludeRule" {`
`141`	`141`
`142`	`142`	`it "includes 2 wildcardrules" {`
`143`	`143`	`$includeWildcard = Invoke-ScriptAnalyzer $directory\..\Rules\BadCmdlet.ps1 -IncludeRule $avoidRules, $useRules`
`144`		`- $includeWildcard.Count \| Should be 7`
	`144`	`+ $includeWildcard.Count \| Should be 9`
`145`	`145`	`}`
`146`	`146`	`}`
`147`	`147`	`}`
`@@ -169,6 +169,11 @@ Describe "Test Severity" {`
`169`	`169`	`$errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity Information, Warning`
`170`	`170`	`$errors.Count \| Should Be 2`
`171`	`171`	`}`
	`172`	`+`
	`173`	`+ It "works with lowercase argument"{`
	`174`	`+ $errors = Invoke-ScriptAnalyzer $directory\TestScript.ps1 -Severity information, warning`
	`175`	`+ $errors.Count \| Should Be 2`
	`176`	`+ }`
`172`	`177`	`}`
`173`	`178`
`174`	`179`	`Context "When used incorrectly" {`