Skip to content

Conversation

@keynmol
Copy link
Contributor

@keynmol keynmol commented Aug 14, 2023

I've noticed that Scala 3 projects don't work, and identified the issue (semanticdb location calculation).

[info] [error] (ThisBuild / sourcegraphTargetRoots) com.sourcegraph.sbtsourcegraph.SourcegraphPlugin$TaskException: Can't upload LSIF index to Sourcegraph because there are no SemanticDB directories. To fix this problem, run the `sourcegraphEnable` command before `sourcegraphCompile` like this: sbt sourcegraphEnable sourcegraphCompile
[info] [error] Total time: 1 s, completed 14 Aug 2023, 13:26:11
[error] x sbt-sourcegraph / scala-3
[error]  Cause of test exception: {line 2}  Command failed: sourcegraphCompile failed
[error] stack trace is suppressed; run last scripted for the full output
[error] (scripted) Failed tests:
[error]         sbt-sourcegraph / scala-3

Working on a fix currently

Test plan

  • New scripted plugin test

Our previous logic relied on withDefaultValue in the Scalameta version
map to get the version of scalameta plugin.

Issue is, withDefaultValue does not affect .get operations:

```scala
Welcome to Scala 2.12.18 (OpenJDK 64-Bit Server VM, Java 17.0.6).
Type in expressions for evaluation. Or try :help.

scala> val mp = Map("a" -> 2).withDefaultValue(3)
mp: scala.collection.immutable.Map[String,Int] = Map(a -> 2)

scala> mp.get("c")
res0: Option[Int] = None
```

Moreover, we don't need to add semanticdb plugin at all if we're working
on a Scala 3 project.

This comment fixes the logic to be explicit about Scala 3.

Additionally, we amend the semanticdb parameters used to customise
output location, according to https://github.com/sbt/sbt/blob/1.9.x/main/src/main/scala/sbt/plugins/SemanticdbPlugin.scala#L92
1. Remove Java 17-ism
2. Upgrade SBT to 1.9.3
3. Remove sbt-bloop (it's added automatically by metals)
@keynmol keynmol marked this pull request as ready for review August 15, 2023 08:09
@keynmol keynmol requested a review from olafurpg August 15, 2023 08:16
@keynmol keynmol mentioned this pull request Aug 15, 2023
Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏻 Nice to get this working with Scala 3!

@keynmol keynmol merged commit 3b5938f into main Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants