From 91f08be24663bae66ef7d8988b65d7cc84df454c Mon Sep 17 00:00:00 2001 From: Xavier Pinho Date: Thu, 6 Feb 2025 15:31:39 +0000 Subject: [PATCH] [c#] support setter assignments whose lhs is an identifier (#5296) --- .../AstForExpressionsCreator.scala | 205 +++++++++++++----- .../querying/ast/PropertySetterTests.scala | 135 ++++++++++++ 2 files changed, 280 insertions(+), 60 deletions(-) diff --git a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/astcreation/AstForExpressionsCreator.scala b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/astcreation/AstForExpressionsCreator.scala index 7ca27c2dad37..7107cef2042b 100644 --- a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/astcreation/AstForExpressionsCreator.scala +++ b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/astcreation/AstForExpressionsCreator.scala @@ -54,6 +54,8 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) { case SimpleMemberAccessExpression => val base = createDotNetNodeInfo(expr.json(ParserKeys.Expression)) Some(nodeTypeFullName(base)) + case IdentifierName => + scope.surroundingTypeDeclFullName case _ => None } @@ -78,89 +80,167 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) { case _ => None } - private def astForSetterAssignmentExpression( - assignExpr: DotNetNodeInfo, + /** Mainly to abstract the lowering of +=, *=, etc. assignments when the LHS is a property. Takes care of building the + * RHS appropriately, e.g. by expanding `P += RHS` into `set_P(get_P() + RHS)`, etc. + * @param expr + * the full assignment expression, for `code`, `line`, etc. + * @param assignOp + * the assignment operator, cf. [[Operators]] + * @param setterInfo + * the setter meta-data, cf. [[tryResolveSetterInvocation]] + */ + private def lowerSetterAssignmentRhs( + expr: DotNetNodeInfo, + assignOp: String, setterInfo: (CSharpMethod, String), - lhs: DotNetNodeInfo, - opName: String, - rhsNode: DotNetNodeInfo + receiver: Option[Ast], + rhs: DotNetNodeInfo ): Seq[Ast] = { val (setterMethod, setterBaseType) = setterInfo + val propertyName = setterMethod.name.stripPrefix("set_") + val originalRhs = astForOperand(rhs) - lhs.node match { - case SimpleMemberAccessExpression => - 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. + assignOp 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(expr)}") + originalRhs + case Some(getterMethod) => + stripAssignmentFromOperator(assignOp) match { case None => - logger.warn(s"Couldn't find matching getter for $propertyName in ${code(assignExpr)}") + logger.warn(s"Unrecognized assignment in ${code(expr)}") 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 - } + case Some(opName) => + val getterInvocation = + createInvocationAst(expr, getterMethod.name, Nil, receiver, Some(getterMethod), Some(setterBaseType)) + val operatorCall = + newOperatorCallNode(opName, code(expr), Some(setterMethod.returnType), line(expr), column(expr)) + callAst(operatorCall, getterInvocation +: originalRhs, None, None) :: Nil } } + } + } + + /** Lowers assignments such as `x.P = RHS` and `x.P += RHS` with `P` denoting a setter property into calls + * `x.set_P(RHS)` and `x.set_P(x.get_P() + RHS)`. + * @param assignExpr + * the full assignment expression, for `code`, `line` properties + * @param assignOp + * the final assignment operator name, cf. [[Operators]] + * @param setterInfo + * the setter meta-data, cf. [[tryResolveSetterInvocation]] + */ + private def astForMemberAccessSetterAssignment( + assignExpr: DotNetNodeInfo, + lhs: DotNetNodeInfo, + assignOp: String, + rhs: DotNetNodeInfo, + setterInfo: (CSharpMethod, String) + ): Seq[Ast] = { + val (setterMethod, setterBaseType) = setterInfo + val receiver = if (setterMethod.isStatic) { + None + } else { + val baseNode = createDotNetNodeInfo(lhs.json(ParserKeys.Expression)) + astForNode(baseNode).headOption + } + val rhsAst = lowerSetterAssignmentRhs(assignExpr, assignOp, setterInfo, receiver, rhs) + + createInvocationAst( + assignExpr, + setterMethod.name, + rhsAst, + receiver, + Some(setterMethod), + Some(setterBaseType) + ) :: Nil + } + + /** Lowers assignments such as `P = RHS` and `P += RHS` with `P` an identifier denoting a setter property into calls + * `set_P(RHS)` and `set_P(get_P() + RHS)`, respectively. + * @param assignExpr + * the full assignment expression, for `code`, `line` properties. + * @param assignOp + * the final assignment operator name, cf. [[Operators]] + * @param setterInfo + * the setter meta-data, cf. [[tryResolveSetterInvocation]] + */ + private def astForIdentifierSetterAssignment( + assignExpr: DotNetNodeInfo, + lhs: DotNetNodeInfo, + assignOp: String, + rhs: DotNetNodeInfo, + setterInfo: (CSharpMethod, String) + ): Seq[Ast] = { + val (setterMethod, setterBaseType) = setterInfo + val receiver = Option.when(!setterMethod.isStatic)(astForThisReceiver(lhs, scope.surroundingTypeDeclFullName)) + val rhsAst = lowerSetterAssignmentRhs(assignExpr, assignOp, setterInfo, receiver, rhs) + + createInvocationAst( + assignExpr, + setterMethod.name, + rhsAst, + receiver, + Some(setterMethod), + Some(setterBaseType) + ) :: Nil + } - createInvocationAst( - assignExpr, - setterMethod.name, - rhsAst, - receiver, - Some(setterMethod), - Some(setterBaseType) - ) :: Nil + /** Lowers assignments such as `x.P = RHS` and `P += RHS` where `P` denotes a setter property into a call + * `x.set_P(RHS)` and `set_P(get_P() + RHS)`, respectively. + * + * @param assignExpr + * the full assignment expr, for `code`, `line` properties + * @param setterInfo + * the setter meta-data, cf. [[tryResolveSetterInvocation]] + * @param assignOp + * the final assignment operator name, cf. [[Operators]] + */ + private def astForSetterAssignmentExpression( + assignExpr: DotNetNodeInfo, + setterInfo: (CSharpMethod, String), + lhs: DotNetNodeInfo, + assignOp: String, + rhs: DotNetNodeInfo + ): Seq[Ast] = { + lhs.node match { + case SimpleMemberAccessExpression => + astForMemberAccessSetterAssignment(assignExpr, lhs, assignOp, rhs, setterInfo) + case IdentifierName => astForIdentifierSetterAssignment(assignExpr, lhs, assignOp, rhs, setterInfo) case _ => logger.warn(s"Unsupported setter assignment: ${code(assignExpr)}") Nil } } + /** Lowers arbitrary assignment `LHS = RHS`, `LHS += RHS`, etc. expressions. + * @param assignExpr + * the full assignment, for `code`, `line` properties + * @param assignOp + * the final assignment operator name, cf. [[Operators]] + */ private def astForAssignmentExpression( assignExpr: DotNetNodeInfo, - lhsNode: DotNetNodeInfo, - opName: String, - rhsNode: DotNetNodeInfo + lhs: DotNetNodeInfo, + assignOp: String, + rhs: DotNetNodeInfo ): Seq[Ast] = { - tryResolveSetterInvocation(lhsNode) match { - case Some(setterInfo) => astForSetterAssignmentExpression(assignExpr, setterInfo, lhsNode, opName, rhsNode) - case None => astForRegularAssignmentExpression(assignExpr, lhsNode, opName, rhsNode) + tryResolveSetterInvocation(lhs) match { + case Some(setterInfo) => astForSetterAssignmentExpression(assignExpr, setterInfo, lhs, assignOp, rhs) + case None => astForRegularAssignmentExpression(assignExpr, lhs, assignOp, rhs) } } private def astForRegularAssignmentExpression( assignExpr: DotNetNodeInfo, lhs: DotNetNodeInfo, - opName: String, + assignOp: String, rhs: DotNetNodeInfo ): Seq[Ast] = { - astForRegularBinaryExpression(assignExpr, lhs, opName, rhs) + astForRegularBinaryExpression(assignExpr, lhs, assignOp, rhs) } private def astForParenthesizedExpression(parenExpr: DotNetNodeInfo): Seq[Ast] = { @@ -252,13 +332,18 @@ trait AstForExpressionsCreator(implicit withSchemaValidation: ValidationMode) { } } + /** @param binaryExpr + * the full binary expression, for `code`, `line`, etc. + * @param operatorName + * the final operator name, cf. [[Operators]] + */ private def astForRegularBinaryExpression( binaryExpr: DotNetNodeInfo, - lhsNode: DotNetNodeInfo, + lhs: DotNetNodeInfo, operatorName: String, - rhsNode: DotNetNodeInfo + rhs: DotNetNodeInfo ): Seq[Ast] = { - val args = astForOperand(lhsNode) ++: astForOperand(rhsNode) + val args = astForOperand(lhs) ++: astForOperand(rhs) val typeFullName = fixedTypeOperators.get(operatorName).orElse(Some(getTypeFullNameFromAstNode(args))) val callNode = operatorCallNode(binaryExpr, operatorName, typeFullName) callAst(callNode, args) :: Nil diff --git a/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/querying/ast/PropertySetterTests.scala b/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/querying/ast/PropertySetterTests.scala index 8f79985d3b3d..c7b3fa2de94f 100644 --- a/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/querying/ast/PropertySetterTests.scala +++ b/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/querying/ast/PropertySetterTests.scala @@ -207,6 +207,140 @@ class PropertySetterTests extends CSharpCode2CpgFixture { } } + "setting a previously declared {get;set;} property via `Property = y`" should { + val cpg = code(""" + |class MyData + |{ + | public int MyProperty { get; set; } + | public void DoStuff() + | { + | MyProperty = 3; // rendered as MyData.set_MyProperty(this, 3) + | } + |} + |""".stripMargin) + + "be translated to that property's set_* method" in { + inside(cpg.call.nameExact("set_MyProperty").l) { + case setter :: Nil => + setter.code shouldBe "MyProperty = 3" + 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 (thisId: Identifier) :: (three: Literal) :: Nil => + thisId.typeFullName shouldBe "MyData" + thisId.code shouldBe "this" + thisId.argumentIndex shouldBe 0 + three.code shouldBe "3" + three.typeFullName shouldBe "System.Int32" + three.argumentIndex shouldBe 1 + case xs => fail(s"Unexpected arguments to set_MyProperty, got $xs") + } + } + } + + "setting a previously declared {get;set;} property via `Property *= y" should { + val cpg = code(""" + |class MyData + |{ + | public int MyProperty { get; set; } + | public void DoStuff() + | { + | MyProperty *= 3; // rendered as set_MyProperty(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 "MyProperty *= 3" + 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 (thisId: Identifier) :: (timesCall: Call) :: Nil => + thisId.typeFullName shouldBe "MyData" + thisId.code shouldBe "this" + thisId.argumentIndex shouldBe 0 + + timesCall.argumentIndex shouldBe 1 + timesCall.code shouldBe "MyProperty *= 3" + timesCall.methodFullName shouldBe Operators.multiplication + timesCall.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 "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 "this" + receiver.name shouldBe "this" + case xs => fail(s"Expected single argument to get_MyProperty, but got $xs") + } + } + } + + "setting a previously declared static {get;set;} property via `Property = y`" should { + val cpg = code(""" + |class MyData + |{ + | public static int MyProperty { get; set; } + | public void DoStuff() + | { + | MyProperty = 3; // rendered as MyData.set_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 "MyProperty = 3" + 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 (three: Literal) :: Nil => + three.code shouldBe "3" + three.typeFullName shouldBe "System.Int32" + three.argumentIndex shouldBe 1 + case xs => fail(s"Unexpected arguments to set_MyProperty, got $xs") + } + } + } + "setting a previously declared static {set{}} property via `C.Property = y` where `C` is the class name" should { val cpg = code(""" |class MyData @@ -370,4 +504,5 @@ class PropertySetterTests extends CSharpCode2CpgFixture { cpg.call.nameExact("get_MyProperty").argument shouldBe empty } } + }