Skip to content

Commit cca9a2d

Browse files
author
Kapil Borle
authored
Merge pull request #753 from PowerShell/kapilmb/align-assignments-rule
Add rule to align assignment statements in a hashtable
2 parents 42bbd98 + ab2e031 commit cca9a2d

8 files changed

+437
-3
lines changed

Engine/Settings/CodeFormatting.psd1

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
IncludeRules = @(
33
'PSPlaceOpenBrace',
44
'PSPlaceCloseBrace',
5+
'PSUseConsistentWhitespace',
56
'PSUseConsistentIndentation',
6-
'PSUseConsistentWhitespace'
7+
'PSAlignAssignmentStatement'
78
)
89

910
Rules = @{
@@ -30,5 +31,10 @@
3031
CheckSeparator = $true
3132
}
3233

34+
PSAlignAssignmentStatement = @{
35+
Enable = $true
36+
CheckHashtable = $true
37+
CheckDSCConfiguration = $true
38+
}
3339
}
3440
}

Engine/TokenOperations.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,11 @@ public IEnumerable<Tuple<Token, int>> GetOpenParensWithWhiteSpacesBefore()
272272
return GetTokenAndPrecedingWhitespace(TokenKind.LParen);
273273
}
274274

275+
public static int GetExtentWidth(IScriptExtent extent)
276+
{
277+
return extent.EndOffset - extent.StartOffset;
278+
}
279+
275280
private bool OnSameLine(Token token1, Token token2)
276281
{
277282
return token1.Extent.StartLineNumber == token2.Extent.EndLineNumber;
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# AlignAssignmentStatement
2+
3+
**Severity Level: Warning**
4+
5+
## Description
6+
7+
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.
8+
9+
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.
10+
11+
```powershell
12+
$hashtable = @{
13+
property1 = "value"
14+
anotherProperty = "another value"
15+
}
16+
```
17+
18+
Alignment in this case would look like the following.
19+
20+
```powershell
21+
$hashtable = @{
22+
property1 = "value"
23+
anotherProperty = "another value"
24+
}
25+
```
26+
27+
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}`.
28+
29+
## Configuration
30+
31+
```powershell
32+
Rules = @{
33+
PSAlignAssignmentStatement = @{
34+
Enable = $true
35+
CheckHashtable = $true
36+
}
37+
}
38+
```
39+
40+
### Parameters
41+
42+
#### Enable: bool (Default value is `$false`)
43+
44+
Enable or disable the rule during ScriptAnalyzer invocation.
45+
46+
#### CheckHashtable: bool (Default value is `$false`)
47+
48+
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

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
// Copyright (c) Microsoft Corporation.
2+
//
3+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
4+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
5+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
6+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
7+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
8+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
9+
// THE SOFTWARE.
10+
11+
using System;
12+
using System.Collections.Generic;
13+
#if !CORECLR
14+
using System.ComponentModel.Composition;
15+
#endif
16+
using System.Globalization;
17+
using System.Linq;
18+
using System.Management.Automation.Language;
19+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
20+
21+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
22+
{
23+
/// <summary>
24+
/// A class to walk an AST to check if consecutive assignment statements are aligned.
25+
/// </summary>
26+
#if !CORECLR
27+
[Export(typeof(IScriptRule))]
28+
#endif
29+
class AlignAssignmentStatement : ConfigurableRule
30+
{
31+
// We keep this switch even though the rule has only one switch (this) as of now, because we want
32+
// to let the rule be expandable in the future to allow formatting assignments even
33+
// in variable assignments. But for now we will stick to only one option.
34+
/// <summary>
35+
/// Check if key value pairs in a hashtable are aligned or not.
36+
/// </summary>
37+
/// <returns></returns>
38+
[ConfigurableRuleProperty(defaultValue: true)]
39+
public bool CheckHashtable { get; set; }
40+
41+
private readonly char whitespaceChar = ' ';
42+
43+
private List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>> violationFinders
44+
= new List<Func<TokenOperations, IEnumerable<DiagnosticRecord>>>();
45+
46+
/// <summary>
47+
/// Sets the configurable properties of this rule.
48+
/// </summary>
49+
/// <param name="paramValueMap">A dictionary that maps parameter name to it value. Must be non-null</param>
50+
public override void ConfigureRule(IDictionary<string, object> paramValueMap)
51+
{
52+
base.ConfigureRule(paramValueMap);
53+
if (CheckHashtable)
54+
{
55+
violationFinders.Add(FindHashtableViolations);
56+
}
57+
}
58+
59+
60+
/// <summary>
61+
/// Analyzes the given ast to find if consecutive assignment statements are aligned.
62+
/// </summary>
63+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
64+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
65+
/// <returns>A an enumerable type containing the violations</returns>
66+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
67+
{
68+
if (ast == null)
69+
{
70+
throw new ArgumentNullException("ast");
71+
}
72+
// only handles one line assignments
73+
// if the rule encounters assignment statements that are multi-line, the rule will ignore that block
74+
75+
var tokenOps = new TokenOperations(Helper.Instance.Tokens, ast);
76+
foreach (var violationFinder in violationFinders)
77+
{
78+
foreach (var diagnosticRecord in violationFinder(tokenOps))
79+
{
80+
yield return diagnosticRecord;
81+
}
82+
}
83+
}
84+
85+
/// <summary>
86+
/// Retrieves the common name of this rule.
87+
/// </summary>
88+
public override string GetCommonName()
89+
{
90+
return string.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementCommonName);
91+
}
92+
93+
/// <summary>
94+
/// Retrieves the description of this rule.
95+
/// </summary>
96+
public override string GetDescription()
97+
{
98+
return string.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementDescription);
99+
}
100+
101+
/// <summary>
102+
/// Retrieves the name of this rule.
103+
/// </summary>
104+
public override string GetName()
105+
{
106+
return string.Format(
107+
CultureInfo.CurrentCulture,
108+
Strings.NameSpaceFormat,
109+
GetSourceName(),
110+
Strings.AlignAssignmentStatementName);
111+
}
112+
113+
/// <summary>
114+
/// Retrieves the severity of the rule: error, warning or information.
115+
/// </summary>
116+
public override RuleSeverity GetSeverity()
117+
{
118+
return RuleSeverity.Warning;
119+
}
120+
121+
/// <summary>
122+
/// Gets the severity of the returned diagnostic record: error, warning, or information.
123+
/// </summary>
124+
/// <returns></returns>
125+
public DiagnosticSeverity GetDiagnosticSeverity()
126+
{
127+
return DiagnosticSeverity.Warning;
128+
}
129+
130+
/// <summary>
131+
/// Retrieves the name of the module/assembly the rule is from.
132+
/// </summary>
133+
public override string GetSourceName()
134+
{
135+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
136+
}
137+
138+
/// <summary>
139+
/// Retrieves the type of the rule, Builtin, Managed or Module.
140+
/// </summary>
141+
public override SourceType GetSourceType()
142+
{
143+
return SourceType.Builtin;
144+
}
145+
private IEnumerable<DiagnosticRecord> FindHashtableViolations(TokenOperations tokenOps)
146+
{
147+
var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true);
148+
if (hashtableAsts == null)
149+
{
150+
yield break;
151+
}
152+
153+
// it is probably much easier have a hashtable writer that formats the hashtable and writes it
154+
// but it makes handling comments hard. So we need to use this approach.
155+
156+
// This is how the algorithm actually works:
157+
// if each key value pair are on a separate line
158+
// find all the assignment operators
159+
// if all the assignment operators are aligned (check the column number of each assignment operator)
160+
// skip
161+
// else
162+
// find the distance between the assignment operators and their corresponding LHS
163+
// find the longest left expression
164+
// make sure all the assignment operators are in the same column as that of the longest left hand.
165+
166+
foreach (var astItem in hashtableAsts)
167+
{
168+
var hashtableAst = (HashtableAst)astItem;
169+
if (!HasKeysOnSeparateLines(hashtableAst))
170+
{
171+
continue;
172+
}
173+
174+
var extentTuples = GetExtents(tokenOps, hashtableAst);
175+
if (extentTuples == null
176+
|| extentTuples.Count == 0
177+
|| !extentTuples.All(t => t.Item1.StartLineNumber == t.Item2.EndLineNumber))
178+
{
179+
continue;
180+
}
181+
182+
var widestKeyExtent = extentTuples
183+
.Select(t => t.Item1)
184+
.Aggregate((t1, tAggregate) => {
185+
return TokenOperations.GetExtentWidth(tAggregate) > TokenOperations.GetExtentWidth(t1)
186+
? tAggregate
187+
: t1;
188+
});
189+
var expectedStartColumnNumber = widestKeyExtent.EndColumnNumber + 1;
190+
foreach (var extentTuple in extentTuples)
191+
{
192+
if (extentTuple.Item2.StartColumnNumber != expectedStartColumnNumber)
193+
{
194+
yield return new DiagnosticRecord(
195+
GetError(),
196+
extentTuple.Item2,
197+
GetName(),
198+
GetDiagnosticSeverity(),
199+
extentTuple.Item1.File,
200+
null,
201+
GetHashtableCorrections(extentTuple, expectedStartColumnNumber).ToList());
202+
}
203+
}
204+
}
205+
}
206+
207+
private IEnumerable<CorrectionExtent> GetHashtableCorrections(
208+
Tuple<IScriptExtent, IScriptExtent> extentTuple,
209+
int expectedStartColumnNumber)
210+
{
211+
var equalExtent = extentTuple.Item2;
212+
var lhsExtent = extentTuple.Item1;
213+
var columnDiff = expectedStartColumnNumber - equalExtent.StartColumnNumber;
214+
yield return new CorrectionExtent(
215+
lhsExtent.EndLineNumber,
216+
equalExtent.StartLineNumber,
217+
lhsExtent.EndColumnNumber,
218+
equalExtent.StartColumnNumber,
219+
new String(whitespaceChar, expectedStartColumnNumber - lhsExtent.EndColumnNumber),
220+
GetError());
221+
}
222+
223+
private string GetError()
224+
{
225+
return String.Format(CultureInfo.CurrentCulture, Strings.AlignAssignmentStatementError);
226+
}
227+
228+
private static IList<Tuple<IScriptExtent, IScriptExtent>> GetExtents(
229+
TokenOperations tokenOps,
230+
HashtableAst hashtableAst)
231+
{
232+
var nodeTuples = new List<Tuple<IScriptExtent, IScriptExtent>>();
233+
foreach (var kvp in hashtableAst.KeyValuePairs)
234+
{
235+
var keyStartOffset = kvp.Item1.Extent.StartOffset;
236+
var keyTokenNode = tokenOps.GetTokenNodes(
237+
token => token.Extent.StartOffset == keyStartOffset).FirstOrDefault();
238+
if (keyTokenNode == null
239+
|| keyTokenNode.Next == null
240+
|| keyTokenNode.Next.Value.Kind != TokenKind.Equals)
241+
{
242+
return null;
243+
}
244+
245+
nodeTuples.Add(new Tuple<IScriptExtent, IScriptExtent>(
246+
kvp.Item1.Extent,
247+
keyTokenNode.Next.Value.Extent));
248+
}
249+
250+
return nodeTuples;
251+
}
252+
253+
private bool HasKeysOnSeparateLines(HashtableAst hashtableAst)
254+
{
255+
var lines = new HashSet<int>();
256+
foreach (var kvp in hashtableAst.KeyValuePairs)
257+
{
258+
if (lines.Contains(kvp.Item1.Extent.StartLineNumber))
259+
{
260+
return false;
261+
}
262+
else
263+
{
264+
lines.Add(kvp.Item1.Extent.StartLineNumber);
265+
}
266+
}
267+
268+
return true;
269+
}
270+
}
271+
}

Rules/ScriptAnalyzerBuiltinRules.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@
121121
<Compile Include="PlaceCloseBrace.cs" />
122122
<Compile Include="UseConsistentIndentation.cs" />
123123
<Compile Include="UseConsistentWhitespace.cs" />
124+
<Compile Include="AlignAssignmentStatement.cs" />
124125
</ItemGroup>
125126
<ItemGroup>
126127
<ProjectReference Include="..\Engine\ScriptAnalyzerEngine.csproj">

Rules/Strings.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -942,4 +942,16 @@
942942
<data name="UseConsistentWhitespaceErrorSeparatorSemi" xml:space="preserve">
943943
<value>Use space after a semicolon.</value>
944944
</data>
945+
<data name="AlignAssignmentStatementName" xml:space="preserve">
946+
<value>AlignAssignmentStatement</value>
947+
</data>
948+
<data name="AlignAssignmentStatementCommonName" xml:space="preserve">
949+
<value>Align assignment statement</value>
950+
</data>
951+
<data name="AlignAssignmentStatementDescription" xml:space="preserve">
952+
<value>Line up assignment statements such that the assignment operator are aligned.</value>
953+
</data>
954+
<data name="AlignAssignmentStatementError" xml:space="preserve">
955+
<value>Assignment statements are not aligned</value>
956+
</data>
945957
</root>

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Describe "Test Name parameters" {
6161

6262
It "get Rules with no parameters supplied" {
6363
$defaultRules = Get-ScriptAnalyzerRule
64-
$expectedNumRules = 49
64+
$expectedNumRules = 50
6565
if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4))
6666
{
6767
# for PSv3 PSAvoidGlobalAliases is not shipped because
@@ -70,7 +70,7 @@ Describe "Test Name parameters" {
7070
# for PowerShell Core, PSUseSingularNouns is not
7171
# shipped because it uses APIs that are not present
7272
# in dotnet core.
73-
$expectedNumRules = 48
73+
$expectedNumRules = 49
7474
}
7575
$defaultRules.Count | Should be $expectedNumRules
7676
}

0 commit comments

Comments
 (0)