Skip to content

Added rule: AvoidLongLines #1329

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 16 commits into from
Oct 4, 2019
Merged
Show file tree
Hide file tree
Changes from 7 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
30 changes: 30 additions & 0 deletions RuleDocumentation/AvoidLongLines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# AvoidLongLines

**Severity Level: Warning**

## Description

Lines should be no longer than a configured number of characters (default: 120), including leading whitespace (indentation).

**Note**: This rule is not enabled by default. The user needs to enable it through settings.

## Configuration

```powershell
Rules = @{
PSAvoidLongLines = @{
Enable = $true
LineLength = 120
}
}
```

### Parameters

#### Enable: bool (Default value is `$false`)

Enable or disable the rule during ScriptAnalyzer invocation.

#### LineLength: int (Default value is 120)

Optional parameter do override the default line length.
1 change: 1 addition & 0 deletions RuleDocumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
|[AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | |
|[AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | |
|[AvoidInvokingEmptyMembers](./AvoidInvokingEmptyMembers.md) | Warning | |
|[AvoidLongLines](./AvoidLongLines.md) | Warning | |
|[AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | |
|[AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | |
|[AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md) | Warning | Yes |
Expand Down
153 changes: 153 additions & 0 deletions Rules/AvoidLongLines.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Text.RegularExpressions;
#if !CORECLR
using System.ComponentModel.Composition;
#endif
using System.Globalization;
using System.Management.Automation.Language;
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
{
/// <summary>
/// AvoidLongLines: Checks for lines longer than 120 characters
/// </summary>
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif
public class AvoidLongLines : ConfigurableRule
{
/// <summary>
/// Construct an object of AvoidLongLines type.
/// </summary>
public AvoidLongLines() : base()
{
// Enable the rule by default
Enable = false;
}

[ConfigurableRuleProperty(defaultValue: 120)]
public int LineLength { get; set; }
/// <summary>
/// Analyzes the given ast to find violations.
/// </summary>
/// <param name="ast">AST to be analyzed. This should be non-null</param>
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
/// <returns>A an enumerable type containing the violations</returns>
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast == null)
{
throw new ArgumentNullException("ast");
}

var diagnosticRecords = new List<DiagnosticRecord>();

string[] lines = Regex.Split(ast.Extent.Text, @"\r?\n");

for (int lineNumber = 0; lineNumber < lines.Length; lineNumber++)
{
var line = lines[lineNumber];

if (line.Length > LineLength)
{
var startLine = lineNumber + 1;
var endLine = startLine;
var startColumn = 1;
var endColumn = line.Length;

var violationExtent = new ScriptExtent(
new ScriptPosition(
ast.Extent.File,
startLine,
startColumn,
line
),
new ScriptPosition(
ast.Extent.File,
endLine,
endColumn,
line
));

var record = new DiagnosticRecord(
String.Format(CultureInfo.CurrentCulture, Strings.AvoidLongLinesError),
violationExtent,
GetName(),
GetDiagnosticSeverity(),
ast.Extent.File,
null
);
diagnosticRecords.Add(record);
}
}

return diagnosticRecords;
}

/// <summary>
/// Retrieves the common name of this rule.
/// </summary>
public override string GetCommonName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidLongLinesCommonName);
}

/// <summary>
/// Retrieves the description of this rule.
/// </summary>
public override string GetDescription()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidLongLinesDescription);
}

/// <summary>
/// Retrieves the name of this rule.
/// </summary>
public override string GetName()
{
return string.Format(
CultureInfo.CurrentCulture,
Strings.NameSpaceFormat,
GetSourceName(),
Strings.AvoidLongLinesName);
}

/// <summary>
/// Retrieves the severity of the rule: error, warning or information.
/// </summary>
public override RuleSeverity GetSeverity()
{
return RuleSeverity.Warning;
}

/// <summary>
/// Gets the severity of the returned diagnostic record: error, warning, or information.
/// </summary>
/// <returns></returns>
public DiagnosticSeverity GetDiagnosticSeverity()
{
return DiagnosticSeverity.Warning;
}

/// <summary>
/// Retrieves the name of the module/assembly the rule is from.
/// </summary>
public override string GetSourceName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}

/// <summary>
/// Retrieves the type of the rule, Builtin, Managed or Module.
/// </summary>
public override SourceType GetSourceType()
{
return SourceType.Builtin;
}
}
}
55 changes: 55 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 @@ -897,6 +897,18 @@
<data name="AvoidTrailingWhitespaceError" xml:space="preserve">
<value>Line has trailing whitespace</value>
</data>
<data name="AvoidLongLinesName" xml:space="preserve">
<value>AvoidLongLines</value>
</data>
<data name="AvoidLongLinesCommonName" xml:space="preserve">
<value>Avoid long lines</value>
</data>
<data name="AvoidLongLinesDescription" xml:space="preserve">
<value>Each line should be not be too long.</value>
</data>
<data name="AvoidLongLinesError" xml:space="preserve">
<value>Line is too long</value>
</data>
<data name="PlaceOpenBraceName" xml:space="preserve">
<value>PlaceOpenBrace</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Describe "Test Name parameters" {

It "get Rules with no parameters supplied" {
$defaultRules = Get-ScriptAnalyzerRule
$expectedNumRules = 59
$expectedNumRules = 60
if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4))
{
# for PSv3 PSAvoidGlobalAliases is not shipped because
Expand Down
43 changes: 43 additions & 0 deletions Tests/Rules/AvoidLongLines.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
$ruleName = "PSAvoidLongLines"

$ruleSettings = @{
Enable = $true
}
$settings = @{
IncludeRules = @($ruleName)
Rules = @{ $ruleName = $ruleSettings }
}

Describe "AvoidLongLines" {
it 'Should be off by default' {
$def = "a" * 500
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def
$violations.Count | Should -Be 0
}

it 'Should find a violation when a line is longer than 120 characters (no whitespace)' {
$def = "a" * 125
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should -Be 1
}

it 'Should find a violation when a line is longer than 120 characters (leading whitespace)' {
$def = " " * 100 + "a" * 25
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should -Be 1
}

it 'Should not find a violation for lines under 120 characters' {
$def = "a" * 120
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should -Be 0
}

it 'Should find a violation with a configured line length' {
$ruleSettings.Add('LineLength', 10)
$settings['Rules'] = @{ $ruleName = $ruleSettings }
$def = "a" * 15
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
$violations.Count | Should -Be 1
}
}