diff --git a/RuleDocumentation/AvoidLongLines.md b/RuleDocumentation/AvoidLongLines.md new file mode 100644 index 000000000..0ea2c9a65 --- /dev/null +++ b/RuleDocumentation/AvoidLongLines.md @@ -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. + +#### MaximumLineLength: int (Default value is 120) + +Optional parameter to override the default maximum line length. diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md index 02f56a394..e9d3f66ce 100644 --- a/RuleDocumentation/README.md +++ b/RuleDocumentation/README.md @@ -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 | diff --git a/Rules/AvoidLongLines.cs b/Rules/AvoidLongLines.cs new file mode 100644 index 000000000..e0860a75e --- /dev/null +++ b/Rules/AvoidLongLines.cs @@ -0,0 +1,156 @@ +// 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 +{ + /// + /// AvoidLongLines: Checks for lines longer than 120 characters + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class AvoidLongLines : ConfigurableRule + { + /// + /// Construct an object of AvoidLongLines type. + /// + public AvoidLongLines() + { } + + [ConfigurableRuleProperty(defaultValue: 120)] + public int MaximumLineLength { get; set; } + + private readonly string[] s_lineSeparators = new[] { "\r\n", "\n" }; + + /// + /// Analyzes the given ast to find violations. + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) + { + throw new ArgumentNullException(nameof(ast)); + } + + var diagnosticRecords = new List(); + + string[] lines = ast.Extent.Text.Split(s_lineSeparators, StringSplitOptions.None); + + for (int lineNumber = 0; lineNumber < lines.Length; lineNumber++) + { + string line = lines[lineNumber]; + + if (line.Length <= MaximumLineLength) + { + continue; + } + + int startLine = lineNumber + 1; + int endLine = startLine; + int startColumn = 1; + int 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, + String.Format(Strings.AvoidLongLinesError, MaximumLineLength)), + violationExtent, + GetName(), + GetDiagnosticSeverity(), + ast.Extent.File, + null + ); + diagnosticRecords.Add(record); + } + + return diagnosticRecords; + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidLongLinesCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidLongLinesDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidLongLinesName); + } + + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 8c4f6fc5f..42d5a846e 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -536,6 +536,61 @@ internal static string AvoidTrailingWhitespaceName { return ResourceManager.GetString("AvoidTrailingWhitespaceName", resourceCulture); } } + + /// + /// Looks up a localized string similar to AvoidLongLines. + /// + internal static string AvoidLongLinesName + { + get + { + return ResourceManager.GetString("AvoidLongLinesName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid long lines. + /// + internal static string AvoidLongLinesCommonName + { + get + { + return ResourceManager.GetString("AvoidLongLinesCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Each line should be under 120 characters. + /// + internal static string AvoidLongLinesDescription + { + get + { + return ResourceManager.GetString("AvoidLongLinesDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Line is longer than 120 characters. + /// + internal static string AvoidLongLinesError + { + get + { + return ResourceManager.GetString("AvoidLongLinesError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidLongLines. + /// + internal static string AvoidLongLinesWhitespaceName + { + get + { + return ResourceManager.GetString("AvoidLongLinesName", resourceCulture); + } + } /// /// Looks up a localized string similar to Module Must Be Loadable. diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 4f6a29df3..dd5825e53 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -897,6 +897,18 @@ Line has trailing whitespace + + AvoidLongLines + + + Avoid long lines + + + Line lengths should be less than the configured maximum + + + Line exceeds the configured maximum length of {0} characters + PlaceOpenBrace diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 668590672..8b68c63f3 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -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 diff --git a/Tests/Rules/AvoidLongLines.tests.ps1 b/Tests/Rules/AvoidLongLines.tests.ps1 new file mode 100644 index 000000000..5a6c8e189 --- /dev/null +++ b/Tests/Rules/AvoidLongLines.tests.ps1 @@ -0,0 +1,61 @@ +$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" * 500) +this line is short enough +"@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + } + + it 'Should get the correct extent of the violation' { + $def = @" +this line is short enough +$("a" * 500) +"@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations[0].Extent.StartLineNumber | Should -Be 2 + $violations[0].Extent.EndLineNumber | Should -Be 2 + $violations[0].Extent.StartColumnNumber | Should -Be 1 + $violations[0].Extent.EndColumnNumber | Should -Be 500 + } + + it 'Should find a violation when a line is longer than 120 characters (leading whitespace)' { + $def = @" +$(" " * 100 + "a" * 25) +this line is short enough +"@ + $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('MaximumLineLength', 10) + $settings['Rules'] = @{ $ruleName = $ruleSettings } + $def = "a" * 15 + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + } +}