Rferguson better test detection

metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala

@@ -147,7 +147,7 @@ class BuildServerConnection private (
   /* Currently only Bloop and sbt support running single test cases
    * and ScalaCLI uses Bloop underneath.
    */
-  def supportsTestSelection: Boolean = isBloop || isSbt || isScalaCLI
+  def supportsTestSelection: Boolean = isBloop || isSbt || isScalaCLI || isBazel
 
   /* Some users may still use an old version of Bloop that relies on scala-debug-adapter 1.x.
    * Metals does not support scala-debug-adapter 1.x anymore.

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

@@ -317,7 +317,11 @@ abstract class MetalsLspService(
   )
 
   protected val testFrameworkProvider: TestFrameworkProvider =
-    new TestFrameworkProvider(semanticdbs, trees)
+    new TestFrameworkProvider(
+      semanticdbs,
+      definitionIndex,
+      trees,
+    )
 
   protected val testProvider: TestSuitesProvider = new TestSuitesProvider(
     buildTargets,

metals/src/main/scala/scala/meta/internal/metals/debug/BuildTargetClasses.scala

@@ -9,6 +9,7 @@ import scala.meta.internal.metals.BuildTargets
 import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.debug.BuildTargetClasses.Classes
 import scala.meta.internal.metals.debug.BuildTargetClasses.TestSymbolInfo
+import scala.meta.internal.metals.testProvider.TestEntry
 import scala.meta.internal.semanticdb.Scala.Descriptor
 import scala.meta.internal.semanticdb.Scala.Symbols
 
@@ -171,6 +172,31 @@ final class BuildTargetClasses(val buildTargets: BuildTargets)(implicit
     }
   }
 
+  /*
+   * Caches test entries found by metals when the build server fails to provide them
+   */
+  def cacheTestClasses(
+      target: b.BuildTarget,
+      testEntries: List[TestEntry],
+  ): Unit = {
+    val targetId = target.getId()
+    val classes = new Classes
+    testEntries.foreach { testEntry =>
+      val details = testEntry.suiteDetails
+      val testInfo = BuildTargetClasses.TestSymbolInfo(
+        details.className
+          .asInstanceOf[BuildTargetClasses.FullyQualifiedClassName],
+        details.framework,
+      )
+      classes.testClasses.put(
+        details.symbol.value.asInstanceOf[BuildTargetClasses.Symbol],
+        testInfo,
+      )
+    }
+    index.put(targetId, classes)
+
+  }
+
   private def cacheTestClasses(
       classes: Map[b.BuildTargetIdentifier, Classes],
       result: b.ScalaTestClassesResult,

metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala

@@ -107,7 +107,7 @@ final class Doctor(
     scribe.info(s"running doctor check")
     val scalaTargets = buildTargets.allScala.toList
     val javaTargets = buildTargets.allJava.toList
-    scribe.info(
+    scribe.debug(
       s"java targets: ${javaTargets.map(_.info.getDisplayName()).mkString(", ")}"
     )
     val summary = problemResolver.problemMessage(scalaTargets, javaTargets)

metals/src/main/scala/scala/meta/internal/metals/testProvider/TestFrameworkProvider.scala

@@ -3,6 +3,8 @@ package scala.meta.internal.metals.testProvider
 import ch.epfl.scala.bsp4j.BuildTarget
 import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.debug.Specs2
+import scala.meta.internal.mtags
+import scala.meta.internal.mtags.GlobalSymbolIndex
 import scala.meta.internal.mtags.Semanticdbs
 import scala.meta.internal.mtags.Symbol
 import scala.meta.internal.parsing.Trees
@@ -12,29 +14,20 @@ import scala.meta.transversers._
 
 final class TestFrameworkProvider(
     semanticdbs: () => Semanticdbs,
+    symbolIndex: GlobalSymbolIndex,
     trees: Trees,
 ) {
   def getTestSuiteDetailsForPath(
       path: AbsolutePath,
       textDocument: TextDocument,
   ): List[TestSuiteDetails] = {
     val location = Range.defaultInstance.toLocation(path.toURI.toString())
-    def isASpecsTest(symbol: SymbolInformation) = {
-      symbol.signature match {
-        case klass @ ClassSignature(_, parents: Iterable[Type], _, _) => {
-          parents.exists { case TypeRef(_, name: String, _) =>
-            name == "sbt/testing/Framework#"
-          }
-        }
-        case _ => false
-      }
-    }
     textDocument.symbols.collect {
       case symbol if isASpecsTest(symbol) => {
-        val className = ClassName(symbol.displayName.split('.').last)
+        val className = ClassName(symbol.symbol.split('.').last)
         TestSuiteDetails(
           fullyQualifiedName = FullyQualifiedName(
-            symbol.displayName
+            symbol.symbol
           ), // fix maybe? not sure this is the right fully qualified name
           framework = Specs2,
           className = className,
@@ -44,4 +37,34 @@ final class TestFrameworkProvider(
       }
     }.toList
   }
+
+  private val specsNames =
+    Set("org/specs2/Specification#", "org/specs2/mutable/Specification#")
+
+  private def isASpecsTest(
+      symbol: SymbolInformation,
+      depth: Int = 0,
+  ): Boolean = {
+    if (specsNames.contains(symbol.symbol)) {
+      return true
+    }
+    symbol.signature match {
+      case ClassSignature(_, parents: Iterable[Type], _, _) => {
+        parents.exists { case TypeRef(_, name: String, _) =>
+          (for {
+            definition <- symbolIndex.definition(mtags.Symbol(name))
+            document <- semanticdbs()
+              .textDocument(definition.path)
+              .documentIncludingStale
+            parentSymbol <- document.symbols.find { s => s.symbol == name }
+          } yield {
+            isASpecsTest(parentSymbol, depth + 1)
+          })
+            .getOrElse(false)
+        }
+      }
+      case _ => false
+    }
+  }
+
 }

metals/src/main/scala/scala/meta/internal/metals/testProvider/TestSuitesIndex.scala

@@ -28,7 +28,7 @@ private[testProvider] final case class TestFileMetadata(
     hasTestCasesGranularity: Boolean,
 )
 
-private[testProvider] final case class TestEntry(
+private[metals] final case class TestEntry(
     buildTarget: BuildTarget,
     path: AbsolutePath,
     suiteDetails: TestSuiteDetails,
@@ -60,12 +60,9 @@ private[testProvider] final class TestSuitesIndex {
   /**
    * Cached, already discovered test suites per build target.
    *
-   * For every test suite a.TestSuiteName Metals returns 2 symbols,
-   * one for object and one for class.
-   * However, only one of them has location in source code and it's useful.
-   * That's why FullyQualifiedClassName is used as a key.
-   * "a/TestSuiteName., a.TestSuiteName"
-   * "a/TestSuiteName#, a.TestSuiteName"
+   * For every test suite a.TestSuiteName Metals returns 2 symbols, one for object and one for class. However, only one
+   * of them has location in source code and it's useful. That's why FullyQualifiedClassName is used as a key.
+   * "a/TestSuiteName., a.TestSuiteName" "a/TestSuiteName#, a.TestSuiteName"
    */
   private val cachedTestSuites =
     TrieMap[
@@ -109,8 +106,10 @@ private[testProvider] final class TestSuitesIndex {
 
   /**
    * Determine if test cases should be updated for a given file after compilation
-   * @param path - file path
-   * @param md5 - md5 of updated file
+   * @param path
+   *   \- file path
+   * @param md5
+   *   \- md5 of updated file
    */
   def shouldBeUpdated(path: AbsolutePath, md5: String): Boolean =
     fileToMetadata

metals/src/main/scala/scala/meta/internal/metals/testProvider/TestSuitesProvider.scala

@@ -22,17 +22,23 @@ import scala.meta.internal.metals.UserConfiguration
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.codelenses.CodeLens
 import scala.meta.internal.metals.debug.BuildTargetClasses
-import scala.meta.internal.metals.debug.JUnit4
-import scala.meta.internal.metals.debug.MUnit
-import scala.meta.internal.metals.debug.Scalatest
-import scala.meta.internal.metals.debug.TestFramework
-import scala.meta.internal.metals.debug.Unknown
-import scala.meta.internal.metals.debug.WeaverCatsEffect
+import scala.meta.internal.metals.debug.{
+  JUnit4,
+  MUnit,
+  Scalatest,
+  Specs2,
+  TestFramework,
+  Unknown,
+  WeaverCatsEffect,
+}
 import scala.meta.internal.metals.testProvider.TestExplorerEvent._
-import scala.meta.internal.metals.testProvider.frameworks.JunitTestFinder
-import scala.meta.internal.metals.testProvider.frameworks.MunitTestFinder
-import scala.meta.internal.metals.testProvider.frameworks.ScalatestTestFinder
-import scala.meta.internal.metals.testProvider.frameworks.WeaverCatsEffectTestFinder
+import scala.meta.internal.metals.testProvider.frameworks.{
+  JunitTestFinder,
+  MunitTestFinder,
+  ScalatestTestFinder,
+  SpecsTestFinder,
+  WeaverCatsEffectTestFinder,
+}
 import scala.meta.internal.mtags
 import scala.meta.internal.mtags.GlobalSymbolIndex
 import scala.meta.internal.mtags.Semanticdbs
@@ -72,6 +78,8 @@ final class TestSuitesProvider(
     new ScalatestTestFinder(trees, symbolIndex, semanticdbs)
   private val weaverCatsEffect =
     new WeaverCatsEffectTestFinder(trees, symbolIndex, semanticdbs)
+  private val specsTestFinder =
+    new SpecsTestFinder(trees, symbolIndex, semanticdbs)
 
   private def isExplorerEnabled = clientConfig.isTestExplorerProvider() &&
     userConfig().testUserInterface == TestUserInterfaceKind.TestExplorer
@@ -169,13 +177,45 @@ final class TestSuitesProvider(
   /**
    * Check if opened file contains test suite and update test cases if yes.
    */
-  def didOpen(file: AbsolutePath): Future[Unit] =
+  def didOpen(file: AbsolutePath): Future[Unit] = {
     // no need to check the index, checking for tests in a single file is cheap
     if (isExplorerEnabled) Future {
-      val buildTargetUpdates = getTestCasesForPath(file, None)
-      updateClientIfNonEmpty(buildTargetUpdates)
+      val targetOpt =
+        buildTargets.inverseSources(file).flatMap(buildTargets.info)
+      targetOpt.map { target =>
+        if (target.getCapabilities().getCanTest()) {
+          scribe.info(s"Calculating test cases for ${target.getId.getUri}")
+          // Note: this is terrible, bu t it's the best I could find.
+          // This could obviously use some cleanup if someone knows a way
+          val entries = Map(target -> computeTestEntries(target))
+          entries.values.foreach(_.foreach(index.put(_)))
+          val addedEntries = getTestCasesForPath(file, None)
+          val addedSuites =
+            entries.mapValues(_.map(_.suiteDetails.asAddEvent)).toMap
+          val addedTestCases = entries.mapValues {
+            _.flatMap { entry =>
+              val canResolve = entry.suiteDetails.framework.canResolveChildren
+              if (canResolve && buffers.contains(entry.path))
+                getTestCasesForSuites(
+                  entry.path,
+                  Vector(entry.suiteDetails),
+                  None,
+                )
+              else Nil
+            }
+          }.toMap
+          val buildTargetUpdates =
+            getBuildTargetUpdates(
+              Map.empty,
+              addedSuites,
+              addedTestCases,
+            )
+          updateClientIfNonEmpty(buildTargetUpdates ++ addedEntries)
+        }
+      }
     }
     else Future.unit
+  }
 
   /**
    * Discover tests:
@@ -336,6 +376,13 @@ final class TestSuitesProvider(
                 suiteName = suite.fullyQualifiedName,
                 symbol = suite.symbol,
               )
+            case Specs2 =>
+              specsTestFinder.findTests(
+                doc = semanticdb,
+                path = path,
+                suiteName = suite.fullyQualifiedName,
+                symbol = suite.symbol,
+              )
             case Unknown => Vector.empty
           }
 
@@ -375,7 +422,8 @@ final class TestSuitesProvider(
       }
 
     val deletedSuites = removeStaleTestSuites(symbolsPerTarget)
-    val addedEntries = getTestEntries(symbolsPerTarget)
+    val addedEntries: Map[BuildTarget, List[TestEntry]] =
+      getTestEntries(symbolsPerTarget)
 
     // update cached suites with currently discovered
     addedEntries.foreach { case (_, entries) =>
@@ -424,7 +472,7 @@ final class TestSuitesProvider(
       )
     // when test suite is deleted it has to be removed from cache
     symbolsPerTargetsWithEmpty.map {
-      case SymbolsPerTarget(buildTarget, testSymbols) =>
+      case SymbolsPerTarget(buildTarget, testSymbols) => {
         val fromBSP =
           testSymbols.values
             .map(info => FullyQualifiedName(info.fullyQualifiedName))
@@ -439,6 +487,7 @@ final class TestSuitesProvider(
             }
         }
         (buildTarget, removed)
+      }
     }.toMap
   }
 
@@ -454,46 +503,34 @@ final class TestSuitesProvider(
       val currentlyCached = index.getSuiteNames(currentTarget.target)
       val cachedSuites = mutable.Set.from(currentlyCached)
 
-      /* buildTarget/scalaTestClasses is deprecated in favor of buildTargetjvmTestEnvironment
-       * which doesn't provide a framework in the response.
-       * Since we'll need to find the framework here anyway, relying on it to get the "main" classes seems unnecessary.
-       */
-      if (
-        currentTarget.testSymbols.isEmpty && currentTarget.target
-          .getCapabilities()
-          .getCanTest()
-      ) {
-        computeTestEntries(currentTarget.target)
-      } else {
-        currentTarget.testSymbols
-          .readOnlySnapshot()
-          .toList
-          // sort the symbols lexically so that symbols with the same fullyQualifiedName
-          // will be grouped, and the class will come before companion object (i.e.
-          // `a.b.WordSpec#` < `a.b.WordSpec.`). This ensures that the class is put into the cache
-          // instead of the companion object.
-          .sortBy { case (symbol, _) => symbol }
-          .foldLeft(List.empty[TestEntry]) {
-            case (entries, (symbol, testSymbolInfo)) =>
-              val fullyQualifiedName =
-                FullyQualifiedName(testSymbolInfo.fullyQualifiedName)
-              if (cachedSuites.contains(fullyQualifiedName)) entries
-              else {
-                val entryOpt = computeTestEntry(
-                  currentTarget.target,
-                  mtags.Symbol(symbol),
-                  fullyQualifiedName,
-                  testSymbolInfo,
-                )
-                entryOpt match {
-                  case Some(entry) =>
-                    cachedSuites.add(entry.suiteDetails.fullyQualifiedName)
-                    entry :: entries
-                  case None => entries
-                }
+      currentTarget.testSymbols
+        .readOnlySnapshot()
+        .toList
+        // sort the symbols lexically so that symbols with the same fullyQualifiedName
+        // will be grouped, and the class will come before companion object (i.e.
+        // `a.b.WordSpec#` < `a.b.WordSpec.`). This ensures that the class is put into the cache
+        // instead of the companion object.
+        .sortBy { case (symbol, _) => symbol }
+        .foldLeft(List.empty[TestEntry]) {
+          case (entries, (symbol, testSymbolInfo)) =>
+            val fullyQualifiedName =
+              FullyQualifiedName(testSymbolInfo.fullyQualifiedName)
+            if (cachedSuites.contains(fullyQualifiedName)) entries
+            else {
+              val entryOpt = computeTestEntry(
+                currentTarget.target,
+                mtags.Symbol(symbol),
+                fullyQualifiedName,
+                testSymbolInfo,
+              )
+              entryOpt match {
+                case Some(entry) =>
+                  cachedSuites.add(entry.suiteDetails.fullyQualifiedName)
+                  entry :: entries
+                case None => entries
               }
-          }
-      }
+            }
+        }
     }
 
     entries.groupBy(_.buildTarget)