Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 19 additions & 1 deletion csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ predicate negatedOperators(string op, string negated) {
negatedOperators(negated, op)
}

/** Holds if replacing `expr` with `operatorName` could call an enclosing operator. */
private predicate couldCallEnclosingOperator(LogicalNotExpr expr, string operatorName) {
exists(BinaryOperation binary, Operator enclosingOperator |
binary = expr.getOperand() and
enclosingOperator = expr.getEnclosingCallable().getEnclosingCallable*() and
enclosingOperator.getName() = operatorName and
binary
.getLeftOperand()
.getType()
.isImplicitlyConvertibleTo(enclosingOperator.getParameter(0).getType()) and
binary
.getRightOperand()
.getType()
.isImplicitlyConvertibleTo(enclosingOperator.getParameter(1).getType())
)
}

predicate simplifyBinaryExpr(string op, string withFalseOperand, string withTrueOperand) {
op = "==" and withTrueOperand = "A" and withFalseOperand = "!A"
or
Expand All @@ -90,7 +107,8 @@ predicate pushNegation(LogicalNotExpr expr, string oldPattern, string newPattern
or
exists(string oldOperator, string newOperator |
oldOperator = expr.getOperand().(BinaryOperation).getOperator() and
negatedOperators(oldOperator, newOperator)
negatedOperators(oldOperator, newOperator) and
not couldCallEnclosingOperator(expr, newOperator)
|
oldPattern = "!(A " + oldOperator + " B)" and
newPattern = "A " + newOperator + " B"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cs/simplifiable-boolean-expression` query no longer suggests replacing a negated comparison when the replacement could recursively call an enclosing user-defined operator in `build-mode: none` databases.
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
using System;

// The missing matching operators deliberately model an incompletely compiled database.
class IncompleteOperatorTest
{
// GOOD: Rewriting this as `left != right` could recursively call this operator.
public static bool operator !=(IncompleteOperatorTest left, IncompleteOperatorTest right)
{
// BAD: Rewriting this built-in comparison cannot call the enclosing operator.
bool valuesDiffer = !(left.Value == right.Value); // $ Alert

// BAD: Explicitly converting both operands prevents a call to the enclosing operator.
bool referencesDiffer = !((object)left == (object)right); // $ Alert

return valuesDiffer && referencesDiffer && !(left == right);
}

// GOOD: Rewriting this as `left >= right` could recursively call this operator.
public static bool operator >=(IncompleteOperatorTest left, IncompleteOperatorTest right) => !(left < right);

int Value { get; }
}

class IncompleteReverseOperatorTest
{
// GOOD: Rewriting this as `left == right` could recursively call this operator.
public static bool operator ==(IncompleteReverseOperatorTest left, IncompleteReverseOperatorTest right) => !(left != right);

// GOOD: Rewriting this as `left < right` could recursively call this operator.
public static bool operator <(IncompleteReverseOperatorTest left, IncompleteReverseOperatorTest right) => !(left >= right);
}

class IncompleteGreaterOperatorTest
{
// GOOD: Rewriting this as `left <= right` could recursively call this operator.
public static bool operator <=(IncompleteGreaterOperatorTest left, IncompleteGreaterOperatorTest right) => !(left > right);

// GOOD: Rewriting this as `left > right` could recursively call this operator.
public static bool operator >(IncompleteGreaterOperatorTest left, IncompleteGreaterOperatorTest right) => !(left <= right);
}

class IncompleteDifferentOperatorTest
{
// BAD: The suggested operator differs from the enclosing operator.
public static bool operator >(IncompleteDifferentOperatorTest left, IncompleteDifferentOperatorTest right) => !(left == right); // $ Alert
}

class IncompleteNestedOperatorTest
{
public static bool operator !=(IncompleteNestedOperatorTest left, IncompleteNestedOperatorTest right)
{
// GOOD: The replacement could call the enclosing operator from this lambda.
Func<bool> lambda = () => !(left == right);

// GOOD: The replacement could call the enclosing operator from this local function.
bool Local() => !(left == right);

return lambda() || Local();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
| SimplifyBoolExpr.cs:10:29:10:56 | !... | The expression '!(A == B)' can be simplified to 'A != B'. |
| SimplifyBoolExpr.cs:13:33:13:64 | !... | The expression '!(A == B)' can be simplified to 'A != B'. |
| SimplifyBoolExpr.cs:45:115:45:130 | !... | The expression '!(A == B)' can be simplified to 'A != B'. |
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Language Abuse/SimplifyBoolExpr.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
semmle-extractor-options: --standalone
Loading