Skip to content

Commit 2104f05

Browse files
rjmholtJamesWTruher
rjmholt
authored and
JamesWTruher
committed
Fix settings file array parsing when no commas are present (#1161)
* Fix settings file array parsing when no commas are present * Add extra test * Improve comment * Add test file * Change test to accept null * Fix new-item call in PS 4
1 parent c366328 commit 2104f05

File tree

3 files changed

+170
-17
lines changed

3 files changed

+170
-17
lines changed

‎Engine/Settings.cs

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,9 @@ private static object GetSafeValueFromExpressionAst(ExpressionAst exprAst)
522522
case "false":
523523
return false;
524524

525+
case "null":
526+
return null;
527+
525528
default:
526529
throw CreateInvalidDataExceptionFromAst(varExprAst);
527530
}
@@ -540,24 +543,36 @@ private static object GetSafeValueFromExpressionAst(ExpressionAst exprAst)
540543
return new object[0];
541544
}
542545

543-
PipelineAst pipelineAst = arrExprAst.SubExpression.Statements[0] as PipelineAst;
544-
if (pipelineAst == null)
546+
var listComponents = new List<object>();
547+
// Arrays can either be array expressions (1, 2, 3) or array literals with statements @(1 `n 2 `n 3)
548+
// Or they can be a combination of these
549+
// We go through each statement (line) in an array and read the whole subarray
550+
// This will also mean that @(1; 2) is parsed as an array of two elements, but there's not much point defending against this
551+
foreach (StatementAst statement in arrExprAst.SubExpression.Statements)
545552
{
546-
throw CreateInvalidDataExceptionFromAst(arrExprAst);
547-
}
553+
if (!(statement is PipelineAst pipelineAst))
554+
{
555+
throw CreateInvalidDataExceptionFromAst(arrExprAst);
556+
}
548557

549-
ExpressionAst pipelineExpressionAst = pipelineAst.GetPureExpression();
550-
if (pipelineExpressionAst == null)
551-
{
552-
throw CreateInvalidDataExceptionFromAst(arrExprAst);
558+
ExpressionAst pipelineExpressionAst = pipelineAst.GetPureExpression();
559+
if (pipelineExpressionAst == null)
560+
{
561+
throw CreateInvalidDataExceptionFromAst(arrExprAst);
562+
}
563+
564+
object arrayValue = GetSafeValueFromExpressionAst(pipelineExpressionAst);
565+
// We might hit arrays like @(\n1,2,3\n4,5,6), which the parser sees as two statements containing array expressions
566+
if (arrayValue is object[] subArray)
567+
{
568+
listComponents.AddRange(subArray);
569+
continue;
570+
}
571+
572+
listComponents.Add(arrayValue);
553573
}
574+
return listComponents.ToArray();
554575

555-
// Array expressions may not really be arrays (like `@('a')`, which has no ArrayLiteralAst within)
556-
// However, some rules depend on this always being an array
557-
object arrayValue = GetSafeValueFromExpressionAst(pipelineExpressionAst);
558-
return arrayValue.GetType().IsArray
559-
? arrayValue
560-
: new object[] { arrayValue };
561576

562577
case ArrayLiteralAst arrLiteralAst:
563578
return GetSafeValuesFromArrayAst(arrLiteralAst);
@@ -647,10 +662,10 @@ private static InvalidDataException CreateInvalidDataExceptionFromAst(Ast ast)
647662
throw new ArgumentNullException(nameof(ast));
648663
}
649664

650-
return ThrowInvalidDataException(ast.Extent);
665+
return CreateInvalidDataException(ast.Extent);
651666
}
652667

