Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correctly report error position on unknown directive without values #3518

Merged
merged 2 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ case class DirectivesPreprocessor(
val res = directives
.iterator
.flatMap {
case d @ StrictDirective(k, _, _) =>
case d @ StrictDirective(k, _, _, _) =>
handlersMap.get(k).iterator.map(_(ScopedDirective(d, path, cwd), logger))
}
.toVector
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,13 @@ object ExtractedDirectives {
case sl: StringLiteral => Seq(StringValue(sl.getValue(), sl))
case bl: BooleanLiteral => Seq(BooleanValue(bl.getValue(), bl))
}
def toStrictDirective(ud: UsingDef) = StrictDirective(
ud.getKey(),
toStrictValue(ud.getValue()),
ud.getPosition().getColumn()
)
def toStrictDirective(ud: UsingDef) =
StrictDirective(
ud.getKey(),
toStrictValue(ud.getValue()),
ud.getPosition().getColumn(),
ud.getPosition().getLine()
)

directives.getAst match
case uds: UsingDefs => uds.getUsingDefs.asScala.toSeq.map(toStrictDirective)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ object BuiltInRules extends CommandHelpers {
val (withTestEquivalent, noTestEquivalent) =
noInitialTestPrefix.partition(_.existsTestEquivalent)
val transformedToTestEquivalents = withTestEquivalent.map {
case StrictDirective(key, values, _) => StrictDirective("test." + key, values)
case StrictDirective(key, values, _, _) => StrictDirective("test." + key, values)
}

TransformedTestDirectives(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package scala.build.errors

import scala.build.Position

final class UnusedDirectiveError(key: String, values: Seq[String], positions: Seq[Position])
final class UnusedDirectiveError(key: String, values: Seq[String], position: Position)
extends BuildException(
s"Unrecognized directive: $key with values: ${values.mkString(", ")}",
positions = positions
s"Unrecognized directive: $key${
if values.isEmpty then "" else s" with values: ${values.mkString(", ")}"
}",
positions = List(position)
)
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package scala.build.internal

import bloop.rifle.VersionUtil.parseJavaVersion
import coursier.jvm.{JavaHome, JvmChannel}
import coursier.jvm.{JavaHome, JvmChannel, JvmIndex}

import java.io.IOException
import java.nio.charset.Charset
Expand Down Expand Up @@ -56,7 +56,7 @@ object OsLibc {
// FIXME These values should be the default ones in coursier-jvm

lazy val jvmIndexOs: String = {
val default = JvmChannel.defaultOs
val default = JvmIndex.defaultOs
if (default == "linux" && isMusl.getOrElse(false)) "linux-musl"
else default
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,10 @@ object DirectiveUtil {

def concatAllValues(
scopedDirective: ScopedDirective
): Seq[Positioned[String]] =
scopedDirective.directive.values.map {
case v: StringValue =>
val pos = position(v, scopedDirective.maybePath)
Positioned(pos, v.get)
case v: BooleanValue =>
val pos = position(v, scopedDirective.maybePath)
Positioned(pos, v.get.toString)
case v: EmptyValue =>
val pos = position(v, scopedDirective.maybePath)
Positioned(pos, v.get)
}
): Seq[String] =
scopedDirective.directive.values.collect:
case v: StringValue => v.get
case v: BooleanValue => v.get.toString

def positions(
values: Seq[Value[_]],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package scala.build.preprocessing.directives

import scala.build.Position
import scala.build.errors.UnusedDirectiveError
import scala.build.preprocessing.ScopePath

Expand All @@ -9,12 +10,16 @@ case class ScopedDirective(
cwd: ScopePath
) {
def unusedDirectiveError: UnusedDirectiveError = {
val values =
DirectiveUtil.concatAllValues(this)
val values = DirectiveUtil.concatAllValues(this)
val keyPos = Position.File(
maybePath,
(directive.startLine, directive.startColumn),
(directive.startLine, directive.startColumn + directive.key.length())
)
new UnusedDirectiveError(
directive.key,
values.map(_.value),
values.flatMap(_.positions)
values,
keyPos
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import scala.build.Position
case class StrictDirective(
key: String,
values: Seq[Value[?]],
startColumn: Int = 0
startColumn: Int = 0,
startLine: Int = 0
) {
override def toString: String = {
val suffix = if validValues.isEmpty then "" else s" ${validValues.mkString(" ")}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,9 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
expectedMessage = "Unrecognized directive: resource with values: ./resources",
expectedSeverity = b.DiagnosticSeverity.ERROR,
expectedStartLine = 0,
expectedStartCharacter = 19,
expectedStartCharacter = 10,
expectedEndLine = 0,
expectedEndCharacter = 30,
expectedEndCharacter = 18,
strictlyCheckMessage = false
)
}
Expand Down Expand Up @@ -1374,9 +1374,9 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
s"Unrecognized directive: $directiveKey with values: $directiveValue",
expectedSeverity = b.DiagnosticSeverity.ERROR,
expectedStartLine = 0,
expectedStartCharacter = 33,
expectedStartCharacter = 10,
expectedEndLine = 0,
expectedEndCharacter = 38
expectedEndCharacter = 32
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -821,4 +821,52 @@ abstract class CompileTestDefinitions

}
}

test("i3389") {
val filename = "Main.scala"
val inputs = TestInputs(
os.rel / filename ->
"""//> using optionsdeprecation
|""".stripMargin
)

inputs.fromRoot { root =>
val result = os.proc(TestUtil.cli, "compile", ".", extraOptions).call(
cwd = root,
check = false,
mergeErrIntoOut = true
)
assertEquals(
TestUtil.fullStableOutput(result).trim(),
s"""|[error] .${File.separatorChar}Main.scala:1:11
|[error] Unrecognized directive: optionsdeprecation
|[error] //> using optionsdeprecation
|[error] ^^^^^^^^^^^^^^^^^^""".stripMargin
)
}
}

test("i3389-2") {
val filename = "Main.scala"
val inputs = TestInputs(
os.rel / filename ->
"""//> using unrecognised.directive value1 value2
|""".stripMargin
)

inputs.fromRoot { root =>
val result = os.proc(TestUtil.cli, "compile", ".", extraOptions).call(
cwd = root,
check = false,
mergeErrIntoOut = true
)
assertEquals(
TestUtil.fullStableOutput(result).trim(),
s"""|[error] .${File.separatorChar}Main.scala:1:11
|[error] Unrecognized directive: unrecognised.directive with values: value1, value2
|[error] //> using unrecognised.directive value1 value2
|[error] ^^^^^^^^^^^^^^^^^^^^^^""".stripMargin
)
}
}
}
Loading