Skip to content

Commit

Permalink
make --strict-import-syntax opt-in to ease migration
Browse files Browse the repository at this point in the history
  • Loading branch information
lihaoyi-databricks committed Jul 8, 2023
1 parent ea8720f commit ccbe6e3
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 16 deletions.
2 changes: 1 addition & 1 deletion bench/src/main/scala/sjsonnet/OptimizerBenchmark.scala
Expand Up @@ -26,7 +26,7 @@ class OptimizerBenchmark {
def setup(): Unit = {
val (allFiles, ev) = MainBenchmark.findFiles()
this.inputs = allFiles.map { case (p, s) =>
fastparse.parse(s, new Parser(p).document(_)) match {
fastparse.parse(s, new Parser(p, true).document(_)) match {
case Success(v, _) => v
}
}
Expand Down
2 changes: 1 addition & 1 deletion bench/src/main/scala/sjsonnet/ParserBenchmark.scala
Expand Up @@ -25,7 +25,7 @@ class ParserBenchmark {
@Benchmark
def main(bh: Blackhole): Unit = {
bh.consume(allFiles.foreach { case (p, s) =>
val res = fastparse.parse(s, new Parser(p).document(_))
val res = fastparse.parse(s, new Parser(p, true).document(_))
bh.consume(res.asInstanceOf[Success[_]])
})
}
Expand Down
2 changes: 1 addition & 1 deletion build.sc
@@ -1,5 +1,5 @@
import mill._, scalalib._, publish._, scalajslib._, scalanativelib._, scalanativelib.api._
val sjsonnetVersion = "0.4.5-RC4"
val sjsonnetVersion = "0.4.5-RC5"

object sjsonnet extends Cross[SjsonnetModule]("2.12.13", "2.13.4")
class SjsonnetModule(val crossScalaVersion: String) extends Module {
Expand Down
5 changes: 5 additions & 0 deletions sjsonnet/src-jvm-native/sjsonnet/Config.scala
Expand Up @@ -128,4 +128,9 @@ case class Config(
doc = "Raise an error if an object comprehension contains duplicate keys"
)
noDuplicateKeysInComprehension: Flag = Flag(),
@arg(
name = "strict-import-syntax",
doc = """Raise an error if import expressions are used without proper parentheses, e.g. import "foo".bar rather than (import "foo").bar"""
)
strictImportSyntax: Flag = Flag(),
)
1 change: 1 addition & 0 deletions sjsonnet/src-jvm-native/sjsonnet/SjsonnetMain.scala
Expand Up @@ -216,6 +216,7 @@ object SjsonnetMain {
strict = config.strict.value,
noStaticErrors = config.noStaticErrors.value,
noDuplicateKeysInComprehension = config.noDuplicateKeysInComprehension.value,
strictImportSyntax = config.strictImportSyntax.value
),
storePos = if (config.yamlDebug.value) currentPos = _ else null,
warnLogger = warnLogger,
Expand Down
6 changes: 3 additions & 3 deletions sjsonnet/src/sjsonnet/Importer.scala
Expand Up @@ -43,12 +43,12 @@ class CachedImporter(parent: Importer) extends Importer {

class CachedResolver(
parentImporter: Importer,
val parseCache: ParseCache
) extends CachedImporter(parentImporter) {
val parseCache: ParseCache,
strictImportSyntax: Boolean) extends CachedImporter(parentImporter) {

def parse(path: Path, txt: String)(implicit ev: EvalErrorScope): Either[Error, (Expr, FileScope)] = {
parseCache.getOrElseUpdate((path, txt), {
val parsed = fastparse.parse(txt, new Parser(path).document(_)) match {
val parsed = fastparse.parse(txt, new Parser(path, strictImportSyntax).document(_)) match {
case f @ Parsed.Failure(_, _, _) =>
val traced = f.trace()
val pos = new Position(new FileScope(path), traced.index)
Expand Down
2 changes: 1 addition & 1 deletion sjsonnet/src/sjsonnet/Interpreter.scala
Expand Up @@ -21,7 +21,7 @@ class Interpreter(extVars: Map[String, String],
std: Val.Obj = new Std().Std
) { self =>

val resolver = new CachedResolver(importer, parseCache) {
val resolver = new CachedResolver(importer, parseCache, settings.strictImportSyntax) {
override def process(expr: Expr, fs: FileScope): Either[Error, (Expr, FileScope)] =
handleException(new StaticOptimizer(evaluator, std).optimize(expr), fs)
}
Expand Down
14 changes: 9 additions & 5 deletions sjsonnet/src/sjsonnet/Parser.scala
Expand Up @@ -44,7 +44,8 @@ object Parser {
private val emptyLazyArray = new Array[Lazy](0)
}

class Parser(val currentFile: Path) {
class Parser(val currentFile: Path,
strictImportSyntax: Boolean) {
import Parser._

private val fileScope = new FileScope(currentFile)
Expand Down Expand Up @@ -241,10 +242,13 @@ class Parser(val currentFile: Path) {
def `import`[_: P](pos: Position) = P( importExpr.map(Expr.Import(pos, _)) )
def error[_: P](pos: Position) = P(expr.map(Expr.Error(pos, _)) )

def importExpr[_: P]: P[String] = P(expr.flatMap {
case Val.Str(_, s) => Pass(s)
case _ => Fail.opaque("string literal (computed imports are not allowed)")
})
def importExpr[_: P]: P[String] = P(
if (!strictImportSyntax) string
else expr.flatMap {
case Val.Str(_, s) => Pass(s)
case _ => Fail.opaque("string literal (computed imports are not allowed)")
}
)

def unaryOpExpr[_: P](pos: Position, op: Char) = P(
expr1.map{ e =>
Expand Down
1 change: 1 addition & 0 deletions sjsonnet/src/sjsonnet/Settings.scala
Expand Up @@ -7,6 +7,7 @@ class Settings(
val strict: Boolean = false,
val noStaticErrors: Boolean = false,
val noDuplicateKeysInComprehension: Boolean = false,
val strictImportSyntax: Boolean = false,
)

object Settings {
Expand Down
17 changes: 13 additions & 4 deletions sjsonnet/test/src/sjsonnet/ParserTests.scala
Expand Up @@ -4,8 +4,8 @@ import Expr._
import fastparse.Parsed
import Val.{True, Num}
object ParserTests extends TestSuite{
def parse(s: String) = fastparse.parse(s, new Parser(null).document(_)).get.value._1
def parseErr(s: String) = fastparse.parse(s, new Parser(null).document(_), verboseFailures = true).asInstanceOf[Parsed.Failure].msg
def parse(s: String, strictImportSyntax: Boolean = false) = fastparse.parse(s, new Parser(null, strictImportSyntax).document(_)).get.value._1
def parseErr(s: String, strictImportSyntax: Boolean = false) = fastparse.parse(s, new Parser(null, strictImportSyntax).document(_), verboseFailures = true).asInstanceOf[Parsed.Failure].msg
val dummyFS = new FileScope(null)
def pos(i: Int) = new Position(dummyFS, i)
def tests = Tests{
Expand Down Expand Up @@ -33,8 +33,17 @@ object ParserTests extends TestSuite{
LocalExpr(pos(6), Array(Bind(pos(6), "foo", null, Import(pos(12), "foo"))), Num(pos(26),0.0))
parse("""local foo = (import "foo") + bar; 0""") ==>
LocalExpr(pos(6), Array(Bind(pos(6), "foo", null, BinaryOp(pos(27), Import(pos(13), "foo"), 3, Id(pos(29), "bar")))), Num(pos(34),0.0))
parseErr("""local foo = import ("foo" + bar); 0""") ==> """Expected string literal (computed imports are not allowed):1:33, found "; 0""""
parseErr("""local foo = import "foo" + bar; 0""") ==> """Expected string literal (computed imports are not allowed):1:31, found "; 0""""

parse("""import "foo".bar""", strictImportSyntax = false) ==> Select(pos(12),Import(pos(0),"foo"), "bar")
parse("""import "foo"[1]""", strictImportSyntax = false) ==> Lookup(pos(12),Import(pos(0), "foo"), Num(pos(13), 1))

parseErr("""import "foo".bar""", strictImportSyntax = true)
parseErr("""import "foo"[1]""", strictImportSyntax = true)

parseErr("""local foo = import ("foo" + bar); 0""", strictImportSyntax = true) ==>
"""Expected string literal (computed imports are not allowed):1:33, found "; 0""""
parseErr("""local foo = import "foo" + bar; 0""", strictImportSyntax = true) ==>
"""Expected string literal (computed imports are not allowed):1:31, found "; 0""""
}

test("id starts with number") {
Expand Down

0 comments on commit ccbe6e3

Please sign in to comment.