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.

Sign up for GitHub

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

Jump to bottom

Don't create block on indentation with a single statement #2183

Merged
merged 1 commit into from Dec 1, 2020
Merged

Don't create block on indentation with a single statement #2183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 48 additions & 36 deletions scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala
View file
Open in desktop
Expand Up @@ -2062,7 +2062,7 @@ class ScalametaParser(input: Input, dialect: Dialect) { parser =>

private def exprMaybeIndented(): Term = {
if (token.is[Indentation.Indent]) {
indented(block())
block()
} else {
expr()
}
Expand All @@ -2087,42 +2087,55 @@ class ScalametaParser(input: Input, dialect: Dialect) { parser =>
atPos(t, auto)(Term.Match(t, inBracesOrNil(caseClauses())))
}
}

def ifClause(isInline: Boolean = false) = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is moved out from expr

accept[KwIf]
val (cond, thenp) = if (token.isNot[LeftParen] && dialect.allowSignificantIndentation) {
val cond = expr()
acceptOpt[LF]
accept[KwThen]
(cond, exprMaybeIndented())
} else {
val forked = in.fork
var cond = condExpr()
if ((token.is[Ident] && isLeadingInfixOperator(token)) || token.is[Dot]) {
in = forked
cond = expr()
}
newLinesOpt()
acceptOpt[KwThen]
(cond, exprMaybeIndented())
}

val ifExpr = if (tryAcceptWithOptLF[KwElse]) {
Term.If(cond, thenp, exprMaybeIndented())
} else if (token.is[Semicolon] && ahead { token.is[KwElse] }) {
next(); next(); Term.If(cond, thenp, expr())
} else {
Term.If(cond, thenp, atPos(in.tokenPos, in.prevTokenPos)(Lit.Unit()))
}

if (isInline) {
ifExpr.setMods(List(Mod.Inline()))
}
ifExpr
}

