[c#] support setter assignments via += et al assignments (#5295)

* [c#] fix synthetic set_* method call signature * [c#] support setter assignments via += et al assignments
joernio · Feb 5, 2025 · ec17221 · ec17221
1 parent a58462f
commit ec17221
Show file tree

Hide file tree

Showing 2 changed files with 197 additions and 31 deletions.
diff --git a/...psrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/astcreation/AstForExpressionsCreator.scala b/...psrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/astcreation/AstForExpressionsCreator.scala
@@ -62,6 +62,22 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
     baseType.flatMap(x => scope.tryResolveSetterInvocation(fieldName, Some(x)).map((_, x)))
   }
 
+  private def stripAssignmentFromOperator(operatorName: String): Option[String] = operatorName match {
+    case Operators.assignmentPlus                 => Some(Operators.plus)
+    case Operators.assignmentMinus                => Some(Operators.minus)
+    case Operators.assignmentMultiplication       => Some(Operators.multiplication)
+    case Operators.assignmentDivision             => Some(Operators.division)
+    case Operators.assignmentExponentiation       => Some(Operators.exponentiation)
+    case Operators.assignmentModulo               => Some(Operators.modulo)
+    case Operators.assignmentShiftLeft            => Some(Operators.shiftLeft)
+    case Operators.assignmentLogicalShiftRight    => Some(Operators.logicalShiftRight)
+    case Operators.assignmentArithmeticShiftRight => Some(Operators.arithmeticShiftRight)
+    case Operators.assignmentAnd                  => Some(Operators.and)
+    case Operators.assignmentOr                   => Some(Operators.or)
+    case Operators.assignmentXor                  => Some(Operators.xor)
+    case _                                        => None
+  }
+
   private def astForSetterAssignmentExpression(
     assignExpr: DotNetNodeInfo,
     setterInfo: (CSharpMethod, String),
@@ -70,32 +86,56 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
     rhsNode: DotNetNodeInfo
   ): Seq[Ast] = {
     val (setterMethod, setterBaseType) = setterInfo
-    val returnType                     = DotNetTypeMap.getOrElse(setterMethod.returnType, setterMethod.returnType)
-    // FIXME: signature should not contain the receiver
-    val signature = composeMethodLikeSignature(returnType, setterMethod.parameterTypes.map(_._2))
-    val fullName  = composeMethodFullName(setterBaseType, setterMethod.name, signature)
-    val dispatch  = if setterMethod.isStatic then DispatchTypes.STATIC_DISPATCH else DispatchTypes.DYNAMIC_DISPATCH
-    val rhsAst = opName match {
-      case Operators.assignment => astForOperand(rhsNode)
-      case _                    =>
-        // TODO: should become `get_Property() <opName> rhsNode`. For now emit rhsNode.
-        logger.warn(s"Unsupported setter operation '$opName' in ${code(assignExpr)}")
-        astForOperand(rhsNode)
-    }
-
-    val setterCallNode = callNode(
-      node = assignExpr,
-      code = code(assignExpr),
-      name = setterMethod.name,
-      methodFullName = fullName,
-      dispatchType = dispatch
-    )
 
     lhs.node match {
       case SimpleMemberAccessExpression =>
-        val baseNode = astForNode(createDotNetNodeInfo(lhs.json(ParserKeys.Expression)))
-        val receiver = if setterMethod.isStatic then None else baseNode.headOption
-        callAst(setterCallNode, rhsAst, receiver) :: Nil
+        val baseNode     = astForNode(createDotNetNodeInfo(lhs.json(ParserKeys.Expression)))
+        val receiver     = if setterMethod.isStatic then None else baseNode.headOption
+        val propertyName = setterMethod.name.stripPrefix("set_")
+        val originalRhs  = astForOperand(rhsNode)
+
+        val rhsAst = opName match {
+          case Operators.assignment => originalRhs
+          case _ =>
+            scope.tryResolveGetterInvocation(propertyName, Some(setterBaseType)) match {
+              // Shouldn't happen, provided it is valid code. At any rate, log and emit the RHS verbatim.
+              case None =>
+                logger.warn(s"Couldn't find matching getter for $propertyName in ${code(assignExpr)}")
+                originalRhs
+              case Some(getterMethod) =>
+                stripAssignmentFromOperator(opName) match {
+                  case None =>
+                    logger.warn(s"Unrecognized assignment in ${code(assignExpr)}")
+                    originalRhs
+                  case Some(opName) =>
+                    val getterInvocation = createInvocationAst(
+                      assignExpr,
+                      getterMethod.name,
+                      Nil,
+                      receiver,
+                      Some(getterMethod),
+                      Some(setterBaseType)
+                    )
+                    val operatorCall = newOperatorCallNode(
+                      opName,
+                      code(assignExpr),
+                      Some(setterMethod.returnType),
+                      line(assignExpr),
+                      column(assignExpr)
+                    )
+                    callAst(operatorCall, getterInvocation +: originalRhs, None, None) :: Nil
+                }
+            }
+        }
+
+        createInvocationAst(
+          assignExpr,
+          setterMethod.name,
+          rhsAst,
+          receiver,
+          Some(setterMethod),
+          Some(setterBaseType)
+        ) :: Nil
       case _ =>
         logger.warn(s"Unsupported setter assignment: ${code(assignExpr)}")
         Nil
@@ -297,7 +337,8 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) {
   ): Ast = {
     val methodSignature = methodMetaData match {
       case Some(m) =>
-        composeMethodLikeSignature(m.returnType, m.parameterTypes.filterNot(_._1 == Constants.This).map(_._2))
+        val returnType = DotNetTypeMap.getOrElse(m.returnType, m.returnType)
+        composeMethodLikeSignature(returnType, m.parameterTypes.filterNot(_._1 == Constants.This).map(_._2))
       case None => Defines.UnresolvedSignature
     }
 

diff --git a/...sharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/querying/ast/PropertySetterTests.scala b/...sharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/querying/ast/PropertySetterTests.scala
@@ -1,7 +1,7 @@
 package io.joern.csharpsrc2cpg.querying.ast
 
 import io.joern.csharpsrc2cpg.testfixtures.CSharpCode2CpgFixture
-import io.shiftleft.codepropertygraph.generated.{DispatchTypes, ModifierTypes}
+import io.shiftleft.codepropertygraph.generated.{DispatchTypes, ModifierTypes, Operators}
 import io.shiftleft.codepropertygraph.generated.nodes.{Call, Identifier, Literal}
 import io.shiftleft.semanticcpg.language.*
 
@@ -112,8 +112,7 @@ class PropertySetterTests extends CSharpCode2CpgFixture {
       inside(cpg.call.nameExact("set_MyProperty").l) {
         case setter :: Nil =>
           setter.code shouldBe "m.MyProperty = 3"
-          // FIXME: signature
-          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(MyData,System.Int32)"
+          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(System.Int32)"
           setter.dispatchType shouldBe DispatchTypes.DYNAMIC_DISPATCH
         case xs => fail(s"Expected single call to set_MyProperty, but got $xs")
       }
@@ -152,8 +151,7 @@ class PropertySetterTests extends CSharpCode2CpgFixture {
       inside(cpg.call.nameExact("set_MyProperty").l) {
         case setter :: Nil =>
           setter.code shouldBe "m.MyProperty = 3"
-          // FIXME: signature
-          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(MyData,System.Int32)"
+          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(System.Int32)"
           setter.dispatchType shouldBe DispatchTypes.DYNAMIC_DISPATCH
         case xs => fail(s"Expected single call to set_MyProperty, but got $xs")
       }
@@ -189,8 +187,7 @@ class PropertySetterTests extends CSharpCode2CpgFixture {
       inside(cpg.call.nameExact("set_MyProperty").l) {
         case setter :: Nil =>
           setter.code shouldBe "this.MyProperty = 3"
-          // FIXME: signature
-          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(MyData,System.Int32)"
+          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(System.Int32)"
           setter.dispatchType shouldBe DispatchTypes.DYNAMIC_DISPATCH
         case xs => fail(s"Expected single call to set_MyProperty, but got $xs")
       }
@@ -245,4 +242,132 @@ class PropertySetterTests extends CSharpCode2CpgFixture {
       }
     }
   }
+
+  "setting a previously declared {get;set;} property via `x.Property += y` where `x` is a local variable" should {
+    val cpg = code("""
+        |class MyData
+        |{
+        |  public int MyProperty { get; set; }
+        |}
+        |class Main
+        |{
+        |  public static void DoStuff()
+        |  {
+        |    var m = new MyData();
+        |    m.MyProperty += 3; // rendered as MyData.set_MyProperty(m, MyData.get_MyProperty() + 3)
+        |  }
+        |}
+        |""".stripMargin)
+
+    "be translated to that property's set_* method" in {
+      inside(cpg.call.nameExact("set_MyProperty").l) {
+        case setter :: Nil =>
+          setter.code shouldBe "m.MyProperty += 3"
+          setter.name shouldBe "set_MyProperty"
+          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(System.Int32)"
+          setter.dispatchType shouldBe DispatchTypes.DYNAMIC_DISPATCH
+        case xs => fail(s"Expected single call to set_MyProperty, but got $xs")
+      }
+    }
+
+    "have correct arguments to the set_* method" in {
+      inside(cpg.call.nameExact("set_MyProperty").argument.sortBy(_.argumentIndex).l) {
+        case (m: Identifier) :: (plusCall: Call) :: Nil =>
+          m.typeFullName shouldBe "MyData"
+          m.code shouldBe "m"
+          m.argumentIndex shouldBe 0
+
+          plusCall.argumentIndex shouldBe 1
+          plusCall.code shouldBe "m.MyProperty += 3"
+          plusCall.methodFullName shouldBe Operators.plus
+          plusCall.dispatchType shouldBe DispatchTypes.STATIC_DISPATCH
+        case xs => fail(s"Unexpected arguments to set_MyProperty, got $xs")
+      }
+    }
+
+    "have correct arguments to the synthetic `+` call" in {
+      inside(cpg.call.nameExact("set_MyProperty").argument(1).isCall.argument.sortBy(_.argumentIndex).l) {
+        case (getter: Call) :: (three: Literal) :: Nil =>
+          getter.argumentIndex shouldBe 1
+          getter.code shouldBe "m.MyProperty += 3"
+          getter.methodFullName shouldBe "MyData.get_MyProperty:System.Int32()"
+          getter.name shouldBe "get_MyProperty"
+          getter.dispatchType shouldBe DispatchTypes.DYNAMIC_DISPATCH
+
+          three.argumentIndex shouldBe 2
+          three.code shouldBe "3"
+          three.typeFullName shouldBe "System.Int32"
+        case xs => fail(s"Expected two arguments for +, but got $xs")
+      }
+    }
+
+    "have correct arguments to the synthetic getter call" in {
+      inside(cpg.call.nameExact("get_MyProperty").argument.sortBy(_.argumentIndex).l) {
+        case (receiver: Identifier) :: Nil =>
+          receiver.argumentIndex shouldBe 0
+          receiver.typeFullName shouldBe "MyData"
+          receiver.code shouldBe "m"
+          receiver.name shouldBe "m"
+        case xs => fail(s"Expected single argument to get_MyProperty, but got $xs")
+      }
+    }
+  }
+
+  "setting a previously declared static {get;set;} property via `C.Property += y` where `C` is the class name" should {
+    val cpg = code("""
+        |class MyData
+        |{
+        |  public static int MyProperty { get; set; }
+        |}
+        |class Main
+        |{
+        |  public static void DoStuff()
+        |  {
+        |    MyData.MyProperty += 3; // rendered as MyData.set_MyProperty(MyData.get_MyProperty() + 3)
+        |  }
+        |}
+        |""".stripMargin)
+
+    "be translated to that property's set_* method" in {
+      inside(cpg.call.nameExact("set_MyProperty").l) {
+        case setter :: Nil =>
+          setter.code shouldBe "MyData.MyProperty += 3"
+          setter.name shouldBe "set_MyProperty"
+          setter.methodFullName shouldBe "MyData.set_MyProperty:System.Void(System.Int32)"
+          setter.dispatchType shouldBe DispatchTypes.STATIC_DISPATCH
+        case xs => fail(s"Expected single call to set_MyProperty, but got $xs")
+      }
+    }
+
+    "have correct arguments to the set_* method" in {
+      inside(cpg.call.nameExact("set_MyProperty").argument.sortBy(_.argumentIndex).l) {
+        case (plusCall: Call) :: Nil =>
+          plusCall.argumentIndex shouldBe 1
+          plusCall.code shouldBe "MyData.MyProperty += 3"
+          plusCall.methodFullName shouldBe Operators.plus
+          plusCall.dispatchType shouldBe DispatchTypes.STATIC_DISPATCH
+        case xs => fail(s"Unexpected arguments to set_MyProperty, got $xs")
+      }
+    }
+
+    "have correct arguments to the synthetic `+` call" in {
+      inside(cpg.call.nameExact("set_MyProperty").argument(1).isCall.argument.sortBy(_.argumentIndex).l) {
+        case (getter: Call) :: (three: Literal) :: Nil =>
+          getter.argumentIndex shouldBe 1
+          getter.code shouldBe "MyData.MyProperty += 3"
+          getter.methodFullName shouldBe "MyData.get_MyProperty:System.Int32()"
+          getter.name shouldBe "get_MyProperty"
+          getter.dispatchType shouldBe DispatchTypes.STATIC_DISPATCH
+
+          three.argumentIndex shouldBe 2
+          three.code shouldBe "3"
+          three.typeFullName shouldBe "System.Int32"
+        case xs => fail(s"Expected two arguments for +, but got $xs")
+      }
+    }
+
+    "have correct arguments to the synthetic getter call" in {
+      cpg.call.nameExact("get_MyProperty").argument shouldBe empty
+    }
+  }
 }