From ab31260f4e85266a7a9d161f66293a4129ada436 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 13:28:25 -0700 Subject: [PATCH 01/10] Add switch to lauch debugger in DEBUG configuration --- .../Commands/InvokeScriptAnalyzerCommand.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index db42db740..c52776a3a 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -206,6 +206,16 @@ public SwitchParameter SaveDscDependency } private bool saveDscDependency; #endif // !PSV3 + +#if DEBUG + [Parameter(Mandatory = false)] + public SwitchParameter AttachAndDebug + { + get { return attachAndDebug; } + set { attachAndDebug = value; } + } + private bool attachAndDebug = false; +#endif #endregion Parameters #region Overrides @@ -216,6 +226,19 @@ public SwitchParameter SaveDscDependency protected override void BeginProcessing() { // Initialize helper +#if DEBUG + if (attachAndDebug) + { + if (System.Diagnostics.Debugger.IsAttached) + { + System.Diagnostics.Debugger.Break(); + } + else + { + System.Diagnostics.Debugger.Launch(); + } + } +#endif Helper.Instance = new Helper( SessionState.InvokeCommand, this); From 352f7449730ff24eea6cd93f76abe22591ee7503 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 14:47:44 -0700 Subject: [PATCH 02/10] Get groups of CommandAsts that resemble prop value pairs --- Rules/AlignAssignmentStatement.cs | 62 ++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index 83827f433..9cb88d592 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -145,8 +145,26 @@ public override SourceType GetSourceType() private IEnumerable FindHashtableViolations(TokenOperations tokenOps) { var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true); - if (hashtableAsts == null) + if (hashtableAsts == null || !hashtableAsts.Any()) { + var configAsts = tokenOps.Ast.FindAll(ast => ast is ConfigurationDefinitionAst, true); + if (configAsts != null) + { + // There are probably parse errors caused by an "Undefined DSC resource" + // which prevents the parser from detecting the property value pairs as + // hashtable. Hence, this is a workaround to format configurations which + // have "Undefined DSC resource" parse errors. + + // TODO break down the function and reuse the alignment logic. + + // find all commandAsts of the form "prop" "=" "val" that have the same parent + // and format those pairs. + foreach (var configAst in configAsts) + { + var groups = GetCommandElementGroups(configAst); + } + } + yield break; } @@ -204,6 +222,48 @@ private IEnumerable FindHashtableViolations(TokenOperations to } } + private List> GetCommandElementGroups(Ast configAst) + { + var result = new List>(); + var parents = new HashSet(); + var parentChildrenGroup = configAst.FindAll(ast => IsPropertyValueCommandAst(ast), true)? + .Select(ast => ast as CommandAst) + .GroupBy(ast => ast.Parent.Parent); // parent is pipeline and pipeline's parent is namedblockast + if (parentChildrenGroup == null) + { + return result; + } + + // var parentChildrenMap = new Dictionary>(); + // foreach (var commandAst in commandAsts) + // { + // var parent = commandAst.Parent; + // if (parentChildrenMap.ContainsKey(parent)) + // { + // parentChildrenMap[parent].Add(commandAst); + // } + // else + // { + // parentChildrenMap.Add(parent, new List()) + // } + // } + + foreach (var group in parentChildrenGroup) + { + result.Add(group.ToList()); + } + + return result; + } + + private bool IsPropertyValueCommandAst(Ast ast) + { + var commandAst = ast as CommandAst; + return commandAst != null + && commandAst.CommandElements.Count() == 3 + && commandAst.CommandElements[1].Extent.Text.Equals("="); + } + private IEnumerable GetHashtableCorrections( Tuple extentTuple, int expectedStartColumnNumber) From f0fd2ed95e79b2efcfa3d42ad8134383f99869f6 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 15:05:47 -0700 Subject: [PATCH 03/10] Update GetCommandElementGroups implementation --- Rules/AlignAssignmentStatement.cs | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index 9cb88d592..c848a42f5 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -225,29 +225,15 @@ private IEnumerable FindHashtableViolations(TokenOperations to private List> GetCommandElementGroups(Ast configAst) { var result = new List>(); - var parents = new HashSet(); - var parentChildrenGroup = configAst.FindAll(ast => IsPropertyValueCommandAst(ast), true)? - .Select(ast => ast as CommandAst) - .GroupBy(ast => ast.Parent.Parent); // parent is pipeline and pipeline's parent is namedblockast - if (parentChildrenGroup == null) + var astsFound = configAst.FindAll(ast => IsPropertyValueCommandAst(ast), true); + if (astsFound == null) { return result; } - // var parentChildrenMap = new Dictionary>(); - // foreach (var commandAst in commandAsts) - // { - // var parent = commandAst.Parent; - // if (parentChildrenMap.ContainsKey(parent)) - // { - // parentChildrenMap[parent].Add(commandAst); - // } - // else - // { - // parentChildrenMap.Add(parent, new List()) - // } - // } - + var parentChildrenGroup = from ast in astsFound + select (CommandAst)ast into commandAst + group commandAst by commandAst.Parent.Parent; // parent is pipeline and pipeline's parent is namedblockast foreach (var group in parentChildrenGroup) { result.Add(group.ToList()); From e4e219de826b23381603562910669553cf874978 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 15:06:37 -0700 Subject: [PATCH 04/10] Use extent tuples to check keys on different lines --- Rules/AlignAssignmentStatement.cs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index c848a42f5..78af69cc6 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -184,12 +184,13 @@ private IEnumerable FindHashtableViolations(TokenOperations to foreach (var astItem in hashtableAsts) { var hashtableAst = (HashtableAst)astItem; - if (!HasKeysOnSeparateLines(hashtableAst)) + var extentTuples = GetExtents(tokenOps, hashtableAst); + + if (!HasPropertiesOnSeparateLines(extentTuples)) { continue; } - var extentTuples = GetExtents(tokenOps, hashtableAst); if (extentTuples == null || extentTuples.Count == 0 || !extentTuples.All(t => t.Item1.StartLineNumber == t.Item2.EndLineNumber)) @@ -296,6 +297,24 @@ private static IList> GetExtents( return nodeTuples; } + private bool HasPropertiesOnSeparateLines(IEnumerable> tuples) + { + var lines = new HashSet(); + foreach (var kvp in tuples) + { + if (lines.Contains(kvp.Item1.StartLineNumber)) + { + return false; + } + else + { + lines.Add(kvp.Item1.StartLineNumber); + } + } + + return true; + } + private bool HasKeysOnSeparateLines(HashtableAst hashtableAst) { var lines = new HashSet(); From d4bc9ef53b8190bc5298626f54161d3e9dd286e7 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 15:33:17 -0700 Subject: [PATCH 05/10] Add method to get extent tuples from command asts --- Rules/AlignAssignmentStatement.cs | 34 +++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index 78af69cc6..f42d37481 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -145,6 +145,7 @@ public override SourceType GetSourceType() private IEnumerable FindHashtableViolations(TokenOperations tokenOps) { var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true); + var groups = new List>>(); if (hashtableAsts == null || !hashtableAsts.Any()) { var configAsts = tokenOps.Ast.FindAll(ast => ast is ConfigurationDefinitionAst, true); @@ -161,7 +162,7 @@ private IEnumerable FindHashtableViolations(TokenOperations to // and format those pairs. foreach (var configAst in configAsts) { - var groups = GetCommandElementGroups(configAst); + groups.AddRange(GetCommandElementExtentGroups(configAst)); } } @@ -181,6 +182,7 @@ private IEnumerable FindHashtableViolations(TokenOperations to // find the longest left expression // make sure all the assignment operators are in the same column as that of the longest left hand. + foreach (var astItem in hashtableAsts) { var hashtableAst = (HashtableAst)astItem; @@ -200,11 +202,12 @@ private IEnumerable FindHashtableViolations(TokenOperations to var widestKeyExtent = extentTuples .Select(t => t.Item1) - .Aggregate((t1, tAggregate) => { - return TokenOperations.GetExtentWidth(tAggregate) > TokenOperations.GetExtentWidth(t1) - ? tAggregate - : t1; - }); + .Aggregate((t1, tAggregate) => + { + return TokenOperations.GetExtentWidth(tAggregate) > TokenOperations.GetExtentWidth(t1) + ? tAggregate + : t1; + }); var expectedStartColumnNumber = widestKeyExtent.EndColumnNumber + 1; foreach (var extentTuple in extentTuples) { @@ -223,6 +226,25 @@ private IEnumerable FindHashtableViolations(TokenOperations to } } + private List>> GetCommandElementExtentGroups(Ast configAst) + { + var result = new List>>(); + var commandAstGroups = GetCommandElementGroups(configAst); + foreach (var commandAstGroup in commandAstGroups) + { + var list = new List>(); + foreach (var commandAst in commandAstGroup) + { + var elems = commandAst.CommandElements; + list.Add(new Tuple(elems[0].Extent, elems[1].Extent)); + } + + result.Add(list); + } + + return result; + } + private List> GetCommandElementGroups(Ast configAst) { var result = new List>(); From be2deddbed9772a638a5c2500d8df7810e85321b Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 15:59:06 -0700 Subject: [PATCH 06/10] Handle DSC configurations with parse errors --- Rules/AlignAssignmentStatement.cs | 43 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index f42d37481..7c681d17b 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -146,29 +146,31 @@ private IEnumerable FindHashtableViolations(TokenOperations to { var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true); var groups = new List>>(); - if (hashtableAsts == null || !hashtableAsts.Any()) + if (hashtableAsts != null) { - var configAsts = tokenOps.Ast.FindAll(ast => ast is ConfigurationDefinitionAst, true); - if (configAsts != null) + foreach (var astItem in hashtableAsts) { - // There are probably parse errors caused by an "Undefined DSC resource" - // which prevents the parser from detecting the property value pairs as - // hashtable. Hence, this is a workaround to format configurations which - // have "Undefined DSC resource" parse errors. - - // TODO break down the function and reuse the alignment logic. - - // find all commandAsts of the form "prop" "=" "val" that have the same parent - // and format those pairs. - foreach (var configAst in configAsts) - { - groups.AddRange(GetCommandElementExtentGroups(configAst)); - } + groups.Add(GetExtents(tokenOps, (HashtableAst)astItem)); } + } - yield break; + var configAsts = tokenOps.Ast.FindAll(ast => ast is ConfigurationDefinitionAst, true); + if (configAsts != null) + { + // There are probably parse errors caused by an "Undefined DSC resource" + // which prevents the parser from detecting the property value pairs as + // hashtable. Hence, this is a workaround to format configurations which + // have "Undefined DSC resource" parse errors. + + // find all commandAsts of the form "prop" "=" "val" that have the same parent + // and format those pairs. + foreach (var configAst in configAsts) + { + groups.AddRange(GetCommandElementExtentGroups(configAst)); + } } + // it is probably much easier have a hashtable writer that formats the hashtable and writes it // but it makes handling comments hard. So we need to use this approach. @@ -183,11 +185,8 @@ private IEnumerable FindHashtableViolations(TokenOperations to // make sure all the assignment operators are in the same column as that of the longest left hand. - foreach (var astItem in hashtableAsts) + foreach (var extentTuples in groups) { - var hashtableAst = (HashtableAst)astItem; - var extentTuples = GetExtents(tokenOps, hashtableAst); - if (!HasPropertiesOnSeparateLines(extentTuples)) { continue; @@ -294,7 +293,7 @@ private string GetError() return String.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementError); } - private static IList> GetExtents( + private static List> GetExtents( TokenOperations tokenOps, HashtableAst hashtableAst) { From 622899ec95a0927ce48197bbe8a93af63391913b Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 16:00:50 -0700 Subject: [PATCH 07/10] Add tests for dsc configurations with parse errors --- .../Rules/AlignAssignmentStatement.tests.ps1 | 58 ++++++++++++++----- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/Tests/Rules/AlignAssignmentStatement.tests.ps1 b/Tests/Rules/AlignAssignmentStatement.tests.ps1 index 207b5f088..5266969ec 100644 --- a/Tests/Rules/AlignAssignmentStatement.tests.ps1 +++ b/Tests/Rules/AlignAssignmentStatement.tests.ps1 @@ -5,13 +5,13 @@ Import-Module PSScriptAnalyzer Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1") $ruleConfiguration = @{ - Enable = $true + Enable = $true CheckHashtable = $true } $settings = @{ IncludeRules = @("PSAlignAssignmentStatement") - Rules = @{ + Rules = @{ PSAlignAssignmentStatement = $ruleConfiguration } } @@ -26,18 +26,18 @@ $hashtable = @{ } '@ -# Expected output after correction should be the following -# $hashtable = @{ -# property1 = "value" -# anotherProperty = "another value" -# } + # Expected output after correction should be the following + # $hashtable = @{ + # property1 = "value" + # anotherProperty = "another value" + # } $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings $violations.Count | Should Be 1 Test-CorrectionExtentFromContent $def $violations 1 ' ' ' ' } - It "Should find violation when assignment statements are not aligned (whitespace needs to be removed)" { + It "Should find violation when assignment statements are not aligned (whitespace needs to be removed)" { $def = @' $hashtable = @{ property1 = "value" @@ -45,11 +45,11 @@ $hashtable = @{ } '@ -# Expected output should be the following -# $hashtable = @{ -# property1 = "value" -# anotherProperty = "another value" -# } + # Expected output should be the following + # $hashtable = @{ + # property1 = "value" + # anotherProperty = "another value" + # } $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings $violations.Count | Should Be 1 @@ -87,5 +87,37 @@ Configuration MyDscConfiguration { '@ Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Get-Count | Should Be 2 } + } + + Context "When assignment statements are in DSC Configuration that has parse errors" { + It "Sdhould find violations when assignment statements are not aligned" { + $def = @' +Configuration Sample_ChangeDescriptionAndPermissions +{ + Import-DscResource -Module NonExistentModule + # A Configuration block can have zero or more Node blocks + Node $NodeName + { + # Next, specify one or more resource blocks + + NonExistentModule MySMBShare + { + Ensure = "Present" + Name = "MyShare" + Path = "C:\Demo\Temp" + ReadAccess = "author" + FullAccess = "some other author" + Description = "This is an updated description for this share" + } + } } +'@ + # This invocation will throw parse error caused by "Undefined DSC resource" because + # NonExistentModule is not really avaiable to load. Therefore we set erroraction to + # SilentlyContinue + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue | + Get-Count | + Should Be 4 + } + } } From 07603d03a431711c4af4192884fe0fdb4da372fb Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 16:57:11 -0700 Subject: [PATCH 08/10] Update changelog --- CHANGELOG.MD | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index 2e9da62c2..ae21bc2f6 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -1,4 +1,9 @@ -## [1.12.0](https://github.com/PowerShell/PSScriptAnalyzer/tree/1.11.1) - 2017-05-09 +## [unreleased] + +## Fixed +- `PSAlignAssignmentStatement` to align assignment statements in DSC configurations that have *Undefined DSC Resource* parse errors. + +## [1.12.0](https://github.com/PowerShell/PSScriptAnalyzer/tree/1.11.1) - 2017-05-09 ### Added - [PSAlignAssignmentRuleStatement](https://github.com/PowerShell/PSScriptAnalyzer/blob/cca9a2d7ee35be7322f8c5a09b6c500a0a8bd101/RuleDocumentation/AlignAssignmentStatement.md) rule to align assignment statements in property value pairs (#753). From 25741c85b15a6e0c02d48fc85c4180385c961139 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 16:58:52 -0700 Subject: [PATCH 09/10] Remove unused method from AlignAssignmentStatement class --- Rules/AlignAssignmentStatement.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index 7c681d17b..006fa3fc7 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -335,23 +335,5 @@ private bool HasPropertiesOnSeparateLines(IEnumerable(); - foreach (var kvp in hashtableAst.KeyValuePairs) - { - if (lines.Contains(kvp.Item1.Extent.StartLineNumber)) - { - return false; - } - else - { - lines.Add(kvp.Item1.Extent.StartLineNumber); - } - } - - return true; - } } } From 3fcb22674bfda2a8bf34185cdf4a9f19dddc82ce Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Wed, 10 May 2017 17:40:44 -0700 Subject: [PATCH 10/10] Disable DSC Configuration fix on PSv3 and PSv4 --- Rules/AlignAssignmentStatement.cs | 5 ++--- .../Rules/AlignAssignmentStatement.tests.ps1 | 20 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index 006fa3fc7..37566862e 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -154,6 +154,7 @@ private IEnumerable FindHashtableViolations(TokenOperations to } } +#if !PSV3 var configAsts = tokenOps.Ast.FindAll(ast => ast is ConfigurationDefinitionAst, true); if (configAsts != null) { @@ -169,7 +170,7 @@ private IEnumerable FindHashtableViolations(TokenOperations to groups.AddRange(GetCommandElementExtentGroups(configAst)); } } - +#endif // it is probably much easier have a hashtable writer that formats the hashtable and writes it // but it makes handling comments hard. So we need to use this approach. @@ -183,8 +184,6 @@ private IEnumerable FindHashtableViolations(TokenOperations to // find the distance between the assignment operators and their corresponding LHS // find the longest left expression // make sure all the assignment operators are in the same column as that of the longest left hand. - - foreach (var extentTuples in groups) { if (!HasPropertiesOnSeparateLines(extentTuples)) diff --git a/Tests/Rules/AlignAssignmentStatement.tests.ps1 b/Tests/Rules/AlignAssignmentStatement.tests.ps1 index 5266969ec..6dd22304b 100644 --- a/Tests/Rules/AlignAssignmentStatement.tests.ps1 +++ b/Tests/Rules/AlignAssignmentStatement.tests.ps1 @@ -89,9 +89,10 @@ Configuration MyDscConfiguration { } } - Context "When assignment statements are in DSC Configuration that has parse errors" { - It "Sdhould find violations when assignment statements are not aligned" { - $def = @' + if ($PSVersionTable.PSVersion.Major -ge 5) { + Context "When assignment statements are in DSC Configuration that has parse errors" { + It "Should find violations when assignment statements are not aligned" { + $def = @' Configuration Sample_ChangeDescriptionAndPermissions { Import-DscResource -Module NonExistentModule @@ -112,12 +113,13 @@ Configuration Sample_ChangeDescriptionAndPermissions } } '@ - # This invocation will throw parse error caused by "Undefined DSC resource" because - # NonExistentModule is not really avaiable to load. Therefore we set erroraction to - # SilentlyContinue - Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue | - Get-Count | - Should Be 4 + # This invocation will throw parse error caused by "Undefined DSC resource" because + # NonExistentModule is not really avaiable to load. Therefore we set erroraction to + # SilentlyContinue + Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings -ErrorAction SilentlyContinue | + Get-Count | + Should Be 4 + } } } }