From 49c0e1e2441faba24efe05860386890d0a3491b7 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Tue, 5 May 2015 11:01:51 -0700 Subject: [PATCH] Suppress write host for function that starts with show --- Rules/AvoidUsingWriteHost.cs | 91 ++++++++++++++----- Tests/Rules/AvoidUsingWriteHost.ps1 | 7 +- Tests/Rules/AvoidUsingWriteHost.tests.ps1 | 4 +- .../Rules/AvoidUsingWriteHostNoViolations.ps1 | 8 +- 4 files changed, 84 insertions(+), 26 deletions(-) diff --git a/Rules/AvoidUsingWriteHost.cs b/Rules/AvoidUsingWriteHost.cs index 597d7cf4a..fcc06a38b 100644 --- a/Rules/AvoidUsingWriteHost.cs +++ b/Rules/AvoidUsingWriteHost.cs @@ -23,8 +23,11 @@ namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules /// AvoidUsingWriteHost: Check that Write-Host or Console.Write are not used /// [Export(typeof(IScriptRule))] - public class AvoidUsingWriteHost : IScriptRule + public class AvoidUsingWriteHost : AstVisitor, IScriptRule { + List records; + string fileName; + /// /// AnalyzeScript: check that Write-Host or Console.Write are not used. /// @@ -32,34 +35,78 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - // Finds all CommandAsts. - IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); + records = new List(); + this.fileName = fileName; + + ast.Visit(this); + + return records; + } + + + /// + /// Visit function and skips any function that starts with show + /// + /// + /// + public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) + { + if (funcAst == null || funcAst.Name == null) + { + return AstVisitAction.SkipChildren; + } + + if (funcAst.Name.StartsWith("show", StringComparison.OrdinalIgnoreCase)) + { + return AstVisitAction.SkipChildren; + } + + return AstVisitAction.Continue; + } + + /// + /// Checks that write-host command is not used + /// + /// + /// + public override AstVisitAction VisitCommand(CommandAst cmdAst) + { + if (cmdAst == null) + { + return AstVisitAction.SkipChildren; + } - // Iterrates all CommandAsts and check the command name. - foreach (CommandAst cmdAst in commandAsts) + if (cmdAst.GetCommandName() != null && String.Equals(cmdAst.GetCommandName(), "write-host", StringComparison.OrdinalIgnoreCase)) { - if (cmdAst.GetCommandName() != null && String.Equals(cmdAst.GetCommandName(), "write-host", StringComparison.OrdinalIgnoreCase)) - { - yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWriteHostError, System.IO.Path.GetFileName(fileName)), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); - } + records.Add(new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWriteHostError, System.IO.Path.GetFileName(fileName)), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName)); } - // Finds all InvokeMemberExpressionAst - IEnumerable invokeAsts = ast.FindAll(testAst => testAst is InvokeMemberExpressionAst, true); + return AstVisitAction.Continue; + } - foreach (InvokeMemberExpressionAst invokeAst in invokeAsts) + public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst imeAst) + { + if (imeAst == null) { - TypeExpressionAst typeAst = invokeAst.Expression as TypeExpressionAst; - if (typeAst == null || typeAst.TypeName == null || typeAst.TypeName.FullName == null) continue; - - if (typeAst.TypeName.FullName.EndsWith("console", StringComparison.OrdinalIgnoreCase) - && !String.IsNullOrWhiteSpace(invokeAst.Member.Extent.Text) && invokeAst.Member.Extent.Text.StartsWith("Write", StringComparison.OrdinalIgnoreCase)) - { - yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingConsoleWriteError, System.IO.Path.GetFileName(fileName), invokeAst.Member.Extent.Text), - invokeAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); - } + return AstVisitAction.SkipChildren; } + + TypeExpressionAst typeAst = imeAst.Expression as TypeExpressionAst; + + if (typeAst == null || typeAst.TypeName == null || typeAst.TypeName.FullName == null) + { + return AstVisitAction.SkipChildren; + } + + if (typeAst.TypeName.FullName.EndsWith("console", StringComparison.OrdinalIgnoreCase) + && !String.IsNullOrWhiteSpace(imeAst.Member.Extent.Text) && imeAst.Member.Extent.Text.StartsWith("Write", StringComparison.OrdinalIgnoreCase)) + { + records.Add(new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingConsoleWriteError, System.IO.Path.GetFileName(fileName), imeAst.Member.Extent.Text), + imeAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName)); + } + + return AstVisitAction.Continue; } /// diff --git a/Tests/Rules/AvoidUsingWriteHost.ps1 b/Tests/Rules/AvoidUsingWriteHost.ps1 index b9ea56344..d8f73259c 100644 --- a/Tests/Rules/AvoidUsingWriteHost.ps1 +++ b/Tests/Rules/AvoidUsingWriteHost.ps1 @@ -3,4 +3,9 @@ cls Write-Host "aaa" clear [System.Console]::Write("abcdefg"); -[System.Console]::WriteLine("No console.writeline plz!"); \ No newline at end of file +[System.Console]::WriteLine("No console.writeline plz!"); + +function Test +{ + Write-Host "aaaa" +} \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingWriteHost.tests.ps1 b/Tests/Rules/AvoidUsingWriteHost.tests.ps1 index f9cd926a2..2e28f5c37 100644 --- a/Tests/Rules/AvoidUsingWriteHost.tests.ps1 +++ b/Tests/Rules/AvoidUsingWriteHost.tests.ps1 @@ -8,8 +8,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingWriteHostNoViolations Describe "AvoidUsingWriteHost" { Context "When there are violations" { - It "has 3 Write-Host violations" { - $violations.Count | Should Be 3 + It "has 4 Write-Host violations" { + $violations.Count | Should Be 4 } It "has the correct description message for Write-Host" { diff --git a/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 b/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 index 4dadd1faa..28b18eeae 100644 --- a/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 +++ b/Tests/Rules/AvoidUsingWriteHostNoViolations.ps1 @@ -1 +1,7 @@ -Write-Output "This is the correct way to write output" \ No newline at end of file +Write-Output "This is the correct way to write output" + +# Even if write-host is used, error should not be raised in this function +function Show-Something +{ + Write-Host "show something on screen"; +} \ No newline at end of file