Skip to content

Commit b0383ee

Browse files
liamjpetersbergmeister
liamjpeters
and
bergmeister
authored
Add the AvoidExclaimOperator rule to warn about the use of the ! negation operator. Fixes #1826 (#1922)
* Added the AvoidExclamationPointOperator rule to warn about the use of the negation operator !. Fixes #1826 * Updated license header of Rules/AvoidExclamationPointOperator.cs * Updated the description of the rule in docs Co-authored-by: Christoph Bergmeister <[email protected]> * Fix alignment Co-authored-by: Christoph Bergmeister <[email protected]> * Update Rules/Strings.resx Co-authored-by: Christoph Bergmeister <[email protected]> * Renamed rule from AvoidExclamationPointOperator to AvoidExclaimOperator * Added comment explanation of the replacement suggestion logic and renamed loop variable for clarity --------- Co-authored-by: Christoph Bergmeister <[email protected]>
1 parent 4b976a2 commit b0383ee

8 files changed

+310
-3
lines changed

Diff for: ‎Engine/Formatter.cs

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public static string Format<TCmdlet>(
4646
"PSAvoidUsingCmdletAliases",
4747
"PSAvoidUsingDoubleQuotesForConstantString",
4848
"PSAvoidSemicolonsAsLineTerminators",
49+
"PSAvoidExclaimOperator",
4950
};
5051

5152
var text = new EditableText(scriptDefinition);

Diff for: ‎Rules/AvoidExclaimOperator.cs

+147
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
#if !CORECLR
7+
using System.ComponentModel.Composition;
8+
#endif
9+
using System.Globalization;
10+
using System.Linq;
11+
using System.Management.Automation;
12+
using System.Management.Automation.Language;
13+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
14+
15+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
16+
{
17+
/// <summary>
18+
/// AvoidExclaimOperator: Checks for use of the exclaim operator
19+
/// </summary>
20+
#if !CORECLR
21+
[Export(typeof(IScriptRule))]
22+
#endif
23+
public class AvoidExclaimOperator : ConfigurableRule
24+
{
25+
26+
/// <summary>
27+
/// Construct an object of AvoidExclaimOperator type.
28+
/// </summary>
29+
public AvoidExclaimOperator() {
30+
Enable = false;
31+
}
32+
33+
/// <summary>
34+
/// Analyzes the given ast to find the [violation]
35+
/// </summary>
36+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
37+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
38+
/// <returns>A an enumerable type containing the violations</returns>
39+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
40+
{
41+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
42+
43+
var diagnosticRecords = new List<DiagnosticRecord>();
44+
45+
IEnumerable<Ast> foundAsts = ast.FindAll(testAst => testAst is UnaryExpressionAst, true);
46+
if (foundAsts != null) {
47+
var correctionDescription = Strings.AvoidExclaimOperatorCorrectionDescription;
48+
foreach (UnaryExpressionAst unaryExpressionAst in foundAsts) {
49+
if (unaryExpressionAst.TokenKind == TokenKind.Exclaim) {
50+
var replaceWith = "-not";
51+
// The UnaryExpressionAST should have a single child, the argument that the unary operator is acting upon.
52+
// If the child's extent starts 1 after the parent's extent then there's no whitespace between the exclaim
53+
// token and any variable/expression; in that case the replacement -not should include a space
54+
if (unaryExpressionAst.Child != null && unaryExpressionAst.Child.Extent.StartColumnNumber == unaryExpressionAst.Extent.StartColumnNumber + 1) {
55+
replaceWith = "-not ";
56+
}
57+
var corrections = new List<CorrectionExtent> {
58+
new CorrectionExtent(
59+
unaryExpressionAst.Extent.StartLineNumber,
60+
unaryExpressionAst.Extent.EndLineNumber,
61+
unaryExpressionAst.Extent.StartColumnNumber,
62+
unaryExpressionAst.Extent.StartColumnNumber + 1,
63+
replaceWith,
64+
fileName,
65+
correctionDescription
66+
)
67+
};
68+
diagnosticRecords.Add(new DiagnosticRecord(
69+
string.Format(
70+
CultureInfo.CurrentCulture,
71+
Strings.AvoidExclaimOperatorError
72+
),
73+
unaryExpressionAst.Extent,
74+
GetName(),
75+
GetDiagnosticSeverity(),
76+
fileName,
77+
suggestedCorrections: corrections
78+
));
79+
}
80+
}
81+
}
82+
return diagnosticRecords;
83+
}
84+
85+
/// <summary>
86+
/// Retrieves the common name of this rule.
87+
/// </summary>
88+
public override string GetCommonName()
89+
{
90+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorCommonName);
91+
}
92+
93+
/// <summary>
94+
/// Retrieves the description of this rule.
95+
/// </summary>
96+
public override string GetDescription()
97+
{
98+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorDescription);
99+
}
100+
101+
/// <summary>
102+
/// Retrieves the name of this rule.
103+
/// </summary>
104+
public override string GetName()
105+
{
106+
return string.Format(
107+
CultureInfo.CurrentCulture,
108+
Strings.NameSpaceFormat,
109+
GetSourceName(),
110+
Strings.AvoidExclaimOperatorName);
111+
}
112+
113+
/// <summary>
114+
/// Retrieves the severity of the rule: error, warning or information.
115+
/// </summary>
116+
public override RuleSeverity GetSeverity()
117+
{
118+
return RuleSeverity.Warning;
119+
}
120+
121+
/// <summary>
122+
/// Gets the severity of the returned diagnostic record: error, warning, or information.
123+
/// </summary>
124+
/// <returns></returns>
125+
public DiagnosticSeverity GetDiagnosticSeverity()
126+
{
127+
return DiagnosticSeverity.Warning;
128+
}
129+
130+
/// <summary>
131+
/// Retrieves the name of the module/assembly the rule is from.
132+
/// </summary>
133+
public override string GetSourceName()
134+
{
135+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
136+
}
137+
138+
/// <summary>
139+
/// Retrieves the type of the rule, Builtin, Managed or Module.
140+
/// </summary>
141+
public override SourceType GetSourceType()
142+
{
143+
return SourceType.Builtin;
144+
}
145+
}
146+
}
147+

