Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enhance check for assignment by rather checking if assigned variable is used in assignment block #1

Merged
merged 10 commits into from
Feb 17, 2018
Merged

Enhance check for assignment by rather checking if assigned variable is used in assignment block #1

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a6285b4
prototype of detecting variable usage on LHS to avoid false positives
bergmeister Feb 13, 2018
2ea4546
simplify and reduce nesting
bergmeister Feb 13, 2018
3d33169
fix regression by continue statements
bergmeister Feb 13, 2018
f9fd379
fix last regression
bergmeister Feb 13, 2018
4f7dbe6
add simple test case for new improvemeent
bergmeister Feb 13, 2018
135be11
finish it off and adapt tests
bergmeister Feb 17, 2018
d8666b0
update docs
bergmeister Feb 17, 2018
bd72cc3
minor style update
bergmeister Feb 17, 2018
0269ac0
fix typo
bergmeister Feb 17, 2018
edd37c0
fix test due to warning level change
bergmeister Feb 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix regression by continue statements
  • Loading branch information
@bergmeister
bergmeister committed Feb 13, 2018
commit 3d3316959866df5109dca77a7b775a5164e0c9c4
71 changes: 34 additions & 37 deletions Rules/PossibleIncorrectUsageOfAssignmentOperator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,54 +45,51 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
var ifStatementCondition = clause.Item1;
var assignmentStatementAst = ifStatementCondition.Find(testAst => testAst is AssignmentStatementAst, searchNestedScriptBlocks: false) as AssignmentStatementAst;
if (assignmentStatementAst == null)
if (assignmentStatementAst != null)
{
continue;
}
// 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.PossibleIncorrectUsageOfComparisonOperatorAssignmentOperatorError, assignmentStatementAst.ErrorPosition,
GetName(), DiagnosticSeverity.Warning, fileName);
}
else
{
var leftVariableExpressionAstOfAssignment = assignmentStatementAst.Left as VariableExpressionAst;
if (leftVariableExpressionAstOfAssignment == null)
{
continue;
}
var statementBlockOfIfStatenent = clause.Item2;
var variableExpressionAstsInStatementBlockOfIfStatement = statementBlockOfIfStatenent.FindAll(testAst => testAst is VariableExpressionAst, searchNestedScriptBlocks: true);
if (variableExpressionAstsInStatementBlockOfIfStatement == null) // no variable uages
// 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.PossibleIncorrectUsageOfComparisonOperatorAssignmentOperatorError, assignmentStatementAst.ErrorPosition,
GetName(), DiagnosticSeverity.Information, fileName);
GetName(), DiagnosticSeverity.Warning, fileName);
}
else
{
var variableOnLHSIsBeingUsed = variableExpressionAstsInStatementBlockOfIfStatement.Where(x => x is VariableExpressionAst)
.Any(x => ((VariableExpressionAst)x).VariablePath.UserPath.Equals(leftVariableExpressionAstOfAssignment.VariablePath.UserPath, StringComparison.OrdinalIgnoreCase));

if (variableOnLHSIsBeingUsed)
{
continue;
}
// 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 CommandExpressionAst;
if (commandAst == null && !variableOnLHSIsBeingUsed)
var leftVariableExpressionAstOfAssignment = assignmentStatementAst.Left as VariableExpressionAst;
if (leftVariableExpressionAstOfAssignment != null)
{
yield return new DiagnosticRecord(
Strings.PossibleIncorrectUsageOfComparisonOperatorAssignmentOperatorError, assignmentStatementAst.ErrorPosition,
GetName(), DiagnosticSeverity.Information, fileName);
var statementBlockOfIfStatenent = clause.Item2;
var variableExpressionAstsInStatementBlockOfIfStatement = statementBlockOfIfStatenent.FindAll(testAst => testAst is VariableExpressionAst, searchNestedScriptBlocks: true);
if (variableExpressionAstsInStatementBlockOfIfStatement == null) // no variable uages
Copy link
Owner Author

@bergmeister bergmeister Feb 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in comment

{
yield return new DiagnosticRecord(
Strings.PossibleIncorrectUsageOfComparisonOperatorAssignmentOperatorError, assignmentStatementAst.ErrorPosition,
GetName(), DiagnosticSeverity.Information, fileName);
}
else
{
var variableOnLHSIsBeingUsed = variableExpressionAstsInStatementBlockOfIfStatement.Where(x => x is VariableExpressionAst)
.Any(x => ((VariableExpressionAst)x).VariablePath.UserPath.Equals(leftVariableExpressionAstOfAssignment.VariablePath.UserPath, StringComparison.OrdinalIgnoreCase));

if (!variableOnLHSIsBeingUsed)
{
// 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 CommandExpressionAst;
if (commandAst == null)
{
yield return new DiagnosticRecord(
Strings.PossibleIncorrectUsageOfComparisonOperatorAssignmentOperatorError, assignmentStatementAst.ErrorPosition,
GetName(), DiagnosticSeverity.Information, fileName);
}
}
}
}
}
}

}
var fileRedirectionAst = clause.Item1.Find(testAst => testAst is FileRedirectionAst, searchNestedScriptBlocks: false) as FileRedirectionAst;
if (fileRedirectionAst != null)
{
Expand Down