From db7a75fc6e0db6e5f504bac252673bad593d6f5f Mon Sep 17 00:00:00 2001 From: Andrew Gaspar Date: Thu, 7 Jan 2016 23:19:13 -0800 Subject: [PATCH 1/2] Add rule for warning about backticks that look like line-continuations but are not. --- Rules/MisleadingBacktick.cs | 120 +++++++++++++++++++++++ Rules/ScriptAnalyzerBuiltinRules.csproj | 3 +- Rules/Strings.Designer.cs | 36 +++++++ Rules/Strings.resx | 12 +++ Tests/Rules/MisleadingBacktick.ps1 | 12 +++ Tests/Rules/MisleadingBacktick.tests.ps1 | 23 +++++ Tests/Rules/NoMisleadingBacktick.ps1 | 7 ++ 7 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 Rules/MisleadingBacktick.cs create mode 100644 Tests/Rules/MisleadingBacktick.ps1 create mode 100644 Tests/Rules/MisleadingBacktick.tests.ps1 create mode 100644 Tests/Rules/NoMisleadingBacktick.ps1 diff --git a/Rules/MisleadingBacktick.cs b/Rules/MisleadingBacktick.cs new file mode 100644 index 000000000..1da366faf --- /dev/null +++ b/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 +{ + /// + /// MisleadingBacktick: Checks that lines don't end with a backtick followed by whitespace + /// + [Export(typeof(IScriptRule))] + public class MisleadingBacktick : IScriptRule + { + private static Regex TrailingEscapedWhitespaceRegex = new Regex(@"`\s+$"); + private static Regex NewlineRegex = new Regex("\r?\n"); + + /// + /// MisleadingBacktick: Checks that lines don't end with a backtick followed by a whitespace + /// + public IEnumerable 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); + } + } + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.MisleadingBacktickName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.MisleadingBacktickCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.MisleadingBacktickDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index 1213ed998..d5828643a 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -87,6 +87,7 @@ + True True @@ -129,7 +130,7 @@ - + if "PSV3 Release" == "$(ConfigurationName)" ( GOTO psv3Release ) else if "PSV3 Debug" == "$(ConfigurationName)" ( diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 55a65fd0a..fe00b561c 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1014,6 +1014,42 @@ internal static string DscTestsPresentNoTestsError { } } + /// + /// Looks up a localized string similar to Misleading Backtick. + /// + internal static string MisleadingBacktickCommonName { + get { + return ResourceManager.GetString("MisleadingBacktickCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to 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.. + /// + internal static string MisleadingBacktickDescription { + get { + return ResourceManager.GetString("MisleadingBacktickDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to This line has a backtick at the end trailed by a whitespace character. Did you mean for this to be a line continuation?. + /// + internal static string MisleadingBacktickError { + get { + return ResourceManager.GetString("MisleadingBacktickError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to MisleadingBacktick. + /// + internal static string MisleadingBacktickName { + get { + return ResourceManager.GetString("MisleadingBacktickName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Module Manifest Fields. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 319d72d27..9d92c1ee4 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -774,4 +774,16 @@ ScriptDefinition + + Misleading Backtick + + + 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. + + + MisleadingBacktick + + + This line has a backtick at the end trailed by a whitespace character. Did you mean for this to be a line continuation? + \ No newline at end of file diff --git a/Tests/Rules/MisleadingBacktick.ps1 b/Tests/Rules/MisleadingBacktick.ps1 new file mode 100644 index 000000000..eb29a00f1 --- /dev/null +++ b/Tests/Rules/MisleadingBacktick.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 ` \ No newline at end of file diff --git a/Tests/Rules/MisleadingBacktick.tests.ps1 b/Tests/Rules/MisleadingBacktick.tests.ps1 new file mode 100644 index 000000000..7f0c6744f --- /dev/null +++ b/Tests/Rules/MisleadingBacktick.tests.ps1 @@ -0,0 +1,23 @@ +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 + } + + It "is " { + $violations[0].Message | Should Match $writeHostMessage + } + } + + Context "When there are no violations" { + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + } +} \ No newline at end of file diff --git a/Tests/Rules/NoMisleadingBacktick.ps1 b/Tests/Rules/NoMisleadingBacktick.ps1 new file mode 100644 index 000000000..216e6af1f --- /dev/null +++ b/Tests/Rules/NoMisleadingBacktick.ps1 @@ -0,0 +1,7 @@ +New-NetLbfoTeam ` + -Name NetTeam ` + -TeamingMode SwitchIndependent ` + -TeamMembers Ethernet* + + +"this ` backtick is just fine, though" \ No newline at end of file From 05e36689ab8f30a32fddd3da361184fa3cae456e Mon Sep 17 00:00:00 2001 From: Andrew Gaspar Date: Mon, 11 Jan 2016 23:56:12 -0800 Subject: [PATCH 2/2] Fix tests for MisleadingBacktick. --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- Tests/Rules/MisleadingBacktick.tests.ps1 | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index d992f5ca4..7acf5743b 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -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 } } diff --git a/Tests/Rules/MisleadingBacktick.tests.ps1 b/Tests/Rules/MisleadingBacktick.tests.ps1 index 7f0c6744f..659d6fdb3 100644 --- a/Tests/Rules/MisleadingBacktick.tests.ps1 +++ b/Tests/Rules/MisleadingBacktick.tests.ps1 @@ -9,10 +9,6 @@ Describe "Avoid Misleading Backticks" { It "has 5 misleading backtick violations" { $violations.Count | Should Be 5 } - - It "is " { - $violations[0].Message | Should Match $writeHostMessage - } } Context "When there are no violations" {