Skip to content

Commit 07da3ae

Browse files
Copilotbergmeister
Copilot
and
bergmeister
authored
Fix PlaceOpenBrace/PlaceCloseBrace corrections leaving stray whitespace and wrong indentation
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
1 parent 70c9c34 commit 07da3ae

4 files changed

Lines changed: 114 additions & 9 deletions

File tree

‎Rules/PlaceCloseBrace.cs‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,15 @@ private List<CorrectionExtent> GetCorrectionsForBraceShouldHaveNewLineAfter(
277277
var corrections = new List<CorrectionExtent>();
278278
var nextToken = tokens[closeBracePos + 1];
279279
var closeBraceToken = tokens[closeBracePos];
280+
281+
// Also remove the whitespace between the close brace and the next token
282+
// and indent the next token to the same level as the close brace
280283
corrections.Add(new CorrectionExtent(
281-
closeBraceToken.Extent,
282-
closeBraceToken.Text + Environment.NewLine,
284+
closeBraceToken.Extent.StartLineNumber,
285+
nextToken.Extent.StartLineNumber,
286+
closeBraceToken.Extent.StartColumnNumber,
287+
nextToken.Extent.StartColumnNumber,
288+
closeBraceToken.Text + Environment.NewLine + GetIndentation(tokens, closeBracePos, openBracePos),
283289
fileName));
284290
return corrections;
285291
}
@@ -296,7 +302,7 @@ private string GetIndentation(Token[] tokens, int closeBracePos, int openBracePo
296302
// if open brace follows any keywords use the identation of the first keyword
297303
// on the line containing the open brace
298304
Token firstTokenOnOpenBraceLine = openBraceToken;
299-
for (int k = openBracePos; k > 0; --k)
305+
for (int k = openBracePos; k >= 0; --k)
300306
{
301307
if (tokens[k].Extent.StartLineNumber == firstTokenOnOpenBraceLine.Extent.StartLineNumber)
302308
{
@@ -433,10 +439,21 @@ private List<CorrectionExtent> GetCorrectionsForBraceShouldBeOnNewLine(
433439
{
434440
var corrections = new List<CorrectionExtent>();
435441
var closeBraceToken = tokens[closeBracePos];
442+
443+
// Also remove any whitespace between the preceding token and the close brace
444+
// so that the preceding line does not end with a trailing whitespace
445+
var startLineNumber = closeBraceToken.Extent.StartLineNumber;
446+
var startColumnNumber = closeBraceToken.Extent.StartColumnNumber;
447+
var precedingTokenExtent = tokens[closeBracePos - 1].Extent;
448+
if (precedingTokenExtent.EndLineNumber == startLineNumber)
449+
{
450+
startColumnNumber = precedingTokenExtent.EndColumnNumber;
451+
}
452+
436453
corrections.Add(new CorrectionExtent(
437-
closeBraceToken.Extent.StartLineNumber,
454+
startLineNumber,
438455
closeBraceToken.Extent.EndLineNumber,
439-
closeBraceToken.Extent.StartColumnNumber,
456+
startColumnNumber,
440457
closeBraceToken.Extent.EndColumnNumber,
441458
Environment.NewLine + GetIndentation(tokens, closeBracePos, openBracePos) + closeBraceToken.Extent.Text,
442459
fileName));

‎Rules/PlaceOpenBrace.cs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,12 +288,24 @@ private List<CorrectionExtent> GetCorrectionsForNoNewLineAfterBrace(
288288
var corrections = new List<CorrectionExtent>();
289289
var extent = tokens[openBracePos].Extent;
290290

291+
// Also remove any whitespace between the open brace and the next token on the same line
292+
// so that the next token does not start with a leading whitespace on the new line
293+
var endColumnNumber = extent.EndColumnNumber;
294+
if (openBracePos + 1 < tokens.Length)
295+
{
296+
var nextTokenExtent = tokens[openBracePos + 1].Extent;
297+
if (nextTokenExtent.StartLineNumber == extent.EndLineNumber)
298+
{
299+
endColumnNumber = nextTokenExtent.StartColumnNumber;
300+
}
301+
}
302+
291303
corrections.Add(
292304
new CorrectionExtent(
293305
extent.StartLineNumber,
294306
extent.EndLineNumber,
295307
extent.StartColumnNumber,
296-
extent.EndColumnNumber,
308+
endColumnNumber,
297309
new StringBuilder().Append(extent.Text).Append(Environment.NewLine).ToString(),
298310
fileName));
299311
return corrections;

‎Tests/Rules/PlaceCloseBrace.tests.ps1‎

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ if (Test-Path "blah") {
174174
RawContent = $def
175175
DiagnosticRecord = $violations[0]
176176
CorrectionsCount = 1
177-
ViolationText = '}'
177+
ViolationText = '} '
178178
CorrectionText = '}' + [System.Environment]::NewLine
179179
}
180180
Test-CorrectionExtentFromContent @params
@@ -194,7 +194,7 @@ if (Test-Path "blah") {
194194
RawContent = $def
195195
DiagnosticRecord = $violations[0]
196196
CorrectionsCount = 1
197-
ViolationText = '}'
197+
ViolationText = '} '
198198
CorrectionText = '}' + [System.Environment]::NewLine
199199
}
200200
Test-CorrectionExtentFromContent @params
@@ -215,7 +215,7 @@ try {
215215
RawContent = $def
216216
DiagnosticRecord = $violations[0]
217217
CorrectionsCount = 1
218-
ViolationText = '}'
218+
ViolationText = '} '
219219
CorrectionText = '}' + [System.Environment]::NewLine
220220
}
221221
Test-CorrectionExtentFromContent @params
@@ -428,4 +428,71 @@ else {
428428
Invoke-Formatter -ScriptDefinition $OTBSDefinition -Settings 'CodeFormattingStroustrup' | Should -Be $StroustrupDefinition
429429
}
430430
}
431+
432+
Context "When formatting a one line if/else statement with IgnoreOneLineBlock set to false" {
433+
BeforeAll {
434+
$oneLineDefinition = "if (`$RegEx) { '-match' } else { '-like' }"
435+
}
436+
437+
It "Should not leave stray whitespace or misplace the close braces" {
438+
$formatterSettings = @{
439+
Rules = @{
440+
PSPlaceOpenBrace = @{ Enable = $true; IgnoreOneLineBlock = $false }
441+
PSPlaceCloseBrace = @{ Enable = $true; IgnoreOneLineBlock = $false }
442+
}
443+
}
444+
$expected = @"
445+
if (`$RegEx) {
446+
'-match'
447+
}
448+
else {
449+
'-like'
450+
}
451+
"@
452+
Invoke-Formatter -ScriptDefinition $oneLineDefinition -Settings $formatterSettings | Should -Be $expected
453+
}
454+
455+
It "Should indent the close brace to the level of its statement" {
456+
$formatterSettings = @{
457+
Rules = @{
458+
PSPlaceOpenBrace = @{ Enable = $true; IgnoreOneLineBlock = $false }
459+
PSPlaceCloseBrace = @{ Enable = $true; IgnoreOneLineBlock = $false }
460+
}
461+
}
462+
$def = @"
463+
function foo {
464+
if (`$RegEx) {
465+
'-match'
466+
} else {
467+
'-like'
468+
}
469+
}
470+
"@
471+
$expected = @"
472+
function foo {
473+
if (`$RegEx) {
474+
'-match'
475+
}
476+
else {
477+
'-like'
478+
}
479+
}
480+
"@
481+
Invoke-Formatter -ScriptDefinition $def -Settings $formatterSettings | Should -Be $expected
482+
}
483+
484+
It "Should format to OTBS style" {
485+
$formatterSettings = Import-PowerShellDataFile (Join-Path (Get-Module PSScriptAnalyzer).ModuleBase 'Settings/CodeFormattingOTBS.psd1')
486+
$formatterSettings.Rules.PSPlaceOpenBrace.IgnoreOneLineBlock = $false
487+
$formatterSettings.Rules.PSPlaceCloseBrace.IgnoreOneLineBlock = $false
488+
$expected = @"
489+
if (`$RegEx) {
490+
'-match'
491+
} else {
492+
'-like'
493+
}
494+
"@
495+
Invoke-Formatter -ScriptDefinition $oneLineDefinition -Settings $formatterSettings | Should -Be $expected
496+
}
497+
}
431498
}

‎Tests/Rules/PlaceOpenBrace.tests.ps1‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,5 +168,14 @@ function foo { }
168168
It "Should mark only the open brace" {
169169
$violations[0].Extent.Text | Should -Be '{'
170170
}
171+
172+
It "Should remove the whitespace following the open brace when correcting" {
173+
$def = "if (`$true) { 'blah' }"
174+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
175+
$violations.Count | Should -Be 1
176+
$violations[0].SuggestedCorrections[0].Text | Should -Be ('{' + [System.Environment]::NewLine)
177+
$violations[0].SuggestedCorrections[0].StartColumnNumber | Should -Be 12
178+
$violations[0].SuggestedCorrections[0].EndColumnNumber | Should -Be 14
179+
}
171180
}
172181
}

0 commit comments

Comments
 (0)