C#: Modify compiler generated strip logic to also take generated ToString calls into account.

Author: michaelnebel

csharp/ql/examples/snippets/ternary_conditional.ql

@@ -11,7 +11,7 @@ import csharp
 
 from ConditionalExpr e
 where
-  e.getThen().stripImplicitCasts() != e.getElse().stripImplicitCasts() and
+  e.getThen().stripImplicit() != e.getElse().stripImplicit() and
   not e.getThen().getType() instanceof NullType and
   not e.getElse().getType() instanceof NullType
 select e

csharp/ql/lib/semmle/code/csharp/commons/Constants.qll

@@ -43,7 +43,7 @@ predicate isConstantComparison(ComparisonOperation co, boolean b) {
 private module ConstantComparisonOperation {
   private import semmle.code.csharp.commons.ComparisonTest
 
-  private SimpleType convertedType(Expr expr) { result = expr.stripImplicitCasts().getType() }
+  private SimpleType convertedType(Expr expr) { result = expr.stripImplicit().getType() }
 
   private int maxValue(Expr expr) {
     if convertedType(expr) instanceof IntegralType and exists(expr.getValue())

csharp/ql/lib/semmle/code/csharp/commons/Strings.qll

@@ -44,11 +44,11 @@ class ImplicitToStringExpr extends Expr {
     )
     or
     exists(AddExpr add, Expr o | o = add.getAnOperand() |
-      o.stripImplicitCasts().getType() instanceof StringType and
-      this = add.getOtherOperand(o)
+      o.stripImplicit().getType() instanceof StringType and
+      this = add.getOtherOperand(o).stripImplicit()
     )
     or
-    this = any(InterpolatedStringExpr ise).getAnInsert()
+    this = any(InterpolatedStringExpr ise).getAnInsert().stripImplicit()
   }
 }
 

csharp/ql/lib/semmle/code/csharp/dispatch/Dispatch.qll

@@ -857,7 +857,7 @@ private module Internal {
     private predicate hasDynamicArg(int i, Type argumentType) {
       exists(Expr argument |
         argument = this.getArgument(i) and
-        argument.stripImplicitCasts().getType() instanceof DynamicType and
+        argument.stripImplicit().getType() instanceof DynamicType and
         argumentType = getAPossibleType(argument, _)
       )
     }

csharp/ql/lib/semmle/code/csharp/exprs/Call.qll

@@ -281,6 +281,10 @@ class MethodCall extends Call, QualifiableExpr, LateBindableExpr, @method_invoca
       result = this.getArgument(i - 1)
     else result = this.getArgument(i)
   }
+
+  override Expr stripImplicit() {
+    if this.isImplicit() then result = this.getQualifier().stripImplicit() else result = this
+  }
 }
 
 /**

csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll

@@ -82,10 +82,18 @@ class Expr extends ControlFlowElement, @expr {
   Expr stripCasts() { result = this }
 
   /**
+   * DEPRECATED: Use `stripImplicit` instead.
+   *
    * Gets an expression that is the result of stripping (recursively) all
    * implicit casts from this expression, if any.
    */
-  Expr stripImplicitCasts() { result = this }
+  deprecated Expr stripImplicitCasts() { result = this.stripImplicit() }
+
+  /**
+   * Gets an expression that is the result of stripping (recursively) all
+   * implicit casts and implicit ToString calls from this expression, if any.
+   */
+  Expr stripImplicit() { result = this }
 
   /**
    * Gets the explicit parameter name used to pass this expression as an
@@ -714,8 +722,8 @@ class Cast extends Expr {
 
   override Expr stripCasts() { result = this.getExpr().stripCasts() }
 
-  override Expr stripImplicitCasts() {
-    if this.isImplicit() then result = this.getExpr().stripImplicitCasts() else result = this
+  override Expr stripImplicit() {
+    if this.isImplicit() then result = this.getExpr().stripImplicit() else result = this
   }
 }
 

csharp/ql/src/Likely Bugs/Dynamic/BadDynamicCall.ql

@@ -44,11 +44,11 @@ abstract class BadDynamicCall extends DynamicExpr {
       ultimateSsaDef = ssaDef.getAnUltimateDefinition()
     |
       ultimateSsaDef.getADefinition() =
-        any(AssignableDefinition def | source = def.getSource().stripImplicitCasts())
+        any(AssignableDefinition def | source = def.getSource().stripImplicit())
       or
       ultimateSsaDef.getADefinition() =
         any(AssignableDefinitions::ImplicitParameterDefinition p |
-          source = p.getParameter().getAnAssignedValue().stripImplicitCasts()
+          source = p.getParameter().getAnAssignedValue().stripImplicit()
         )
     )
   }

csharp/ql/src/Likely Bugs/ObjectComparison.ql

@@ -28,7 +28,7 @@ class ReferenceEqualityTestOnObject extends EqualityOperation {
     exists(getObjectOperand(this)) and
     // Neither operand is 'null'.
     not this.getAnOperand() instanceof NullLiteral and
-    not exists(Type t | t = this.getAnOperand().stripImplicitCasts().getType() |
+    not exists(Type t | t = this.getAnOperand().stripImplicit().getType() |
       t instanceof NullType or
       t instanceof ValueType
     ) and