Merge pull request #759 from PowerShell/kapilmb/fix-align-assignment-rule

Fix align assignment statements in DSC configurations

Author: Kapil Borle

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).

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);

Rules/AlignAssignmentStatement.cs

@@ -145,11 +145,33 @@ public override SourceType GetSourceType()
         private IEnumerable<DiagnosticRecord> FindHashtableViolations(TokenOperations tokenOps)
         {
             var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true);
-            if (hashtableAsts == null)
+            var groups = new List<List<Tuple<IScriptExtent, IScriptExtent>>>();
+            if (hashtableAsts != null)
             {
-                yield break;
+                foreach (var astItem in hashtableAsts)
+                {
+                    groups.Add(GetExtents(tokenOps, (HashtableAst)astItem));
+                }
             }
 
+#if !PSV3
+            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));
+                }
+            }
+#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.
 
@@ -162,16 +184,13 @@ private IEnumerable<DiagnosticRecord> 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 astItem in hashtableAsts)
+            foreach (var extentTuples in groups)
             {
-                var hashtableAst = (HashtableAst)astItem;
-                if (!HasKeysOnSeparateLines(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))
@@ -181,11 +200,12 @@ private IEnumerable<DiagnosticRecord> 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)
                 {
@@ -204,6 +224,53 @@ private IEnumerable<DiagnosticRecord> FindHashtableViolations(TokenOperations to
             }
         }
 
+        private List<List<Tuple<IScriptExtent, IScriptExtent>>> GetCommandElementExtentGroups(Ast configAst)
+        {
+            var result = new List<List<Tuple<IScriptExtent, IScriptExtent>>>();
+            var commandAstGroups = GetCommandElementGroups(configAst);
+            foreach (var commandAstGroup in commandAstGroups)
+            {
+                var list = new List<Tuple<IScriptExtent, IScriptExtent>>();
+                foreach (var commandAst in commandAstGroup)
+                {
+                    var elems = commandAst.CommandElements;
+                    list.Add(new Tuple<IScriptExtent, IScriptExtent>(elems[0].Extent, elems[1].Extent));
+                }
+
+                result.Add(list);
+            }
+
+            return result;
+        }
+
+        private List<List<CommandAst>> GetCommandElementGroups(Ast configAst)
+        {
+            var result = new List<List<CommandAst>>();
+            var astsFound = configAst.FindAll(ast => IsPropertyValueCommandAst(ast), true);
+            if (astsFound == null)
+            {
+                return result;
+            }
+
+            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());
+            }
+
+            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<CorrectionExtent> GetHashtableCorrections(
             Tuple<IScriptExtent, IScriptExtent> extentTuple,
             int expectedStartColumnNumber)
@@ -225,7 +292,7 @@ private string GetError()
             return String.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementError);
         }
 
-        private static IList<Tuple<IScriptExtent, IScriptExtent>> GetExtents(
+        private static List<Tuple<IScriptExtent, IScriptExtent>> GetExtents(
             TokenOperations tokenOps,
             HashtableAst hashtableAst)
         {
@@ -250,18 +317,18 @@ private static IList<Tuple<IScriptExtent, IScriptExtent>> GetExtents(
             return nodeTuples;
         }
 
-        private bool HasKeysOnSeparateLines(HashtableAst hashtableAst)
+        private bool HasPropertiesOnSeparateLines(IEnumerable<Tuple<IScriptExtent, IScriptExtent>> tuples)
         {
             var lines = new HashSet<int>();
-            foreach (var kvp in hashtableAst.KeyValuePairs)
+            foreach (var kvp in tuples)
             {
-                if (lines.Contains(kvp.Item1.Extent.StartLineNumber))
+                if (lines.Contains(kvp.Item1.StartLineNumber))
                 {
                     return false;
                 }
                 else
                 {
-                    lines.Add(kvp.Item1.Extent.StartLineNumber);
+                    lines.Add(kvp.Item1.StartLineNumber);
                 }
             }
 

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,30 +26,30 @@ $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"
     anotherProperty = "another value"
 }
 '@
 
-# 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,39 @@ Configuration MyDscConfiguration {
 '@
             Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Get-Count | Should Be 2
         }
+    }
+
+    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
+    # 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
+            }
+        }
+    }
 }