Add the AvoidExclaimOperator rule to warn about the use of the ! negation operator. Fixes #1826

Engine/Formatter.cs

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

Rules/AvoidExclaimOperator.cs

@@ -0,0 +1,147 @@
+// 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;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// AvoidExclaimOperator: Checks for use of the exclaim operator
+    /// </summary>
+#if !CORECLR
+    [Export(typeof(IScriptRule))]
+#endif
+    public class AvoidExclaimOperator : ConfigurableRule
+    {
+
+        /// <summary>
+        /// Construct an object of AvoidExclaimOperator type.
+        /// </summary>
+        public AvoidExclaimOperator() {
+            Enable = false;
+        }
+
+        /// <summary>
+        /// Analyzes the given ast to find the [violation]
+        /// </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(Strings.NullAstErrorMessage);
+
+            var diagnosticRecords = new List<DiagnosticRecord>();
+
+            IEnumerable<Ast> foundAsts = ast.FindAll(testAst => testAst is UnaryExpressionAst, true);
+            if (foundAsts != null) {
+                var correctionDescription = Strings.AvoidExclaimOperatorCorrectionDescription;
+                foreach (UnaryExpressionAst unaryExpressionAst in foundAsts) {
+                    if (unaryExpressionAst.TokenKind == TokenKind.Exclaim) {
+                        var replaceWith = "-not";
+                        // The UnaryExpressionAST should have a single child, the argument that the unary operator is acting upon.
+                        // If the child's extent starts 1 after the parent's extent then there's no whitespace between the exclaim
+                        // token and any variable/expression; in that case the replacement -not should include a space
+                        if (unaryExpressionAst.Child != null && unaryExpressionAst.Child.Extent.StartColumnNumber == unaryExpressionAst.Extent.StartColumnNumber + 1) {
+                            replaceWith = "-not ";
+                        }
+                        var corrections = new List<CorrectionExtent> {
+                            new CorrectionExtent(
+                                unaryExpressionAst.Extent.StartLineNumber,
+                                unaryExpressionAst.Extent.EndLineNumber,
+                                unaryExpressionAst.Extent.StartColumnNumber,
+                                unaryExpressionAst.Extent.StartColumnNumber + 1,
+                                replaceWith,
+                                fileName,
+                                correctionDescription
+                            )
+                        };
+                        diagnosticRecords.Add(new DiagnosticRecord(
+                                string.Format(
+                                    CultureInfo.CurrentCulture, 
+                                    Strings.AvoidExclaimOperatorError
+                                ), 
+                                unaryExpressionAst.Extent, 
+                                GetName(),
+                                GetDiagnosticSeverity(), 
+                                fileName,
+                                suggestedCorrections: corrections
+                        ));
+                    }
+                }
+            }
+            return diagnosticRecords;
+        }
+
+        /// <summary>
+        /// Retrieves the common name of this rule.
+        /// </summary>
+        public override string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorCommonName);
+        }
+
+        /// <summary>
+        /// Retrieves the description of this rule.
+        /// </summary>
+        public override string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorDescription);
+        }
+
+        /// <summary>
+        /// Retrieves the name of this rule.
+        /// </summary>
+        public override string GetName()
+        {
+            return string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.AvoidExclaimOperatorName);
+        }
+
+        /// <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

@@ -1191,4 +1191,19 @@
   <data name="UseCorrectCasingParameterError" xml:space="preserve">
     <value>Parameter '{0}' of function/cmdlet '{1}' does not match its exact casing '{2}'.</value>
   </data>
-</root>
+  <data name="AvoidExclaimOperatorName" xml:space="preserve">
+    <value>AvoidExclaimOperator</value>
+  </data>
+  <data name="AvoidExclaimOperatorCommonName" xml:space="preserve">
+    <value>Avoid exclaim operator</value>
+  </data>
+  <data name="AvoidExclaimOperatorDescription" xml:space="preserve">
+    <value>The negation operator ! should not be used for readability purposes. Use -not instead.</value>
+  </data>
+  <data name="AvoidExclaimOperatorError" xml:space="preserve">
+    <value>Avoid using the ! negation operator</value>
+  </data>
+  <data name="AvoidExclaimOperatorCorrectionDescription" xml:space="preserve">
+    <value>Replace ! with -not</value>
+  </data>
+</root>

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

