Skip to content

Add new line after close brace #713

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 9 commits into from
Feb 21, 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
15 changes: 15 additions & 0 deletions Engine/Generic/CorrectionExtent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,21 @@ public CorrectionExtent(

}

public CorrectionExtent(
IScriptExtent violationExtent,
string replacementText,
string filePath)
: this(
violationExtent.StartLineNumber,
violationExtent.EndLineNumber,
violationExtent.StartColumnNumber,
violationExtent.EndColumnNumber,
replacementText,
filePath)
{

}

private void ThrowIfInvalidArguments()
{
ThrowIfNull<string>(text, "text");
Expand Down
6 changes: 5 additions & 1 deletion RuleDocumentation/PlaceCloseBrace.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Close brace placement should follow a consistent style. It should be on a new li
Enable = $true
NoEmptyLineBefore = $false
IgnoreOneLineBlock = $true
NewLineAfter = $true
}
}
```
Expand All @@ -28,4 +29,7 @@ Create violation if there is an empty line before a close brace.
#### IgnoreOneLineBlock: bool (Default value is `$true`)
Indicates if close braces in a one line block should be ignored or not.
E.g. $x = if ($true) { "blah" } else { "blah blah" }
In the above example, if the property is set to true then the rule will not fire a violation.
In the above example, if the property is set to true then the rule will not fire a violation.

#### NewLineAfter: bool (Default value is `$true`)
Indicates if a new line should follow a close brace. If set to true a close brace should be followed by a new line.
72 changes: 69 additions & 3 deletions Rules/PlaceCloseBrace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class PlaceCloseBrace : ConfigurableRule
/// <summary>
/// Indicates if there should or should not be an empty line before a close brace.
///
/// Default value if false.
/// Default value is false.
/// </summary>
[ConfigurableRuleProperty(defaultValue:false)]
public bool NoEmptyLineBefore { get; protected set; }
Expand All @@ -42,11 +42,21 @@ public class PlaceCloseBrace : ConfigurableRule
/// In the above example, if the property is set to true then the rule will
/// not fire a violation.
///
/// Default value if true.
/// Default value is true.
/// </summary>
[ConfigurableRuleProperty(defaultValue: true)]
public bool IgnoreOneLineBlock { get; protected set; }

/// <summary>
/// Indicates if a new line should follow a close brace.
///
/// If set to true a close brace should be followed by a new line.
///
/// Default value is true.
/// </summary>
[ConfigurableRuleProperty(defaultValue: true)]
public bool NewLineAfter { get; protected set; }

private HashSet<Token> tokensToIgnore;

/// <summary>
Expand Down Expand Up @@ -121,6 +131,13 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
GetViolationForBraceShouldNotFollowEmptyLine(tokens, k, openBracePos, fileName),
ref diagnosticRecords);
}

if (NewLineAfter)
{
AddToDiagnosticRecords(
GetViolationForBraceShouldHaveNewLineAfter(tokens, k, openBracePos, fileName),
ref diagnosticRecords);
}
}
else
{
Expand Down Expand Up @@ -244,6 +261,22 @@ private List<CorrectionExtent> GetCorrectionsForBraceShouldNotFollowEmptyLine(
return corrections;
}

private List<CorrectionExtent> GetCorrectionsForBraceShouldHaveNewLineAfter(
Token[] tokens,
int closeBracePos,
int openBracePos,
string fileName)
{
var corrections = new List<CorrectionExtent>();
var nextToken = tokens[closeBracePos + 1];
var closeBraceToken = tokens[closeBracePos];
corrections.Add(new CorrectionExtent(
closeBraceToken.Extent,
closeBraceToken.Text + Environment.NewLine,
fileName));
return corrections;
}

private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePos)
{
// if open brace on a new line by itself, use its indentation
Expand Down Expand Up @@ -271,7 +304,40 @@ private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePo
return new String(' ', firstTokenOnOpenBraceLine.Extent.StartColumnNumber - 1);
}

private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine(Token[] tokens, int closeBracePos, int openBracePos, string fileName)
private DiagnosticRecord GetViolationForBraceShouldHaveNewLineAfter(
Token[] tokens,
int closeBracePos,
int openBracePos,
string fileName)
{
var expectedNewLinePos = closeBracePos + 1;
if (tokens.Length > 1 && tokens.Length > expectedNewLinePos)
{
var closeBraceToken = tokens[closeBracePos];
if (tokens[expectedNewLinePos].Kind != TokenKind.NewLine
&& tokens[expectedNewLinePos].Kind != TokenKind.Comment
&& !tokensToIgnore.Contains(tokens[closeBracePos]))
{

return new DiagnosticRecord(
GetError(Strings.PlaceCloseBraceErrorShouldFollowNewLine),
closeBraceToken.Extent,
GetName(),
GetDiagnosticSeverity(),
fileName,
null,
GetCorrectionsForBraceShouldHaveNewLineAfter(tokens, closeBracePos, openBracePos, fileName));
}
}

return null;
}

private DiagnosticRecord GetViolationForBraceShouldBeOnNewLine(
Token[] tokens,
int closeBracePos,
int openBracePos,
string fileName)
{
if (tokens.Length > 1 && tokens.Length > closeBracePos)
{
Expand Down
3 changes: 3 additions & 0 deletions Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,9 @@
<data name="PlaceCloseBraceErrorShouldNotFollowEmptyLine" xml:space="preserve">
<value>Close brace does not follow a non-empty line.</value>
</data>
<data name="PlaceCloseBraceErrorShouldFollowNewLine" xml:space="preserve">
<value>Close brace does not follow a new line.</value>
</data>
<data name="UseConsistentIndentationName" xml:space="preserve">
<value>UseConsistentIndentation</value>
</data>
Expand Down
57 changes: 56 additions & 1 deletion Tests/Rules/PlaceCloseBrace.tests.ps1
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
Import-Module PSScriptAnalyzer
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
$testRootDirectory = Split-Path -Parent $directory

Import-Module PSScriptAnalyzer
Import-Module (Join-Path $testRootDirectory "PSScriptAnalyzerTestHelper.psm1")

$ruleConfiguration = @{
Enable = $true
NoEmptyLineBefore = $true
IgnoreOneLineBlock = $true
NewLineAfter = $true
}

$settings = @{
Expand All @@ -19,6 +25,9 @@ Describe "PlaceCloseBrace" {
function foo {
Write-Output "close brace not on a new line"}
'@
$ruleConfiguration.'NoEmptyLineBefore' = $false
$ruleConfiguration.'IgnoreOneLineBlock' = $false
$ruleConfiguration.'NewLineAfter' = $false
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
}

Expand All @@ -39,6 +48,9 @@ function foo {

}
'@
$ruleConfiguration.'NoEmptyLineBefore' = $true
$ruleConfiguration.'IgnoreOneLineBlock' = $false
$ruleConfiguration.'NewLineAfter' = $false
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
}

Expand All @@ -56,6 +68,9 @@ function foo {
$def = @'
$hashtable = @{a = 1; b = 2}
'@
$ruleConfiguration.'NoEmptyLineBefore' = $false
$ruleConfiguration.'IgnoreOneLineBlock' = $true
$ruleConfiguration.'NewLineAfter' = $false
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
}

Expand All @@ -71,6 +86,9 @@ $hashtable = @{
a = 1
b = 2}
'@
$ruleConfiguration.'NoEmptyLineBefore' = $false
$ruleConfiguration.'IgnoreOneLineBlock' = $true
$ruleConfiguration.'NewLineAfter' = $false
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
}

Expand Down Expand Up @@ -101,4 +119,41 @@ $x = if ($true) { "blah" } else { "blah blah" }
$violations.Count | Should Be 0
}
}

Context "When a close brace should be follow a new line" {
BeforeAll {
$def = @'
if (Test-Path "blah") {
"blah"
} else {
"blah blah"
}
'@
$ruleConfiguration.'NoEmptyLineBefore' = $false
$ruleConfiguration.'IgnoreOneLineBlock' = $false
$ruleConfiguration.'NewLineAfter' = $true
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
}

It "Should find two violations" {
$violations.Count | Should Be 2
$params = @{
RawContent = $def
DiagnosticRecord = $violations[0]
CorrectionsCount = 1
ViolationText = '}'
CorrectionText = '}' + [System.Environment]::NewLine
}
Test-CorrectionExtentFromContent @params

$params = @{
RawContent = $def
DiagnosticRecord = $violations[1]
CorrectionsCount = 1
ViolationText = '}'
CorrectionText = '}' + [System.Environment]::NewLine
}
Test-CorrectionExtentFromContent @params
}
}
}