Merge pull request #635 from adpi2/fix-631

Fix variables and rework runtime evaluation errors

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/EvaluationProvider.scala

@@ -8,7 +8,6 @@ import ch.epfl.scala.debugadapter.EvaluationFailed
 import ch.epfl.scala.debugadapter.JavaRuntime
 import ch.epfl.scala.debugadapter.Logger
 import ch.epfl.scala.debugadapter.ManagedEntry
-import ch.epfl.scala.debugadapter.UnmanagedEntry
 import ch.epfl.scala.debugadapter.internal.evaluator.*
 import com.microsoft.java.debug.core.IEvaluatableBreakpoint
 import com.microsoft.java.debug.core.adapter.IDebugAdapterContext
@@ -27,9 +26,9 @@ import ScalaExtension.*
 
 private[internal] class EvaluationProvider(
     sourceLookUp: SourceLookUpProvider,
-    messageLogger: MessageLogger,
-    scalaEvaluators: Map[ClassEntry, ScalaEvaluator],
-    mode: DebugConfig.EvaluationMode,
+    compilers: Map[ClassEntry, ExpressionCompiler],
+    evaluationMode: DebugConfig.EvaluationMode,
+    testMode: Boolean,
     logger: Logger
 ) extends IEvaluationProvider {
 
@@ -65,15 +64,13 @@ private[internal] class EvaluationProvider(
     val location = frame.current().location
     val locationCode = (location.method.name, location.codeIndex).hashCode
     val expression =
-      if (breakpoint.getCompiledExpression(locationCode) != null) {
+      if (breakpoint.getCompiledExpression(locationCode) != null)
         breakpoint.getCompiledExpression(locationCode).asInstanceOf[Try[PreparedExpression]]
-      } else if (breakpoint.containsConditionalExpression) {
+      else if (breakpoint.containsConditionalExpression)
         prepare(breakpoint.getCondition, frame, preEvaluation = false)
-      } else if (breakpoint.containsLogpointExpression) {
+      else if (breakpoint.containsLogpointExpression)
         prepareLogMessage(breakpoint.getLogMessage, frame)
-      } else {
-        Failure(new Exception("Missing expression"))
-      }
+      else Failure(new Exception("Missing expression"))
     breakpoint.setCompiledExpression(locationCode, expression)
     val evaluation = for {
       expression <- expression
@@ -104,59 +101,60 @@ private[internal] class EvaluationProvider(
     completeFuture(invocation.getResult, thread)
   }
 
-  private def getScalaEvaluator(fqcn: String): Try[ScalaEvaluator] =
-    for {
-      entry <- sourceLookUp.getClassEntry(fqcn).toTry(s"Unknown class $fqcn")
-      evaluator <- scalaEvaluators.get(entry).toTry(missingEvaluatorMessage(entry))
-    } yield evaluator
-
-  private def missingEvaluatorMessage(entry: ClassEntry): String =
-    entry match {
-      case m: ManagedEntry =>
-        m.scalaVersion match {
-          case None => s"Unsupported evaluation in Java classpath entry: ${entry.name}"
-          case Some(sv) =>
-            s"""|Missing scala-expression-compiler_$sv with version ${BuildInfo.version}.
-                |You can open an issue at https://github.com/scalacenter/scala-debug-adapter.""".stripMargin
-        }
-      case _: JavaRuntime => s"Unsupported evaluation in JDK: ${entry.name}"
-      case _: UnmanagedEntry => s"Unsupported evaluation in unmanaged classpath entry: ${entry.name}"
-      case _ => s"Unsupported evaluation in ${entry.name}"
-    }
-
   private def prepareLogMessage(message: String, frame: JdiFrame): Try[PreparedExpression] = {
     if (!message.contains("$")) {
       Success(PlainLogMessage(message))
-    } else if (mode.allowScalaEvaluation) {
+    } else if (evaluationMode.allowScalaEvaluation) {
       val tripleQuote = "\"\"\""
       val expression = s"""println(s$tripleQuote$message$tripleQuote)"""
-      compile(expression, frame)
-    } else Failure(new EvaluationFailed(s"Cannot evaluate logpoint '$message' with $mode mode"))
+      getScalaEvaluator(frame).flatMap(_.compile(expression))
+    } else Failure(new EvaluationFailed(s"Cannot evaluate logpoint '$message' with $evaluationMode mode"))
   }
 
-  private def prepare(expression: String, frame: JdiFrame, preEvaluation: Boolean): Try[PreparedExpression] =
-    if (mode.allowRuntimeEvaluation)
+  private def prepare(expression: String, frame: JdiFrame, preEvaluation: Boolean): Try[PreparedExpression] = {
+    val scalaEvaluator = getScalaEvaluator(frame)
+    def compiledExpression = scalaEvaluator.flatMap(_.compile(expression))
+    if (evaluationMode.allowRuntimeEvaluation) {
       runtimeEvaluator.validate(expression, frame, preEvaluation) match {
-        case Success(expr) if mode.allowScalaEvaluation && containsMethodCall(expr.tree) =>
-          compile(expression, frame).orElse(Success(expr))
+        case Success(expr) if scalaEvaluator.isSuccess && containsMethodCall(expr.tree) =>
+          Success(compiledExpression.getOrElse(expr))
         case success: Success[RuntimeExpression] => success
         case failure: Failure[RuntimeExpression] =>
-          if (mode.allowScalaEvaluation) compile(expression, frame) else failure
+          if (scalaEvaluator.isSuccess) compiledExpression
+          else failure
       }
-    else if (mode.allowScalaEvaluation) compile(expression, frame)
+    } else if (evaluationMode.allowScalaEvaluation) compiledExpression
     else Failure(new EvaluationFailed(s"Evaluation is disabled"))
-
-  private def compile(expression: String, frame: JdiFrame): Try[CompiledExpression] = {
-    val fqcn = frame.current().location.declaringType.name
-    for {
-      evaluator <- getScalaEvaluator(fqcn)
-      sourceContent <- sourceLookUp
-        .getSourceContentFromClassName(fqcn)
-        .toTry(s"Cannot find source file of class $fqcn")
-      preparedExpression <- evaluator.compile(sourceContent, expression, frame)
-    } yield preparedExpression
   }
 
+  private def getScalaEvaluator(frame: JdiFrame): Try[ScalaEvaluator] =
+    if (evaluationMode.allowScalaEvaluation)
+      for {
+        fqcn <- Try(frame.current().location.declaringType.name)
+        entry <- sourceLookUp.getClassEntry(fqcn).toTry(s"Unknown class $fqcn")
+        compiler <- compilers.get(entry).toTry(missingCompilerMessage(entry))
+        sourceContent <- sourceLookUp
+          .getSourceContentFromClassName(fqcn)
+          .toTry(s"Cannot find source file of class $fqcn")
+      } yield new ScalaEvaluator(sourceContent, frame, compiler, logger, testMode)
+    else Failure(new Exception("Scala evaluation is not allowed"))
+
+  private def missingCompilerMessage(entry: ClassEntry): String =
+    entry match {
+      case m: ManagedEntry =>
+        m.scalaVersion match {
+          case None =>
+            s"Failed resolving scala-expression-compiler:${BuildInfo.version} for ${entry.name} (Missing Scala Version)"
+          case Some(sv) =>
+            s"""|Failed resolving scala-expression-compiler:${BuildInfo.version} for Scala $sv.
+                |Please open an issue at https://github.com/scalacenter/scala-debug-adapter.""".stripMargin
+        }
+      case _: JavaRuntime =>
+        s"Failed resolving scala-expression-compiler:${BuildInfo.version} for ${entry.name} (Missing Scala Version)"
+      case _ =>
+        s"Failed resolving scala-expression-compiler:${BuildInfo.version} for ${entry.name} (Unknown Scala Version)"
+    }
+
   private def containsMethodCall(tree: RuntimeEvaluationTree): Boolean = {
     import RuntimeEvaluationTree.*
     tree match {
@@ -173,13 +171,12 @@ private[internal] class EvaluationProvider(
 
   private def evaluate(expression: PreparedExpression, frame: JdiFrame): Try[Value] = evaluationBlock {
     expression match {
-      case logMessage: PlainLogMessage => messageLogger.log(logMessage, frame)
+      case logMessage: PlainLogMessage => MessageLogger.log(logMessage, frame)
       case expr: RuntimeExpression => runtimeEvaluator.evaluate(expr, frame)
       case expr: CompiledExpression =>
-        val fqcn = frame.current().location.declaringType.name
         for {
-          scalaEvaluator <- getScalaEvaluator(fqcn)
-          compiledExpression <- scalaEvaluator.evaluate(expr, frame)
+          scalaEvaluator <- getScalaEvaluator(frame)
+          compiledExpression <- scalaEvaluator.evaluate(expr)
         } yield compiledExpression
     }
   }
@@ -204,22 +201,12 @@ private[internal] class EvaluationProvider(
 }
 
 private[internal] object EvaluationProvider {
-  def apply(
-      debuggee: Debuggee,
-      tools: DebugTools,
-      logger: Logger,
-      config: DebugConfig
-  ): IEvaluationProvider = {
-    val scalaEvaluators = tools.expressionCompilers.view.map { case (entry, compiler) =>
-      (entry, new ScalaEvaluator(entry, compiler, logger, config.testMode))
-    }.toMap
-    val messageLogger = new MessageLogger()
+  def apply(debuggee: Debuggee, tools: DebugTools, logger: Logger, config: DebugConfig): IEvaluationProvider =
     new EvaluationProvider(
       tools.sourceLookUp,
-      messageLogger,
-      scalaEvaluators,
+      tools.expressionCompilers,
       config.evaluationMode,
+      config.testMode,
       logger
     )
-  }
 }

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/JdiFrame.scala

@@ -1,12 +1,6 @@
 package ch.epfl.scala.debugadapter.internal.evaluator
 
-import com.sun.jdi.ClassLoaderReference
-import com.sun.jdi.LocalVariable
-import com.sun.jdi.StackFrame
-import com.sun.jdi.ThreadReference
-import com.sun.jdi.ReferenceType
-import com.sun.jdi.PrimitiveType
-import com.sun.jdi.{BooleanType, ByteType, CharType, DoubleType, FloatType, IntegerType, LongType, ShortType}
+import com.sun.jdi.*
 
 import scala.jdk.CollectionConverters.*
 
@@ -34,7 +28,10 @@ final case class JdiFrame(thread: ThreadReference, depth: Int) {
     Option(current().thisObject).map(JdiObject(_, thread))
 
   def variables(): Seq[LocalVariable] =
-    current().visibleVariables.asScala.toSeq
+    try current().visibleVariables.asScala.toSeq
+    catch {
+      case _: AbsentInformationException => Seq.empty
+    }
 
   def variablesAndValues(): Seq[(LocalVariable, JdiValue)] =
     variables().map(v => v -> JdiValue(current().getValue(v), thread))

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/MessageLogger.scala

@@ -4,7 +4,7 @@ import com.sun.jdi.*
 
 import scala.util.Try
 
-private[internal] class MessageLogger() {
+private[internal] object MessageLogger {
   def log(logMessage: PlainLogMessage, frame: JdiFrame): Try[Value] = {
     val result = for {
       classLoader <- frame.classLoader()

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimePrimitiveOps.scala

@@ -55,25 +55,17 @@ object RuntimePrimitiveOps {
       } yield result
   }
 
-  /* -------------------------------------------------------------------------- */
-  /*                         Primitive binary operations                        */
-  /* -------------------------------------------------------------------------- */
   object BinaryOp {
-    def apply(
-        lhs: RuntimeEvaluationTree,
-        rhs: RuntimeEvaluationTree,
-        op: String
-    ): Validation[BinaryOp] = {
+    def apply(lhs: Type, rhs: Type, op: String): Validation[BinaryOp] = {
+      def notDefined = Recoverable(s"The $op operator is not defined on ${lhs.name} and ${rhs.name}")
       op match {
         case "==" => Valid(Eq)
         case "!=" => Valid(Neq)
-        case _ if !isPrimitive(lhs.`type`) || !isPrimitive(rhs.`type`) =>
-          Recoverable("Primitive operations don't support reference types")
-        case "&&" if isBoolean(lhs.`type`) && isBoolean(rhs.`type`) => Valid(And)
-        case "||" if isBoolean(lhs.`type`) && isBoolean(rhs.`type`) => Valid(Or)
-        case "&&" | "||" => Recoverable("Boolean operations don't support numeric types")
-        case _ if !isNumeric(lhs.`type`) || !isNumeric(rhs.`type`) =>
-          Recoverable("Numeric operations don't support boolean types")
+        case _ if !isPrimitive(lhs) || !isPrimitive(rhs) => notDefined
+        case "&&" if isBoolean(lhs) && isBoolean(rhs) => Valid(And)
+        case "||" if isBoolean(lhs) && isBoolean(rhs) => Valid(Or)
+        case "&&" | "||" => notDefined
+        case _ if !isNumeric(lhs) || !isNumeric(rhs) => notDefined
         case "+" => Valid(Plus)
         case "-" => Valid(Minus)
         case "*" => Valid(Times)
@@ -83,19 +75,9 @@ object RuntimePrimitiveOps {
         case "<=" => Valid(LessOrEqual)
         case ">" => Valid(Greater)
         case ">=" => Valid(GreaterOrEqual)
-        case _ => Recoverable(s"$op is not a primitive binary operation")
+        case _ => notDefined
       }
     }
-
-    def apply(
-        lhs: RuntimeEvaluationTree,
-        args: Seq[RuntimeEvaluationTree],
-        op: String
-    ): Validation[BinaryOp] =
-      args match {
-        case Seq(rhs) => apply(lhs, rhs, op)
-        case _ => Recoverable("Too many arguments")
-      }
   }
 
   /* --------------------------- Numeric operations --------------------------- */
@@ -272,7 +254,6 @@ object RuntimePrimitiveOps {
   case object And extends BooleanOp
   case object Or extends BooleanOp
 
-  /* --------------------------- Object operations --------------------------- */
   sealed trait ObjectOp extends BinaryOp {
     override def typeCheck(lhs: Type, rhs: Type): PrimitiveType =
       lhs.virtualMachine().mirrorOf(true).`type`().asInstanceOf[PrimitiveType]
@@ -294,25 +275,21 @@ object RuntimePrimitiveOps {
   case object Eq extends ObjectOp
   case object Neq extends ObjectOp
 
-  /* -------------------------------------------------------------------------- */
-  /*                         Primitive unary operations                         */
-  /* -------------------------------------------------------------------------- */
   case object Not extends UnaryOp {
     override def evaluate(rhs: JdiValue, loader: JdiClassLoader) =
       rhs.toBoolean.map(v => loader.mirrorOf(!v))
     override def typeCheck(rhs: Type): Type = rhs.virtualMachine().mirrorOf(true).`type`().asInstanceOf[BooleanType]
   }
 
   object UnaryOp {
-    def apply(rhs: RuntimeEvaluationTree, op: String): Validation[UnaryOp] = {
+    def apply(rhs: Type, op: String): Validation[UnaryOp] =
       op match {
-        case "unary_+" if isNumeric(rhs.`type`) => Valid(UnaryPlus)
-        case "unary_-" if isNumeric(rhs.`type`) => Valid(UnaryMinus)
-        case "unary_~" if isIntegral(rhs.`type`) => Valid(UnaryBitwiseNot)
-        case "unary_!" if isBoolean(rhs.`type`) => Valid(Not)
-        case _ => Recoverable("Not a primitive unary operation")
+        case "unary_+" if isNumeric(rhs) => Valid(UnaryPlus)
+        case "unary_-" if isNumeric(rhs) => Valid(UnaryMinus)
+        case "unary_~" if isIntegral(rhs) => Valid(UnaryBitwiseNot)
+        case "unary_!" if isBoolean(rhs) => Valid(Not)
+        case _ => Recoverable(s"$op is not defined on ${rhs.name}")
       }
-    }
   }
 
   /* -------------------------------------------------------------------------- */

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/RuntimeTree.scala

@@ -1,7 +1,6 @@
 package ch.epfl.scala.debugadapter.internal.evaluator
 
 import com.sun.jdi
-import ch.epfl.scala.debugadapter.Logger
 import ch.epfl.scala.debugadapter.internal.evaluator.RuntimePrimitiveOps.*
 
 sealed trait RuntimeEvaluationTree {
@@ -20,7 +19,7 @@ object RuntimeEvaluationTree {
 
   sealed trait Field extends Assignable {
     def field: jdi.Field
-    def immutable: Boolean = field.isFinal
+    def isMutable: Boolean = !field.isFinal
   }
 
   case class LocalVar(name: String, `type`: jdi.Type) extends Assignable {
@@ -100,16 +99,6 @@ object RuntimeEvaluationTree {
     }
   }
 
-  object CallBinaryOp {
-    def apply(lhs: RuntimeEvaluationTree, args: Seq[RuntimeEvaluationTree], name: String)(implicit
-        logger: Logger
-    ): Validation[CallBinaryOp] =
-      args match {
-        case Seq(rhs) => BinaryOp(lhs, rhs, name).map(CallBinaryOp(lhs, rhs, _))
-        case _ => Recoverable(s"Too many arguments")
-      }
-  }
-
   case class ArrayElem(array: RuntimeEvaluationTree, index: RuntimeEvaluationTree, `type`: jdi.Type)
       extends RuntimeEvaluationTree {
     override def prettyPrint(depth: Int): String = {
@@ -154,12 +143,6 @@ object RuntimeEvaluationTree {
     }
   }
 
-  object CallUnaryOp {
-    def apply(rhs: RuntimeEvaluationTree, name: String)(implicit logger: Logger): Validation[CallUnaryOp] = {
-      UnaryOp(rhs, name).map(CallUnaryOp(rhs, _))
-    }
-  }
-
   case class NewInstance(init: CallStaticMethod) extends RuntimeEvaluationTree {
     override lazy val `type`: jdi.ReferenceType = init.method.declaringType() // .asInstanceOf[jdi.ClassType]
     override def prettyPrint(depth: Int): String = {
@@ -174,9 +157,7 @@ object RuntimeEvaluationTree {
     override def prettyPrint(depth: Int): String = s"This(${`type`})"
   }
 
-  case class StaticModule(
-      `type`: jdi.ReferenceType
-  ) extends RuntimeEvaluationTree {
+  case class StaticModule(`type`: jdi.ClassType) extends RuntimeEvaluationTree {
     override def prettyPrint(depth: Int): String = {
       val indent = "\t" * (depth + 1)
       s"""|StaticModule(
@@ -238,12 +219,4 @@ object RuntimeEvaluationTree {
           |${indent.dropRight(1)})""".stripMargin
     }
   }
-
-  object Assign {
-    def apply(lhs: RuntimeEvaluationTree, rhs: RuntimeEvaluationTree, tpe: jdi.Type): Validation[Assign] =
-      lhs match {
-        case lhs: Assignable => Valid(Assign(lhs, rhs, tpe))
-        case _ => Recoverable("Left hand side of an assignment must be assignable")
-      }
-  }
 }