@@ -63,7 +63,7 @@ Describe "Test Name parameters" {
 
         It "get Rules with no parameters supplied" {
             $defaultRules = Get-ScriptAnalyzerRule
-            $expectedNumRules = 68
+            $expectedNumRules = 69
             if ($PSVersionTable.PSVersion.Major -le 4)
             {
                 # for PSv3 PSAvoidGlobalAliases is not shipped because

Tests/Rules/AvoidExclaimOperator.tests.ps1

@@ -0,0 +1,54 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+BeforeAll {
+    $ruleName = "PSAvoidExclaimOperator"
+
+    $ruleSettings = @{
+        Enable = $true
+    }
+    $settings = @{
+        IncludeRules = @($ruleName)
+        Rules        = @{ $ruleName = $ruleSettings }
+    }
+}
+
+Describe "AvoidExclaimOperator" {
+    Context "When the rule is not enabled explicitly" {
+        It "Should not find violations" {
+            $def = '!$true'
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def
+            $violations.Count | Should -Be 0
+        }
+    }
+
+    Context "Given a line with the exclaim operator" {
+        It "Should find one violation" {
+            $def = '!$true'
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
+            $violations.Count | Should -Be 1
+        }
+    }
+
+    Context "Given a line with the exclaim operator" {
+        It "Should replace the exclaim operator with the -not operator" {
+            $def = '!$true'
+            $expected = '-not $true'
+            Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
+        }
+    }
+    Context "Given a line with the exlaim operator followed by a space" {
+        It "Should replace the exclaim operator without adding an additional space" {
+            $def = '! $true'
+            $expected = '-not $true'
+            Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
+        }
+    }
+    Context "Given a line with a string containing an exclamation point" {
+        It "Should not replace it" {
+            $def = '$MyVar = "Should not replace!"'
+            $expected = '$MyVar = "Should not replace!"'
+            Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
+        }
+    }
+}

docs/Rules/AvoidExclaimOperator.md

@@ -0,0 +1,44 @@
+---
+description: Avoid exclaim operator
+ms.custom: PSSA v1.21.0
+ms.date: 06/14/2023
+ms.topic: reference
+title: AvoidExclaimOperator
+---
+# AvoidExclaimOperator
+**Severity Level: Warning**
+
+## Description
+
+The negation operator `!` should not be used for readability purposes. Use `-not` instead.
+
+**Note**: This rule is not enabled by default. The user needs to enable it through settings.
+
+## How to Fix
+
+## Example
+### Wrong：
+```PowerShell
+$MyVar = !$true
+```
+
+### Correct:
+```PowerShell
+$MyVar = -not $true
+```
+
+## Configuration
+
+```powershell
+Rules = @{
+    PSAvoidExclaimOperator  = @{
+        Enable = $true
+    }
+}
+```
+
+### Parameters
+
+#### Enable: bool (Default value is `$false`)
+
+Enable or disable the rule during ScriptAnalyzer invocation.

docs/Rules/README.md

@@ -15,6 +15,7 @@ The PSScriptAnalyzer contains the following rule definitions.
 | [AvoidAssignmentToAutomaticVariable](./AvoidAssignmentToAutomaticVariable.md)                     | Warning     |        Yes         |                 |
 | [AvoidDefaultValueForMandatoryParameter](./AvoidDefaultValueForMandatoryParameter.md)             | Warning     |        Yes         |                 |
 | [AvoidDefaultValueSwitchParameter](./AvoidDefaultValueSwitchParameter.md)                         | Warning     |        Yes         |                 |
+| [AvoidExclaimOperator](./AvoidExclaimOperator.md)                                                 | Warning     |         No         |                 |
 | [AvoidGlobalAliases<sup>1</sup>](./AvoidGlobalAliases.md)                                         | Warning     |        Yes         |                 |
 | [AvoidGlobalFunctions](./AvoidGlobalFunctions.md)                                                 | Warning     |        Yes         |                 |
 | [AvoidGlobalVars](./AvoidGlobalVars.md)                                                           | Warning     |        Yes         |                 |