Skip to content

Commit c60e9d5

Browse files
author
Quoc Truong
committed
Merge pull request #112 from PowerShell/FixUninitializedVariableDSC
Suppress AvoidUninitializedVariable for variables in the param block of get, set and test-targetresource
2 parents 1ea6a31 + e7ee76d commit c60e9d5

File tree

4 files changed

+94
-57
lines changed

4 files changed

+94
-57
lines changed

‎Engine/Commands/InvokeScriptAnalyzerCommand.cs

Lines changed: 35 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -502,68 +502,48 @@ private void AnalyzeFile(string filePath)
502502
}
503503

504504
// Check if the supplied artifact is indeed part of the DSC resource
505-
// Step 1: Check if the artifact is under the "DSCResources" folder
506-
DirectoryInfo dscResourceParent = Directory.GetParent(filePath);
507-
if (null != dscResourceParent)
505+
if (Helper.Instance.IsDscResourceModule(filePath))
508506
{
509-
DirectoryInfo dscResourcesFolder = Directory.GetParent(dscResourceParent.ToString());
510-
if (null != dscResourcesFolder)
511-
{
512-
if (String.Equals(dscResourcesFolder.Name, "dscresources",StringComparison.OrdinalIgnoreCase))
507+
// Run all DSC Rules
508+
foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules)
509+
{
510+
bool includeRegexMatch = false;
511+
bool excludeRegexMatch = false;
512+
foreach (Regex include in includeRegexList)
513+
{
514+
if (include.IsMatch(dscResourceRule.GetName()))
515+
{
516+
includeRegexMatch = true;
517+
break;
518+
}
519+
}
520+
foreach (Regex exclude in excludeRegexList)
521+
{
522+
if (exclude.IsMatch(dscResourceRule.GetName()))
523+
{
524+
excludeRegexMatch = true;
525+
}
526+
}
527+
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
513528
{
514-
// Step 2: Ensure there is a Schema.mof in the same folder as the artifact
515-
string schemaMofParentFolder = Directory.GetParent(filePath).ToString();
516-
string[] schemaMofFile = Directory.GetFiles(schemaMofParentFolder, "*.schema.mof");
529+
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.GetName()));
517530

518-
// Ensure Schema file exists and is the only one in the DSCResource folder
519-
if (schemaMofFile != null && schemaMofFile.Count() == 1)
531+
// Ensure that any unhandled errors from Rules are converted to non-terminating errors
532+
// We want the Engine to continue functioning even if one or more Rules throws an exception
533+
try
520534
{
521-
// Run DSC Rules only on module that matches the schema.mof file name without extension
522-
if (schemaMofFile[0].Replace("schema.mof", "psm1") == filePath)
523-
{
524-
// Run all DSC Rules
525-
foreach (IDSCResourceRule dscResourceRule in ScriptAnalyzer.Instance.DSCResourceRules)
526-
{
527-
bool includeRegexMatch = false;
528-
bool excludeRegexMatch = false;
529-
foreach (Regex include in includeRegexList)
530-
{
531-
if (include.IsMatch(dscResourceRule.GetName()))
532-
{
533-
includeRegexMatch = true;
534-
break;
535-
}
536-
}
537-
foreach (Regex exclude in excludeRegexList)
538-
{
539-
if (exclude.IsMatch(dscResourceRule.GetName()))
540-
{
541-
excludeRegexMatch = true;
542-
}
543-
}
544-
if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch))
545-
{
546-
WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, dscResourceRule.GetName()));
547-
548-
// Ensure that any unhandled errors from Rules are converted to non-terminating errors
549-
// We want the Engine to continue functioning even if one or more Rules throws an exception
550-
try
551-
{
552-
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
553-
diagnostics.AddRange(records.Item2);
554-
suppressed.AddRange(records.Item1);
555-
}
556-
catch (Exception dscResourceRuleException)
557-
{
558-
WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
559-
}
560-
}
561-
}
562-
}
535+
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
536+
diagnostics.AddRange(records.Item2);
537+
suppressed.AddRange(records.Item1);
538+
}
539+
catch (Exception dscResourceRuleException)
540+
{
541+
WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath));
563542
}
564543
}
565544
}
566-
}
545+
546+
}
567547
}
568548
#endregion
569549

‎Engine/Helper.cs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,41 @@ public string GetCmdletNameFromAlias(String Alias)
156156
return String.Empty;
157157
}
158158

