diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index 251060a6d..3c1325a5d 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -45,6 +45,7 @@ public static string Format( "PSUseCorrectCasing", "PSAvoidUsingCmdletAliases", "PSAvoidUsingDoubleQuotesForConstantString", + "PSAvoidSemicolonsAsLineTerminators", }; var text = new EditableText(scriptDefinition); diff --git a/Rules/AvoidSemicolonsAsLineTerminators.cs b/Rules/AvoidSemicolonsAsLineTerminators.cs new file mode 100644 index 000000000..68a121d77 --- /dev/null +++ b/Rules/AvoidSemicolonsAsLineTerminators.cs @@ -0,0 +1,175 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; +using System.Linq; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidSemicolonsAsLineTerminators: Checks for lines that end with a semicolon. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class AvoidSemicolonsAsLineTerminators : ConfigurableRule + { + /// + /// Construct an object of AvoidSemicolonsAsLineTerminators type. + /// + public AvoidSemicolonsAsLineTerminators() + { + Enable = false; + } + + /// + /// Checks for lines that end with a semicolon. + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// The diagnostic results of this rule + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) + { + throw new ArgumentNullException(nameof(ast)); + } + + + var diagnosticRecords = new List(); + + IEnumerable assignmentStatements = ast.FindAll(item => item is AssignmentStatementAst, true); + + var tokens = Helper.Instance.Tokens; + for (int tokenIndex = 0; tokenIndex < tokens.Length; tokenIndex++) + { + + var token = tokens[tokenIndex]; + var semicolonTokenExtent = token.Extent; + + var isSemicolonToken = token.Kind is TokenKind.Semi; + if (!isSemicolonToken) + { + continue; + } + + var isPartOfAnyAssignmentStatement = assignmentStatements.Any(assignmentStatement => (assignmentStatement.Extent.EndOffset == semicolonTokenExtent.StartOffset + 1)); + if (isPartOfAnyAssignmentStatement) + { + continue; + } + + var nextTokenIndex = tokenIndex + 1; + var isNextTokenIsNewLine = tokens[nextTokenIndex].Kind is TokenKind.NewLine; + var isNextTokenIsEndOfInput = tokens[nextTokenIndex].Kind is TokenKind.EndOfInput; + + if (!isNextTokenIsNewLine && !isNextTokenIsEndOfInput) + { + continue; + } + + var violationExtent = new ScriptExtent( + new ScriptPosition( + ast.Extent.File, + semicolonTokenExtent.StartLineNumber, + semicolonTokenExtent.StartColumnNumber, + semicolonTokenExtent.StartScriptPosition.Line + ), + new ScriptPosition( + ast.Extent.File, + semicolonTokenExtent.EndLineNumber, + semicolonTokenExtent.EndColumnNumber, + semicolonTokenExtent.EndScriptPosition.Line + )); + + var suggestedCorrections = new List(); + suggestedCorrections.Add(new CorrectionExtent( + violationExtent, + string.Empty, + ast.Extent.File + )); + + var record = new DiagnosticRecord( + String.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsError), + violationExtent, + GetName(), + GetDiagnosticSeverity(), + ast.Extent.File, + null, + suggestedCorrections + ); + diagnosticRecords.Add(record); + } + + return diagnosticRecords; + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidSemicolonsAsLineTerminatorsName); + } + + /// + /// 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 9fead171c..42c1d79af 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -465,6 +465,42 @@ internal static string AvoidLongLinesName { } } + /// + /// Looks up a localized string similar to Avoid semicolons as line terminators. + /// + internal static string AvoidSemicolonsAsLineTerminatorsCommonName { + get { + return ResourceManager.GetString("AvoidSemicolonsAsLineTerminatorsCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Line should not end with a semicolon. + /// + internal static string AvoidSemicolonsAsLineTerminatorsDescription { + get { + return ResourceManager.GetString("AvoidSemicolonsAsLineTerminatorsDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Line ends with a semicolon. + /// + internal static string AvoidSemicolonsAsLineTerminatorsError { + get { + return ResourceManager.GetString("AvoidSemicolonsAsLineTerminatorsError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidSemicolonsAsLineTerminators. + /// + internal static string AvoidSemicolonsAsLineTerminatorsName { + get { + return ResourceManager.GetString("AvoidSemicolonsAsLineTerminatorsName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid multiple type specifiers on parameters. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index da1032671..b0cf08518 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -921,6 +921,18 @@ Line exceeds the configured maximum length of {0} characters + + AvoidSemicolonsAsLineTerminators + + + Avoid semicolons as line terminators + + + Line should not end with a semicolon + + + Line ends with a semicolon + PlaceOpenBrace diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 50a6f0077..de9e6e326 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -63,7 +63,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 66 + $expectedNumRules = 67 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/AvoidSemicolonsAsLineTerminators.tests.ps1 b/Tests/Rules/AvoidSemicolonsAsLineTerminators.tests.ps1 new file mode 100644 index 000000000..9f88c1957 --- /dev/null +++ b/Tests/Rules/AvoidSemicolonsAsLineTerminators.tests.ps1 @@ -0,0 +1,135 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +BeforeAll { + $ruleName = "PSAvoidSemicolonsAsLineTerminators" + + $ruleSettings = @{ + Enable = $true + } + $settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = $ruleSettings } + } +} + +Describe "AvoidSemicolonsAsLineTerminators" { + Context "When the rule is not enabled explicitly" { + It "Should not find violations" { + $def = "'test';" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def + $violations.Count | Should -Be 0 + } + } + + Context "Given a line ending with a semicolon" { + It "Should find one violation" { + $def = "'test';" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 1 + } + } + + Context "Given a line with a semicolon in the middle and one at the end" { + It "Should find one violation" { + $def = "'test';'test';" + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations[0].Extent.StartLineNumber | Should -Be 1 + $violations[0].Extent.EndLineNumber | Should -Be 1 + $violations[0].Extent.StartColumnNumber | Should -Be 14 + $violations[0].Extent.EndColumnNumber | Should -Be 15 + } + } + + + Context "Given a multiline string with a line ending with a semicolon" { + It "Should get the correct extent of the violation for a single semicolon" { + $def = @" +'this line has no semicolon' +'test'; +"@ + $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 7 + $violations[0].Extent.EndColumnNumber | Should -Be 8 + } + } + + Context "Given a multiline string with a line having a semicolon in the middle" { + It "Should not find any violations" { + $def = @" +'this line has no semicolon' +'line with a semicolon; in the middle' +"@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 0 + } + } + + Context "Given a multiline string with a line having a semicolon in C# code" { + It "Should not find any violations" { + $def = @" +`$calcCode = `@" +public class Calc{ + public int Add(int x,int y){ + return x+y; + } +} +"`@ + +Add-Type -TypeDefinition `$calcCode -Language CSharp + +`$c = New-Object Calc +`$c.Add(1,2) +"@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 0 + } + } + + Context "Given a multiline string with a line having a semicolon in variable assignment" { + It "Should not find any violations" { + $def = @" +`$allowPopupsOption=`@" +lockPref("dom.disable_open_during_load", false); +"`@ + Write `$allowPopupsOption | Out-File -Encoding UTF8 -FilePath `$pathToMozillaCfg -Append +"@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings + $violations.Count | Should -Be 0 + } + } + + Context "Given a line ending with a semicolon" { + It "Should remove the semicolon at the end" { + $def = "'test';" + $expected = "'test'" + + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected + } + } + + Context "Given a line with a semicolon in the middle and one at the end" { + It "Should remove the semicolon at the end" { + $def = "'test';'test';" + $expected = "'test';'test'" + + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected + } + } + + Context "Given a multiline string with a line ending with a semicolon" { + It "Should remove the semicolon at the end of the line with a semicolon" { + $def = @" +'this line has no semicolon at the end' +'test'; +"@ + $expected = @" +'this line has no semicolon at the end' +'test' +"@ + Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected + } + } +} diff --git a/docs/Rules/AvoidSemicolonsAsLineTerminators.md b/docs/Rules/AvoidSemicolonsAsLineTerminators.md new file mode 100644 index 000000000..0ce1cbc7a --- /dev/null +++ b/docs/Rules/AvoidSemicolonsAsLineTerminators.md @@ -0,0 +1,56 @@ +--- +description: Avoid semicolons as line terminators +ms.custom: PSSA v1.21.0 +ms.date: 06/15/2022 +ms.topic: reference +title: AvoidSemicolonsAsLineTerminators +--- +# AvoidSemicolonsAsLineTerminators + +**Severity Level: Warning** + +## Description + +Lines should not end with a semicolon. + +**Note**: This rule is not enabled by default. The user needs to enable it through settings. + +## Example + +### Wrong + +```powershell +Install-Module -Name PSScriptAnalyzer; $a = 1 + $b; +``` + +```powershell +Install-Module -Name PSScriptAnalyzer; +$a = 1 + $b +``` + +### Correct + +```powershell +Install-Module -Name PSScriptAnalyzer; $a = 1 + $b +``` + +```powershell +Install-Module -Name PSScriptAnalyzer +$a = 1 + $b +``` + +## Configuration + +```powershell +Rules = @{ + PSAvoidSemicolonsAsLineTerminators = @{ + Enable = $true + } +} +``` + +### Parameters + +#### Enable: bool (Default value is `$false`) + +Enable or disable the rule during ScriptAnalyzer invocation. diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 6622ac679..3a838f3f9 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -23,6 +23,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidMultipleTypeAttributes1](./AvoidMultipleTypeAttributes.md) | Warning | Yes | | | [AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | Yes | | | [AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | Yes | Yes | +| [AvoidSemicolonsAsLineTerminators](./AvoidSemicolonsAsLineTerminators.md) | Warning | No | | | [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | | | [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | | | [AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md) | Warning | Yes | Yes2 |