Add the AvoidSemicolonsAsLineTerminators rule to warn about lines ending with a semicolon. Fix (#824)

Engine/Formatter.cs

@@ -45,6 +45,7 @@ public static string Format<TCmdlet>(
                 "PSUseCorrectCasing",
                 "PSAvoidUsingCmdletAliases",
                 "PSAvoidUsingDoubleQuotesForConstantString",
+                "PSAvoidSemicolonsAsLineTerminators",
             };
 
             var text = new EditableText(scriptDefinition);

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
+{
+    /// <summary>
+    /// AvoidSemicolonsAsLineTerminators: Checks for lines that end with a semicolon.
+    /// </summary>
+#if !CORECLR
+    [Export(typeof(IScriptRule))]
+#endif
+    public class AvoidSemicolonsAsLineTerminators : ConfigurableRule
+    {
+        /// <summary>
+        /// Construct an object of AvoidSemicolonsAsLineTerminators type.
+        /// </summary>
+        public AvoidSemicolonsAsLineTerminators()
+        {
+            Enable = false;
+        }
+
+        /// <summary>
+        /// Checks for lines that end with a semicolon.
+        /// </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>The diagnostic results of this rule</returns>
+        public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null)
+            {
+                throw new ArgumentNullException(nameof(ast));
+            }
+
+
+            var diagnosticRecords = new List<DiagnosticRecord>();
+
+            IEnumerable<Ast> 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<CorrectionExtent>();
+                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;
+        }
+
+        /// <summary>
+        /// Retrieves the common name of this rule.
+        /// </summary>
+        public override string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsCommonName);
+        }
+
+        /// <summary>
+        /// Retrieves the description of this rule.
+        /// </summary>
+        public override string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidSemicolonsAsLineTerminatorsDescription);
+        }
+
+        /// <summary>
+        /// Retrieves the name of this rule.
+        /// </summary>
+        public override string GetName()
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.AvoidSemicolonsAsLineTerminatorsName);
+        }
+
+        /// <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;
+        }
+    }
+}

Rules/Strings.resx

@@ -921,6 +921,18 @@
   <data name="AvoidLongLinesError" xml:space="preserve">
     <value>Line exceeds the configured maximum length of {0} characters</value>
   </data>
+  <data name="AvoidSemicolonsAsLineTerminatorsName" xml:space="preserve">
+    <value>AvoidSemicolonsAsLineTerminators</value>
+  </data>
+  <data name="AvoidSemicolonsAsLineTerminatorsCommonName" xml:space="preserve">
+    <value>Avoid semicolons as line terminators</value>
+  </data>
+  <data name="AvoidSemicolonsAsLineTerminatorsDescription" xml:space="preserve">
+    <value>Line should not end with a semicolon</value>
+  </data>
+  <data name="AvoidSemicolonsAsLineTerminatorsError" xml:space="preserve">
+    <value>Line ends with a semicolon</value>
+  </data>
   <data name="PlaceOpenBraceName" xml:space="preserve">
     <value>PlaceOpenBrace</value>
   </data>

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

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