159+
/// <summary>
160+
/// Given a file path, checks whether the file is part of a dsc resource module
161+
/// </summary>
162+
/// <param name="fileName"></param>
163+
/// <returns></returns>
164+
public bool IsDscResourceModule(string filePath)
165+
{
166+
DirectoryInfo dscResourceParent = Directory.GetParent(filePath);
167+
if (null != dscResourceParent)
168+
{
169+
DirectoryInfo dscResourcesFolder = Directory.GetParent(dscResourceParent.ToString());
170+
if (null != dscResourcesFolder)
171+
{
172+
if (String.Equals(dscResourcesFolder.Name, "dscresources", StringComparison.OrdinalIgnoreCase))
173+
{
174+
// Step 2: Ensure there is a Schema.mof in the same folder as the artifact
175+
string schemaMofParentFolder = Directory.GetParent(filePath).ToString();
176+
string[] schemaMofFile = Directory.GetFiles(schemaMofParentFolder, "*.schema.mof");
177+
178+
// Ensure Schema file exists and is the only one in the DSCResource folder
179+
if (schemaMofFile != null && schemaMofFile.Count() == 1)
180+
{
181+
// Run DSC Rules only on module that matches the schema.mof file name without extension
182+
if (String.Equals(schemaMofFile[0].Replace("schema.mof", "psm1"), filePath, StringComparison.OrdinalIgnoreCase))
183+
{
184+
return true;
185+
}
186+
}
187+
}
188+
}
189+
}
190+
191+
return false;
192+
}
193+
159194
/// <summary>
160195
/// Given a commandast, checks whether positional parameters are used or not.
161196
/// </summary>

‎Rules/AvoidUninitializedVariable.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//
1212

1313
using System;
14+
using System.Linq;
1415
using System.Collections.Generic;
1516
using System.Management.Automation.Language;
1617
using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
@@ -51,15 +52,31 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5152

5253
IEnumerable<Ast> funcAsts = ast.FindAll(item => item is FunctionDefinitionAst || item is FunctionMemberAst, true);
5354

54-
foreach (var funcAst in funcAsts)
55+
// Checks whether this is a dsc resource file (we don't raise this rule for get, set and test-target resource
56+
bool isDscResourceFile = Helper.Instance.IsDscResourceModule(fileName);
57+
58+
List<string> targetResourcesFunctions = new List<string>( new string[] { "get-targetresource", "set-targetresource", "test-targetresource" });
59+
60+
foreach (FunctionDefinitionAst funcAst in funcAsts)
5561
{
5662
// Finds all VariableExpressionAst.
5763
IEnumerable<Ast> varAsts = funcAst.FindAll(testAst => testAst is VariableExpressionAst, true);
5864

65+
HashSet<string> paramVariables = new HashSet<string>();
66+
67+
if (isDscResourceFile && targetResourcesFunctions.Contains(funcAst.Name, StringComparer.OrdinalIgnoreCase))
68+
{
69+
// don't raise the rules for variables in the param block.
70+
if (funcAst.Body != null && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Parameters != null)
71+
{
72+
paramVariables.UnionWith(funcAst.Body.ParamBlock.Parameters.Select(paramAst => paramAst.Name.VariablePath.UserPath));
73+
}
74+
}
75+
5976
// Iterates all VariableExpressionAst and check the command name.
6077
foreach (VariableExpressionAst varAst in varAsts)
6178
{
62-
if (Helper.Instance.IsUninitialized(varAst, funcAst))
79+
if (Helper.Instance.IsUninitialized(varAst, funcAst) && !paramVariables.Contains(varAst.VariablePath.UserPath))
6380
{
6481
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUninitializedVariableError, varAst.VariablePath.UserPath),
6582
varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, varAst.VariablePath.UserPath);

‎Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ $nonInitializedName = "PSAvoidUninitializedVariable"
55
$nonInitializedMessage = "Variable 'a' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables."
66
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
77
$violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVars.ps1
8+
$dscResourceViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $nonInitializedName}
89
$globalViolations = $violations | Where-Object {$_.RuleName -eq $globalName}
910
$nonInitializedViolations = $violations | Where-Object {$_.RuleName -eq $nonInitializedName}
1011
$noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVarsNoViolations.ps1
@@ -17,6 +18,10 @@ Describe "AvoidGlobalVars" {
1718
$globalViolations.Count | Should Be 1
1819
}
1920

21+
It "has 4 violations for dsc resources (not counting the variables in parameters)" {
22+
$dscResourceViolations.Count | Should Be 4
23+
}
24+
2025
It "has the correct description message" {
2126
$violations[0].Message | Should Match $globalMessage
2227
}

0 commit comments

Comments
 (0)