Skip to content

Commit e060eb2

Browse files
GedochaoGedochao
authored
Merge pull request #2133 from Gedochao/fix-using-directives-errors
Improve directive parsing errors & special-case `toolkit` directive version parsing
2 parents 7b5680a + 323ba26 commit e060eb2

File tree

5 files changed

+82
-35
lines changed

5 files changed

+82
-35
lines changed

‎modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import scala.build.tests.util.BloopServer
99
import build.Ops.EitherThrowOps
1010
import dependency.AnyDependency
1111

12+
import scala.build.errors.ToolkitDirectiveMissingVersionError
13+
1214
class DirectiveTests extends munit.FunSuite {
1315

1416
val buildThreads = BuildThreads.create()
@@ -87,11 +89,11 @@ class DirectiveTests extends munit.FunSuite {
8789
}
8890
}
8991

90-
test("resolve toolkit & toolkit-test dependency") {
92+
test(s"resolve toolkit & toolkit-test dependency with version passed") {
9193
val testInputs = TestInputs(
9294
os.rel / "simple.sc" ->
93-
"""//> using toolkit latest
94-
|""".stripMargin
95+
s"""//> using toolkit latest
96+
|""".stripMargin
9597
)
9698
testInputs.withBuilds(baseOptions, buildThreads, bloopConfigOpt) {
9799
(_, _, maybeBuilds) =>
@@ -111,6 +113,21 @@ class DirectiveTests extends munit.FunSuite {
111113
expect(toolkitTestDep.version == expectedVersion)
112114
}
113115
}
116+
117+
for (toolkitDirectiveKey <- Seq("toolkit", "test.toolkit"))
118+
test(s"missing $toolkitDirectiveKey version produces an informative error message") {
119+
val testInputs = TestInputs(
120+
os.rel / "simple.sc" ->
121+
s"""//> using $toolkitDirectiveKey
122+
|""".stripMargin
123+
)
124+
testInputs.withBuilds(baseOptions, buildThreads, bloopConfigOpt) {
125+
(_, _, maybeBuilds) =>
126+
maybeBuilds match
127+
case Left(ToolkitDirectiveMissingVersionError(_, errorKey)) =>
128+
expect(errorKey == toolkitDirectiveKey)
129+
}
130+
}
114131
for (scope <- Scope.all) {
115132
def withProjectFile[T](projectFileContent: String)(f: (Build, Boolean) => T): T = TestInputs(
116133
os.rel / "project.scala" -> projectFileContent,

‎modules/directives/src/main/scala/scala/build/directives/DirectiveValueParser.scala‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import scala.build.errors.{
77
BuildException,
88
CompositeBuildException,
99
MalformedDirectiveError,
10+
ToolkitDirectiveMissingVersionError,
1011
UsingDirectiveValueNumError,
1112
UsingDirectiveWrongValueTypeError
1213
}
@@ -16,6 +17,7 @@ import scala.build.{Position, Positioned}
1617

1718
abstract class DirectiveValueParser[+T] {
1819
def parse(
20+
key: String,
1921
values: Seq[Value[_]],
2022
scopePath: ScopePath,
2123
path: Either[String, os.Path]
@@ -30,40 +32,46 @@ object DirectiveValueParser {
3032
private final class Mapped[T, +U](underlying: DirectiveValueParser[T], f: T => U)
3133
extends DirectiveValueParser[U] {
3234
def parse(
35+
key: String,
3336
values: Seq[Value[_]],
3437
scopePath: ScopePath,
3538
path: Either[String, os.Path]
3639
): Either[BuildException, U] =
37-
underlying.parse(values, scopePath, path).map(f)
40+
underlying.parse(key, values, scopePath, path).map(f)
3841
}
3942

4043
abstract class DirectiveSingleValueParser[+T] extends DirectiveValueParser[T] {
4144
def parseValue(
45+
key: String,
4246
value: Value[_],
4347
cwd: ScopePath,
4448
path: Either[String, os.Path]
4549
): Either[BuildException, T]
4650

4751
final def parse(
52+
key: String,
4853
values: Seq[Value[_]],
4954
scopePath: ScopePath,
5055
path: Either[String, os.Path]
5156
): Either[BuildException, T] =
52-
values.filter(!_.isEmpty) match {
53-
case Seq(value) => parseValue(value, scopePath, path)
54-
case _ =>
57+
values match {
58+
case Seq(value) if !value.isEmpty => parseValue(key, value, scopePath, path)
59+
case Seq(value) if value.isEmpty && (key == "toolkit" || key == "test.toolkit") =>
60+
// FIXME: handle similar parsing errors in the directive declaration instead of hacks like this one
61+
Left(ToolkitDirectiveMissingVersionError(maybePath = path, key = key))
62+
case resultValues @ _ =>
5563
Left(
5664
new UsingDirectiveValueNumError(
57-
path,
58-
"",
59-
"1",
60-
values.length
65+
maybePath = path,
66+
key = key,
67+
expectedValueNum = 1,
68+
providedValueNum = resultValues.count(!_.isEmpty)
6169
)
6270
)
6371
}
6472
}
6573

66-
given DirectiveValueParser[Unit] = { (values, scopePath, path) =>
74+
given DirectiveValueParser[Unit] = { (key, values, scopePath, path) =>
6775
values match {
6876
case Seq() => Right(())
6977
case Seq(value, _*) =>
@@ -105,16 +113,16 @@ object DirectiveValueParser {
105113
DirectiveUtil.position(value, path, skipQuotes = isString)
106114
}
107115

108-
given DirectiveValueParser[Boolean] = { (values, scopePath, path) =>
116+
given DirectiveValueParser[Boolean] = { (key, values, scopePath, path) =>
109117
values.filter(!_.isEmpty) match {
110118
case Seq() => Right(true)
111119
case Seq(v) =>
112120
v.asBoolean.toRight {
113121
new UsingDirectiveWrongValueTypeError(
114-
path,
115-
"",
116-
Seq("boolean"),
117-
""
122+
maybePath = path,
123+
key = key,
124+
expectedTypes = Seq("boolean"),
125+
hint = ""
118126
)
119127
}
120128
case values0 =>
@@ -128,23 +136,26 @@ object DirectiveValueParser {
128136
}
129137

130138
given DirectiveSingleValueParser[String] =
131-
(value, scopePath, path) =>
139+
(key, value, scopePath, path) =>
132140
value.asString.toRight {
133141
val pos = value.position(path)
134142
new MalformedDirectiveError(
135-
s"Expected a string, got '${value.getRelatedASTNode.toString}'",
136-
Seq(pos)
143+
message =
144+
s"""Encountered an error for the $key using directive.
145+
|Expected a string, got '${value.getRelatedASTNode.toString}'""".stripMargin,
146+
positions = Seq(pos)
137147
)
138148
}.map(DirectiveSpecialSyntax.handlingSpecialPathSyntax(_, path))
139149

140150
final case class MaybeNumericalString(value: String)
141151

142152
given DirectiveSingleValueParser[MaybeNumericalString] =
143-
(value, scopePath, path) =>
153+
(key, value, scopePath, path) =>
144154
value.asString.map(MaybeNumericalString(_)).toRight {
145155
val pos = value.position(path)
146156
new MalformedDirectiveError(
147-
s"Expected a string value, got '${value.getRelatedASTNode.toString}'",
157+
s"""Encountered an error for the $key using directive.
158+
|Expected a string value, got '${value.getRelatedASTNode.toString}'""".stripMargin,
148159
Seq(pos)
149160
)
150161
}
@@ -157,22 +168,22 @@ object DirectiveValueParser {
157168
}
158169

159170
given [T](using underlying: DirectiveValueParser[T]): DirectiveValueParser[WithScopePath[T]] = {
160-
(values, scopePath, path) =>
161-
underlying.parse(values, scopePath, path)
171+
(key, values, scopePath, path) =>
172+
underlying.parse(key, values, scopePath, path)
162173
.map(WithScopePath(scopePath, _))
163174
}
164175
given [T](using
165176
underlying: DirectiveSingleValueParser[T]
166177
): DirectiveSingleValueParser[Positioned[T]] = {
167-
(value, scopePath, path) =>
168-
underlying.parseValue(value, scopePath, path)
178+
(key, value, scopePath, path) =>
179+
underlying.parseValue(key, value, scopePath, path)
169180
.map(Positioned(value.position(path), _))
170181
}
171182
given [T](using underlying: DirectiveValueParser[T]): DirectiveValueParser[Option[T]] =
172183
underlying.map(Some(_))
173184
given [T](using underlying: DirectiveSingleValueParser[T]): DirectiveValueParser[List[T]] = {
174-
(values, scopePath, path) =>
175-
val res = values.filter(!_.isEmpty).map(underlying.parseValue(_, scopePath, path))
185+
(key, values, scopePath, path) =>
186+
val res = values.filter(!_.isEmpty).map(underlying.parseValue(key, _, scopePath, path))
176187
val errors = res.collect {
177188
case Left(e) => e
178189
}

‎modules/directives/src/main/scala/scala/build/errors/UsingDirectiveExpectationError.scala‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,27 @@ final class UsingDirectiveWrongValueTypeError(
2121
final class UsingDirectiveValueNumError(
2222
maybePath: Either[String, os.Path],
2323
key: String,
24-
expectedBounds: String,
24+
expectedValueNum: Int,
2525
providedValueNum: Int
26-
) extends UsingDirectiveExpectationError(
27-
s"expected $expectedBounds for the $key using directive key${maybePath.map(path => s" at $path").getOrElse(
28-
""
29-
)}; but got $providedValueNum values, instead."
30-
) {}
26+
) extends UsingDirectiveExpectationError({
27+
val pathString = maybePath.map(p => s" at $p").getOrElse("")
28+
s"""Encountered an error when parsing the `$key` using directive$pathString.
29+
|Expected $expectedValueNum values, but got $providedValueNum values instead.""".stripMargin
30+
})
31+
32+
final class ToolkitDirectiveMissingVersionError(
33+
val maybePath: Either[String, os.Path],
34+
val key: String
35+
) extends UsingDirectiveExpectationError({
36+
val pathString = maybePath.map(p => s" at $p").getOrElse("")
37+
s"""Encountered an error when parsing the `$key` using directive$pathString.
38+
|Expected a version to be passed.
39+
|Example: `//> using $key latest`
40+
|""".stripMargin
41+
})
42+
object ToolkitDirectiveMissingVersionError {
43+
def apply(maybePath: Either[String, os.Path], key: String): ToolkitDirectiveMissingVersionError =
44+
new ToolkitDirectiveMissingVersionError(maybePath, key)
45+
def unapply(arg: ToolkitDirectiveMissingVersionError): (Either[String, os.Path], String) =
46+
arg.maybePath -> arg.key
47+
}

‎modules/directives/src/main/scala/scala/build/preprocessing/directives/DirectiveHandler.scala‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ object DirectiveHandler {
381381
.map {
382382
case (scopeOpt, values) =>
383383
$parser.parse(
384+
$scopedDirective.directive.key,
384385
$scopedDirective.directive.values,
385386
$scopedDirective.cwd,
386387
$scopedDirective.maybePath

‎modules/integration/src/test/scala/scala/cli/integration/BspTestDefinitions.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1179,7 +1179,8 @@ abstract class BspTestDefinitions(val scalaVersionOpt: Option[String])
11791179
checkDiagnostic(
11801180
diagnostic = diagnostics.head,
11811181
expectedMessage =
1182-
"Expected a string value, got 'true'",
1182+
"""Encountered an error for the scala using directive.
1183+
|Expected a string value, got 'true'""".stripMargin,
11831184
expectedSeverity = b.DiagnosticSeverity.ERROR,
11841185
expectedStartLine = 0,
11851186
expectedStartCharacter = 16,

0 commit comments

Comments
 (0)