-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Correctly detect colon lambda eol indent for optional brace of argument #22477
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -638,7 +638,6 @@ object Scanners { | |
insert(OUTDENT, offset) | ||
else if r.isInstanceOf[InBraces] && !closingRegionTokens.contains(token) then | ||
report.warning("Line is indented too far to the left, or a `}` is missing", sourcePos()) | ||
|
||
else if lastWidth < nextWidth | ||
|| lastWidth == nextWidth && (lastToken == MATCH || lastToken == CATCH) && token == CASE then | ||
if canStartIndentTokens.contains(lastToken) then | ||
|
@@ -658,7 +657,7 @@ object Scanners { | |
def spaceTabMismatchMsg(lastWidth: IndentWidth, nextWidth: IndentWidth): Message = | ||
em"""Incompatible combinations of tabs and spaces in indentation prefixes. | ||
|Previous indent : $lastWidth | ||
|Latest indent : $nextWidth""" | ||
|Latest indent : $nextWidth""" | ||
|
||
def observeColonEOL(inTemplate: Boolean): Unit = | ||
val enabled = | ||
|
@@ -672,6 +671,20 @@ object Scanners { | |
reset() | ||
if atEOL then token = COLONeol | ||
|
||
// consume => and insert <indent> if applicable | ||
def observeArrowIndented(): Unit = | ||
if isArrow && indentSyntax then | ||
peekAhead() | ||
val atEOL = isAfterLineEnd || token == EOF | ||
reset() | ||
if atEOL then | ||
val nextWidth = indentWidth(next.offset) | ||
val lastWidth = currentRegion.indentWidth | ||
if lastWidth < nextWidth then | ||
currentRegion = Indented(nextWidth, COLONeol, currentRegion) | ||
offset = next.offset | ||
token = INDENT | ||
|
||
def observeIndented(): Unit = | ||
if indentSyntax && isNewLine then | ||
val nextWidth = indentWidth(next.offset) | ||
|
@@ -680,7 +693,6 @@ object Scanners { | |
currentRegion = Indented(nextWidth, COLONeol, currentRegion) | ||
offset = next.offset | ||
token = INDENT | ||
end observeIndented | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, I also don't like these end markers 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the method used to be longer and had a blank line. I must have deleted it during other edits while experimenting, but here it's no longer needed. |
||
|
||
/** Insert an <outdent> token if next token closes an indentation region. | ||
* Exception: continue if indentation region belongs to a `match` and next token is `case`. | ||
|
@@ -1100,7 +1112,7 @@ object Scanners { | |
reset() | ||
next | ||
|
||
class LookaheadScanner(val allowIndent: Boolean = false) extends Scanner(source, offset, allowIndent = allowIndent) { | ||
class LookaheadScanner(allowIndent: Boolean = false) extends Scanner(source, offset, allowIndent = allowIndent) { | ||
override protected def initialCharBufferSize = 8 | ||
override def languageImportContext = Scanner.this.languageImportContext | ||
} | ||
|
@@ -1652,7 +1664,7 @@ object Scanners { | |
case class InCase(outer: Region) extends Region(OUTDENT) | ||
|
||
/** A class describing an indentation region. | ||
* @param width The principal indendation width | ||
* @param width The principal indentation width | ||
* @param prefix The token before the initial <indent> of the region | ||
*/ | ||
case class Indented(width: IndentWidth, prefix: Token, outer: Region | Null) extends Region(OUTDENT): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
|
||
def fn2(arg: String, arg2: String)(f: String => Unit): Unit = f(arg) | ||
|
||
def fn3(arg: String, arg2: String)(f: => Unit): Unit = f | ||
|
||
def test1() = | ||
|
||
// ok baseline | ||
fn2(arg = "blue sleeps faster than tuesday", arg2 = "the quick brown fox jumped over the lazy dog"): env => | ||
val x = env | ||
som-snytt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
println(x) | ||
|
||
fn2( // error not a legal formal parameter for a function literal | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): env => | ||
val x = env // error | ||
println(x) | ||
|
||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
env => // error indented definitions expected, identifier env found | ||
val x = env | ||
println(x) | ||
|
||
def test2() = | ||
|
||
fn3( // error missing argument list for value of type (=> Unit) => Unit | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" // error | ||
println(x) // error |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
|
||
def fn2(arg: String, arg2: String)(f: String => Unit): Unit = f(arg) | ||
|
||
def fn3(arg: String, arg2: String)(f: => Unit): Unit = f | ||
|
||
def test() = | ||
|
||
fn2(arg = "blue sleeps faster than tuesday", arg2 = "the quick brown fox jumped over the lazy dog"): env => | ||
val x = env | ||
println(x) | ||
|
||
// doesn't compile | ||
som-snytt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): env => | ||
val x = env | ||
println(x) | ||
|
||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): env => | ||
val x = env | ||
println(x) | ||
|
||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): env => | ||
val x = env | ||
println(x) | ||
|
||
// does compile | ||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
env => | ||
val x = env | ||
println(x) | ||
|
||
// does compile | ||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog" | ||
): env => | ||
val x = env | ||
println(x) | ||
|
||
fn2( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog" | ||
): env => | ||
val x = env | ||
println(x) | ||
|
||
fn3( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" | ||
println(x) | ||
|
||
fn3( | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" | ||
println(x) | ||
|
||
som-snytt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fn3( // arg at 3, body at 3 | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" | ||
println(x) | ||
|
||
fn3( // arg at 3, body at 1: not sure if sig indent of 1 is allowed, saw some comments from odersky | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" | ||
println(x) | ||
|
||
fn3( // arg at 3, body at 2: even if sig indent of 1 is not allowed, body is at fn3+2, not arg2-1 | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" | ||
println(x) | ||
|
||
fn3( // arg at 3, body at 4 | ||
arg = "blue sleeps faster than tuesday", | ||
arg2 = "the quick brown fox jumped over the lazy dog"): | ||
val x = "Hello" | ||
println(x) | ||
|
||
// don't turn innocent empty cases into functions | ||
def regress(x: Int) = | ||
x match | ||
case 42 => | ||
case _ => | ||
|
||
// previously lookahead calculated indent width at the colon | ||
def k(xs: List[Int]) = | ||
xs.foldLeft( | ||
0) | ||
: (acc, x) => | ||
acc + x | ||
|
||
def `test kit`(xs: List[Int]): Unit = | ||
def addOne(i: Int): Int = i + 1 | ||
def isPositive(i: Int): Boolean = i > 0 | ||
// doesn't compile but would be nice | ||
// first body is indented "twice", or, rather, first outdent establishes an intermediate indentation level | ||
xs.map: x => | ||
x + 1 | ||
.filter: x => | ||
x > 0 | ||
xs.map: | ||
addOne | ||
.filter: | ||
isPositive | ||
|
||
// does compile | ||
xs | ||
.map: x => | ||
x + 1 | ||
.filter: x => | ||
x > 0 | ||
|
||
// does compile but doesn't look good, at least, to some people | ||
xs.map: x => | ||
x + 1 | ||
.filter: x => | ||
x > 0 | ||
|
||
def `tested kit`(xs: List[Int]): Unit = | ||
{ | ||
def addOne(i: Int): Int = i.+(1) | ||
def isPositive(i: Int): Boolean = i.>(0) | ||
xs.map[Int]((x: Int) => x.+(1)).filter((x: Int) => x.>(0)) | ||
xs.map[Int]((i: Int) => addOne(i)).filter((i: Int) => isPositive(i)) | ||
xs.map[Int]((x: Int) => x.+(1)).filter((x: Int) => x.>(0)) | ||
{ | ||
xs.map[Int]((x: Int) => x.+(1)).filter((x: Int) => x.>(0)) | ||
() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected to use
COLONeol
here? Or could useARROW
instead?At first sight, it only seems to be used here where it's compared to
MATCH
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first attempt used ARROW and used it during newline handling, but that was not needed. I changed to colon for uniformity only; the result here is only produced and discarded in lookahead scanner. Actually on my WIP branch, I used a new token
ARROWeol
for when it is not a proper indent, as afterMATCH
. (Region prefix is usedscala3/compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Line 615 in b57549e
There is experimental (dubious) use of the prefix at https://github.com/scala/scala3/pull/22530/files just to show that the prefix, if observed, could have semantics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let's get that merged then!