Skip to content

Commit 850fae4

Browse files
andyleejordanandyleejordan
authored
Merge branch 'main' into test-noprofile
2 parents 38b9e3e + 9fa10d4 commit 850fae4

7 files changed

+113
-20
lines changed

‎Engine/Commands/InvokeScriptAnalyzerCommand.cs‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -434,29 +434,39 @@ private void ProcessInput()
434434
WriteToOutput(RunAnalysis());
435435
}
436436

437-
private List<DiagnosticRecord> RunAnalysis()
437+
private IEnumerable<DiagnosticRecord> RunAnalysis()
438438
{
439439
if (!IsFileParameterSet())
440440
{
441-
return ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _);
441+
foreach (var record in ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _))
442+
{
443+
yield return record;
444+
}
445+
yield break;
442446
}
443447

444-
var diagnostics = new List<DiagnosticRecord>();
445-
foreach (string path in this.processedPaths)
448+
foreach (var path in this.processedPaths)
446449
{
450+
if (!ShouldProcess(path, $"Analyzing path with Fix={this.fix} and Recurse={this.recurse}"))
451+
{
452+
continue;
453+
}
454+
447455
if (fix)
448456
{
449-
ShouldProcess(path, $"Analyzing and fixing path with Recurse={this.recurse}");
450-
diagnostics.AddRange(ScriptAnalyzer.Instance.AnalyzeAndFixPath(path, this.ShouldProcess, this.recurse));
457+
foreach (var record in ScriptAnalyzer.Instance.AnalyzeAndFixPath(path, this.ShouldProcess, this.recurse))
458+
{
459+
yield return record;
460+
}
451461
}
452462
else
453463
{
454-
ShouldProcess(path, $"Analyzing path with Recurse={this.recurse}");
455-
diagnostics.AddRange(ScriptAnalyzer.Instance.AnalyzePath(path, this.ShouldProcess, this.recurse));
464+
foreach (var record in ScriptAnalyzer.Instance.AnalyzePath(path, this.ShouldProcess, this.recurse))
465+
{
466+
yield return record;
467+
}
456468
}
457469
}
458-
459-
return diagnostics;
460470
}
461471

462472
private void WriteToOutput(IEnumerable<DiagnosticRecord> diagnosticRecords)

‎Rules/AvoidAssignmentToAutomaticVariable.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,31 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
7979
}
8080
}
8181

