Skip to content
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

Added rule: AvoidLongLines #1329

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

**Severity Level: Warning**

## Description

Lines should be no longer than 120 characters, including leading whitespace (indentation).
142 changes: 142 additions & 0 deletions Rules/AvoidLongLines.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
// 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 : IScriptRule
{
/// <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 IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast == null)
{
throw new ArgumentNullException("ast");
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved
}

var diagnosticRecords = new List<DiagnosticRecord>();

string[] lines = Regex.Split(ast.Extent.Text, @"\r?\n");
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved

for (int lineNumber = 0; lineNumber < lines.Length; lineNumber++)
{
var line = lines[lineNumber];
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved

if (line.Length > 120)
{
var startLine = lineNumber + 1;
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved
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 string GetCommonName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidLongLinesCommonName);
}

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

/// <summary>
/// Retrieves the name of this rule.
/// </summary>
public 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 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 string GetSourceName()
{
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}

/// <summary>
/// Retrieves the type of the rule, Builtin, Managed or Module.
/// </summary>
public 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 no longer than 120 characters.</value>
</data>
<data name="AvoidLongLinesError" xml:space="preserve">
<value>Line is longer than 120 characters which is too long</value>
</data>
<data name="PlaceOpenBraceName" xml:space="preserve">
<value>PlaceOpenBrace</value>
</data>
Expand Down
25 changes: 25 additions & 0 deletions Tests/Rules/AvoidLongLines.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
$ruleName = "PSAvoidLongLines"

$settings = @{
IncludeRules = @($ruleName)
}

Describe "AvoidLongLines" {
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
thomasrayner marked this conversation as resolved.
Show resolved Hide resolved
}

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
}
}