Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions Rules/MisleadingBacktick.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
3 changes: 2 additions & 1 deletion Rules/ScriptAnalyzerBuiltinRules.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -129,7 +130,7 @@
<VisualStudio AllowExistingFolder="true" />
</ProjectExtensions>
<PropertyGroup>
<PostBuildEvent>
<PostBuildEvent>
if "PSV3 Release" == "$(ConfigurationName)" (
GOTO psv3Release
) else if "PSV3 Debug" == "$(ConfigurationName)" (
Expand Down
36 changes: 36 additions & 0 deletions Rules/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
2 changes: 1 addition & 1 deletion Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
12 changes: 12 additions & 0 deletions Tests/Rules/MisleadingBacktick.ps1
Original file line number Diff line number Diff line change
@@ -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 `
19 changes: 19 additions & 0 deletions Tests/Rules/MisleadingBacktick.tests.ps1
Original file line number Diff line number Diff line change
@@ -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
}
}
}
7 changes: 7 additions & 0 deletions Tests/Rules/NoMisleadingBacktick.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
New-NetLbfoTeam `
-Name NetTeam `
-TeamingMode SwitchIndependent `
-TeamMembers Ethernet*


"this ` backtick is just fine, though"