82+
IEnumerable<Ast> forEachStatementAsts = ast.FindAll(testAst => testAst is ForEachStatementAst, searchNestedScriptBlocks: true);
83+
foreach (ForEachStatementAst forEachStatementAst in forEachStatementAsts)
84+
{
85+
var variableExpressionAst = forEachStatementAst.Variable;
86+
var variableName = variableExpressionAst.VariablePath.UserPath;
87+
if (_readOnlyAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase))
88+
{
89+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableError, variableName),
90+
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Error, fileName, variableName);
91+
}
92+
93+
if (_readOnlyAutomaticVariablesIntroducedInVersion6_0.Contains(variableName, StringComparer.OrdinalIgnoreCase))
94+
{
95+
var severity = IsPowerShellVersion6OrGreater() ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning;
96+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToReadOnlyAutomaticVariableIntroducedInPowerShell6_0Error, variableName),
97+
variableExpressionAst.Extent, GetName(), severity, fileName, variableName);
98+
}
99+
100+
if (_writableAutomaticVariables.Contains(variableName, StringComparer.OrdinalIgnoreCase))
101+
{
102+
yield return new DiagnosticRecord(DiagnosticRecordHelper.FormatError(Strings.AvoidAssignmentToWritableAutomaticVariableError, variableName),
103+
variableExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, variableName);
104+
}
105+
}
106+
82107
IEnumerable<Ast> parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true);
83108
foreach (ParameterAst parameterAst in parameterAsts)
84109
{

‎Rules/UseConsistentIndentation.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
163163
break;
164164

165165
case TokenKind.LParen:
166+
AddViolation(token, indentationLevel, diagnosticRecords, ref onNewLine);
166167
// When a line starts with a parenthesis and it is not the last non-comment token of that line,
167168
// then indentation does not need to be increased.
168169
if ((tokenIndex == 0 || tokens[tokenIndex - 1].Kind == TokenKind.NewLine) &&
@@ -173,7 +174,7 @@ caused by tokens that require a closing RParen (which are LParen, AtParen and Do
173174
break;
174175
}
175176
lParenSkippedIndentation.Push(false);
176-
AddViolation(token, indentationLevel++, diagnosticRecords, ref onNewLine);
177+
indentationLevel++;
177178
break;
178179

179180
case TokenKind.Pipe:

‎Rules/UseConsistentWhitespace.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,8 @@ private IEnumerable<DiagnosticRecord> FindParameterViolations(Ast ast)
421421
{
422422
int numberOfRedundantWhiteSpaces = rightExtent.StartColumnNumber - expectedStartColumnNumberOfRightExtent;
423423
var correction = new CorrectionExtent(
424-
startLineNumber: leftExtent.StartLineNumber,
425-
endLineNumber: leftExtent.EndLineNumber,
424+
startLineNumber: leftExtent.EndLineNumber,
425+
endLineNumber: rightExtent.StartLineNumber,
426426
startColumnNumber: leftExtent.EndColumnNumber + 1,
427427
endColumnNumber: leftExtent.EndColumnNumber + 1 + numberOfRedundantWhiteSpaces,
428428
text: string.Empty,

‎Tests/Rules/AvoidAssignmentToAutomaticVariable.tests.ps1‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,22 @@ Describe "AvoidAssignmentToAutomaticVariables" {
6565
It "Variable <VariableName> produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_AutomaticVariables {
6666
param ($VariableName, $ExpectedSeverity)
6767

68-
$warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" -ExcludeRule PSUseDeclaredVarsMoreThanAssignments
68+
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "`$${VariableName} = 'foo'" -ExcludeRule PSUseDeclaredVarsMoreThanAssignments
6969
$warnings.Count | Should -Be 1
7070
$warnings.Severity | Should -Be $ExpectedSeverity
7171
$warnings.RuleName | Should -Be $ruleName
7272
}
7373

74-
It "Using Variable <VariableName> as parameter name produces warning of Severity error" -TestCases $testCases_AutomaticVariables {
74+
It "Using Variable <VariableName> as foreach assignment produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_AutomaticVariables {
75+
param ($VariableName, $ExpectedSeverity)
76+
77+
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "foreach (`$$VariableName in `$foo) {}"
78+
$warnings.Count | Should -Be 1
79+
$warnings.Severity | Should -Be $ExpectedSeverity
80+
$warnings.RuleName | Should -Be $ruleName
81+
}
82+
83+
It "Using Variable <VariableName> as parameter name produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_AutomaticVariables {
7584
param ($VariableName, $ExpectedSeverity)
7685

7786
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo{Param(`$$VariableName)}" -ExcludeRule PSReviewUnusedParameter
@@ -80,7 +89,7 @@ Describe "AvoidAssignmentToAutomaticVariables" {
8089
$warnings.RuleName | Should -Be $ruleName
8190
}
8291

83-
It "Using Variable <VariableName> as parameter name in param block produces warning of Severity error" -TestCases $testCases_AutomaticVariables {
92+
It "Using Variable <VariableName> as parameter name in param block produces warning of Severity <ExpectedSeverity>" -TestCases $testCases_AutomaticVariables {
8493
param ($VariableName, $ExpectedSeverity)
8594

8695
[System.Array] $warnings = Invoke-ScriptAnalyzer -ScriptDefinition "function foo(`$$VariableName){}"

‎Tests/Rules/UseConsistentIndentation.tests.ps1‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,17 @@ Describe "UseConsistentIndentation" {
1111
function Invoke-FormatterAssertion {
1212
param(
1313
[string] $ScriptDefinition,
14-
[string] $ExcpectedScriptDefinition,
14+
[string] $ExpectedScriptDefinition,
1515
[int] $NumberOfExpectedWarnings,
1616
[hashtable] $Settings
1717
)
1818

1919
# Unit test just using this rule only
2020
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings
2121
$violations.Count | Should -Be $NumberOfExpectedWarnings -Because $ScriptDefinition
22-
Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $expected -Because $ScriptDefinition
22+
Invoke-Formatter -ScriptDefinition $scriptDefinition -Settings $settings | Should -Be $ExpectedScriptDefinition -Because $ScriptDefinition
2323
# Integration test with all default formatting rules
24-
Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $expected -Because $ScriptDefinition
24+
Invoke-Formatter -ScriptDefinition $scriptDefinition | Should -Be $ExpectedScriptDefinition -Because $ScriptDefinition
2525
}
2626
}
2727
BeforeEach {
@@ -177,6 +177,18 @@ function test {
177177
'@
178178
Invoke-Formatter -ScriptDefinition $IdempotentScriptDefinition | Should -Be $idempotentScriptDefinition
179179
}
180+
181+
It 'Should find violation in script when LParen is first token on a line and is not followed by Newline' {
182+
$ScriptDefinition = @'
183+
(foo)
184+
(bar)
185+
'@
186+
$ExpectedScriptDefinition = @'
187+
(foo)
188+
(bar)
189+
'@
190+
Invoke-FormatterAssertion $ScriptDefinition $ExpectedScriptDefinition 2 $settings
191+
}
180192
}
181193

182194
Context "When a sub-expression is provided" {

‎Tests/Rules/UseConsistentWhitespace.tests.ps1‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ bar -h i `
535535
Invoke-ScriptAnalyzer -ScriptDefinition "$def" -Settings $settings | Should -Be $null
536536
}
537537

538-
It "Should not find no violation if there is always 1 space between parameters except when using colon syntax" {
538+
It "Should not find a violation if there is always 1 space between parameters except when using colon syntax" {
539539
$def = 'foo -bar $baz @splattedVariable -bat -parameterName:$parameterValue'
540540
Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings | Should -Be $null
541541
}
@@ -585,6 +585,42 @@ bar -h i `
585585
Should -Be "$expected"
586586
}
587587

588+
It "Should fix script when a parameter value is a script block spanning multiple lines" {
589+
$def = {foo {
590+
bar
591+
} -baz}
592+
593+
$expected = {foo {
594+
bar
595+
} -baz}
596+
Invoke-Formatter -ScriptDefinition "$def" -Settings $settings |
597+
Should -Be "$expected"
598+
}
599+
600+
It "Should fix script when a parameter value is a hashtable spanning multiple lines" {
601+
$def = {foo @{
602+
a = 1
603+
} -baz}
604+
605+
$expected = {foo @{
606+
a = 1
607+
} -baz}
608+
Invoke-Formatter -ScriptDefinition "$def" -Settings $settings |
609+
Should -Be "$expected"
610+
}
611+
612+
It "Should fix script when a parameter value is an array spanning multiple lines" {
613+
$def = {foo @(
614+
1
615+
) -baz}
616+
617+
$expected = {foo @(
618+
1
619+
) -baz}
620+
Invoke-Formatter -ScriptDefinition "$def" -Settings $settings |
621+
Should -Be "$expected"
622+
}
623+
588624
It "Should fix script when redirects are involved and whitespace is not consistent" {
589625
# Related to Issue #2000
590626
$def = 'foo 3>&1 1>$null 2>&1'

0 commit comments

Comments
 (0)