From e7ee76d0eedf1d4758b095c905796c5f81d68355 Mon Sep 17 00:00:00 2001 From: quoctruong Date: Wed, 6 May 2015 16:25:06 -0700 Subject: [PATCH] Suppress AvoidUninitializedVariable for variables in the param block of get, set and test targetresource --- .../Commands/InvokeScriptAnalyzerCommand.cs | 90 ++++++++----------- Engine/Helper.cs | 35 ++++++++ Rules/AvoidUninitializedVariable.cs | 21 ++++- .../AvoidGlobalOrUnitializedVars.tests.ps1 | 5 ++ 4 files changed, 94 insertions(+), 57 deletions(-) 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 0e59ad803..7433ce8eb 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/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/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 }