Skip to content

Commit 60f358a

Browse files
Copilotbergmeister
andauthored
Fix AvoidAlias DSC review feedback
Co-authored-by: bergmeister <9250262+bergmeister@users.noreply.github.com>
1 parent 9a987dc commit 60f358a

2 files changed

Lines changed: 111 additions & 2 deletions

File tree

‎Rules/AvoidAlias.cs‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,25 @@ private bool IgnoreCommandast(CommandAst cmdAst)
195195

196196
private bool IsDscResourceDeclaration(CommandAst cmdAst)
197197
{
198-
if (GetDscResourceBody(cmdAst) == null)
198+
bool hasConfigurationAncestor = false;
199+
for (Ast parent = cmdAst.Parent; parent != null; parent = parent.Parent)
200+
{
201+
if (parent is ConfigurationDefinitionAst)
202+
{
203+
hasConfigurationAncestor = true;
204+
break;
205+
}
206+
207+
if (parent is FunctionDefinitionAst
208+
|| parent is HashtableAst
209+
|| parent is AssignmentStatementAst
210+
|| parent is ParenExpressionAst)
211+
{
212+
return false;
213+
}
214+
}
215+
216+
if (!hasConfigurationAncestor || GetDscResourceBody(cmdAst) == null)
199217
{
200218
return false;
201219
}
@@ -207,7 +225,17 @@ private bool IsDscResourceDeclaration(CommandAst cmdAst)
207225
return true;
208226
}
209227

210-
if (parent is FunctionDefinitionAst || parent is HashtableAst)
228+
if (parent is FunctionDefinitionAst
229+
|| parent is HashtableAst
230+
|| parent is AssignmentStatementAst
231+
|| parent is ParenExpressionAst)
232+
{
233+
return false;
234+
}
235+
236+
if (parent is CommandExpressionAst commandExpression
237+
&& (!(commandExpression.Expression is ScriptBlockExpressionAst body)
238+
|| !IsFollowingLineDscResourceBody(body)))
211239
{
212240
return false;
213241
}
@@ -236,6 +264,41 @@ private bool IsDscResourceDeclaration(CommandAst cmdAst)
236264
return false;
237265
}
238266

267+
private bool IsFollowingLineDscResourceBody(ScriptBlockExpressionAst body)
268+
{
269+
if (!(body.Parent is CommandExpressionAst bodyExpression)
270+
|| !(bodyExpression.Parent is PipelineAst bodyPipeline)
271+
|| bodyPipeline.PipelineElements.Count != 1
272+
|| bodyPipeline.PipelineElements[0] != bodyExpression)
273+
{
274+
return false;
275+
}
276+
277+
Ast statement = bodyPipeline;
278+
for (Ast parent = bodyPipeline.Parent; parent != null; parent = parent.Parent)
279+
{
280+
var statements = (parent as NamedBlockAst)?.Statements
281+
?? (parent as StatementBlockAst)?.Statements;
282+
if (statements == null)
283+
{
284+
statement = parent;
285+
continue;
286+
}
287+
288+
int index = statements.IndexOf(statement as StatementAst);
289+
if (index <= 0 || !(statements[index - 1] is PipelineAst declarationPipeline)
290+
|| declarationPipeline.PipelineElements.Count != 1
291+
|| !(declarationPipeline.PipelineElements[0] is CommandAst declaration))
292+
{
293+
return false;
294+
}
295+
296+
return GetDscResourceBody(declaration) == body;
297+
}
298+
299+
return false;
300+
}
301+
239302
private ScriptBlockExpressionAst GetDscResourceBody(CommandAst cmdAst)
240303
{
241304
if (cmdAst.InvocationOperator != TokenKind.Unknown
@@ -246,6 +309,13 @@ private ScriptBlockExpressionAst GetDscResourceBody(CommandAst cmdAst)
246309
return null;
247310
}
248311

312+
if (!(cmdAst.Parent is PipelineAst commandPipeline)
313+
|| commandPipeline.PipelineElements.Count != 1
314+
|| commandPipeline.PipelineElements[0] != cmdAst)
315+
{
316+
return null;
317+
}
318+
249319
if (cmdAst.CommandElements.Count == 3)
250320
{
251321
return cmdAst.CommandElements[2] as ScriptBlockExpressionAst;

‎Tests/Rules/AvoidUsingAlias.tests.ps1‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,45 @@ Configuration PackageTest {
146146
$violations[5].Extent.Text | Should -Be 'Verb'
147147
$violations[6].Extent.Text | Should -Be 'Verb'
148148
}
149+
150+
It "does not ignore executable commands in assignment and expression contexts or pipelines" {
151+
$target = @'
152+
{
153+
$items = ChildItem . {}
154+
$result = Verb Argument {}
155+
$action = { Verb Argument {} }
156+
Name = (Verb Argument {})
157+
Script ScriptExample {
158+
SetScript =
159+
{
160+
Verb Argument {}
161+
}
162+
}
163+
Verb Argument {} | Out-Null
164+
gci . | Out-Null
165+
{
166+
}
167+
}
168+
'@
169+
$ast = [System.Management.Automation.Language.Parser]::ParseInput($target, [ref]$null, [ref]$null)
170+
$body = $ast.EndBlock.Statements[0].PipelineElements[0].Expression.Copy()
171+
$configuration = [System.Management.Automation.Language.ConfigurationDefinitionAst]::new(
172+
$ast.Extent, $body, 'Resource',
173+
[System.Management.Automation.Language.StringConstantExpressionAst]::new($ast.Extent, 'Test', 'BareWord'))
174+
$rule = [Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias]::new()
175+
176+
$violations = @($rule.AnalyzeScript($configuration, $null))
177+
178+
$violations.Count | Should -Be 7
179+
$violations[0].Extent.Text | Should -Be 'ChildItem'
180+
$violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-ChildItem'
181+
$violations[1].Extent.Text | Should -Be 'Verb'
182+
$violations[2].Extent.Text | Should -Be 'Verb'
183+
$violations[3].Extent.Text | Should -Be 'Verb'
184+
$violations[4].Extent.Text | Should -Be 'Verb'
185+
$violations[5].Extent.Text | Should -Be 'Verb'
186+
$violations[6].Extent.Text | Should -Be 'gci'
187+
}
149188
}
150189

151190
Context "Settings file provides allowlist" {

0 commit comments

Comments
 (0)