From 9132c9de76725adf57b9da2410c4c4ef62af461d Mon Sep 17 00:00:00 2001 From: Lucas Nouguier Date: Fri, 19 May 2023 16:09:42 +0200 Subject: [PATCH] fix: module evaluation fix: module evaluation --- .../internal/evaluator/RuntimeEvaluator.scala | 53 ++++++++----------- .../RuntimeEvaluatorExtractors.scala | 5 +- .../internal/evaluator/RuntimeTree.scala | 26 +++++---- ...timeEvaluatorPrimitiveOperationTests.scala | 3 +- .../internal/RuntimeEvaluatorTests.scala | 9 ++-- 5 files changed, 47 insertions(+), 49 deletions(-) diff --git a/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluator.scala b/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluator.scala index f8ed5078d..265475762 100644 --- a/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluator.scala +++ b/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluator.scala @@ -52,7 +52,7 @@ case class RuntimeEvaluator( case LocalVarTree(varName, _) => Safe.successful(frame.variableByName(varName).map(frame.variableValue).get) case primitive: PrimitiveBinaryOpTree => invokePrimitive(primitive) case primitive: PrimitiveUnaryOpTree => invokePrimitive(primitive) - case module: ModuleTree => evaluateModule(module, module.of.map(evaluate)) + case module: ModuleTree => evaluateModule(module) case literal: LiteralTree => evaluateLiteral(literal) case ThisTree(obj) => Safe(JdiValue(obj.instances(1).get(0), frame.thread)) case field: InstanceFieldTree => evaluateField(field) @@ -77,14 +77,10 @@ case class RuntimeEvaluator( /* Outer evaluation */ /* -------------------------------------------------------------------------- */ private def evaluateOuter(tree: OuterTree): Safe[JdiValue] = - evaluate(tree.inner).flatMap { innerValue => - tree match { - case OuterModuleTree(_, module) => - evaluateModule(module, Some(Safe(innerValue))) - case _: OuterClassTree => - Safe(innerValue.asObject.getField("$outer")) - } - + tree match { + case OuterModuleTree(module) => evaluateModule(module) + case outerClass: OuterClassTree => + evaluate(outerClass.inner).map(_.asObject.getField("$outer")) } /* -------------------------------------------------------------------------- */ @@ -145,20 +141,20 @@ case class RuntimeEvaluator( * @param of the potential parent of the module * @return an instance of the module in a Safe */ - private def evaluateModule(tree: ModuleTree, of: Option[Safe[JdiValue]]): Safe[JdiValue] = { - val module = Safe(JdiObject(tree.`type`.instances(1).get(0), frame.thread)) - (module, of) match { - case (Safe(Success(_)), _) => module - case (Safe(Failure(e: ArrayIndexOutOfBoundsException)), Some(qual)) => + private def evaluateModule(tree: ModuleTree): Safe[JdiValue] = + tree.of match { + case None | Some(Module(_)) => Safe(JdiObject(tree.`type`.instances(1).get(0), frame.thread)) + case Some(of) => for { - ofValue <- qual + ofValue <- evaluate(of) loader <- frame.classLoader() initMethodName <- Safe(getLastInnerType(tree.`type`.name()).get) - instance <- ofValue.asObject.invoke(initMethodName, Seq.empty) + instance <- ofValue.value.`type` match { + case tree.`type` => Safe(ofValue) + case _ => ofValue.asObject.invoke(initMethodName, Seq.empty) + } } yield instance - case _ => Safe(Failure(new Exception("Can't evaluate or initialize module"))) } - } /* -------------------------------------------------------------------------- */ /* Instantiation */ @@ -277,7 +273,7 @@ case class RuntimeEvaluator( /** * Returns a [[ModuleTree]], if name is a (nested) module * - * If name is in the companion class, returns an [[Invalid]] to avoid static access to instance members + * Should the module be a nested module, then the qualifier must be an instance of the outer module (to avoid accessing a field from a static context) * * @param name * @param of an option representing the qualifier of the module. It helps resolving name conflicts by selecting the most suitable one @@ -286,14 +282,9 @@ case class RuntimeEvaluator( private def validateModule(name: String, of: Option[RuntimeTree]): Validation[ModuleTree] = { val moduleName = if (name.endsWith("$")) name else name + "$" searchAllClassesFor(moduleName, of.map(_.`type`.name()), frame).flatMap { cls => - val isInClass = loadClass(removeLastInnerTypeFromFQCN(cls.name()), frame) - .withFilterNot { _.cls.methodsByName(moduleName.stripSuffix("$")).isEmpty() } - - // TODO: understand why I can't merge n°1 and n°3 - (isInClass, cls, of) match { - case (Safe(Success(_)), _, Instance(instance)) => ModuleTree(cls, Some(instance)) - case (_, Module(module), _) => ModuleTree(module, of) - case (_, _, Instance(instance)) => ModuleTree(cls, Some(instance)) + (cls, of) match { + case (Module(module), _) => ModuleTree(module, None) + case (_, Instance(instance)) => ModuleTree(cls, Some(instance)) case _ => Recoverable(s"Cannot access module $cls") } } @@ -390,9 +381,11 @@ case class RuntimeEvaluator( ): Validation[MethodTree] = tree.`type` match { case ref: ReferenceType => - methodsByNameAndArgs(ref, name, args.map(_.`type`), frame) - .map(toStaticIfNeeded(_, args, tree)) - .orElse(validateApplyCall(name, tree, args)) + validateApplyCall(name, tree, args) + .orElse { + methodsByNameAndArgs(ref, name, args.map(_.`type`), frame) + .map(toStaticIfNeeded(_, args, tree)) + } .orElse(validateImplicitApplyCall(ref, tree, name, args)) .orElse(findOuter(tree, frame).flatMap(findMethod(_, name, args))) case t => illegalAccess(t, "ReferenceType") diff --git a/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluatorExtractors.scala b/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluatorExtractors.scala index 2b5ed5dc2..9546923b7 100644 --- a/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluatorExtractors.scala +++ b/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeEvaluatorExtractors.scala @@ -62,9 +62,10 @@ protected[internal] object RuntimeEvaluatorExtractors { object MethodCall { def unapply(tree: RuntimeTree): Option[RuntimeTree] = tree match { - case mt: ModuleTree => mt.of.flatMap(t => unapply(t)) + case mt: ModuleTree => mt.of.flatMap(unapply) case ft: InstanceFieldTree => unapply(ft.qual) - case ot: OuterTree => unapply(ot.inner) + case oct: OuterClassTree => unapply(oct.inner) + case OuterModuleTree(module) => module.of.flatMap(unapply) case _: MethodTree | _: NewInstanceTree => Some(tree) case _: LiteralTree | _: LocalVarTree | _: ThisTree | _: StaticFieldTree | _: ClassTree | _: PrimitiveBinaryOpTree | _: PrimitiveUnaryOpTree => diff --git a/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeTree.scala b/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeTree.scala index e28bdb702..9bf5ad554 100644 --- a/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeTree.scala +++ b/modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeTree.scala @@ -15,6 +15,10 @@ sealed trait RuntimeTree { sealed trait RuntimeValidationTree extends RuntimeTree sealed trait RuntimeEvaluationTree extends RuntimeTree +sealed trait TypeTree extends RuntimeTree { + override def `type`: ClassType +} + sealed trait MethodTree extends RuntimeEvaluationTree { def method: Method } @@ -23,9 +27,7 @@ sealed trait FieldTree extends RuntimeEvaluationTree { def field: Field } -sealed trait OuterTree extends RuntimeEvaluationTree { - def inner: RuntimeEvaluationTree -} +sealed trait OuterTree extends RuntimeEvaluationTree /* -------------------------------------------------------------------------- */ /* Simple trees */ @@ -134,8 +136,8 @@ case class InstanceMethodTree( override lazy val `type` = method.returnType() override def prettyPrint(depth: Int): String = { val indent = "\t" * (depth + 1) - s"""|MethodTree( - |${indent}m= $method, + s"""|InstanceMethodTree( + |${indent}m= $method -> ${method.returnType()}, |${indent}args= ${args.map(_.prettyPrint(depth + 1)).mkString(",\n" + indent)}, |${indent}qual= ${qual.prettyPrint(depth + 1)} |${indent.dropRight(1)})""".stripMargin @@ -179,26 +181,26 @@ case class OuterClassTree( val indent = "\t" * (depth + 1) s"""|OuterClassTree( |${indent}of= ${inner.prettyPrint(depth + 1)} + |${indent}type= ${`type`} |${indent.dropRight(1)})""".stripMargin } } case class OuterModuleTree( - inner: RuntimeEvaluationTree, module: ModuleTree ) extends OuterTree { override def `type`: ClassType = module.`type` override def prettyPrint(depth: Int): String = { val indent = "\t" * (depth + 1) s"""|OuterModuleTree( - |${indent}of= ${inner.prettyPrint(depth + 1)} + |${indent}module= ${module.prettyPrint(depth + 1)} |${indent.dropRight(1)})""".stripMargin } } object OuterTree { def apply(of: RuntimeTree, tpe: Type): Validation[OuterTree] = (of, tpe) match { - case (tree: RuntimeEvaluationTree, Module(module)) => Valid(new OuterModuleTree(tree, ModuleTree(module, None))) + case (tree: RuntimeEvaluationTree, Module(module)) => Valid(new OuterModuleTree(ModuleTree(module, None))) case (tree: RuntimeEvaluationTree, ct: ClassType) => Valid(new OuterClassTree(tree, ct)) case _ => Recoverable("No valid outer can be found") } @@ -213,12 +215,13 @@ case class ThisTree( case class ModuleTree( `type`: ClassType, of: Option[RuntimeEvaluationTree] -) extends RuntimeEvaluationTree { +) extends RuntimeEvaluationTree + with TypeTree { override def prettyPrint(depth: Int): String = { val indent = "\t" * (depth + 1) s"""|ModuleTree( |${indent}mod= ${`type`} - |${indent}of= $of + |${indent}of= ${of.map(_.prettyPrint(depth + 1)).getOrElse("None")} |${indent.dropRight(1)})""".stripMargin } } @@ -238,7 +241,8 @@ object ModuleTree { case class ClassTree( `type`: ClassType -) extends RuntimeValidationTree { +) extends RuntimeValidationTree + with TypeTree { override def prettyPrint(depth: Int): String = { val indent = "\t" * (depth + 1) s"""|ClassTree( diff --git a/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorPrimitiveOperationTests.scala b/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorPrimitiveOperationTests.scala index 61f62a6f0..b82e64fd3 100644 --- a/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorPrimitiveOperationTests.scala +++ b/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorPrimitiveOperationTests.scala @@ -37,8 +37,7 @@ object RuntimeEvaluatorPrimitiveEnvironment { |""".stripMargin } -class Scala31RuntimeEvaluatorPrimitiveOperationTests - extends RuntimeEvaluatorPrimitiveOperationTests(ScalaVersion.`3.1+`) +class ScalaRuntimeEvaluatorPrimitiveOperationTests extends RuntimeEvaluatorPrimitiveOperationTests(ScalaVersion.`3.1+`) abstract class RuntimeEvaluatorPrimitiveOperationTests(val scalaVersion: ScalaVersion) extends DebugTestSuite { lazy val localVar = diff --git a/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorTests.scala b/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorTests.scala index 3c3641a94..95f1493a5 100644 --- a/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorTests.scala +++ b/modules/tests/src/test/scala/ch/epfl/scala/debugadapter/internal/RuntimeEvaluatorTests.scala @@ -418,6 +418,7 @@ abstract class RuntimeEvaluatorTests(val scalaVersion: ScalaVersion) extends Deb ) } + // TODO: patch to make the last test work test( "Should access to multiple layers of nested types. However when the nested class take no parameters there is a conflict with its companion object" ) { @@ -431,11 +432,11 @@ abstract class RuntimeEvaluatorTests(val scalaVersion: ScalaVersion) extends Deb Evaluation.success("Main.Inner.DoubleInner(84).str", "double inner 84"), Evaluation.success("Main.Inner.DoubleInner.str", "double inner"), Evaluation.success("Main.Inner(42).InnerInner.str", "inner inner 42"), - Evaluation.successOrIgnore("Main.Inner(41).InnerInner().str", "inner inner 42", true), + Evaluation.success("Main.Inner(41).InnerInner().str", "inner inner 42"), Evaluation.success("Foo().FriendFoo(Foo()).InnerFriendFoo.str", "object inner friend foo 42"), - Evaluation.successOrIgnore("Foo().FriendFoo(Foo()).InnerFriendFoo().str", "inner friend foo 42", true), - Evaluation.success("Foo().FriendFoo.ObjectFriendFoo.str", "object object friend foo 42"), - Evaluation.successOrIgnore("Foo().FriendFoo.ObjectFriendFoo().str", "object friend foo 42", true) + Evaluation.success("Foo().FriendFoo(Foo()).InnerFriendFoo().str", "inner friend foo 42"), + Evaluation.success("Foo().FriendFoo.ObjectFriendFoo().str", "object friend foo 42"), + Evaluation.success("Foo().FriendFoo.ObjectFriendFoo.str", "object object friend foo 42") ) ) }