Merge pull request #93 from PowerShell/writehostbug

Suppress write host for function that starts with show

Author: Quoc Truong

Rules/AvoidUsingWriteHost.cs

@@ -23,43 +23,90 @@ namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
     /// AvoidUsingWriteHost: Check that Write-Host or Console.Write are not used
     /// </summary>
     [Export(typeof(IScriptRule))]
-    public class AvoidUsingWriteHost : IScriptRule
+    public class AvoidUsingWriteHost : AstVisitor, IScriptRule
     {
+        List<DiagnosticRecord> records;
+        string fileName;
+
         /// <summary>
         /// AnalyzeScript: check that Write-Host or Console.Write are not used.
         /// </summary>
         public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
         {
             if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
 
-            // Finds all CommandAsts.
-            IEnumerable<Ast> commandAsts = ast.FindAll(testAst => testAst is CommandAst, true);
+            records = new List<DiagnosticRecord>();
+            this.fileName = fileName;
+
+            ast.Visit(this);
+
+            return records;
+        }
+
+
+        /// <summary>
+        /// Visit function and skips any function that starts with show
+        /// </summary>
+        /// <param name="funcAst"></param>
+        /// <returns></returns>
+        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;
+        }
+
+        /// <summary>
+        /// Checks that write-host command is not used
+        /// </summary>
+        /// <param name="cmdAst"></param>
+        /// <returns></returns>
+        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<Ast> 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;
         }
 
         /// <summary>

Tests/Rules/AvoidUsingWriteHost.ps1

@@ -3,4 +3,9 @@ cls
 Write-Host "aaa"
 clear
 [System.Console]::Write("abcdefg");
-[System.Console]::WriteLine("No console.writeline plz!");
+[System.Console]::WriteLine("No console.writeline plz!");
+
+function Test
+{
+    Write-Host "aaaa"
+}

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" {

Tests/Rules/AvoidUsingWriteHostNoViolations.ps1

@@ -1 +1,7 @@
-Write-Output "This is the correct way to write output"
+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";
+}