Merge pull request #119 from PowerShell/master

raghushantha · raghushantha · commit 6ea858a97207 · 2015-05-07T14:51:23.000-07:00
Forward Integrate: Master to Development
diff --git a/CHANGELOG.MD b/CHANGELOG.MD
@@ -0,0 +1,46 @@
+## Unreleased (May.7, 2015)
+###Features:
+- Integrated with waffle.io for Project Management.
+- Added documentation for writing script rules.
+
+###Rules:
+- AvoidUsingWMICmdlet rule: For PowerShell 3.0 and above, usage of WMI cmdlets is not recommended. This rule is to detect WMI cmdlet usage in scripts that are written for PS 3.0 and above.
+- DSCTestsPresent rule: Resource module contains Tests folder with tests for given resource.
+- UseOutputTypeCorrectly rule: If we can identify the type of an object that is outputted to the pipeline by a cmdlet, then that type must be listed in the OutputType attribute.
+
+###Fixes:
+
+- PSProvideVerboseMessage only throws warnings in non-advanced functions.
+- Fix the issue in importing customized rule
+
+
+
+
+##Relesed 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.
+- Added wild card supports for rules in Invoke-ScriptAnalyzer and Get-ScriptAnalyzer. Eg. Invoke-ScriptAnalyzer -IncludeRule PSAvoid* will apply all rules starting with PSAvoid* in built in rule assemblies. 
+- Added -Severity to Get-ScriptAnalyzerRules. Get-ScriptAnalyzer -Severity will filter rules based on the severity given.
+- Added Suppression functionality. Users are now able to specify suppression on certain parts of the scripts by specifying "SupressMessageAttribute" in the scripts. More details and documentations will be coming soon in blog posts. Also comes with this feature is the ability for users to display a list of suppressed messages.
+
+###Rules:
+
+- Added DSC Rules for resources including Parameter validation, Usage of standard DSC functions and return type validation. Rule checkings also support for DSC classes. Built-in DSC rules include:
+    + UseStandardDSCFunctionsInResource
+    + UseIdenticalParametersDSC
+    + UseIdenticalMandatoryParametersDSC
+    + ReturnCorrectTypesForDSCFunctions
+- Added support in the engine to detect DSC configuration/resource files and disable default rule checkings on DSC configuration and resource files.
+- UseShouldProcessForStateChangingFunctions - If an advanced function has Verbs like New/Start/Stop/Restart/Reset/Set- that will change system state, it should support ShouldProcess attribute.
+
+
+###Fixes:
+
+- Improved heuristics to detect usage of Username and Password instead of PSCredential type.
+- Improved accuracy in the detection of uninitialized variables.
+- Improved error messages to include error line numbers and file names.
+- Identified usage of PSBound parameters and PowerShell supplied variables such as $MyInvocation to avoid unnecessary noise in the results returned by some of the built-in rules.
+- Fixed terminating errors including "Illegal characters in Path".
+
diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs
@@ -502,68 +502,48 @@ private void AnalyzeFile(string filePath)
                 }
 
                 // Check if the supplied artifact is indeed part of the DSC resource
