From 68dd141d28dd6d2f3f17b32cdb6fcdb0ae5fa6b7 Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Wed, 4 Jan 2023 11:50:59 +0100 Subject: [PATCH 1/8] red test on mapped prededence, using SPAssoc with map --- .../scala/diesel/PrecedenceMappedTest.scala | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala diff --git a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala new file mode 100644 index 0000000..9cbba39 --- /dev/null +++ b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala @@ -0,0 +1,68 @@ +/* + * Copyright 2018 The Diesel Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package diesel + +import diesel.Dsl.{Axiom, Syntax} +import diesel.samples.calc.Ast._ +import diesel.samples.calc.MathBase + +object MyDslWithMappedPrededence extends MathBase { + case class Div(d1: Expr, d2: Expr) extends Expr + + val div: Syntax[Expr] = syntax(number)( + number ~ ("/".leftAssoc(13) map { case (_, t) => t }) ~ number map { + case (c, (n1, plus, n2)) => + c.setTokenStyle(plus, KeywordStyle) + Div(n1, n2) + } + ) + + val expr: Axiom[Expr] = axiom(number) + +} + +class PrecedenceMappedTest extends DslTestFunSuite { + import MyDslWithMappedPrededence.Div + + type Ast = Expr + override def dsl = MyDslWithMappedPrededence + + test("unmapped precedence") { + assertAst("1 + 2 + 3") { + Add( + Add( + Value(1), + Value(2) + ), + Value(3) + ) + } + } + + test("mapped precedence") { + assertAst("1 / 2 / 3") { + Div( + Div( + Value(1), + Value(2) + ), + Value(3) + ) + } + } + +} From 79fdbf66e781d7c02e084346de518286a28df73c Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Wed, 4 Jan 2023 11:51:29 +0100 Subject: [PATCH 2/8] propagate feature on SPMapped during BNF generation --- diesel/shared/src/main/scala/diesel/Bnf.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/diesel/shared/src/main/scala/diesel/Bnf.scala b/diesel/shared/src/main/scala/diesel/Bnf.scala index a69141c..1095635 100644 --- a/diesel/shared/src/main/scala/diesel/Bnf.scala +++ b/diesel/shared/src/main/scala/diesel/Bnf.scala @@ -866,8 +866,9 @@ object Bnf { case SPMapped(p, _) => val mapRule = getOrCreateRule(owner.name, "map." + counter) counter += 1 - mapAction(mapRule, p, mapSyntaxProduction(mapRule, p, ctx, None, first), element) - Partial(Seq(mapRule)) + val partial = mapSyntaxProduction(mapRule, p, ctx, None, first) + mapAction(mapRule, p, partial, element) + Partial(Seq(mapRule)).merge(0, partial.feature) case SPAssoc(p, associativity, level) => val partial = mapSyntaxProduction(owner, p, ctx, None, first) From 4c9f692525dffe80dc27e6da7458a2808252aa7f Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Wed, 4 Jan 2023 16:56:06 +0100 Subject: [PATCH 3/8] improve dev experience on unused imports --- .scalafix.conf | 1 + build.sbt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.scalafix.conf b/.scalafix.conf index a2038e8..318e053 100644 --- a/.scalafix.conf +++ b/.scalafix.conf @@ -4,6 +4,7 @@ rules = [ LeakingImplicitClassVal NoAutoTupling NoValInForComprehension + RemoveUnused ] # find more at https://scalacenter.github.io/scalafix/docs/rules/community-rules.html diff --git a/build.sbt b/build.sbt index 3cbb924..0790122 100644 --- a/build.sbt +++ b/build.sbt @@ -58,7 +58,7 @@ lazy val sharedSettings_scalac = Seq( "-unchecked", "-deprecation", "-feature", - "-Xfatal-warnings", +// "-Xfatal-warnings", "-Wconf:cat=deprecation:i", "-language:existentials", "-Wunused:imports", From a27e7ff1683362026c56a8a3e97cc42083d779af Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Wed, 4 Jan 2023 17:04:10 +0100 Subject: [PATCH 4/8] infinit loop on nested precedence --- .../scala/diesel/PrecedenceMappedTest.scala | 49 +++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala index 9cbba39..a27828e 100644 --- a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala +++ b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala @@ -20,8 +20,9 @@ import diesel.Dsl.{Axiom, Syntax} import diesel.samples.calc.Ast._ import diesel.samples.calc.MathBase -object MyDslWithMappedPrededence extends MathBase { +object MyDslWithMappedPrecedence extends MathBase { case class Div(d1: Expr, d2: Expr) extends Expr + case class Abs(d: Expr) extends Expr val div: Syntax[Expr] = syntax(number)( number ~ ("/".leftAssoc(13) map { case (_, t) => t }) ~ number map { @@ -31,15 +32,22 @@ object MyDslWithMappedPrededence extends MathBase { } ) + val abs: Syntax[Expr] = syntax(number)( + "abs".leftAssoc(9) ~ number map { + case (_, (_, n)) => + Abs(n) + } + ) + val expr: Axiom[Expr] = axiom(number) } class PrecedenceMappedTest extends DslTestFunSuite { - import MyDslWithMappedPrededence.Div + import MyDslWithMappedPrecedence.{Div, Abs} type Ast = Expr - override def dsl = MyDslWithMappedPrededence + override def dsl = MyDslWithMappedPrecedence test("unmapped precedence") { assertAst("1 + 2 + 3") { @@ -65,4 +73,39 @@ class PrecedenceMappedTest extends DslTestFunSuite { } } + test("mixed precedence") { + assertAst("abs 1 + 2") { + Abs( + Add( + Value(1), + Value(2) + ) + ) + } + } + + test("nested precedence".only) { + assertAst("abs abs 1 + 2") { + Abs( + Abs( + Add( + Value(1), + Value(2) + ) + ) + ) + } + } + + test("mapped mixed precedence") { + assertAst("abs 1 / 2") { + Abs( + Div( + Value(1), + Value(2) + ) + ) + } + } + } From ef18ce35e970f5c3ea6100af39aa606815f83892 Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Fri, 6 Jan 2023 08:30:02 +0100 Subject: [PATCH 5/8] allow asserting for multiple AST values --- .../src/main/scala/diesel/AstHelpers.scala | 12 ++++++++++++ .../test/scala/diesel/DslTestFunSuite.scala | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/diesel/shared/src/main/scala/diesel/AstHelpers.scala b/diesel/shared/src/main/scala/diesel/AstHelpers.scala index d6b1104..12f1c47 100644 --- a/diesel/shared/src/main/scala/diesel/AstHelpers.scala +++ b/diesel/shared/src/main/scala/diesel/AstHelpers.scala @@ -101,6 +101,18 @@ object AstHelpers { } } + def assertAllAsts( + dsl: Dsl, + verbalizer: Option[Verbalizer] = None, + axiom: Option[Axiom[_]] = None, + navigatorFactory: Result => Navigator = Navigator(_) + )(s: String)(f: Seq[GenericTree] => Unit): Unit = { + assertAsts(dsl, verbalizer, axiom, navigatorFactory)(s) { n: Navigator => + val as = Seq.from(n.toIterator) + f(as) + } + } + def withAst[T]( dsl: Dsl, verbalizer: Option[Verbalizer] = None, diff --git a/diesel/shared/src/test/scala/diesel/DslTestFunSuite.scala b/diesel/shared/src/test/scala/diesel/DslTestFunSuite.scala index c4f27f2..9e41d19 100644 --- a/diesel/shared/src/test/scala/diesel/DslTestFunSuite.scala +++ b/diesel/shared/src/test/scala/diesel/DslTestFunSuite.scala @@ -41,6 +41,12 @@ abstract class DslTestFunSuite extends FunSuite { } } + protected def assertAllAsts(text: String)(expected: => Seq[Ast]): Unit = { + withAllAsts(text) { asts => + assertEquals(expected, asts) + } + } + protected def assertMarkers(text: String)(expected: => Seq[Marker]): Unit = { withTree(text) { tree => assertEquals(tree.markers, expected) @@ -54,6 +60,12 @@ abstract class DslTestFunSuite extends FunSuite { } } + protected def withAllAsts(text: String)(f: Seq[Ast] => Unit): Unit = { + withAllTrees(text) { trees => + f(trees.map(ast)) + } + } + protected def withAsts(text: String)(f: Navigator => Unit): Unit = { AstHelpers.assertAsts(dsl, axiom = axiom)(text) { navigator => f(navigator) @@ -72,6 +84,12 @@ abstract class DslTestFunSuite extends FunSuite { } } + protected def withAllTrees(text: String)(f: Seq[GenericTree] => Unit): Unit = { + AstHelpers.assertAllAsts(dsl, verbalizer = verbalizer, axiom = axiom)(text) { trees => + f(trees) + } + } + // IntellliJ hint // replace: testAst\(("[^"]*")\) \{([^\}]+) // by: test($1) { assertAst($1) { $2 } From 191c0ed33ef8c226f2b66f8ab860e11ee14e199b Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Fri, 6 Jan 2023 08:30:37 +0100 Subject: [PATCH 6/8] adjust tests to what we decided --- .../scala/diesel/PrecedenceMappedTest.scala | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala index a27828e..bb2bbd8 100644 --- a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala +++ b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala @@ -17,6 +17,7 @@ package diesel import diesel.Dsl.{Axiom, Syntax} +import diesel.MyDslWithMappedPrecedence.Div import diesel.samples.calc.Ast._ import diesel.samples.calc.MathBase @@ -33,7 +34,7 @@ object MyDslWithMappedPrecedence extends MathBase { ) val abs: Syntax[Expr] = syntax(number)( - "abs".leftAssoc(9) ~ number map { + "abs".rightAssoc(1) ~ number map { case (_, (_, n)) => Abs(n) } @@ -61,14 +62,13 @@ class PrecedenceMappedTest extends DslTestFunSuite { } } - test("mapped precedence") { - assertAst("1 / 2 / 3") { - Div( - Div( - Value(1), - Value(2) - ), - Value(3) + test("mapped precedence is lost") { + // mapping a token does not propagate its precedence + // we will find ways to not even allow that situation via API + assertAllAsts("1 / 2 / 3") { + Seq( + Div(Div(Value(1), Value(2)), Value(3)), + Div(Value(1), Div(Value(2), Value(3))) ) } } @@ -84,7 +84,7 @@ class PrecedenceMappedTest extends DslTestFunSuite { } } - test("nested precedence".only) { + test("nested precedence") { assertAst("abs abs 1 + 2") { Abs( Abs( @@ -97,15 +97,4 @@ class PrecedenceMappedTest extends DslTestFunSuite { } } - test("mapped mixed precedence") { - assertAst("abs 1 / 2") { - Abs( - Div( - Value(1), - Value(2) - ) - ) - } - } - } From 160c15a3084fa1e4b11d0cd700cb97fd6c69a7a8 Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Fri, 6 Jan 2023 08:33:17 +0100 Subject: [PATCH 7/8] undo temptative fix, keep unchanged --- diesel/shared/src/main/scala/diesel/Bnf.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/diesel/shared/src/main/scala/diesel/Bnf.scala b/diesel/shared/src/main/scala/diesel/Bnf.scala index 1095635..a69141c 100644 --- a/diesel/shared/src/main/scala/diesel/Bnf.scala +++ b/diesel/shared/src/main/scala/diesel/Bnf.scala @@ -866,9 +866,8 @@ object Bnf { case SPMapped(p, _) => val mapRule = getOrCreateRule(owner.name, "map." + counter) counter += 1 - val partial = mapSyntaxProduction(mapRule, p, ctx, None, first) - mapAction(mapRule, p, partial, element) - Partial(Seq(mapRule)).merge(0, partial.feature) + mapAction(mapRule, p, mapSyntaxProduction(mapRule, p, ctx, None, first), element) + Partial(Seq(mapRule)) case SPAssoc(p, associativity, level) => val partial = mapSyntaxProduction(owner, p, ctx, None, first) From be47c2eb90ae8d233f948343693be320772a3fee Mon Sep 17 00:00:00 2001 From: Frank Wagner Date: Fri, 6 Jan 2023 08:33:33 +0100 Subject: [PATCH 8/8] lint fix --- diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala index bb2bbd8..f0de4b1 100644 --- a/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala +++ b/diesel/shared/src/test/scala/diesel/PrecedenceMappedTest.scala @@ -17,7 +17,6 @@ package diesel import diesel.Dsl.{Axiom, Syntax} -import diesel.MyDslWithMappedPrecedence.Div import diesel.samples.calc.Ast._ import diesel.samples.calc.MathBase