Diff for: ‎Rules/Strings.Designer.cs

+46-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎Rules/Strings.resx

+16-1
Original file line numberDiff line numberDiff line change
@@ -1191,4 +1191,19 @@
11911191
<data name="UseCorrectCasingParameterError" xml:space="preserve">
11921192
<value>Parameter '{0}' of function/cmdlet '{1}' does not match its exact casing '{2}'.</value>
11931193
</data>
1194-
</root>
1194+
<data name="AvoidExclaimOperatorName" xml:space="preserve">
1195+
<value>AvoidExclaimOperator</value>
1196+
</data>
1197+
<data name="AvoidExclaimOperatorCommonName" xml:space="preserve">
1198+
<value>Avoid exclaim operator</value>
1199+
</data>
1200+
<data name="AvoidExclaimOperatorDescription" xml:space="preserve">
1201+
<value>The negation operator ! should not be used for readability purposes. Use -not instead.</value>
1202+
</data>
1203+
<data name="AvoidExclaimOperatorError" xml:space="preserve">
1204+
<value>Avoid using the ! negation operator</value>
1205+
</data>
1206+
<data name="AvoidExclaimOperatorCorrectionDescription" xml:space="preserve">
1207+
<value>Replace ! with -not</value>
1208+
</data>
1209+
</root>

