Skip to content

Fix align assignment statements in DSC configurations #759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 11, 2017
Merged
7 changes: 6 additions & 1 deletion CHANGELOG.MD
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
23 changes: 23 additions & 0 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
101 changes: 84 additions & 17 deletions Rules/AlignAssignmentStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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))
Expand All @@ -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)
{
Expand All @@ -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)
Expand All @@ -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)
{
Expand All @@ -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);
}
}

Expand Down
60 changes: 47 additions & 13 deletions Tests/Rules/AlignAssignmentStatement.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}
}
}
}