Skip to content

Commit be98895

Browse files
Copilotbergmeister
andauthored
Avoid implicit Get-prefix corrections for DSC resource declarations
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
1 parent cafb813 commit be98895

2 files changed

Lines changed: 157 additions & 0 deletions

File tree

‎Rules/AvoidAlias.cs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
138138
continue;
139139
}
140140

141+
if (IsDscResourceDeclaration(cmdAst))
142+
{
143+
continue;
144+
}
145+
141146
var commdNameWithGetPrefix = $"Get-{commandName}";
142147
var cmdletNameIfCommandWasMissingGetPrefix = Helper.Instance.GetCommandInfo(name: commdNameWithGetPrefix,
143148
commandType: CommandTypes.Cmdlet | CommandTypes.Function | CommandTypes.Script);
@@ -186,6 +191,82 @@ private bool IgnoreCommandast(CommandAst cmdAst)
186191
return false;
187192
}
188193

194+
private bool IsDscResourceDeclaration(CommandAst cmdAst)
195+
{
196+
if (GetDscResourceBody(cmdAst) == null)
197+
{
198+
return false;
199+
}
200+
201+
for (Ast parent = cmdAst.Parent; parent != null; parent = parent.Parent)
202+
{
203+
if (parent is ConfigurationDefinitionAst)
204+
{
205+
return true;
206+
}
207+
208+
if (parent is FunctionDefinitionAst || parent is HashtableAst)
209+
{
210+
return false;
211+
}
212+
213+
// Script resource properties contain executable code, not resource declarations.
214+
if (parent is ScriptBlockExpressionAst scriptBlock)
215+
{
216+
if (scriptBlock.Parent is CommandAst enclosingCommand)
217+
{
218+
if (GetDscResourceBody(enclosingCommand) != scriptBlock
219+
|| !String.IsNullOrEmpty(Helper.Instance.GetCmdletNameFromAlias(enclosingCommand.GetCommandName()))
220+
|| Helper.Instance.GetCommandInfo(enclosingCommand.GetCommandName()) != null)
221+
{
222+
return false;
223+
}
224+
}
225+
else if (!(scriptBlock.Parent is ConfigurationDefinitionAst)
226+
&& !(scriptBlock.Parent is DynamicKeywordStatementAst)
227+
&& !(scriptBlock.Parent is CommandExpressionAst))
228+
{
229+
return false;
230+
}
231+
}
232+
}
233+
234+
return false;
235+
}
236+
237+
private ScriptBlockExpressionAst GetDscResourceBody(CommandAst cmdAst)
238+
{
239+
if (cmdAst.InvocationOperator != TokenKind.Unknown
240+
|| cmdAst.CommandElements.Count < 2
241+
|| !(cmdAst.CommandElements[1] is ExpressionAst)
242+
|| cmdAst.CommandElements[1].Extent.Text == "=")
243+
{
244+
return null;
245+
}
246+
247+
if (cmdAst.CommandElements.Count == 3)
248+
{
249+
return cmdAst.CommandElements[2] as ScriptBlockExpressionAst;
250+
}
251+
252+
// A resource's opening brace can be on the following line.
253+
if (cmdAst.CommandElements.Count == 2 && cmdAst.Parent is PipelineAst pipeline)
254+
{
255+
var statements = (pipeline.Parent as NamedBlockAst)?.Statements
256+
?? (pipeline.Parent as StatementBlockAst)?.Statements;
257+
int index = statements == null ? -1 : statements.IndexOf(pipeline);
258+
if (index >= 0 && index + 1 < statements.Count
259+
&& statements[index + 1] is PipelineAst bodyPipeline
260+
&& bodyPipeline.PipelineElements.Count == 1
261+
&& bodyPipeline.PipelineElements[0] is CommandExpressionAst bodyExpression)
262+
{
263+
return bodyExpression.Expression as ScriptBlockExpressionAst;
264+
}
265+
}
266+
267+
return null;
268+
}
269+
189270
/// <summary>
190271
/// For a command like "gci -path c:", returns the extent of "gci" in the command
191272
/// </summary>

‎Tests/Rules/AvoidUsingAlias.tests.ps1‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,19 @@ Describe "AvoidUsingAlias" {
3333
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'verb' | Where-Object { $_.RuleName -eq $violationName }
3434
$violations.Count | Should -Be 1
3535
}
36+
37+
It "still expands aliases with resource-shaped arguments" {
38+
$target = 'icm Server01 { Get-Process }'
39+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName
40+
$violations.Count | Should -Be 1
41+
$violations[0].SuggestedCorrections[0].Text | Should -Be 'Invoke-Command'
42+
}
43+
44+
It "still adds the Get prefix to ordinary commands with resource-shaped arguments" {
45+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Verb Argument {}' -IncludeRule $violationName
46+
$violations.Count | Should -Be 1
47+
$violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb'
48+
}
3649
}
3750

3851
Context "Violation Extent" {
@@ -65,6 +78,69 @@ Configuration MyDscConfiguration {
6578
Get-Count | `
6679
Should -Be 0
6780
}
81+
82+
It "does not expand DSC resource names during formatting" -skip:($IsLinux -or $IsMacOS) {
83+
$target = @'
84+
Configuration PackageTest {
85+
Package PackageExample
86+
{
87+
Name = "TestPackage"
88+
}
89+
}
90+
'@
91+
$settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } }
92+
Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName |
93+
Get-Count | Should -Be 0
94+
Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target
95+
}
96+
}
97+
98+
Context "DSC resource ASTs" {
99+
It "ignores resource declarations but not executable commands" {
100+
$target = @'
101+
{
102+
Verb VerbExample {}
103+
Verb NextLineExample
104+
{
105+
}
106+
icm Server01 { Verb Argument {} }
107+
& Verb Argument {}
108+
function Get-Example {
109+
Verb Argument {}
110+
}
111+
Node localhost
112+
{
113+
Verb NestedExample {}
114+
Script ScriptExample {
115+
SetScript = {
116+
icm Server01 {}
117+
Verb Argument {}
118+
}
119+
}
120+
}
121+
$scriptProperties = @{
122+
SetScript = { Verb Argument {} }
123+
}
124+
}
125+
'@
126+
$ast = [System.Management.Automation.Language.Parser]::ParseInput($target, [ref]$null, [ref]$null)
127+
$body = $ast.EndBlock.Statements[0].PipelineElements[0].Expression.Copy()
128+
$configuration = [System.Management.Automation.Language.ConfigurationDefinitionAst]::new(
129+
$ast.Extent, $body, 'Resource',
130+
[System.Management.Automation.Language.StringConstantExpressionAst]::new($ast.Extent, 'Test', 'BareWord'))
131+
$rule = [Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias]::new()
132+
133+
$violations = @($rule.AnalyzeScript($configuration, $null))
134+
135+
$violations.Count | Should -Be 7
136+
$violations[0].Extent.Text | Should -Be 'icm'
137+
$violations[1].Extent.Text | Should -Be 'Verb'
138+
$violations[2].Extent.Text | Should -Be 'Verb'
139+
$violations[3].Extent.Text | Should -Be 'Verb'
140+
$violations[4].Extent.Text | Should -Be 'icm'
141+
$violations[5].Extent.Text | Should -Be 'Verb'
142+
$violations[6].Extent.Text | Should -Be 'Verb'
143+
}
68144
}
69145

70146
Context "Settings file provides allowlist" {

0 commit comments

Comments
 (0)