Diff for: ‎Tests/Engine/GetScriptAnalyzerRule.tests.ps1

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Describe "Test Name parameters" {
6363

6464
It "get Rules with no parameters supplied" {
6565
$defaultRules = Get-ScriptAnalyzerRule
66-
$expectedNumRules = 68
66+
$expectedNumRules = 69
6767
if ($PSVersionTable.PSVersion.Major -le 4)
6868
{
6969
# for PSv3 PSAvoidGlobalAliases is not shipped because

Diff for: ‎Tests/Rules/AvoidExclaimOperator.tests.ps1

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
BeforeAll {
5+
$ruleName = "PSAvoidExclaimOperator"
6+
7+
$ruleSettings = @{
8+
Enable = $true
9+
}
10+
$settings = @{
11+
IncludeRules = @($ruleName)
12+
Rules = @{ $ruleName = $ruleSettings }
13+
}
14+
}
15+
16+
Describe "AvoidExclaimOperator" {
17+
Context "When the rule is not enabled explicitly" {
18+
It "Should not find violations" {
19+
$def = '!$true'
20+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def
21+
$violations.Count | Should -Be 0
22+
}
23+
}
24+
25+
Context "Given a line with the exclaim operator" {
26+
It "Should find one violation" {
27+
$def = '!$true'
28+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings
29+
$violations.Count | Should -Be 1
30+
}
31+
}
32+
33+
Context "Given a line with the exclaim operator" {
34+
It "Should replace the exclaim operator with the -not operator" {
35+
$def = '!$true'
36+
$expected = '-not $true'
37+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
38+
}
39+
}
40+
Context "Given a line with the exlaim operator followed by a space" {
41+
It "Should replace the exclaim operator without adding an additional space" {
42+
$def = '! $true'
43+
$expected = '-not $true'
44+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
45+
}
46+
}
47+
Context "Given a line with a string containing an exclamation point" {
48+
It "Should not replace it" {
49+
$def = '$MyVar = "Should not replace!"'
50+
$expected = '$MyVar = "Should not replace!"'
51+
Invoke-Formatter -ScriptDefinition $def -Settings $settings | Should -Be $expected
52+
}
53+
}
54+
}

Diff for: ‎docs/Rules/AvoidExclaimOperator.md

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
description: Avoid exclaim operator
3+
ms.custom: PSSA v1.21.0
4+
ms.date: 06/14/2023
5+
ms.topic: reference
6+
title: AvoidExclaimOperator
7+
---
8+
# AvoidExclaimOperator
9+
**Severity Level: Warning**
10+
11+
## Description
12+
13+
The negation operator `!` should not be used for readability purposes. Use `-not` instead.
14+
15+
**Note**: This rule is not enabled by default. The user needs to enable it through settings.
16+
17+
## How to Fix
18+
19+
## Example
20+
### Wrong：
21+
```PowerShell
22+
$MyVar = !$true
23+
```
24+
25+
### Correct:
26+
```PowerShell
27+
$MyVar = -not $true
28+
```
29+
30+
## Configuration
31+
32+
```powershell
33+
Rules = @{
34+
PSAvoidExclaimOperator = @{
35+
Enable = $true
36+
}
37+
}
38+
```
39+
40+
### Parameters
41+
42+
#### Enable: bool (Default value is `$false`)
43+
44+
Enable or disable the rule during ScriptAnalyzer invocation.

Diff for: ‎docs/Rules/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The PSScriptAnalyzer contains the following rule definitions.
1515
| [AvoidAssignmentToAutomaticVariable](./AvoidAssignmentToAutomaticVariable.md) | Warning | Yes | |
1616
| [AvoidDefaultValueForMandatoryParameter](./AvoidDefaultValueForMandatoryParameter.md) | Warning | Yes | |
1717
| [AvoidDefaultValueSwitchParameter](./AvoidDefaultValueSwitchParameter.md) | Warning | Yes | |
18+
| [AvoidExclaimOperator](./AvoidExclaimOperator.md) | Warning | No | |
1819
| [AvoidGlobalAliases<sup>1</sup>](./AvoidGlobalAliases.md) | Warning | Yes | |
1920
| [AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | Yes | |
2021
| [AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | Yes | |

0 commit comments

Comments
 (0)