-                // Step 1: Check if the artifact is under the "DSCResources" folder                
-                DirectoryInfo dscResourceParent = Directory.GetParent(filePath);
-                if (null != dscResourceParent)
+                if (Helper.Instance.IsDscResourceModule(filePath))
                 {
-                    DirectoryInfo dscResourcesFolder = Directory.GetParent(dscResourceParent.ToString());
-                    if (null != dscResourcesFolder)
-                    {                        
-                        if (String.Equals(dscResourcesFolder.Name, "dscresources",StringComparison.OrdinalIgnoreCase))
+                    // Run all DSC Rules
+                    foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules)
+                    {
+                        bool includeRegexMatch = false;
+                        bool excludeRegexMatch = false;
+                        foreach (Regex include in includeRegexList)
+                        {
+                            if (include.IsMatch(dscResourceRule.GetName()))
+                            {
+                                includeRegexMatch = true;
+                                break;
+                            }
+                        }
+                        foreach (Regex exclude in excludeRegexList)
+                        {
+                            if (exclude.IsMatch(dscResourceRule.GetName()))
+                            {
+                                excludeRegexMatch = true;
+                            }
+                        }
+                        if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
                         {
-                            // Step 2: Ensure there is a Schema.mof in the same folder as the artifact
-                            string schemaMofParentFolder = Directory.GetParent(filePath).ToString();
-                            string[] schemaMofFile = Directory.GetFiles(schemaMofParentFolder, "*.schema.mof");
+                            WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.GetName()));
 
-                            // Ensure Schema file exists and is the only one in the DSCResource folder
-                            if (schemaMofFile != null && schemaMofFile.Count() == 1)
+                            // Ensure that any unhandled errors from Rules are converted to non-terminating errors
+                            // We want the Engine to continue functioning even if one or more Rules throws an exception
+                            try
                             {
-                                // Run DSC Rules only on module that matches the schema.mof file name without extension
-                                if (schemaMofFile[0].Replace("schema.mof", "psm1") == filePath)
-                                {
-                                    // Run all DSC Rules
-                                    foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules)
-                                    {
-                                        bool includeRegexMatch = false;
-                                        bool excludeRegexMatch = false;
-                                        foreach (Regex include in includeRegexList)
-                                        {
-                                            if (include.IsMatch(dscResourceRule.GetName()))
-                                            {
-                                                includeRegexMatch = true;
-                                                break;
-                                            }
-                                        }
-                                        foreach (Regex exclude in excludeRegexList)
-                                        {
-                                            if (exclude.IsMatch(dscResourceRule.GetName()))
-                                            {
-                                                excludeRegexMatch = true;
-                                            }
-                                        }
-                                        if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
-                                        {
-                                            WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.GetName()));
-
-                                            // Ensure that any unhandled errors from Rules are converted to non-terminating errors
-                                            // We want the Engine to continue functioning even if one or more Rules throws an exception
-                                            try
-                                            {
-                                                var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
-                                                diagnostics.AddRange(records.Item2);
-                                                suppressed.AddRange(records.Item1);
-                                            }
-                                            catch (Exception dscResourceRuleException)
-                                            {
-                                                WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
-                                            }  
-                                        }
-                                    }
-                                }
+                                var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
+                                diagnostics.AddRange(records.Item2);
+                                suppressed.AddRange(records.Item1);
+                            }
+                            catch (Exception dscResourceRuleException)
+                            {
+                                WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
                             }
                         }
                     }
-                }                
+
+                }         
             }
             #endregion
 
diff --git a/Engine/Helper.cs b/Engine/Helper.cs
@@ -156,6 +156,41 @@ public string GetCmdletNameFromAlias(String Alias)
             return String.Empty;
         }
 
+        /// <summary>
+        /// Given a file path, checks whether the file is part of a dsc resource module
+        /// </summary>
+        /// <param name="fileName"></param>
+        /// <returns></returns>
+        public bool IsDscResourceModule(string filePath)
+        {
+            DirectoryInfo dscResourceParent = Directory.GetParent(filePath);
+            if (null != dscResourceParent)
+            {
+                DirectoryInfo dscResourcesFolder = Directory.GetParent(dscResourceParent.ToString());
+                if (null != dscResourcesFolder)
+                {
+                    if (String.Equals(dscResourcesFolder.Name, "dscresources", StringComparison.OrdinalIgnoreCase))
+                    {
+                        // Step 2: Ensure there is a Schema.mof in the same folder as the artifact
+                        string schemaMofParentFolder = Directory.GetParent(filePath).ToString();
+                        string[] schemaMofFile = Directory.GetFiles(schemaMofParentFolder, "*.schema.mof");
+
+                        // Ensure Schema file exists and is the only one in the DSCResource folder
+                        if (schemaMofFile != null && schemaMofFile.Count() == 1)
+                        {
+                            // Run DSC Rules only on module that matches the schema.mof file name without extension
+                            if (String.Equals(schemaMofFile[0].Replace("schema.mof", "psm1"), filePath, StringComparison.OrdinalIgnoreCase))
+                            {
+                                return true;
+                            }
+                        }
+                    }
+                }
+            }
+
+            return false;
+        }
+
         /// <summary>
         /// Given a commandast, checks whether positional parameters are used or not.
         /// </summary>
