Merge pull request #252 from PowerShell/BugFixes

Merge BugFixes to Master
PowerShell · Jun 24, 2015 · 07b1e14 · 07b1e14
2 parents 0805538 + 610b12c
commit 07b1e14
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 12 deletions.
diff --git a/CHANGELOG.MD b/CHANGELOG.MD
@@ -1,3 +1,30 @@
+## Released v1.0.2 (June.25, 2015)
+###Features:
+- Perf improvements in the Engine to execute rules concurrently.
+
+
+###Rules:
+- New rule to validate the presence of deprecated module manifest fields.
+- Removed PSAvoidTrapStatement rule from the builtin set – since this is not deprecated and using trap is a better choice in certain scenarios.
+
+
+###Fixes:
+- Verbose Message rule applies to only DSC cmdlet based resources.
+- Multiple fixes to AvoidUninitializedVariable to work with non-mandatory parameters, fix in the flow graphs for throw statements;  support for missing preference variables; support for automatic variables.
+- PSAvoidUsingInternalsURLs to work with xPath expressions.
+- UseSingularNouns rule to raise warnings for plural phrases.
+- Added .gitignore to exclude certain files from being reported as untracked.
+- Revisited severity for DSC rules.
+- PSUseOutputTypeCorrectly rule not to get triggered for functions returning system.void type.
+- PSAvoidDefaultTrueValueSwitchParameter to work with switch attribute when supplied as fully qualified.
+- Ignore PSObject and PSCustomObject for UseOutputTypeCorrectly rule.
+- Only raise NullComparisonRule if the RHS is an array or has unknown type.
+- PSUseDeclaredVarsMoreThanAssignments rule to be raised for script variables and for setting the property of a variable.
+- Support for using PSUseCmdletCorrectly rule when mandatory parameters are supplied in a splatted hashtable.
+- AvoidUsingPlainTextForPassword rule to be raised only strings or object types.
+- Fix for PositionalParameterUsed method (Helper.cs) uses unsafe method to exclude ForEach-Object and Where-Object.
+
+
 ## Released v1.0.1 (May.8, 2015)
 ###Features:
 - Integrated with waffle.io for Project Management.
@@ -18,7 +45,6 @@
 
 
 ##Released v1.0.0 on Apr.24, 2015
-
 ###Features:
 - Finalized three levels of Severity - Error/Warning/Information. 
 - Improved PSScriptAnalyzer engine behavior: emits non-terminating errors (Ex: for failed ast parse) and continues rule application when running on multiple scripts.

diff --git a/Engine/Helper.cs b/Engine/Helper.cs
@@ -679,7 +679,7 @@ internal string GetTypeFromMemberExpressionAstHelper(MemberExpressionAst memberA
         /// <param name="varAst"></param>
         /// <param name="ast"></param>
         /// <returns></returns>
-        internal Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast)
+        public Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast)
         {
             try
             {

diff --git a/Engine/PSScriptAnalyzer.psd1 b/Engine/PSScriptAnalyzer.psd1
@@ -11,7 +11,7 @@ Author = 'Microsoft Corporation'
 RootModule = 'Microsoft.Windows.PowerShell.ScriptAnalyzer.dll'
 
 # Version number of this module.
-ModuleVersion = '1.0.1'
+ModuleVersion = '1.0.2'
 
 # ID used to uniquely identify this module
 GUID = '324fc715-36bf-4aee-8e58-72e9b4a08ad9'

diff --git a/Rules/PossibleIncorrectComparisonWithNull.cs b/Rules/PossibleIncorrectComparisonWithNull.cs
@@ -11,6 +11,7 @@
 //
 
 using System;
+using System.Linq;
 using System.Collections.Generic;
 using System.Management.Automation.Language;
 using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
@@ -33,17 +34,67 @@ public class PossibleIncorrectComparisonWithNull : IScriptRule {
         public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName) {
             if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
 
-            IEnumerable<Ast> binExpressionAsts = ast.FindAll(testAst => testAst is BinaryExpressionAst, true);
+            IEnumerable<Ast> binExpressionAsts = ast.FindAll(testAst => testAst is BinaryExpressionAst, false);
 
-            if (binExpressionAsts != null) {
-                foreach (BinaryExpressionAst binExpressionAst in binExpressionAsts) {
-                    if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) 
-                        || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq))
-                        && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) {
-                            yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
+            foreach (BinaryExpressionAst binExpressionAst in binExpressionAsts) {
+                if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) 
+                    || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq))
+                    && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) 
+                {
+                    if (IncorrectComparisonWithNull(binExpressionAst, ast))
+                    {
+                        yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
                     }
                 }
             }
+
+            IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true).Union(ast.FindAll(item => item is FunctionMemberAst, true));
+            foreach (Ast funcAst in funcAsts)
+            {
+                IEnumerable<Ast> binAsts = funcAst.FindAll(item => item is BinaryExpressionAst, true);
+                foreach (BinaryExpressionAst binAst in binAsts)
+                {
+                    if (IncorrectComparisonWithNull(binAst, funcAst))
+                    {
+                        yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
+                    }
+                }
+            }
+        }
+
+        private bool IncorrectComparisonWithNull(BinaryExpressionAst binExpressionAst, Ast ast)
+        {
+            if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) 
+                || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq))
+                && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) 
+            {
+                if (binExpressionAst.Left.StaticType.IsArray)
+                {
+                    return true;
+                }
+                else if (binExpressionAst.Left is VariableExpressionAst)
+                {
+                    // ignores if the variable is a special variable
+                    if (!Helper.Instance.HasSpecialVars((binExpressionAst.Left as VariableExpressionAst).VariablePath.UserPath))
+                    {
+                        Type lhsType = Helper.Instance.GetTypeFromAnalysis(binExpressionAst.Left as VariableExpressionAst, ast);
+                        if (lhsType == null)
+                        {
+                            return true;
+                        }
+                        else if (lhsType.IsArray || lhsType == typeof(object) || lhsType == typeof(Undetermined) || lhsType == typeof(Unreached))
+                        {
+                            return true;
+                        }
+                    }
+                }
+                else if (binExpressionAst.Left.StaticType == typeof(object))
+                {
+                    return true;
+                }
+            }
+
+            return false;
         }
 
         /// <summary>

diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1
@@ -1,4 +1,23 @@
 function CompareWithNull {
     if ($DebugPreference -eq $null) {
     }
+}
+
+if (@("dfd", "eee") -eq $null)
+{
+}
+
+if ($randomUninitializedVariable -eq $null)
+{
+}
+
+function Test
+{
+    $b = "dd", "ddfd";
+    if ($b -ceq $null)
+    {
+        if ("dd","ee" -eq $null)
+        {
+        }
+    }
 }
diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1
@@ -7,8 +7,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\PossibleIncorrectComparisonWith
 
 Describe "PossibleIncorrectComparisonWithNull" {
     Context "When there are violations" {
-        It "has 1 possible incorrect comparison with null violation" {
-            $violations.Count | Should Be 1
+        It "has 4 possible incorrect comparison with null violation" {
+            $violations.Count | Should Be 4
         }
 
         It "has the correct description message" {

diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1
@@ -1,4 +1,15 @@
 function CompareWithNull {
     if ($null -eq $DebugPreference) {
     }
+    if ($DebugPreference -eq $null) {
+    }
+}
+
+$a = 3
+
+if ($a -eq $null)
+{
+    if (3 -eq $null)
+    {
+    }
 }