diff --git a/CHANGELOG.MD b/CHANGELOG.MD new file mode 100644 index 000000000..1602860be --- /dev/null +++ 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 index cdfd7bd4e..1baefa989 100644 --- 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 index 16304ae86..c603b20ce 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -156,6 +156,41 @@ public string GetCmdletNameFromAlias(String Alias) return String.Empty; } + /// + /// Given a file path, checks whether the file is part of a dsc resource module + /// + /// + /// + 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; + } + /// /// Given a commandast, checks whether positional parameters are used or not. /// diff --git a/RuleDocumentation/AvoidUsingWriteHost.md b/RuleDocumentation/AvoidUsingWriteHost.md new file mode 100644 index 000000000..890e16779 --- /dev/null +++ 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 index 24fbf08f4..5579a068e 100644 --- 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 AnalyzeScript(Ast ast, string fileName) IEnumerable 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 targetResourcesFunctions = new List( new string[] { "get-targetresource", "set-targetresource", "test-targetresource" }); + + foreach (FunctionDefinitionAst funcAst in funcAsts) { // Finds all VariableExpressionAst. IEnumerable varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true); + HashSet paramVariables = new HashSet(); + + 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 index a69447c0e..fb4b3f15b 100644 --- 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 index bad960927..0b90d1b71 100644 --- 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 index 73ea8399c..b10b830a4 100644 --- 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 +{ + } \ No newline at end of file