653-
private static InvalidDataException ThrowInvalidDataException(IScriptExtent extent)
668+
private static InvalidDataException CreateInvalidDataException(IScriptExtent extent)
654669
{
655670
return new InvalidDataException(string.Format(
656671
CultureInfo.CurrentCulture,

‎Tests/Engine/Settings.tests.ps1

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ Describe "Settings Class" {
209209
@{ Expr = '0.142' }
210210
@{ Expr = '"Hello"' }
211211
@{ Expr = '"Well then"' }
212+
@{ Expr = '$null' }
212213
)
213214

214215
$gsvArrayTests = @(
@@ -218,6 +219,11 @@ Describe "Settings Class" {
218219
@{ Expr = '@("A","B","C")'; Count = 3 }
219220
@{ Expr = '@()'; Count = 0 }
220221
@{ Expr = '@(7)'; Count = 1 }
222+
@{ Expr = "'1',`n'2',`n'3'"; Count = 3 }
223+
@{ Expr = "@(1`n3`n5`n7)"; Count = 4 }
224+
@{ Expr = "'1',`r`n'2',`r`n'3'"; Count = 3 }
225+
@{ Expr = "@(1`r`n3`r`n5`r`n7)"; Count = 4 }
226+
@{ Expr = "@(1,2,3`n7,8,9)"; Count = 6 }
221227
)
222228

223229
$gsvHashtableTests = @(
@@ -235,7 +241,6 @@ Describe "Settings Class" {
235241
$gsvThrowTests = @(
236242
@{ Expr = '$var' }
237243
@{ Expr = '' }
238-
@{ Expr = '$null' }
239244
@{ Expr = '3+7' }
240245
@{ Expr = '- 2.5'}
241246
@{ Expr = '-not $true' }

‎Tests/Rules/AllCompatibilityRules.Tests.ps1

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
$script:fileContent = @'
5+
$assetDir = 'C:\ImportantFiles\'
6+
$archiveDir = 'C:\Archived\'
7+
8+
$runningOnWindows = -not ($IsLinux -or $IsMacOS)
9+
$zipCompressionLevel = 'Optimal'
10+
$includeBaseDirInZips = $true
11+
12+
if (-not (Test-Path $archiveDir))
13+
{
14+
New-Item -Type Directory $archiveDir
15+
}
16+
17+
Import-Module -FullyQualifiedName @{ ModuleName = 'MyArchiveUtilities'; ModuleVersion = '2.0' }
18+
19+
$sigs = [System.Collections.Generic.List[System.Management.Automation.Signature]]::new()
20+
$filePattern = [System.Management.Automation.WildcardPattern]::Get('system*')
21+
foreach ($file in Get-ChildItem $assetDir -Recurse -Depth 1)
22+
{
23+
if (-not $filePattern.IsMatch($file.Name))
24+
{
25+
continue
26+
}
27+
28+
if ($file.Name -like '*.dll')
29+
{
30+
$sig = Get-AuthenticodeSignature $file
31+
$sigs.Add($sig)
32+
continue
33+
}
34+
35+
if (Test-WithFunctionFromMyModule -File $file)
36+
{
37+
$destZip = Join-Path $archiveDir $file.BaseName
38+
[System.IO.Compression.ZipFile]::CreateFromDirectory($file.FullName, "$destZip.zip", $zipCompressionLevel, $includeBaseDirInZips)
39+
}
40+
}
41+
42+
Write-Output $sigs
43+
'@
44+
45+
$script:settingsContent = @'
46+
@{
47+
Rules = @{
48+
PSUseCompatibleCommands = @{
49+
Enable = $true
50+
TargetProfiles = @(
51+
'win-8_x64_10.0.17763.0_5.1.17763.316_x64_4.0.30319.42000_framework' # Server 2019 - PS 5.1 (the platform it already runs on)
52+
'win-8_x64_6.2.9200.0_3.0_x64_4.0.30319.42000_framework' # Server 2012 - PS 3
53+
'ubuntu_x64_18.04_6.1.3_x64_4.0.30319.42000_core' # Ubuntu 18.04 - PS 6.1
54+
)
55+
}
56+
PSUseCompatibleTypes = @{
57+
Enable = $true
58+
# Same as for command targets
59+
TargetProfiles = @(
60+
'win-8_x64_10.0.17763.0_5.1.17763.316_x64_4.0.30319.42000_framework'
61+
'win-8_x64_6.2.9200.0_3.0_x64_4.0.30319.42000_framework'
62+
'ubuntu_x64_18.04_6.1.3_x64_4.0.30319.42000_core'
63+
)
64+
}
65+
PSUseCompatibleSyntax = @{
66+
Enable = $true
67+
TargetVersions = @('3.0', '5.1', '6.1')
68+
}
69+
}
70+
}
71+
'@
72+
73+
$script:expectedCommandDiagnostics = @(
74+
@{ Command = 'Import-Module'; Parameter = 'FullyQualifiedName'; Line = 13 }
75+
@{ Command = 'Get-ChildItem'; Parameter = 'Depth'; Line = 17 }
76+
@{ Command = 'Get-AuthenticodeSignature'; Line = 26 }
77+
)
78+
79+
$script:expectedTypeDiagnostics = @(
80+
@{ Type = 'System.Management.Automation.WildcardPattern'; Member = 'Get'; Line = 16 }
81+
@{ Type = 'System.IO.Compression.ZipFile'; Line = 34 }
82+
)
83+
84+
$script:expectedSyntaxDiagnostics = @(
85+
@{ Line = 15; CorrectionText = "New-Object 'System.Collections.Generic.List[System.Management.Automation.Signature]'" }
86+
)
87+
88+
Describe "Running all compatibility rules with a settings file" {
89+
BeforeAll {
90+
$testFile = New-Item -Path "$TestDrive/test.ps1" -Value $script:fileContent -ItemType File
91+
$settingsFile = New-Item -Path "$TestDrive/settings.psd1" -Value $script:settingsContent -ItemType File
92+
$diagnostics = Invoke-ScriptAnalyzer -Path $testFile.FullName -Settings $settingsFile.FullName |
93+
Group-Object -Property RuleName -AsHashtable
94+
$commandDiagnostics = $diagnostics.PSUseCompatibleCommands
95+
$typeDiagnostics = $diagnostics.PSUseCompatibleTypes
96+
$syntaxDiagnostics = $diagnostics.PSUseCompatibleSyntax
97+
}
98+
99+
It "Finds the problem with command <Command> on line <Line> in the file" -TestCases $script:expectedCommandDiagnostics {
100+
param([string]$Command, [string]$Parameter, [int]$Line)
101+
102+
$actualDiagnostic = $commandDiagnostics | Where-Object { $_.Command -eq $Command -and $_.Line -eq $Line }
103+
$actualDiagnostic.Command | Should -BeExactly $Command
104+
$actualDiagnostic.Line | Should -Be $Line
105+
if ($Parameter)
106+
{
107+
$actualDiagnostic.Parameter | Should -Be $Parameter
108+
}
109+
}
110+
111+
It "Finds the problem with type <Type> on line <Line> in the file" -TestCases $script:expectedTypeDiagnostics {
112+
param([string]$Type, [string]$Member, [int]$Line)
113+
114+
$actualDiagnostic = $typeDiagnostics | Where-Object { $_.Type -eq $Type -and $_.Line -eq $Line }
115+
$actualDiagnostic.Type | Should -BeExactly $Type
116+
$actualDiagnostic.Line | Should -Be $Line
117+
if ($Member)
118+
{
119+
$actualDiagnostic.Member | Should -Be $Member
120+
}
121+
}
122+
123+
It "Finds the problem with syntax on line <Line> in the file" -TestCases $script:expectedSyntaxDiagnostics {
124+
param([string]$Suggestion, [int]$Line)
125+
126+
$actualDiagnostic = $syntaxDiagnostics | Where-Object { $_.Line -eq $Line }
127+
$actualDiagnostic.Line | Should -Be $Line
128+
if ($Suggestion)
129+
{
130+
$actualDiagnostic.Suggestion | Should -Be $Suggestion
131+
}
132+
}
133+
}

0 commit comments

Comments
 (0)