From fd926e365c0a7957085fe4024cd8029bbb0aabec Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Wed, 14 Jun 2023 12:32:28 +0100 Subject: [PATCH 1/7] Added the AvoidExclamationPointOperator rule to warn about the use of the negation operator !. Fixes #1826 --- Engine/Formatter.cs | 1 + Rules/AvoidExclamationPointOperator.cs | 151 ++++++++++++++++++ Rules/Strings.Designer.cs | 47 +++++- Rules/Strings.resx | 17 +- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- .../AvoidExclamationPointOperator.tests.ps1 | 54 +++++++ docs/Rules/AvoidExclamationPointOperator.md | 44 +++++ docs/Rules/README.md | 1 + 8 files changed, 314 insertions(+), 3 deletions(-) create mode 100644 Rules/AvoidExclamationPointOperator.cs create mode 100644 Tests/Rules/AvoidExclamationPointOperator.tests.ps1 create mode 100644 docs/Rules/AvoidExclamationPointOperator.md diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index 3c1325a5d..26e21c564 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -46,6 +46,7 @@ public static string Format( "PSAvoidUsingCmdletAliases", "PSAvoidUsingDoubleQuotesForConstantString", "PSAvoidSemicolonsAsLineTerminators", + "PSAvoidExclamationPointOperator", }; var text = new EditableText(scriptDefinition); diff --git a/Rules/AvoidExclamationPointOperator.cs b/Rules/AvoidExclamationPointOperator.cs new file mode 100644 index 000000000..43badbd9b --- /dev/null +++ b/Rules/AvoidExclamationPointOperator.cs @@ -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 +{ + /// + /// AvoidExclamationPointOperator: Checks for use of the exclamation point operator + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class AvoidExclamationPointOperator : ConfigurableRule + { + + /// + /// Construct an object of AvoidExclamationPointOperator type. + /// + public AvoidExclamationPointOperator() { + Enable = false; + } + + /// + /// Analyzes the given ast to find the [violation] + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + var diagnosticRecords = new List(); + + IEnumerable 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 { + 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; + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclamationPointOperatorCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclamationPointOperatorDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidExclamationPointOperatorName); + } + + /// + /// 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 30d0a7321..ec41beea5 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -285,6 +285,51 @@ internal static string AvoidEmptyCatchBlockError { } } + /// + /// Looks up a localized string similar to Avoid exclamation point operator. + /// + internal static string AvoidExclamationPointOperatorCommonName { + get { + return ResourceManager.GetString("AvoidExclamationPointOperatorCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Replace ! with -not. + /// + internal static string AvoidExclamationPointOperatorCorrectionDescription { + get { + return ResourceManager.GetString("AvoidExclamationPointOperatorCorrectionDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The negation operator ! should not be used. Use -not instead.. + /// + internal static string AvoidExclamationPointOperatorDescription { + get { + return ResourceManager.GetString("AvoidExclamationPointOperatorDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid using the ! operator. + /// + internal static string AvoidExclamationPointOperatorError { + get { + return ResourceManager.GetString("AvoidExclamationPointOperatorError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidExclamationPointOperator. + /// + internal static string AvoidExclamationPointOperatorName { + get { + return ResourceManager.GetString("AvoidExclamationPointOperatorName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid global aliases.. /// @@ -1132,7 +1177,7 @@ internal static string AvoidUsingPlainTextForPasswordDescription { } /// - /// Looks up a localized string similar to Parameter '{0}' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to to expose this sensitive information.. + /// Looks up a localized string similar to Parameter '{0}' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to expose this sensitive information.. /// internal static string AvoidUsingPlainTextForPasswordError { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index dbba6e570..b2f3a0c00 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1191,4 +1191,19 @@ Parameter '{0}' of function/cmdlet '{1}' does not match its exact casing '{2}'. - + + AvoidExclamationPointOperator + + + Avoid exclamation point operator + + + The negation operator ! should not be used. Use -not instead. + + + Avoid using the ! operator + + + Replace ! with -not + + \ No newline at end of file diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 0ca6e569f..960a2fcd5 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 = 68 + $expectedNumRules = 69 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/AvoidExclamationPointOperator.tests.ps1 b/Tests/Rules/AvoidExclamationPointOperator.tests.ps1 new file mode 100644 index 000000000..1956fddfc --- /dev/null +++ b/Tests/Rules/AvoidExclamationPointOperator.tests.ps1 @@ -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 + } + } +} \ No newline at end of file diff --git a/docs/Rules/AvoidExclamationPointOperator.md b/docs/Rules/AvoidExclamationPointOperator.md new file mode 100644 index 000000000..396c95534 --- /dev/null +++ b/docs/Rules/AvoidExclamationPointOperator.md @@ -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. \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 44062d7c3..0e1913ada 100644 --- a/docs/Rules/README.md +++ b/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 | | +| [AvoidExclamationPointOperator](./AvoidExclamationPointOperator.md) | Warning | No | | | [AvoidGlobalAliases1](./AvoidGlobalAliases.md) | Warning | Yes | | | [AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | Yes | | | [AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | Yes | | From 16ba41481336da0eec788db6d41b199ed455dbca Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Wed, 14 Jun 2023 12:44:18 +0100 Subject: [PATCH 2/7] Updated license header of Rules/AvoidExclamationPointOperator.cs --- Rules/AvoidExclamationPointOperator.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Rules/AvoidExclamationPointOperator.cs b/Rules/AvoidExclamationPointOperator.cs index 43badbd9b..ed9905550 100644 --- a/Rules/AvoidExclamationPointOperator.cs +++ b/Rules/AvoidExclamationPointOperator.cs @@ -1,12 +1,5 @@ -// 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. +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. using System; using System.Collections.Generic; From 01633c50e760827253751ea45276efa8e7caf6bb Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Sun, 18 Jun 2023 14:56:50 +0100 Subject: [PATCH 3/7] Updated the description of the rule in docs Co-authored-by: Christoph Bergmeister --- docs/Rules/AvoidExclamationPointOperator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Rules/AvoidExclamationPointOperator.md b/docs/Rules/AvoidExclamationPointOperator.md index 396c95534..47826fac2 100644 --- a/docs/Rules/AvoidExclamationPointOperator.md +++ b/docs/Rules/AvoidExclamationPointOperator.md @@ -10,7 +10,7 @@ title: AvoidExclamationPointOperator ## Description -The negation operator ! should not be used. Use -not instead. +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. From 1edbcf1cbda883311ccee5bd722cb013e382e671 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Sun, 18 Jun 2023 14:57:22 +0100 Subject: [PATCH 4/7] Fix alignment Co-authored-by: Christoph Bergmeister --- docs/Rules/AvoidExclamationPointOperator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Rules/AvoidExclamationPointOperator.md b/docs/Rules/AvoidExclamationPointOperator.md index 47826fac2..98d4b9d21 100644 --- a/docs/Rules/AvoidExclamationPointOperator.md +++ b/docs/Rules/AvoidExclamationPointOperator.md @@ -32,7 +32,7 @@ $MyVar = -not $true ```powershell Rules = @{ PSAvoidExclamationPointOperator = @{ - Enable = $true + Enable = $true } } ``` From 69af14f924769a53afd7c9667d4446e169c423a4 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Sun, 18 Jun 2023 14:58:07 +0100 Subject: [PATCH 5/7] Update Rules/Strings.resx Co-authored-by: Christoph Bergmeister --- Rules/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/Strings.resx b/Rules/Strings.resx index b2f3a0c00..7ea611a4f 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1201,7 +1201,7 @@ The negation operator ! should not be used. Use -not instead. - Avoid using the ! operator + Avoid using the ! negation operator Replace ! with -not From 5e1b9458fa5611ed99f37360c0d37e14b4b67545 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Sun, 18 Jun 2023 15:45:28 +0100 Subject: [PATCH 6/7] Renamed rule from AvoidExclamationPointOperator to AvoidExclaimOperator --- Engine/Formatter.cs | 2 +- ...intOperator.cs => AvoidExclaimOperator.cs} | 21 +++++++------- Rules/Strings.Designer.cs | 28 +++++++++---------- Rules/Strings.resx | 16 +++++------ ...sts.ps1 => AvoidExclaimOperator.tests.ps1} | 14 +++++----- ...intOperator.md => AvoidExclaimOperator.md} | 8 +++--- docs/Rules/README.md | 2 +- 7 files changed, 46 insertions(+), 45 deletions(-) rename Rules/{AvoidExclamationPointOperator.cs => AvoidExclaimOperator.cs} (87%) rename Tests/Rules/{AvoidExclamationPointOperator.tests.ps1 => AvoidExclaimOperator.tests.ps1} (74%) rename docs/Rules/{AvoidExclamationPointOperator.md => AvoidExclaimOperator.md} (80%) diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index 26e21c564..3db99e79d 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -46,7 +46,7 @@ public static string Format( "PSAvoidUsingCmdletAliases", "PSAvoidUsingDoubleQuotesForConstantString", "PSAvoidSemicolonsAsLineTerminators", - "PSAvoidExclamationPointOperator", + "PSAvoidExclaimOperator", }; var text = new EditableText(scriptDefinition); diff --git a/Rules/AvoidExclamationPointOperator.cs b/Rules/AvoidExclaimOperator.cs similarity index 87% rename from Rules/AvoidExclamationPointOperator.cs rename to Rules/AvoidExclaimOperator.cs index ed9905550..9a013953e 100644 --- a/Rules/AvoidExclamationPointOperator.cs +++ b/Rules/AvoidExclaimOperator.cs @@ -15,18 +15,18 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// AvoidExclamationPointOperator: Checks for use of the exclamation point operator + /// AvoidExclaimOperator: Checks for use of the exclaim operator /// #if !CORECLR [Export(typeof(IScriptRule))] #endif - public class AvoidExclamationPointOperator : ConfigurableRule + public class AvoidExclaimOperator : ConfigurableRule { /// - /// Construct an object of AvoidExclamationPointOperator type. + /// Construct an object of AvoidExclaimOperator type. /// - public AvoidExclamationPointOperator() { + public AvoidExclaimOperator() { Enable = false; } @@ -44,7 +44,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file IEnumerable foundAsts = ast.FindAll(testAst => testAst is UnaryExpressionAst, true); if (foundAsts != null) { - var CorrectionDescription = Strings.AvoidExclamationPointOperatorCorrectionDescription; + var correctionDescription = Strings.AvoidExclaimOperatorCorrectionDescription; foreach (UnaryExpressionAst foundAst in foundAsts) { if (foundAst.TokenKind == TokenKind.Exclaim) { // If the exclaim is not followed by a space, add one @@ -60,13 +60,13 @@ public override IEnumerable AnalyzeScript(Ast ast, string file foundAst.Extent.StartColumnNumber + 1, replaceWith, fileName, - CorrectionDescription + correctionDescription ) }; diagnosticRecords.Add(new DiagnosticRecord( string.Format( CultureInfo.CurrentCulture, - Strings.AvoidExclamationPointOperatorError + Strings.AvoidExclaimOperatorError ), foundAst.Extent, GetName(), @@ -85,7 +85,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file /// public override string GetCommonName() { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclamationPointOperatorCommonName); + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorCommonName); } /// @@ -93,7 +93,7 @@ public override string GetCommonName() /// public override string GetDescription() { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclamationPointOperatorDescription); + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorDescription); } /// @@ -105,7 +105,7 @@ public override string GetName() CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), - Strings.AvoidExclamationPointOperatorName); + Strings.AvoidExclaimOperatorName); } /// @@ -142,3 +142,4 @@ public override SourceType GetSourceType() } } } + diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index ec41beea5..f346e9084 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -286,47 +286,47 @@ internal static string AvoidEmptyCatchBlockError { } /// - /// Looks up a localized string similar to Avoid exclamation point operator. + /// Looks up a localized string similar to Avoid exclaim operator. /// - internal static string AvoidExclamationPointOperatorCommonName { + internal static string AvoidExclaimOperatorCommonName { get { - return ResourceManager.GetString("AvoidExclamationPointOperatorCommonName", resourceCulture); + return ResourceManager.GetString("AvoidExclaimOperatorCommonName", resourceCulture); } } /// /// Looks up a localized string similar to Replace ! with -not. /// - internal static string AvoidExclamationPointOperatorCorrectionDescription { + internal static string AvoidExclaimOperatorCorrectionDescription { get { - return ResourceManager.GetString("AvoidExclamationPointOperatorCorrectionDescription", resourceCulture); + return ResourceManager.GetString("AvoidExclaimOperatorCorrectionDescription", resourceCulture); } } /// - /// Looks up a localized string similar to The negation operator ! should not be used. Use -not instead.. + /// Looks up a localized string similar to The negation operator ! should not be used for readability purposes. Use -not instead.. /// - internal static string AvoidExclamationPointOperatorDescription { + internal static string AvoidExclaimOperatorDescription { get { - return ResourceManager.GetString("AvoidExclamationPointOperatorDescription", resourceCulture); + return ResourceManager.GetString("AvoidExclaimOperatorDescription", resourceCulture); } } /// - /// Looks up a localized string similar to Avoid using the ! operator. + /// Looks up a localized string similar to Avoid using the ! negation operator. /// - internal static string AvoidExclamationPointOperatorError { + internal static string AvoidExclaimOperatorError { get { - return ResourceManager.GetString("AvoidExclamationPointOperatorError", resourceCulture); + return ResourceManager.GetString("AvoidExclaimOperatorError", resourceCulture); } } /// - /// Looks up a localized string similar to AvoidExclamationPointOperator. + /// Looks up a localized string similar to AvoidExclaimOperator. /// - internal static string AvoidExclamationPointOperatorName { + internal static string AvoidExclaimOperatorName { get { - return ResourceManager.GetString("AvoidExclamationPointOperatorName", resourceCulture); + return ResourceManager.GetString("AvoidExclaimOperatorName", resourceCulture); } } diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 7ea611a4f..1e6b0ca80 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1191,19 +1191,19 @@ Parameter '{0}' of function/cmdlet '{1}' does not match its exact casing '{2}'. - - AvoidExclamationPointOperator + + AvoidExclaimOperator - - Avoid exclamation point operator + + Avoid exclaim operator - - The negation operator ! should not be used. Use -not instead. + + The negation operator ! should not be used for readability purposes. Use -not instead. - + Avoid using the ! negation operator - + Replace ! with -not \ No newline at end of file diff --git a/Tests/Rules/AvoidExclamationPointOperator.tests.ps1 b/Tests/Rules/AvoidExclaimOperator.tests.ps1 similarity index 74% rename from Tests/Rules/AvoidExclamationPointOperator.tests.ps1 rename to Tests/Rules/AvoidExclaimOperator.tests.ps1 index 1956fddfc..8307aef5a 100644 --- a/Tests/Rules/AvoidExclamationPointOperator.tests.ps1 +++ b/Tests/Rules/AvoidExclaimOperator.tests.ps1 @@ -2,7 +2,7 @@ # Licensed under the MIT License. BeforeAll { - $ruleName = "PSAvoidExclamationPointOperator" + $ruleName = "PSAvoidExclaimOperator" $ruleSettings = @{ Enable = $true @@ -13,7 +13,7 @@ BeforeAll { } } -Describe "AvoidExclamationPointOperator" { +Describe "AvoidExclaimOperator" { Context "When the rule is not enabled explicitly" { It "Should not find violations" { $def = '!$true' @@ -22,7 +22,7 @@ Describe "AvoidExclamationPointOperator" { } } - Context "Given a line with the exclamation point operator" { + Context "Given a line with the exclaim operator" { It "Should find one violation" { $def = '!$true' $violations = Invoke-ScriptAnalyzer -ScriptDefinition $def -Settings $settings @@ -30,15 +30,15 @@ Describe "AvoidExclamationPointOperator" { } } - Context "Given a line with the exclamation point operator" { - It "Should replace the exclamation point operator with the -not operator" { + 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 exclamation point operator followed by a space" { - It "Should replace the exclamation point operator without adding an additional space" { + 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 diff --git a/docs/Rules/AvoidExclamationPointOperator.md b/docs/Rules/AvoidExclaimOperator.md similarity index 80% rename from docs/Rules/AvoidExclamationPointOperator.md rename to docs/Rules/AvoidExclaimOperator.md index 98d4b9d21..5f858feca 100644 --- a/docs/Rules/AvoidExclamationPointOperator.md +++ b/docs/Rules/AvoidExclaimOperator.md @@ -1,11 +1,11 @@ --- -description: Avoid exclamation point operator +description: Avoid exclaim operator ms.custom: PSSA v1.21.0 ms.date: 06/14/2023 ms.topic: reference -title: AvoidExclamationPointOperator +title: AvoidExclaimOperator --- -# AvoidExclamationPointOperator +# AvoidExclaimOperator **Severity Level: Warning** ## Description @@ -31,7 +31,7 @@ $MyVar = -not $true ```powershell Rules = @{ - PSAvoidExclamationPointOperator = @{ + PSAvoidExclaimOperator = @{ Enable = $true } } diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 0e1913ada..58b2e19d2 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -15,7 +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 | | +| [AvoidExclaimOperator](./AvoidExclaimOperator.md) | Warning | No | | | [AvoidGlobalAliases1](./AvoidGlobalAliases.md) | Warning | Yes | | | [AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | Yes | | | [AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | Yes | | From eb1deed939eca1311edba15ca01139ee42ad1b92 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Wed, 21 Jun 2023 19:36:10 +0100 Subject: [PATCH 7/7] Added comment explanation of the replacement suggestion logic and renamed loop variable for clarity --- Rules/AvoidExclaimOperator.cs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Rules/AvoidExclaimOperator.cs b/Rules/AvoidExclaimOperator.cs index 9a013953e..5521463ea 100644 --- a/Rules/AvoidExclaimOperator.cs +++ b/Rules/AvoidExclaimOperator.cs @@ -45,19 +45,21 @@ public override IEnumerable AnalyzeScript(Ast ast, string file IEnumerable foundAsts = ast.FindAll(testAst => testAst is UnaryExpressionAst, true); if (foundAsts != null) { var correctionDescription = Strings.AvoidExclaimOperatorCorrectionDescription; - foreach (UnaryExpressionAst foundAst in foundAsts) { - if (foundAst.TokenKind == TokenKind.Exclaim) { - // If the exclaim is not followed by a space, add one + foreach (UnaryExpressionAst unaryExpressionAst in foundAsts) { + if (unaryExpressionAst.TokenKind == TokenKind.Exclaim) { var replaceWith = "-not"; - if (foundAst.Child != null && foundAst.Child.Extent.StartColumnNumber == foundAst.Extent.StartColumnNumber + 1) { + // 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 { new CorrectionExtent( - foundAst.Extent.StartLineNumber, - foundAst.Extent.EndLineNumber, - foundAst.Extent.StartColumnNumber, - foundAst.Extent.StartColumnNumber + 1, + unaryExpressionAst.Extent.StartLineNumber, + unaryExpressionAst.Extent.EndLineNumber, + unaryExpressionAst.Extent.StartColumnNumber, + unaryExpressionAst.Extent.StartColumnNumber + 1, replaceWith, fileName, correctionDescription @@ -68,7 +70,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorError ), - foundAst.Extent, + unaryExpressionAst.Extent, GetName(), GetDiagnosticSeverity(), fileName,