// FIXME: when parsing `(2 + 3)`, do we want the ApplyInfix's position to include parentheses?
// if yes, then nothing has to change here
// if no, we need eschew autoPos here, because it forces those parentheses on the result of calling prefixExpr
// see https://github.com/scalameta/scalameta/issues/1083 and https://github.com/scalameta/scalameta/issues/1223
def expr(location: Location, allowRepeated: Boolean): Term =
autoPos(token match {
case SkInline() if ahead(token.is[KwIf]) =>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out there was one more issue with ifs, so a much safer place to check it is here.

The problem was with
def a = inline if cond then a

accept[Ident]
ifClause(isInline = true)
case KwIf() =>
next()
val (cond, thenp) = if (token.isNot[LeftParen] && dialect.allowSignificantIndentation) {
val cond = expr()
acceptOpt[LF]
accept[KwThen]
(cond, exprMaybeIndented())
} else {
val forked = in.fork
var cond = condExpr()
if ((token.is[Ident] && isLeadingInfixOperator(token)) || token.is[Dot]) {
in = forked
cond = expr()
}
newLinesOpt()
acceptOpt[KwThen]
(cond, exprMaybeIndented())
}

if (tryAcceptWithOptLF[KwElse]) {
Term.If(cond, thenp, exprMaybeIndented())
} else if (token.is[Semicolon] && ahead { token.is[KwElse] }) {
next(); next(); Term.If(cond, thenp, expr())
} else {
Term.If(cond, thenp, atPos(in.tokenPos, in.prevTokenPos)(Lit.Unit()))
}
ifClause()
case KwTry() =>
next()
val body: Term = token match {
case Indentation.Indent() => indented(block())
case Indentation.Indent() => block()
case _ if dialect.allowTryWithAnyExpr => expr()
case LeftBrace() => autoPos(inBracesOrUnit(block()))
case LeftParen() => inParensOrUnit(expr())
Expand Down Expand Up @@ -2883,7 +2896,13 @@ class ScalametaParser(input: Input, dialect: Dialect) { parser =>
}

def block(): Term = autoPos {
if (token.is[Indentation.Indent]) indented(Term.Block(blockStatSeq()))
if (token.is[Indentation.Indent]) indented {
val stats = blockStatSeq()
stats match {
case (head: Term) :: Nil => head
case others => Term.Block(others)
}
}
else Term.Block(blockStatSeq())
}

Expand Down Expand Up @@ -3825,14 +3844,7 @@ class ScalametaParser(input: Input, dialect: Dialect) { parser =>
case KwCase() if ahead(token.is[Ident]) && dialect.allowEnums =>
enumCaseDef(mods)
case KwIf() if mods.size == 1 && mods.head.is[Mod.Inline] =>
expr() match {
case ifExpr: Term.If =>
ifExpr.setMods(mods)
ifExpr
case expr => syntaxError("Unexpected expressions after inline modifier", at = expr.pos)
}
case KwIf() if mods.size > 0 =>
syntaxError("If can only contain inline modifier", at = token.pos)
ifClause(isInline = true)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the additional case with non-inline mods - it will be reported anyway as error.

case other =>
tmplDef(mods)
}
Expand Down
122 changes: 42 additions & 80 deletions tests/shared/src/test/scala/scala/meta/tests/parsers/dotty/ControlSyntaxSuite.scala
View file
Open in desktop
Expand Up @@ -34,11 +34,9 @@ class ControlSyntaxSuite extends BaseDottySuite {
|else
| gx
|""".stripMargin
val output = """|if (cond) fx else {
| gx
|}""".stripMargin
val output = """|if (cond) fx else gx""".stripMargin
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.If(Term.Name("cond"), Term.Name("fx"), Term.Block(List(Term.Name("gx"))))
Term.If(Term.Name("cond"), Term.Name("fx"), Term.Name("gx"))
)
}

Expand Down Expand Up @@ -76,16 +74,12 @@ class ControlSyntaxSuite extends BaseDottySuite {
|else
| gx
|""".stripMargin
val output = """|if (cond) {
| fx
|} else {
| gx
|}""".stripMargin
val output = "if (cond) fx else gx"
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.If(
Term.Name("cond"),
Term.Block(List(Term.Name("fx"))),
Term.Block(List(Term.Name("gx")))
Term.Name("fx"),
Term.Name("gx")
)
)
}
Expand All @@ -96,13 +90,11 @@ class ControlSyntaxSuite extends BaseDottySuite {
|then
| gx
|""".stripMargin
val output = """|if (cond1 && cond2) {
| gx
|}""".stripMargin
val output = "if (cond1 && cond2) gx"
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.If(
Term.ApplyInfix(Term.Name("cond1"), Term.Name("&&"), Nil, List(Term.Name("cond2"))),
Term.Block(List(Term.Name("gx"))),
Term.Name("gx"),
Lit.Unit()
)
)
Expand Down Expand Up @@ -166,15 +158,12 @@ class ControlSyntaxSuite extends BaseDottySuite {
| else
| B)
|""".stripMargin
val output = """|fx(if (cond) A else {
| B
|})
|""".stripMargin
val output = "fx(if (cond) A else B)"
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.Apply(
Term.Name("fx"),
List(
Term.If(Term.Name("cond"), Term.Name("A"), Term.Block(List(Term.Name("B"))))
Term.If(Term.Name("cond"), Term.Name("A"), Term.Name("B"))
)
)
)
Expand Down Expand Up @@ -280,17 +269,12 @@ class ControlSyntaxSuite extends BaseDottySuite {
|finally
| ok
|""".stripMargin
val output = """|try {
| fx
|} finally {
| ok
|}
|""".stripMargin
val output = "try fx finally ok"
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.Try(
Term.Block(List(Term.Name("fx"))),
Term.Name("fx"),
Nil,
Some(Term.Block(List(Term.Name("ok"))))
Some(Term.Name("ok"))
)
)
}
Expand Down Expand Up @@ -461,17 +445,15 @@ class ControlSyntaxSuite extends BaseDottySuite {
| case x =>
| ax
| bx
|} finally {
| fx
|}
|} finally fx
|""".stripMargin
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.Try(
Term.Name("fx"),
List(
Case(Pat.Var(Term.Name("x")), None, Term.Block(List(Term.Name("ax"), Term.Name("bx"))))
),
Some(Term.Block(List(Term.Name("fx"))))
Some(Term.Name("fx"))
)
)
}
Expand Down Expand Up @@ -680,13 +662,11 @@ class ControlSyntaxSuite extends BaseDottySuite {
|do
| fx
|""".stripMargin
val output = """|for (a <- gen) {
| fx
|}""".stripMargin
val output = "for (a <- gen) fx"
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.For(
List(Enumerator.Generator(Pat.Var(Term.Name("a")), Term.Name("gen"))),
Term.Block(List(Term.Name("fx")))
Term.Name("fx")
)
)
}
Expand Down Expand Up @@ -772,16 +752,14 @@ class ControlSyntaxSuite extends BaseDottySuite {
|yield
| fx
|""".stripMargin
val output = """|for (a <- gen; if cnd) yield {
| fx
|}""".stripMargin
val output = "for (a <- gen; if cnd) yield fx"
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.ForYield(
List(
Enumerator.Generator(Pat.Var(Term.Name("a")), Term.Name("gen")),
Enumerator.Guard(Term.Name("cnd"))
),
Term.Block(List(Term.Name("fx")))
Term.Name("fx")
)
)
}
Expand Down Expand Up @@ -814,17 +792,15 @@ class ControlSyntaxSuite extends BaseDottySuite {
val code = """|for case a: TP <- iter do
| echo
|""".stripMargin
val output = """|for ( case a: TP <- iter) {
| echo
|}
val output = """|for ( case a: TP <- iter) echo
|""".stripMargin
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.For(
List(
Enumerator
.CaseGenerator(Pat.Typed(Pat.Var(Term.Name("a")), Type.Name("TP")), Term.Name("iter"))
),
Term.Block(List(Term.Name("echo")))
Term.Name("echo")
)
)
}
Expand All @@ -833,9 +809,7 @@ class ControlSyntaxSuite extends BaseDottySuite {
val code = """|for case a: TP <- iter if cnd do
| echo
|""".stripMargin
val output = """|for ( case a: TP <- iter; if cnd) {
| echo
|}
val output = """|for ( case a: TP <- iter; if cnd) echo
|""".stripMargin
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.For(
Expand All @@ -844,7 +818,7 @@ class ControlSyntaxSuite extends BaseDottySuite {
.CaseGenerator(Pat.Typed(Pat.Var(Term.Name("a")), Type.Name("TP")), Term.Name("iter")),
Enumerator.Guard(Term.Name("cnd"))
),
Term.Block(List(Term.Name("echo")))
Term.Name("echo")
)
)
}
Expand Down Expand Up @@ -922,9 +896,7 @@ class ControlSyntaxSuite extends BaseDottySuite {
val code = """|for (arg, param) <- args.zip(vparams) yield
| arg
|""".stripMargin
val output = """|for ((arg, param) <- args.zip(vparams)) yield {
| arg
|}
val output = """|for ((arg, param) <- args.zip(vparams)) yield arg
|""".stripMargin
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.ForYield(
Expand All @@ -934,7 +906,7 @@ class ControlSyntaxSuite extends BaseDottySuite {
Term.Apply(Term.Select(Term.Name("args"), Term.Name("zip")), List(Term.Name("vparams")))
)
),
Term.Block(List(Term.Name("arg")))
Term.Name("arg")
)
)
}
Expand Down Expand Up @@ -988,18 +960,14 @@ class ControlSyntaxSuite extends BaseDottySuite {
| fx
| fy
|""".stripMargin
val output = """|while ({
| fx + fy
|}) {
val output = """|while (fx + fy) {
| fx
| fy
|}
|""".stripMargin
runTestAssert[Stat](code, assertLayout = Some(output))(
Term.While(
Term.Block(
List(Term.ApplyInfix(Term.Name("fx"), Term.Name("+"), Nil, List(Term.Name("fy"))))
),
Term.ApplyInfix(Term.Name("fx"), Term.Name("+"), Nil, List(Term.Name("fy"))),
Term.Block(List(Term.Name("fx"), Term.Name("fy")))
)
)
Expand Down Expand Up @@ -1571,22 +1539,18 @@ class ControlSyntaxSuite extends BaseDottySuite {
Nil,
List(List(Term.Param(Nil, Term.Name("x"), Some(Type.Name("Int")), None))),
Some(Type.Name("String")),
Term.Block(
List(
Term.Apply(
Term.Select(
Term.Match(
Term.Name("x"),
List(
Case(Lit.Int(1), None, Lit.String("1")),
Case(Pat.Wildcard(), None, Lit.String("ERR"))
)
),
Term.Name("trim")
),
Nil
)
)
Term.Apply(
Term.Select(
Term.Match(
Term.Name("x"),
List(
Case(Lit.Int(1), None, Lit.String("1")),
Case(Pat.Wildcard(), None, Lit.String("ERR"))
)
),
Term.Name("trim")
),
Nil
)
)

Expand All @@ -1608,12 +1572,10 @@ class ControlSyntaxSuite extends BaseDottySuite {
| .trim()
|""".stripMargin,
assertLayout = Some(
"""|def mtch(x: Int): String = {
| (x match {
| case 1 => "1"
| case _ => "ERR"
| }).trim()
|}
"""|def mtch(x: Int): String = (x match {
| case 1 => "1"
| case _ => "ERR"
|}).trim()
|""".stripMargin
)
)(expected)
Expand Down