diff --git a/RuleDocumentation/AvoidUsingWriteHost.md b/RuleDocumentation/AvoidUsingWriteHost.md
@@ -0,0 +1,38 @@
+#AvoidUsingWriteHost 
+**Severity Level: Warning**
+
+
+##Description
+
+It is generally accepted that you should never use Write-Host to create any script output whatsoever, unless your script (or function, or whatever) uses the Show verb (as in, Show-Performance). That verb explicitly means “show on the screen, with no other possibilities.” Like Show-Command.
+
+##How to Fix
+
+PTo fix a violation of this rule, please replace Write-Host with Write-Output.
+
+##Example
+
+Wrong： 
+
+```
+function Test
+{
+	...
+	Write-Host "Executing.."
+}
+```
+
+Correct: 
+
+```
+function Test
+{
+	...
+	Write-Output "Executing.."
+}
+
+function Show-Something
+{
+    Write-Host "show something on screen";
+}
+```
diff --git a/Rules/AvoidUninitializedVariable.cs b/Rules/AvoidUninitializedVariable.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;
@@ -51,15 +52,31 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
             IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst || item is FunctionMemberAst, true);
 
-            foreach (var funcAst in funcAsts)
+            // Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
+            bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);
+
+            List<string> targetResourcesFunctions = new List<string>( new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });
+
+            foreach (FunctionDefinitionAst funcAst in funcAsts)
             {
                 // Finds all VariableExpressionAst.
                 IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);
 
+                HashSet<string> paramVariables = new HashSet<string>();
+
+                if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
+                {
+                    // don't raise the rules for variables in the param block.
+                    if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
+                    {
+                        paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
+                    }
+                }
+
                 // Iterates all VariableExpressionAst and check the command name.
                 foreach (VariableExpressionAst varAst in varAsts)
                 {
-                    if (Helper.Instance.IsUninitialized(varAst, funcAst))
+                    if (Helper.Instance.IsUninitialized(varAst, funcAst) && !paramVariables.Contains(varAst.VariablePath.UserPath))
                     {
                         yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUninitializedVariableError, varAst.VariablePath.UserPath),
                             varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, varAst.VariablePath.UserPath);
diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/ProvideVerboseMessage.cs
@@ -12,10 +12,12 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Management.Automation.Language;
 using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
 using System.ComponentModel.Composition;
 using System.Globalization;
+using System.Management.Automation;
 
 namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
 {
@@ -57,6 +59,15 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
                 return AstVisitAction.SkipChildren;
             }
 
+            //Write-Verbose is not required for non-advanced functions
+            if (funcAst.Body == null || funcAst.Body.ParamBlock == null
+           || funcAst.Body.ParamBlock.Attributes == null ||
+           funcAst.Body.ParamBlock.Parameters == null ||
+                !funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute)))
+            {
+                return AstVisitAction.Continue;
+            }
+
             var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false);
             bool hasVerbose = false;
 
diff --git a/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 b/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1
@@ -5,6 +5,7 @@ $nonInitializedName = "PSAvoidUninitializedVariable"
 $nonInitializedMessage = "Variable 'a' 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\AvoidGlobalOrUnitializedVars.ps1
+$dscResourceViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $nonInitializedName}
 $globalViolations = $violations | Where-Object {$_.RuleName -eq $globalName}
 $nonInitializedViolations = $violations | Where-Object {$_.RuleName -eq $nonInitializedName}
 $noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVarsNoViolations.ps1
@@ -17,6 +18,10 @@ Describe "AvoidGlobalVars" {
             $globalViolations.Count | Should Be 1
         }
 
+        It "has 4 violations for dsc resources (not counting the variables in parameters)" {
+            $dscResourceViolations.Count | Should Be 4
+        }
+
         It "has the correct description message" {
             $violations[0].Message | Should Match $globalMessage
         }
diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1
@@ -92,4 +92,10 @@ function Get-File
         if ($pscmdlet.ShouldContinue("Yes", "No")) {
         }
     }
+}
+
+#Write-Verbose should not be required because this is not an advanced function
+function Get-SimpleFunc
+{
+
 }

Original file line number	Diff line number	Diff line change
`@@ -92,4 +92,10 @@ function Get-File`
`92`	`92`	`if ($pscmdlet.ShouldContinue("Yes", "No")) {`
`93`	`93`	`}`
`94`	`94`	`}`
	`95`	`+}`
	`96`	`+`
	`97`	`+#Write-Verbose should not be required because this is not an advanced function`
	`98`	`+function Get-SimpleFunc`
	`99`	`+{`
	`100`	`+`
`95`	`101`	`}`