diff --git a/RuleDocumentation/PossibleIncorrectUsageOAssignmentOperator.md b/RuleDocumentation/PossibleIncorrectUsageOAssignmentOperator.md new file mode 100644 index 000000000..5089c103a --- /dev/null +++ b/RuleDocumentation/PossibleIncorrectUsageOAssignmentOperator.md @@ -0,0 +1,54 @@ +# PossibleIncorrectUsageOfAssignmentOperator + +**Severity Level: Information** + +## Description + +In many programming languages, the equality operator is denoted as `==` or `=` in many programming languages, but `PowerShell` uses `-eq`. Therefore it can easily happen that the wrong operator is used unintentionally and this rule catches a few special cases where the likelihood of that is quite high. + +The rule looks for usages of `==` and `=` operators inside `if`, `else if`, `while` and `do-while` statements but it will not warn if any kind of command or expression is used at the right hand side as this is probably by design. + +## Example + +### Wrong + +```` PowerShell +if ($a = $b) +{ + ... +} +```` + +```` PowerShell +if ($a == $b) +{ + +} +```` + +### Correct + +```` PowerShell +if ($a -eq $b) # Compare $a with $b +{ + ... +} +```` + +```` PowerShell +if ($a = Get-Something) # Only execute action if command returns something and assign result to variable +{ + Do-SomethingWith $a +} +```` + +## Implicit suppresion using Clang style + +There are some rare cases where assignment of variable inside an if statement is by design. Instead of suppression the rule, one can also signal that assignment was intentional by wrapping the expression in extra parenthesis. An exception for this is when `$null` is used on the LHS is used because there is no use case for this. + +```` powershell +if (($shortVariableName = $SuperLongVariableName['SpecialItem']['AnotherItem'])) +{ + ... +} +```` \ No newline at end of file diff --git a/RuleDocumentation/PossibleIncorrectUsageOfAssignmentOperator.md b/RuleDocumentation/PossibleIncorrectUsageOfAssignmentOperator.md deleted file mode 100644 index 42ced0406..000000000 --- a/RuleDocumentation/PossibleIncorrectUsageOfAssignmentOperator.md +++ /dev/null @@ -1,7 +0,0 @@ -# PossibleIncorrectUsageOfAssignmentOperator - -**Severity Level: Information** - -## Description - -In many programming languages, the equality operator is denoted as `==` or `=`, but `PowerShell` uses `-eq`. Since assignment inside if statements are very rare, this rule wants to call out this case because it might have been unintentional. diff --git a/RuleDocumentation/PossibleIncorrectUsageOfRedirectionOperator.md b/RuleDocumentation/PossibleIncorrectUsageOfRedirectionOperator.md new file mode 100644 index 000000000..6af16c0a7 --- /dev/null +++ b/RuleDocumentation/PossibleIncorrectUsageOfRedirectionOperator.md @@ -0,0 +1,29 @@ +# PossibleIncorrectUsageOfRedirectionOperator + +**Severity Level: Information** + +## Description + +In many programming languages, the comparison operator for 'greater than' is `>` but `PowerShell` uses `-gt` for it and `-ge` (greater or equal) for `>=`. Therefore it can easily happen that the wrong operator is used unintentionally and this rule catches a few special cases where the likelihood of that is quite high. + +The rule looks for usages of `>` or `>=` operators inside if, elseif, while and do-while statements because this is likely going to be unintentional usage. + +## Example + +### Wrong + +```` PowerShell +if ($a > $b) +{ + ... +} +```` + +### Correct + +```` PowerShell +if ($a -gt $b) +{ + ... +} +```` \ No newline at end of file diff --git a/Rules/ClangSuppresion.cs b/Rules/ClangSuppresion.cs new file mode 100644 index 000000000..d0fcfec99 --- /dev/null +++ b/Rules/ClangSuppresion.cs @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Management.Automation.Language; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer +{ + /// + /// The idea behind clang suppresion style is to wrap a statement in extra parenthesis to implicitly suppress warnings of its content to signal that the offending operation was deliberate. + /// + internal static class ClangSuppresion + { + /// + /// The community requested an implicit suppression mechanism that follows the clang style where warnings are not issued if the expression is wrapped in extra parenthesis. + /// See here for details: https://github.com/Microsoft/clang/blob/349091162fcf2211a2e55cf81db934978e1c4f0c/test/SemaCXX/warn-assignment-condition.cpp#L15-L18 + /// + /// + /// + internal static bool ScriptExtendIsWrappedInParenthesis(IScriptExtent scriptExtent) + { + return scriptExtent.Text.StartsWith("(") && scriptExtent.Text.EndsWith(")"); + } + } +} diff --git a/Rules/PossibleIncorrectUsageOfAssignmentOperator.cs b/Rules/PossibleIncorrectUsageOfAssignmentOperator.cs index 120186947..e18bf54f9 100644 --- a/Rules/PossibleIncorrectUsageOfAssignmentOperator.cs +++ b/Rules/PossibleIncorrectUsageOfAssignmentOperator.cs @@ -13,7 +13,8 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// PossibleIncorrectUsageOfAssignmentOperator: Warn if someone uses the '=' or '==' by accident in an if statement because in most cases that is not the intention. + /// PossibleIncorrectUsageOfAssignmentOperator: Warn if someone uses '>', '=' or '==' operators inside an if, elseif, while and do-while statement because in most cases that is not the intention. + /// The origin of this rule is that people often forget that operators change when switching between different languages such as C# and PowerShell. /// #if !CORECLR [Export(typeof(IScriptRule))] @@ -21,45 +22,80 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules public class PossibleIncorrectUsageOfAssignmentOperator : AstVisitor, IScriptRule { /// - /// The idea is to get all AssignmentStatementAsts and then check if the parent is an IfStatementAst, which includes if, elseif and else statements. + /// The idea is to get all AssignmentStatementAsts and then check if the parent is an IfStatementAst/WhileStatementAst/DoWhileStatementAst, + /// which includes if, elseif, while and do-while statements. /// public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + var whileStatementAsts = ast.FindAll(testAst => testAst is WhileStatementAst || testAst is DoWhileStatementAst, searchNestedScriptBlocks: true); + foreach (LoopStatementAst whileStatementAst in whileStatementAsts) + { + var diagnosticRecord = AnalyzePipelineBaseAst(whileStatementAst.Condition, fileName); + if (diagnosticRecord != null) + { + yield return diagnosticRecord; + } + } + var ifStatementAsts = ast.FindAll(testAst => testAst is IfStatementAst, searchNestedScriptBlocks: true); foreach (IfStatementAst ifStatementAst in ifStatementAsts) { foreach (var clause in ifStatementAst.Clauses) { - var assignmentStatementAst = clause.Item1.Find(testAst => testAst is AssignmentStatementAst, searchNestedScriptBlocks: false) as AssignmentStatementAst; - if (assignmentStatementAst != null) + var diagnosticRecord = AnalyzePipelineBaseAst(clause.Item1, fileName); + if (diagnosticRecord != null) { - // Check if someone used '==', which can easily happen when the person is used to coding a lot in C#. - // In most cases, this will be a runtime error because PowerShell will look for a cmdlet name starting with '=', which is technically possible to define - if (assignmentStatementAst.Right.Extent.Text.StartsWith("=")) - { - yield return new DiagnosticRecord( - Strings.PossibleIncorrectUsageOfAssignmentOperatorError, assignmentStatementAst.Extent, - GetName(), DiagnosticSeverity.Warning, fileName); - } - else - { - // If the right hand side contains a CommandAst at some point, then we do not want to warn - // because this could be intentional in cases like 'if ($a = Get-ChildItem){ }' - var commandAst = assignmentStatementAst.Right.Find(testAst => testAst is CommandAst, searchNestedScriptBlocks: true) as CommandAst; - if (commandAst == null) - { - yield return new DiagnosticRecord( - Strings.PossibleIncorrectUsageOfAssignmentOperatorError, assignmentStatementAst.Extent, - GetName(), DiagnosticSeverity.Information, fileName); - } - } + yield return diagnosticRecord; } } } } + private DiagnosticRecord AnalyzePipelineBaseAst(PipelineBaseAst pipelineBaseAst, string fileName) + { + var assignmentStatementAst = pipelineBaseAst.Find(testAst => testAst is AssignmentStatementAst, searchNestedScriptBlocks: false) as AssignmentStatementAst; + if (assignmentStatementAst == null) + { + return null; + } + + // Check if someone used '==', which can easily happen when the person is used to coding a lot in C#. + // In most cases, this will be a runtime error because PowerShell will look for a cmdlet name starting with '=', which is technically possible to define + if (assignmentStatementAst.Right.Extent.Text.StartsWith("=")) + { + return new DiagnosticRecord( + Strings.PossibleIncorrectUsageOfAssignmentOperatorError, assignmentStatementAst.ErrorPosition, + GetName(), DiagnosticSeverity.Warning, fileName); + } + + // Check if LHS is $null and then always warn + if (assignmentStatementAst.Left is VariableExpressionAst variableExpressionAst) + { + if (variableExpressionAst.VariablePath.UserPath.Equals("null", StringComparison.OrdinalIgnoreCase)) + { + return new DiagnosticRecord( + Strings.PossibleIncorrectUsageOfAssignmentOperatorError, assignmentStatementAst.ErrorPosition, + GetName(), DiagnosticSeverity.Warning, fileName); + } + } + + // If the RHS contains a CommandAst at some point, then we do not want to warn because this could be intentional in cases like 'if ($a = Get-ChildItem){ }' + var commandAst = assignmentStatementAst.Right.Find(testAst => testAst is CommandAst, searchNestedScriptBlocks: true) as CommandAst; + // If the RHS contains an InvokeMemberExpressionAst, then we also do not want to warn because this could e.g. be 'if ($f = [System.IO.Path]::GetTempFileName()){ }' + var invokeMemberExpressionAst = assignmentStatementAst.Right.Find(testAst => testAst is ExpressionAst, searchNestedScriptBlocks: true) as InvokeMemberExpressionAst; + var doNotWarnBecauseImplicitClangStyleSuppressionWasUsed = ClangSuppresion.ScriptExtendIsWrappedInParenthesis(pipelineBaseAst.Extent); + if (commandAst == null && invokeMemberExpressionAst == null && !doNotWarnBecauseImplicitClangStyleSuppressionWasUsed) + { + return new DiagnosticRecord( + Strings.PossibleIncorrectUsageOfAssignmentOperatorError, assignmentStatementAst.ErrorPosition, + GetName(), DiagnosticSeverity.Information, fileName); + } + + return null; + } + /// /// GetName: Retrieves the name of this rule. /// @@ -84,7 +120,7 @@ public string GetCommonName() /// The description of this rule public string GetDescription() { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingWriteHostDescription); + return string.Format(CultureInfo.CurrentCulture, Strings.PossibleIncorrectUsageOfAssignmentOperatorDescription); } /// diff --git a/Rules/PossibleIncorrectUsageOfRedirectionOperator.cs b/Rules/PossibleIncorrectUsageOfRedirectionOperator.cs new file mode 100644 index 000000000..9e33f19f2 --- /dev/null +++ b/Rules/PossibleIncorrectUsageOfRedirectionOperator.cs @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Management.Automation.Language; +using System.Globalization; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// PossibleIncorrectUsageOfRedirectionOperator: Warn if someone uses '>' or '>=' inside an if, elseif, while or do-while statement because in most cases that is not the intention. + /// The origin of this rule is that people often forget that operators change when switching between different languages such as C# and PowerShell. + /// +#if !CORECLR +[Export(typeof(IScriptRule))] +#endif + public class PossibleIncorrectUsageOfRedirectionOperator : AstVisitor, IScriptRule + { + /// + /// The idea is to get all FileRedirectionAst inside all IfStatementAst clauses. + /// + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + var ifStatementAsts = ast.FindAll(testAst => testAst is IfStatementAst, searchNestedScriptBlocks: true); + foreach (IfStatementAst ifStatementAst in ifStatementAsts) + { + foreach (var clause in ifStatementAst.Clauses) + { + var fileRedirectionAst = clause.Item1.Find(testAst => testAst is FileRedirectionAst, searchNestedScriptBlocks: false) as FileRedirectionAst; + if (fileRedirectionAst != null) + { + yield return new DiagnosticRecord( + Strings.PossibleIncorrectUsageOfRedirectionOperatorError, fileRedirectionAst.Extent, + GetName(), DiagnosticSeverity.Warning, fileName); + } + } + } + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.PossibleIncorrectUsageOfRedirectionOperatorName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.PossibleIncorrectUsageOfRedirectionOperatorCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.PossibleIncorrectUsageOfRedirectionOperatorDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 7c0e53ec3..d9b304f8b 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1592,7 +1592,7 @@ internal static string PossibleIncorrectComparisonWithNullName { } /// - /// Looks up a localized string similar to '=' operator means assignment. Did you mean the equal operator '-eq'?. + /// Looks up a localized string similar to '=' is not an assignment operator. Did you mean the equality operator '-eq'?. /// internal static string PossibleIncorrectUsageOfAssignmentOperatorCommonName { get { @@ -1601,7 +1601,16 @@ internal static string PossibleIncorrectUsageOfAssignmentOperatorCommonName { } /// - /// Looks up a localized string similar to Did you really mean to make an assignment inside an if statement? If you rather meant to check for equality, use the '-eq' operator.. + /// Looks up a localized string similar to '=' or '==' are not comparison operators in the PowerShell language and rarely needed inside conditional statements.. + /// + internal static string PossibleIncorrectUsageOfAssignmentOperatorDescription { + get { + return ResourceManager.GetString("PossibleIncorrectUsageOfAssignmentOperatorDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Did you mean to use the assignment operator '='? The equality operator in PowerShell is 'eq'.. /// internal static string PossibleIncorrectUsageOfAssignmentOperatorError { get { @@ -1618,6 +1627,42 @@ internal static string PossibleIncorrectUsageOfAssignmentOperatorName { } } + /// + /// Looks up a localized string similar to '>' is not a comparison operator. Use '-gt' (greater than) or '-ge' (greater or equal).. + /// + internal static string PossibleIncorrectUsageOfRedirectionOperatorCommonName { + get { + return ResourceManager.GetString("PossibleIncorrectUsageOfRedirectionOperatorCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to When switching between different languages it is easy to forget that '>' does not mean 'great than' in PowerShell.. + /// + internal static string PossibleIncorrectUsageOfRedirectionOperatorDescription { + get { + return ResourceManager.GetString("PossibleIncorrectUsageOfRedirectionOperatorDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Did you mean to use the redirection operator '>'? The comparison operators in PowerShell are '-gt' (greater than) or '-ge' (greater or equal).. + /// + internal static string PossibleIncorrectUsageOfRedirectionOperatorError { + get { + return ResourceManager.GetString("PossibleIncorrectUsageOfRedirectionOperatorError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to PossibleIncorrectUsageOfRedirectionOperator. + /// + internal static string PossibleIncorrectUsageOfRedirectionOperatorName { + get { + return ResourceManager.GetString("PossibleIncorrectUsageOfRedirectionOperatorName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Basic Comment Help. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 352d5ee3f..55790c930 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -982,10 +982,7 @@ Assignment statements are not aligned - '=' operator means assignment. Did you mean the equal operator '-eq'? - - - Did you really mean to make an assignment inside an if statement? If you rather meant to check for equality, use the '-eq' operator. + '=' is not an assignment operator. Did you mean the equality operator '-eq'? PossibleIncorrectUsageOfAssignmentOperator @@ -1008,4 +1005,22 @@ '{0}' is implicitly aliasing '{1}' because it is missing the 'Get-' prefix. This can introduce possible problems and make scripts hard to maintain. Please consider changing command to its full name. + + '=' or '==' are not comparison operators in the PowerShell language and rarely needed inside conditional statements. + + + Did you mean to use the assignment operator '='? The equality operator in PowerShell is 'eq'. + + + '>' is not a comparison operator. Use '-gt' (greater than) or '-ge' (greater or equal). + + + When switching between different languages it is easy to forget that '>' does not mean 'great than' in PowerShell. + + + Did you mean to use the redirection operator '>'? The comparison operators in PowerShell are '-gt' (greater than) or '-ge' (greater or equal). + + + PossibleIncorrectUsageOfRedirectionOperator + \ No newline at end of file diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index f8d80af43..813a3ab8c 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -59,7 +59,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 54 + $expectedNumRules = 55 if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4)) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/PossibleIncorrectUsageOfAssignmentOperator.tests.ps1 b/Tests/Rules/PossibleIncorrectUsageOfAssignmentOperator.tests.ps1 index 353a9994f..8d5996965 100644 --- a/Tests/Rules/PossibleIncorrectUsageOfAssignmentOperator.tests.ps1 +++ b/Tests/Rules/PossibleIncorrectUsageOfAssignmentOperator.tests.ps1 @@ -1,12 +1,22 @@ $ruleName = "PSPossibleIncorrectUsageOfAssignmentOperator" -Describe "PossibleIncorrectUsageOfAssignmentOperator" { +Describe "PossibleIncorrectUsageOfComparisonOperator" { Context "When there are violations" { It "assignment inside if statement causes warning" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a=$b){}' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 1 } + It "assignment inside while statement causes warning" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'while ($a=$b){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "assignment inside do-while statement causes warning" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'do {} while ($a=$b){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + It "assignment inside if statement causes warning when when wrapped in command expression" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a=($b)){}' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 1 @@ -36,26 +46,56 @@ Describe "PossibleIncorrectUsageOfAssignmentOperator" { $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a == "$b"){}' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 1 } + + It "using an expression like a Binaryexpressionastast on the RHS causes warning" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = $b -match $c){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "always returns a violations when LHS is `$null even when RHS is a command that would normally not make it trigger" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($null = Get-ChildItem){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "always returns a violations when LHS is `$null even when using clang style" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if (($null = $b)){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } } Context "When there are no violations" { - It "returns no violations when there is no equality operator" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a -eq $b){$a=$b}' | Where-Object {$_.RuleName -eq $ruleName} + It "returns no violations when correct equality operator is used" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a -eq $b){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 0 + } + + It "returns no violations when using implicit clang style suppresion" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ( ($a = $b) ){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 0 + } + + It "returns no violations when using an InvokeMemberExpressionAst like a .net method on the RHS" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = [System.IO.Path]::GetTempFileName()){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 0 + } + + It "returns no violations when there is an InvokeMemberExpressionAst on the RHS that looks like a variable" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = $PSCmdlet.GetVariableValue($foo)){ }' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 0 } - It "returns no violations when there is an evaluation on the RHS" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = Get-ChildItem){}' | Where-Object {$_.RuleName -eq $ruleName} + It "returns no violations when there is a command on the RHS" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = Get-ChildItem){ }' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 0 } - It "returns no violations when there is an evaluation on the RHS wrapped in an expression" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = (Get-ChildItem)){}' | Where-Object {$_.RuleName -eq $ruleName} + It "returns no violations when there is a command on the RHS wrapped in an expression" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = (Get-ChildItem)){ }' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 0 } - It "returns no violations when there is an evaluation on the RHS wrapped in an expression and also includes a variable" { - $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a = (Get-ChildItem $b)){}' | Where-Object {$_.RuleName -eq $ruleName} + It "returns no violations when using for loop" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'for ($i = 0; $ -lt 42; $i++){ }' | Where-Object {$_.RuleName -eq $ruleName} $warnings.Count | Should -Be 0 } } diff --git a/Tests/Rules/PossibleIncorrectUsageOfRedirectionOperator.tests.ps1 b/Tests/Rules/PossibleIncorrectUsageOfRedirectionOperator.tests.ps1 new file mode 100644 index 000000000..3b1a4791d --- /dev/null +++ b/Tests/Rules/PossibleIncorrectUsageOfRedirectionOperator.tests.ps1 @@ -0,0 +1,38 @@ +Import-Module PSScriptAnalyzer +$ruleName = "PSPossibleIncorrectUsageOfRedirectionOperator" + +Describe "PossibleIncorrectUsageOfComparisonOperator" { + Context "When there are violations" { + It "File redirection operator inside if statement causes warning" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a > $b){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "File redirection operator with equals sign inside if statement causes warning" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a >=){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "File redirection operator inside if statement causes warning when wrapped in command expression" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a > ($b)){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "File redirection operator inside if statement causes warning when wrapped in expression" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a > "$b"){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + + It "File redirection operator inside elseif statement causes warning" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a -eq $b){}elseif($a > $b){}' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 1 + } + } + + Context "When there are no violations" { + It "returns no violations when using correct greater than operator" { + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'if ($a -gt $b){ }' | Where-Object {$_.RuleName -eq $ruleName} + $warnings.Count | Should -Be 0 + } + } +}