diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index 3c1325a5d..3db99e79d 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -46,6 +46,7 @@ public static string Format( "PSAvoidUsingCmdletAliases", "PSAvoidUsingDoubleQuotesForConstantString", "PSAvoidSemicolonsAsLineTerminators", + "PSAvoidExclaimOperator", }; var text = new EditableText(scriptDefinition); diff --git a/Rules/AvoidExclaimOperator.cs b/Rules/AvoidExclaimOperator.cs new file mode 100644 index 000000000..5521463ea --- /dev/null +++ b/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 +{ + /// + /// AvoidExclaimOperator: Checks for use of the exclaim operator + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class AvoidExclaimOperator : ConfigurableRule + { + + /// + /// Construct an object of AvoidExclaimOperator type. + /// + public AvoidExclaimOperator() { + 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.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 { + 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; + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidExclaimOperatorDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidExclaimOperatorName); + } + + /// + /// 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..f346e9084 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 exclaim operator. + /// + internal static string AvoidExclaimOperatorCommonName { + get { + return ResourceManager.GetString("AvoidExclaimOperatorCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Replace ! with -not. + /// + internal static string AvoidExclaimOperatorCorrectionDescription { + get { + return ResourceManager.GetString("AvoidExclaimOperatorCorrectionDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The negation operator ! should not be used for readability purposes. Use -not instead.. + /// + internal static string AvoidExclaimOperatorDescription { + get { + return ResourceManager.GetString("AvoidExclaimOperatorDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid using the ! negation operator. + /// + internal static string AvoidExclaimOperatorError { + get { + return ResourceManager.GetString("AvoidExclaimOperatorError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidExclaimOperator. + /// + internal static string AvoidExclaimOperatorName { + get { + return ResourceManager.GetString("AvoidExclaimOperatorName", 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..1e6b0ca80 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}'. - + + AvoidExclaimOperator + + + Avoid exclaim operator + + + 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/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/AvoidExclaimOperator.tests.ps1 b/Tests/Rules/AvoidExclaimOperator.tests.ps1 new file mode 100644 index 000000000..8307aef5a --- /dev/null +++ b/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 + } + } +} \ No newline at end of file diff --git a/docs/Rules/AvoidExclaimOperator.md b/docs/Rules/AvoidExclaimOperator.md new file mode 100644 index 000000000..5f858feca --- /dev/null +++ b/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. \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 44062d7c3..58b2e19d2 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 | | +| [AvoidExclaimOperator](./AvoidExclaimOperator.md) | Warning | No | | | [AvoidGlobalAliases1](./AvoidGlobalAliases.md) | Warning | Yes | | | [AvoidGlobalFunctions](./AvoidGlobalFunctions.md) | Warning | Yes | | | [AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | Yes | |