Merge pull request #425 from AndrewGaspar/development

Add rule for warning about backticks that look like line-continuations but are not

Author: raghushantha

Rules/MisleadingBacktick.cs

@@ -0,0 +1,120 @@
+//
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+using System.ComponentModel.Composition;
+using System.Globalization;
+using System.Text.RegularExpressions;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// MisleadingBacktick: Checks that lines don't end with a backtick followed by whitespace
+    /// </summary>
+    [Export(typeof(IScriptRule))]
+    public class MisleadingBacktick : IScriptRule
+    {
+        private static Regex TrailingEscapedWhitespaceRegex = new Regex(@"`\s+$");
+        private static Regex NewlineRegex = new Regex("\r?\n");
+
+        /// <summary>
+        /// MisleadingBacktick: Checks that lines don't end with a backtick followed by a whitespace
+        /// </summary>
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
+
+            string[] lines = NewlineRegex.Split(ast.Extent.Text);
+
+            if((ast.Extent.EndLineNumber - ast.Extent.StartLineNumber + 1) != lines.Length)
+            {
+                // Did not match the number of lines that the extent indicated
+                yield break;
+            }
+
+            foreach (int i in Enumerable.Range(0, lines.Length))
+            {
+                string line = lines[i];
+
+                Match match = TrailingEscapedWhitespaceRegex.Match(line);
+
+                if(match.Success)
+                {
+                    int lineNumber = ast.Extent.StartLineNumber + i;
+
+                    ScriptPosition start = new ScriptPosition(fileName, lineNumber, match.Index, line);
+                    ScriptPosition end = new ScriptPosition(fileName, lineNumber, match.Index + match.Length, line);
+                    
+                    yield return new DiagnosticRecord(
+                        string.Format(CultureInfo.CurrentCulture, Strings.MisleadingBacktickError),
+                            new ScriptExtent(start, end), GetName(), DiagnosticSeverity.Warning, fileName);
+                }
+            }
+        }
+
+        /// <summary>
+        /// GetName: Retrieves the name of this rule.
+        /// </summary>
+        /// <returns>The name of this rule</returns>
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.MisleadingBacktickName);
+        }
+
+        /// <summary>
+        /// GetCommonName: Retrieves the common name of this rule.
+        /// </summary>
+        /// <returns>The common name of this rule</returns>
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.MisleadingBacktickCommonName);
+        }
+
+        /// <summary>
+        /// GetDescription: Retrieves the description of this rule.
+        /// </summary>
+        /// <returns>The description of this rule</returns>
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.MisleadingBacktickDescription);
+        }
+
+        /// <summary>
+        /// GetSourceType: Retrieves the type of the rule: builtin, managed or module.
+        /// </summary>
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        /// <summary>
+        /// GetSeverity: Retrieves the severity of the rule: error, warning of information.
+        /// </summary>
+        /// <returns></returns>
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// GetSourceName: Retrieves the module/assembly name the rule is from.
+        /// </summary>
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}

Rules/ScriptAnalyzerBuiltinRules.csproj

@@ -87,6 +87,7 @@
     <Compile Include="AvoidUsingWriteHost.cs" />
     <Compile Include="DscTestsPresent.cs" />
     <Compile Include="DscExamplesPresent.cs" />
+    <Compile Include="MisleadingBacktick.cs" />
     <Compile Include="Strings.Designer.cs">
       <AutoGen>True</AutoGen>
       <DesignTime>True</DesignTime>
@@ -129,7 +130,7 @@
     <VisualStudio AllowExistingFolder="true" />
   </ProjectExtensions>
   <PropertyGroup>
-     <PostBuildEvent>
+    <PostBuildEvent>
 if "PSV3 Release" == "$(ConfigurationName)" (
 	GOTO psv3Release
 ) else if "PSV3 Debug" == "$(ConfigurationName)" (

Rules/Strings.Designer.cs

@@ -774,4 +774,16 @@
   <data name="ScriptDefinitionName" xml:space="preserve">
     <value>ScriptDefinition</value>
   </data>
+  <data name="MisleadingBacktickCommonName" xml:space="preserve">
+    <value>Misleading Backtick</value>
+  </data>
+  <data name="MisleadingBacktickDescription" xml:space="preserve">
+    <value>Ending a line with an escaped whitepsace character is misleading. A trailing backtick is usually used for line continuation. Users typically don't intend to end a line with escaped whitespace.</value>
+  </data>
+  <data name="MisleadingBacktickName" xml:space="preserve">
+    <value>MisleadingBacktick</value>
+  </data>
+  <data name="MisleadingBacktickError" xml:space="preserve">
+    <value>This line has a backtick at the end trailed by a whitespace character. Did you mean for this to be a line continuation?</value>
+  </data>
 </root>

Rules/Strings.resx

@@ -56,7 +56,7 @@ Describe "Test Name parameters" {
 
         It "Get Rules with no parameters supplied" {
 			$defaultRules = Get-ScriptAnalyzerRule
-			$defaultRules.Count | Should be 38
+			$defaultRules.Count | Should be 39
 		}
     }
 

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

@@ -0,0 +1,12 @@
+New-NetLbfoTeam ` 
+    -Name NetTeam ` 
+    -TeamingMode SwitchIndependent ` 
+    -TeamMembers Ethernet*
+    
+    
+"this ` backtick is just fine, though"
+
+and so is this one `
+
+But not this `                     
+or this `      

Tests/Rules/MisleadingBacktick.ps1

@@ -0,0 +1,19 @@
+Import-Module PSScriptAnalyzer
+$writeHostName = "PSMisleadingBacktick"
+$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
+$violations = Invoke-ScriptAnalyzer $directory\MisleadingBacktick.ps1 | Where-Object {$_.RuleName -eq $writeHostName}
+$noViolations = Invoke-ScriptAnalyzer $directory\NoMisleadingBacktick.ps1 | Where-Object {$_.RuleName -eq $clearHostName}
+
+Describe "Avoid Misleading Backticks" {
+    Context "When there are violations" {
+        It "has 5 misleading backtick violations" {
+            $violations.Count | Should Be 5
+        }
+    }
+
+    Context "When there are no violations" {
+        It "returns no violations" {
+            $noViolations.Count | Should Be 0
+        }
+    }
+}

Tests/Rules/MisleadingBacktick.tests.ps1

@@ -0,0 +1,7 @@
+New-NetLbfoTeam `
+    -Name NetTeam `
+    -TeamingMode SwitchIndependent `
+    -TeamMembers Ethernet*
+    
+    
+"this ` backtick is just fine, though"