Warn when an if statement contains an assignment

[bergmeister]

Closes #809
This is a simple prototype that checks if there is an assignment ast inside an if statement.
It currently works and catches cases like if( ($a = $b_){} and if( ($a = $b_){} but also warns if the execution block of the if statement contains an assignment like e.g. in if ($a -eq $b){$a=$b}. This needs to be fixed and is being tracked by the currently failing test.
The idea is to keep it simple to avoid false positives. For example the rule does not catch cases where there is an evalution inside the if statemenet as e.g. in if ( ($a == $b) ){}

[JamesWTruher]

I don't think this is quite ready yet. There are a couple of problems which should be addressed first. There is a pattern which is very useful which will cause warnings to appear:

if ( $files = get-childitem ) {
... do something ...
}
else {
... do something else ...
}

I think if you can be more specific about the test of 2 variables in the ifstatementast (which should help you address the issue you found in the assignment outside of the test.

[bergmeister]

Hmm, such a case was discussed in the issue as well but my thinking was that the helpfulness of the warning makes up for those cases where the warning can be ignored. But you have a fair point, I will have a think if it is possible to get more certainty about the use case of the assignment operator.
But thanks for the feedback, that's exactly why I made a WIP PR because it is better to fail early. :-)
When looking at the warning level, I was not really satisfied with the 3 currently available levels (error, warning, info). Maybe if we had a level that addresses smells or possibe problems, then a user could do a thorough scan of the script e.g. for new code but has to review each warning individually but only require e.g. info level to pass in CI. A good example of warnings where the user has to use its own intelligence on deciding whether the warning can be suppressed or not is the Possible multiple enumeration of IEnumerable warning of ReSharper where ReSharper cannot figure out how an IEnumerable is being used. What are your thoughts on this?

[JamesWTruher]

The IsStatementAst has the Clauses member which should help. Here's a simple example in script:

using namespace System.Management.Automation.Language
param ( [scriptblock]$scriptblock = { if ( $a = $b ) { "the same" } } )
$If = $scriptblock.Ast.Find({$args[0] -is [IfStatementAst]}, $false)
if ( ! $If ) { write-verbose -verbose "no if" ; return }
foreach ( $clause in $If.Clauses ) {
    $assignment = $clause.Item1.Find({$args[0] -is [assignmentstatementast]}, $true)
    if ( $assignment ) {
        $msg =  ("Checking " + $assignment)
        if ( $assignment.Right.Expression -is [variableexpressionast] ) {
            Write-Error "${msg} : Don't assign a variable to a variable"
        }
        else {
            Write-Verbose -verbose "${msg} : Is OK!"
        }
    }
    else {
        Write-Verbose -verbose ("${msg} : no assignment in '" + $clause.item1 + "'")
    
}

when run, you can see what happens!

PS> /tmp/ast1 -scriptblock { if ( $a = get-date ) { 1 } }
VERBOSE: Checking $a = get-date : Is OK!
PS> /tmp/ast1 -scriptblock { if ( $a = $b ) { 1 } }
/tmp/ast1.ps1 : Checking $a = $b : Don't assign a variable to a variable
At line:1 char:1
+ /tmp/ast1 -scriptblock { if ( $a = $b ) { 1 } }
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,ast1.ps1

PS> /tmp/ast1 -scriptblock { if ( $a = get-date ) { 1 } elseif ( $b = $c ) { 2 } else { 3 } }
VERBOSE: Checking $a = get-date : Is OK!
/tmp/ast1.ps1 : Checking $b = $c : Don't assign a variable to a variable
At line:1 char:1
+ /tmp/ast1 -scriptblock { if ( $a = get-date ) { 1 } elseif ( $b = $c  ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,ast1.ps1

[bergmeister]

Thanks for the pointers. I improved it but still need to make sure that the case which you pointed out does not get flagged up (I added a test for it in a TDD manner). I will let you know once the PR is ready for re-review.

[bergmeister]

@JamesWTruher : The PR is ready for review now. I have improved it to not flag false positives like if ($a = (Get-ChildItem)){} and as far as I can see I can detect all positives cases of using = or == inside if statements. Also please feel free to comment on the rule name and the wording of the warning strings.

[JamesWTruher]

this will miss the case of
$a = = $b
but that's probably ok