Skip to content

Commit

Permalink
Remove Usage Of Deprecated lines Method
Browse files Browse the repository at this point in the history
Another instance of the deprecated `lines` method was recently added to the code base. This method causes metals to be not build on JDK >= 11.

This commit also updates the CI to build with JDKs 8 and 11 using AdpotOpenJDK. The majority of tests are JDK 11 as default, with a single test using JDK8 as a legacy compatibility smoke test. This should help prevent future additions of this method.

Finally, this commit drops cross building with deprecated scala versions, as they are unable to parse class files generated with bytecode for JDKs > 8.

See: ce9c890 and 4d78f9c for more context.
  • Loading branch information
isomarcte committed Dec 16, 2019
1 parent 902ca7b commit 9e846ca
Show file tree
Hide file tree
Showing 12 changed files with 377 additions and 312 deletions.
28 changes: 26 additions & 2 deletions .github/workflows/ci.yml
Expand Up @@ -11,10 +11,18 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
os: [windows-latest, macOS-latest, ubuntu-latest]
java: [8, 11]
exclude:
- os: windows-latest
java: 8
- os: macOS-latest
java: 8
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.${{ matrix.java }}"
- name: Run unit tests
run: |
bin/test.sh unit/test
Expand All @@ -25,6 +33,8 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run Sbt tests
run: bin/test.sh 'slow/testOnly -- tests.sbt'
maven:
Expand All @@ -33,6 +43,8 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run Maven tests
run: bin/test.sh 'slow/testOnly -- tests.maven'
gradle:
Expand All @@ -41,6 +53,8 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run Gradle tests
run: bin/test.sh 'slow/testOnly -- tests.gradle'
mill:
Expand All @@ -49,14 +63,18 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run Mill tests
run: bin/test.sh 'slow/testOnly -- tests.mill'
pants:
name: Pants integration
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v5
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run Pants tests
run: bin/test.sh 'slow/testOnly -- tests.pants'
feature:
Expand All @@ -65,6 +83,8 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run LSP tests
run: bin/test.sh 'slow/testOnly -- tests.feature'
cross:
Expand All @@ -73,6 +93,8 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- name: Run cross tests
run: sbt +cross/test
checks:
Expand All @@ -81,6 +103,8 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: olafurpg/setup-scala@v7
with:
java-version: "adopt@1.11"
- uses: actions/setup-node@v1
- run: git fetch --depth=10000
- run: git fetch --tags
Expand Down
5 changes: 3 additions & 2 deletions build.sbt
Expand Up @@ -158,8 +158,9 @@ lazy val V = new {
// List of supported Scala versions in SemanticDB. Needs to be manually updated
// for every SemanticDB upgrade.
def supportedScalaVersions =
Seq("2.13.0", scala213, scala212) ++ deprecatedScalaVersions
nonDeprecatedScalaVersions ++ deprecatedScalaVersions
def deprecatedScalaVersions = Seq("2.12.8", "2.12.9", scala211)
def nonDeprecatedScalaVersions = Seq("2.13.0", scala213, scala212)
def guava = "com.google.guava" % "guava" % "28.1-jre"
def lsp4j = "org.eclipse.lsp4j" % "org.eclipse.lsp4j" % "0.8.1"
def dap4j =
Expand Down Expand Up @@ -384,7 +385,7 @@ lazy val cross = project
List("org.scalamacros" % "paradise" % "2.1.1" cross CrossVersion.full)
case _ => Nil
}),
crossScalaVersions := V.supportedScalaVersions,
crossScalaVersions := V.nonDeprecatedScalaVersions,
scalacOptions ++= crossSetting(
scalaVersion.value,
if211 = List("-Xexperimental", "-Ywarn-unused-import")
Expand Down
14 changes: 0 additions & 14 deletions metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala
Expand Up @@ -2,7 +2,6 @@ package scala.meta.internal.builds

import java.nio.file.{Files, Path}
import scala.meta.io.AbsolutePath
import scala.util.Try
import java.nio.file.StandardCopyOption
import scala.meta.internal.metals.BloopInstallResult
import scala.concurrent.Future
Expand Down Expand Up @@ -47,19 +46,6 @@ abstract class BuildTool {
}

object BuildTool {

def isCompatibleVersion(minimumVersion: String, version: String): Boolean = {
(minimumVersion.split('.'), version.split('.')) match {
case (Array(minMajor, minMinor, minPatch), Array(major, minor, patch)) =>
Try {
(major > minMajor) ||
(major == minMajor && minor > minMinor) ||
(major == minMajor && minor == minMinor && patch >= minPatch)
}.getOrElse(false)
case _ => false
}
}

def copyFromResource(
tempDir: Path,
filePath: String,
Expand Down
4 changes: 2 additions & 2 deletions metals/src/main/scala/scala/meta/internal/metals/Doctor.scala
Expand Up @@ -9,7 +9,7 @@ import scala.meta.internal.metals.Messages.CheckDoctor
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.ScalaVersions._
import scala.meta.io.AbsolutePath
import scala.meta.internal.builds.BuildTool
import scala.meta.internal.semver.SemVer

/**
* Helps the user figure out what is mis-configured in the build through the "Run doctor" command.
Expand Down Expand Up @@ -117,7 +117,7 @@ final class Doctor(
val minimumBloopVersion = "1.3.5"
def isUnsupportedBloopVersion =
bspServerVersion.exists(version =>
!BuildTool.isCompatibleVersion(
!SemVer.isCompatibleVersion(
minimumBloopVersion,
version
)
Expand Down
Expand Up @@ -35,9 +35,10 @@ import scala.meta.internal.implementation.ImplementationProvider
import scala.meta.internal.io.FileIO
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.debug.DebugServer
import scala.meta.internal.tvp._
import scala.meta.internal.mtags._
import scala.meta.internal.semanticdb.Scala._
import scala.meta.internal.semver.SemVer
import scala.meta.internal.tvp._
import scala.meta.io.AbsolutePath
import scala.meta.parsers.ParseException
import scala.meta.pc.CancelToken
Expand Down Expand Up @@ -1289,7 +1290,7 @@ class MetalsLanguageServer(
private def supportedBuildTool(): Option[BuildTool] =
buildTools.loadSupported match {
case Some(buildTool) =>
val isCompatibleVersion = BuildTool.isCompatibleVersion(
val isCompatibleVersion = SemVer.isCompatibleVersion(
buildTool.minimumVersion,
buildTool.version
)
Expand Down
Expand Up @@ -143,7 +143,7 @@ class WorkspaceEditWorksheetPublisher(buffers: Buffers)
private def updateWithEdits(text: String, edits: List[TextEdit]): String = {
val editsMap = edits.map(e => e.getRange().getStart().getLine() -> e).toMap

text.lines.zipWithIndex
text.linesIterator.zipWithIndex
.map {
case (line, i) =>
editsMap.get(i) match {
Expand Down
13 changes: 13 additions & 0 deletions mtags/src/main/scala/scala/meta/internal/semver/SemVer.scala
@@ -0,0 +1,13 @@
package scala.meta.internal.semver

object SemVer {
def isCompatibleVersion(minimumVersion: String, version: String): Boolean = {
(minimumVersion.split('.'), version.split('.')) match {
case (Array(minMajor, minMinor, minPatch), Array(major, minor, patch)) =>
(major > minMajor) ||
(major == minMajor && minor > minMinor) ||
(major == minMajor && minor == minMinor && patch >= minPatch)
case _ => false
}
}
}
19 changes: 15 additions & 4 deletions tests/mtest/src/main/scala/tests/BaseSuite.scala
@@ -1,11 +1,15 @@
package tests

import scala.collection.mutable
import scala.concurrent.Await
import scala.concurrent.Future
import scala.concurrent.duration.Duration
import scala.language.experimental.macros
import scala.meta.io.AbsolutePath
import scala.meta.internal.metals.JdkSources
import scala.meta.internal.metals.Testing
import scala.meta.internal.mtags
import scala.meta.internal.semver.SemVer
import scala.meta.io.AbsolutePath
import scala.reflect.ClassTag
import scala.util.Properties
import utest.TestSuite
Expand All @@ -16,9 +20,6 @@ import utest.framework.TestCallTree
import utest.framework.Tree
import utest.ufansi.Attrs
import utest.ufansi.Str
import scala.meta.internal.metals.JdkSources
import scala.meta.internal.metals.Testing
import scala.collection.mutable

/**
* Test suite that replace utest DSL with FunSuite-style syntax from ScalaTest.
Expand All @@ -37,6 +38,12 @@ class BaseSuite extends TestSuite {
def beforeAll(): Unit = ()
def afterAll(): Unit = ()
def intercept[T: ClassTag](exprs: Unit): T = macro Asserts.interceptProxy[T]
def isValidScalaVersionForEnv(scalaVersion: String): Boolean =
this.isJava8 || SemVer.isCompatibleVersion(
BaseSuite.minScalaVersionForJDK9OrHigher,
scalaVersion
)

def assertNotEmpty(string: String): Unit = {
if (string.isEmpty) {
fail(
Expand Down Expand Up @@ -193,3 +200,7 @@ class BaseSuite extends TestSuite {
postProcess(result)
}
}

object BaseSuite {
val minScalaVersionForJDK9OrHigher: String = "2.12.10"
}
Expand Up @@ -5,11 +5,17 @@ import tests.BaseCompletionLspSuite

object CompletionCrossLspSuite
extends BaseCompletionLspSuite("completion-cross") {
testAsync("basic-211") {
basicTest(V.scala211)

if (super.isValidScalaVersionForEnv(V.scala211)) {
testAsync("basic-211") {
basicTest(V.scala211)
}
}
testAsync("basic-213") {
basicTest(V.scala213)

if (super.isValidScalaVersionForEnv(V.scala213)) {
testAsync("basic-213") {
basicTest(V.scala213)
}
}
testAsync("match-211") {
matchKeywordTest(V.scala213)
Expand Down
Expand Up @@ -7,12 +7,16 @@ import scala.concurrent.Future
object DefinitionCrossLspSuite
extends BaseCompletionLspSuite("definition-cross") {

testAsync("2.11") {
basicDefinitionTest(BuildInfo.scala211)
if (super.isValidScalaVersionForEnv(BuildInfo.scala211)) {
testAsync("2.11") {
basicDefinitionTest(BuildInfo.scala211)
}
}

testAsync("2.13") {
basicDefinitionTest(BuildInfo.scala213)
if (super.isValidScalaVersionForEnv(BuildInfo.scala213)) {
testAsync("2.13") {
basicDefinitionTest(BuildInfo.scala213)
}
}

def basicDefinitionTest(scalaVersion: String): Future[Unit] = {
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/src/main/scala/tests/BaseWorksheetLspSuite.scala
Expand Up @@ -12,6 +12,12 @@ abstract class BaseWorksheetLspSuite(scalaVersion: String)
Some(ClientExperimentalCapabilities(decorationProvider = true))
override def userConfig: UserConfiguration =
super.userConfig.copy(worksheetScreenWidth = 40, worksheetCancelTimeout = 1)

override final def test(name: String)(fun: => Any): Unit =
if (super.isValidScalaVersionForEnv(this.scalaVersion)) {
super.test(name)(fun)
}

if (!isWindows)
testAsync("completion") {
for {
Expand Down

0 comments on commit 9e846ca

Please sign in to comment.