Skip to content

Update PlaceCloseBrace rule behavior for NewLineAfter switch #784

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 2 commits into from
Jun 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 29 additions & 23 deletions Rules/PlaceCloseBrace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
#endif
public class PlaceCloseBrace : ConfigurableRule
{
private HashSet<Token> tokensToIgnore;
private List<Func<Token[], int, int, string, DiagnosticRecord>> violationFinders
= new List<Func<Token[], int, int, string, DiagnosticRecord>>();

/// <summary>
/// Indicates if there should or should not be an empty line before a close brace.
///
Expand Down Expand Up @@ -57,7 +61,28 @@ public class PlaceCloseBrace : ConfigurableRule
[ConfigurableRuleProperty(defaultValue: true)]
public bool NewLineAfter { get; protected set; }

private HashSet<Token> tokensToIgnore;
/// <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);
violationFinders.Add(GetViolationForBraceShouldBeOnNewLine);
if (NoEmptyLineBefore)
{
violationFinders.Add(GetViolationForBraceShouldNotFollowEmptyLine);
}

if (NewLineAfter)
{
violationFinders.Add(GetViolationForBraceShouldHaveNewLineAfter);
}
else
{
violationFinders.Add(GetViolationsForUncuddledBranches);
}
}

/// <summary>
/// Analyzes the given ast to find violations.
Expand Down Expand Up @@ -121,27 +146,10 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
continue;
}

AddToDiagnosticRecords(
GetViolationForBraceShouldBeOnNewLine(tokens, k, openBracePos, fileName),
ref diagnosticRecords);

if (NoEmptyLineBefore)
{
AddToDiagnosticRecords(
GetViolationForBraceShouldNotFollowEmptyLine(tokens, k, openBracePos, fileName),
ref diagnosticRecords);
}

if (NewLineAfter)
{
AddToDiagnosticRecords(
GetViolationForBraceShouldHaveNewLineAfter(tokens, k, openBracePos, fileName),
ref diagnosticRecords);
}
else
foreach (var violationFinder in violationFinders)
{
AddToDiagnosticRecords(
GetViolationsForUncuddledBranches(tokens, k, openBracePos, fileName),
violationFinder(tokens, k, openBracePos, fileName),
ref diagnosticRecords);
}
}
Expand Down Expand Up @@ -320,9 +328,7 @@ private DiagnosticRecord GetViolationForBraceShouldHaveNewLineAfter(
if (tokens.Length > 1 && tokens.Length > expectedNewLinePos)
{
var closeBraceToken = tokens[closeBracePos];
if ((tokens[expectedNewLinePos].Kind == TokenKind.Else
|| tokens[expectedNewLinePos].Kind == TokenKind.ElseIf)
&& !tokensToIgnore.Contains(closeBraceToken))
if (!tokensToIgnore.Contains(closeBraceToken) && IsBranchingStatementToken(tokens[expectedNewLinePos]))
{
return new DiagnosticRecord(
GetError(Strings.PlaceCloseBraceErrorShouldFollowNewLine),
Expand Down
21 changes: 21 additions & 0 deletions Tests/Rules/PlaceCloseBrace.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,27 @@ if (Test-Path "blah") {
Test-CorrectionExtentFromContent @params
}

It "Should find a violation for a close brace followed by a catch statement" {
$def = @'
try {
"try"
} catch {
"catch"
}

'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should Be 1
$params = @{
RawContent = $def
DiagnosticRecord = $violations[0]
CorrectionsCount = 1
ViolationText = '}'
CorrectionText = '}' + [System.Environment]::NewLine
}
Test-CorrectionExtentFromContent @params

}
It "Should not find a violation for a close brace followed by a comma in an array expression" {
$def = @'
Some-Command -Param1 @{
Expand Down