Add rule to align assignment statements in a hashtable

Engine/Settings/CodeFormatting.psd1

@@ -2,8 +2,9 @@
     IncludeRules = @(
         'PSPlaceOpenBrace',
         'PSPlaceCloseBrace',
+        'PSUseConsistentWhitespace',
         'PSUseConsistentIndentation',
-        'PSUseConsistentWhitespace'
+        'PSAlignAssignmentStatement'
     )
 
     Rules = @{
@@ -30,5 +31,10 @@
             CheckSeparator = $true
         }
 
+        PSAlignAssignmentStatement = @{
+            Enable = $true
+            CheckHashtable = $true
+            CheckDSCConfiguration = $true
+        }
     }
 }

Engine/TokenOperations.cs

@@ -272,6 +272,11 @@ public IEnumerable<Tuple<Token, int>> GetOpenParensWithWhiteSpacesBefore()
             return GetTokenAndPrecedingWhitespace(TokenKind.LParen);
         }
 
+        public static int GetExtentWidth(IScriptExtent extent)
+        {
+            return extent.EndOffset - extent.StartOffset;
+        }
+
         private bool OnSameLine(Token token1, Token token2)
         {
             return token1.Extent.StartLineNumber == token2.Extent.EndLineNumber;

RuleDocumentation/AlignAssignmentStatement.md

@@ -0,0 +1,48 @@
+# AlignAssignmentStatement
+
+**Severity Level: Warning**
+
+## Description
+
+Consecutive assignment statements are more readable if they are aligned. By aligned, we imply that the `equal` sign for all the assignment statements should be in the same column.
+
+The rule looks for key (property) value pairs in a hashtable (DSC configuration) to check if they are aligned or not. Consider the following example in which the key value pairs are not aligned.
+
+```powershell
+$hashtable = @{
+    property1 = "value"
+    anotherProperty = "another value"
+}
+```
+
+Alignment in this case would look like the following.
+
+```powershell
+$hashtable = @{
+    property1       = "value"
+    anotherProperty = "another value"
+}
+```
+
+The rule will ignore hashtables in which the assignment statements are on the same line. For example, the rule will ignore `$h = {a = 1; b = 2}`.
+
+## Configuration
+
+```powershell
+    Rules = @{
+        PSAlignAssignmentStatement = @{
+            Enable = $true
+            CheckHashtable = $true
+        }
+    }
+```
+
+### Parameters
+
+#### Enable: bool (Default value is `$false`)
+
+Enable or disable the rule during ScriptAnalyzer invocation.
+
+#### CheckHashtable: bool (Default value is `$false`)
+
+Enforce alignment of assignment statements in a hashtable and in a DSC Configuration. There is only one switch for hasthable and DSC configuration because the property value pairs in a DSC configuration are parsed as key value pairs of a hashtable.

Rules/AlignAssignmentStatement.cs

@@ -0,0 +1,272 @@
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+using System;
+using System.Collections.Generic;
+#if !CORECLR
+using System.ComponentModel.Composition;
+#endif
+using System.Globalization;
+using System.Linq;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// A class to walk an AST to check if consecutive assignment statements are aligned.
+    /// </summary>
+#if !CORECLR
+    [Export(typeof(IScriptRule))]
+#endif
+    class AlignAssignmentStatement : ConfigurableRule
+    {
+        // We keep this switch even though the rule has only one switch (this) as of now, because we want
+        // to let the rule be expandable in the future to allow formatting assignments even
+        // in variable assignments. But for now we will stick to only one option.
+        /// <summary>
+        /// Check if key value pairs in a hashtable are aligned or not.
+        /// </summary>
+        /// <returns></returns>
+        [ConfigurableRuleProperty(defaultValue: true)]
+        public bool CheckHashtable { get; set; }
+
+        private readonly char whitespaceChar = ' ';
+
+        private List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>> violationFinders
+            = new List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>>();
+
+        /// <summary>
+        /// Sets the configurable properties of this rule.
+        /// </summary>
+        /// <param name="paramValueMap">A dictionary that maps parameter name to it value. Must be non-null</param>
+        public override void ConfigureRule(IDictionary<string, object> paramValueMap)
+        {
+            base.ConfigureRule(paramValueMap);
+            if (CheckHashtable)
+            {
+                violationFinders.Add(FindHashtableViolations);
+            }
+        }
+
+
+        /// <summary>
+        /// Analyzes the given ast to find if consecutive assignment statements are aligned.
+        /// </summary>
+        /// <param name="ast">AST to be analyzed. This should be non-null</param>
+        /// <param name="fileName">Name of file that corresponds to the input AST.</param>
+        /// <returns>A an enumerable type containing the violations</returns>
+        public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null)
+            {
+                throw new ArgumentNullException("ast");
+            }
+            // only handles one line assignments
+            // if the rule encounters assignment statements that are multi-line, the rule will ignore that block
+
+            var tokenOps = new TokenOperations(Helper.Instance.Tokens, ast);
+            foreach (var violationFinder in violationFinders)
+            {
+                foreach (var diagnosticRecord in violationFinder(tokenOps))
+                {
+                    yield return diagnosticRecord;
+                }
+            }
+        }
+
+        /// <summary>
+        /// Retrieves the common name of this rule.
+        /// </summary>
+        public override string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementCommonName);
+        }
+
+        /// <summary>
+        /// Retrieves the description of this rule.
+        /// </summary>
+        public override string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementDescription);
+        }
+
+        /// <summary>
+        /// Retrieves the name of this rule.
+        /// </summary>
+        public override string GetName()
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.AlignAssignmentStatementName);
+        }
+
+        /// <summary>
+        /// Retrieves the severity of the rule: error, warning or information.
+        /// </summary>
+        public override RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Gets the severity of the returned diagnostic record: error, warning, or information.
+        /// </summary>
+        /// <returns></returns>
+        public DiagnosticSeverity GetDiagnosticSeverity()
+        {
+            return DiagnosticSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Retrieves the name of the module/assembly the rule is from.
+        /// </summary>
+        public override string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+
+        /// <summary>
+        /// Retrieves the type of the rule, Builtin, Managed or Module.
+        /// </summary>
+        public override SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+        private IEnumerable<DiagnosticRecord> FindHashtableViolations(TokenOperations tokenOps)
+        {
+            var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true);
+            if (hashtableAsts == null)
+            {
+                yield break;
+            }
+
+            // 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.
+
+            // This is how the algorithm actually works:
+            // if each key value pair are on a separate line
+            //   find all the assignment operators
+            //   if all the assignment operators are aligned (check the column number of each alignment operator)
+            //      skip
+            //   else
+            //      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.
+
+            var alignments = new List<int>();
+            foreach (var astItem in hashtableAsts)
+            {
+                var hashtableAst = (HashtableAst)astItem;
+                if (!HasKeysOnSeparateLines(hashtableAst))
+                {
+                    continue;
+                }
+
+                var extentTuples = GetExtents(tokenOps, hashtableAst);
+                if (extentTuples == null
+                    || extentTuples.Count == 0
+                    || !extentTuples.All(t => t.Item1.StartLineNumber == t.Item2.EndLineNumber))
+                {
+                    continue;
+                }
+
+                var widestKeyExtent = extentTuples
+                    .Select(t => t.Item1)
+                    .Aggregate((t1, tAggregate) => {
+                    return TokenOperations.GetExtentWidth(tAggregate) > TokenOperations.GetExtentWidth(t1)
+                        ? tAggregate
+                        : t1;
+                });
+                var expectedStartColumnNumber = widestKeyExtent.EndColumnNumber + 1;
+                foreach (var extentTuple in extentTuples)
+                {
+                    if (extentTuple.Item2.StartColumnNumber != expectedStartColumnNumber)
+                    {
+                        yield return new DiagnosticRecord(
+                            GetError(),
+                            extentTuple.Item2,
+                            GetName(),
+                            GetDiagnosticSeverity(),
+                            extentTuple.Item1.File,
+                            null,
+                            GetHashtableCorrections(extentTuple, expectedStartColumnNumber).ToList());
+                    }
+                }
+            }
+        }
+
+        private IEnumerable<CorrectionExtent> GetHashtableCorrections(
+            Tuple<IScriptExtent, IScriptExtent> extentTuple,
+            int expectedStartColumnNumber)
+        {
+            var equalExtent = extentTuple.Item2;
+            var lhsExtent = extentTuple.Item1;
+            var columnDiff = expectedStartColumnNumber - equalExtent.StartColumnNumber;
+            yield return new CorrectionExtent(
+                lhsExtent.EndLineNumber,
+                equalExtent.StartLineNumber,
+                lhsExtent.EndColumnNumber,
+                equalExtent.StartColumnNumber,
+                new String(whitespaceChar, expectedStartColumnNumber - lhsExtent.EndColumnNumber),
+                GetError());
+        }
+
+        private string GetError()
+        {
+            return String.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementError);
+        }
+
+        private static IList<Tuple<IScriptExtent, IScriptExtent>> GetExtents(
+            TokenOperations tokenOps,
+            HashtableAst hashtableAst)
+        {
+            var nodeTuples = new List<Tuple<IScriptExtent, IScriptExtent>>();
+            foreach (var kvp in hashtableAst.KeyValuePairs)
+            {
+                var keyStartOffset = kvp.Item1.Extent.StartOffset;
+                var keyTokenNode = tokenOps.GetTokenNodes(
+                    token => token.Extent.StartOffset == keyStartOffset).FirstOrDefault();
+                if (keyTokenNode == null
+                    || keyTokenNode.Next == null
+                    || keyTokenNode.Next.Value.Kind != TokenKind.Equals)
+                {
+                    return null;
+                }
+
+                nodeTuples.Add(new Tuple<IScriptExtent, IScriptExtent>(
+                    kvp.Item1.Extent,
+                    keyTokenNode.Next.Value.Extent));
+            }
+
+            return nodeTuples;
+        }
+
+        private bool HasKeysOnSeparateLines(HashtableAst hashtableAst)
+        {
+            var lines = new HashSet<int>();
+            foreach (var kvp in hashtableAst.KeyValuePairs)
+            {
+                if (lines.Contains(kvp.Item1.Extent.StartLineNumber))
+                {
+                    return false;
+                }
+                else
+                {
+                    lines.Add(kvp.Item1.Extent.StartLineNumber);
+                }
+            }
+
+            return true;
+        }
+    }
+}

Rules/ScriptAnalyzerBuiltinRules.csproj

@@ -121,6 +121,7 @@
     <Compile Include="PlaceCloseBrace.cs" />
     <Compile Include="UseConsistentIndentation.cs" />
     <Compile Include="UseConsistentWhitespace.cs" />
+    <Compile Include="AlignAssignmentStatement.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\Engine\ScriptAnalyzerEngine.csproj">

Rules/Strings.resx

@@ -942,4 +942,16 @@
   <data name="UseConsistentWhitespaceErrorSeparatorSemi" xml:space="preserve">
     <value>Use space after a semicolon.</value>
   </data>
+  <data name="AlignAssignmentStatementName" xml:space="preserve">
+    <value>AlignAssignmentStatement</value>
+  </data>
+  <data name="AlignAssignmentStatementCommonName" xml:space="preserve">
+    <value>Align assignment statement</value>
+  </data>
+  <data name="AlignAssignmentStatementDescription" xml:space="preserve">
+    <value>Line up assignment statements such that the assignment operator are aligned.</value>
+  </data>
+  <data name="AlignAssignmentStatementError" xml:space="preserve">
+    <value>Assignment statements are not aligned</value>
+  </data>
 </root>