Skip to content

Commit

Permalink
Added the AvoidExclamationPointOperator rule to warn about the use of…
Browse files Browse the repository at this point in the history
… the negation operator !. Fixes PowerShell#1826
  • Loading branch information
liamjpeters committed Jun 14, 2023
1 parent 5ba330a commit fd926e3
Show file tree
Hide file tree
Showing 8 changed files with 314 additions and 3 deletions.
1 change: 1 addition & 0 deletions Engine/Formatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public static string Format<TCmdlet>(
"PSAvoidUsingCmdletAliases",
"PSAvoidUsingDoubleQuotesForConstantString",
"PSAvoidSemicolonsAsLineTerminators",
"PSAvoidExclamationPointOperator",
};

var text = new EditableText(scriptDefinition);
Expand Down
151 changes: 151 additions & 0 deletions Rules/AvoidExclamationPointOperator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// Copyright (c) Microsoft Corporation.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

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>
/// AvoidExclamationPointOperator: Checks for use of the exclamation point operator
/// </summary>
#if !CORECLR
[Export(typeof(IScriptRule))]
#endif
public class AvoidExclamationPointOperator : ConfigurableRule
{

/// <summary>
/// Construct an object of AvoidExclamationPointOperator type.
/// </summary>
public AvoidExclamationPointOperator() {
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.AvoidExclamationPointOperatorCorrectionDescription;
foreach (UnaryExpressionAst foundAst in foundAsts) {
if (foundAst.TokenKind == TokenKind.Exclaim) {
// If the exclaim is not followed by a space, add one
var replaceWith = "-not";
if (foundAst.Child != null && foundAst.Child.Extent.StartColumnNumber == foundAst.Extent.StartColumnNumber + 1) {
replaceWith = "-not ";
}
var corrections = new List<CorrectionExtent> {
new CorrectionExtent(
foundAst.Extent.StartLineNumber,
foundAst.Extent.EndLineNumber,
foundAst.Extent.StartColumnNumber,
foundAst.Extent.StartColumnNumber + 1,
replaceWith,
fileName,
CorrectionDescription
)
};
diagnosticRecords.Add(new DiagnosticRecord(
string.Format(
CultureInfo.CurrentCulture,
Strings.AvoidExclamationPointOperatorError
),
foundAst.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.AvoidExclamationPointOperatorCommonName);
}

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

/// <summary>
/// Retrieves the name of this rule.
/// </summary>
public override string GetName()
{
return string.Format(
CultureInfo.CurrentCulture,
Strings.NameSpaceFormat,
GetSourceName(),
Strings.AvoidExclamationPointOperatorName);
}

/// <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;
}
}
}
47 changes: 46 additions & 1 deletion Rules/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 16 additions & 1 deletion Rules/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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="AvoidExclamationPointOperatorName" xml:space="preserve">
<value>AvoidExclamationPointOperator</value>
</data>
<data name="AvoidExclamationPointOperatorCommonName" xml:space="preserve">
<value>Avoid exclamation point operator</value>
</data>
<data name="AvoidExclamationPointOperatorDescription" xml:space="preserve">
<value>The negation operator ! should not be used. Use -not instead.</value>
</data>
<data name="AvoidExclamationPointOperatorError" xml:space="preserve">
<value>Avoid using the ! operator</value>
</data>
<data name="AvoidExclamationPointOperatorCorrectionDescription" xml:space="preserve">
<value>Replace ! with -not</value>
</data>
</root>
2 changes: 1 addition & 1 deletion Tests/Engine/GetScriptAnalyzerRule.tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions Tests/Rules/AvoidExclamationPointOperator.tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

BeforeAll {
$ruleName = "PSAvoidExclamationPointOperator"

$ruleSettings = @{
Enable = $true
}
$settings = @{
IncludeRules = @($ruleName)
Rules = @{ $ruleName = $ruleSettings }
}
}

Describe "AvoidExclamationPointOperator" {
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 exclamation point 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 exclamation point operator" {
It "Should replace the exclamation point 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 exclamation point operator followed by a space" {
It "Should replace the exclamation point 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
}
}
}
44 changes: 44 additions & 0 deletions docs/Rules/AvoidExclamationPointOperator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
description: Avoid exclamation point operator
ms.custom: PSSA v1.21.0
ms.date: 06/14/2023
ms.topic: reference
title: AvoidExclamationPointOperator
---
# AvoidExclamationPointOperator
**Severity Level: Warning**

## Description

The negation operator ! should not be used. 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 = @{
PSAvoidExclamationPointOperator = @{
Enable = $true
}
}
```

### Parameters

#### Enable: bool (Default value is `$false`)

Enable or disable the rule during ScriptAnalyzer invocation.
1 change: 1 addition & 0 deletions docs/Rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
| [AvoidExclamationPointOperator](./AvoidExclamationPointOperator.md) | Warning | No | |
| [AvoidGlobalAliases<sup>1</sup>](./AvoidGlobalAliases.md) | Warning | Yes | |
| [AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | Yes | |
| [AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | Yes | |
Expand Down

0 comments on commit fd926e3

Please sign in to comment.