From 77dde82a8fbccfd96c6b9116adff19285917b141 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 5 Feb 2025 16:06:48 -0800 Subject: [PATCH 1/4] Restrict outdent in parens for certain region prefixes --- .../dotty/tools/dotc/parsing/Scanners.scala | 9 ++- tests/neg/i22527.scala | 24 +++++++ tests/pos/i22527.scala | 65 +++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 tests/neg/i22527.scala create mode 100644 tests/pos/i22527.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index e5bba6c3b73b..dfa8e4a8a1bb 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -747,7 +747,14 @@ object Scanners { case _ => false currentRegion match case r: Indented if isEnclosedInParens(r.outer) => - insert(OUTDENT, offset) + // For some region prefixes (COLONeol, EQUALS) only OUTDENT if COMMA at EOL + if canStartIndentTokens.contains(r.prefix) && !statCtdTokens.contains(r.prefix) then + val lookahead = LookaheadScanner() + lookahead.nextToken() + if lookahead.isAfterLineEnd then + insert(OUTDENT, offset) + else + insert(OUTDENT, offset) case _ => peekAhead() if isAfterLineEnd diff --git a/tests/neg/i22527.scala b/tests/neg/i22527.scala new file mode 100644 index 000000000000..b9d9166e8fc2 --- /dev/null +++ b/tests/neg/i22527.scala @@ -0,0 +1,24 @@ + +//rule of thumb is COLONeol was at EOL, so COMMA must be at EOL +def test: Unit = + assert( + identity: + true, "ok" // error end of statement expected but ',' found + ) + +def toss: Unit = + assert( + throw + null, "ok" // error same + ) + +def callme[A](x: => A, msg: String) = try x.toString catch case t: RuntimeException => msg + +// not all indented regions require COMMA at EOL for OUTDENT +def orElse(x: Int): Unit = + callme( + if x > 0 then + class X extends AnyRef, Serializable // error Not found: Serializable - did you mean Specializable? + true // error ',' or ')' expected, but 'true' found + else + false, "fail") diff --git a/tests/pos/i22527.scala b/tests/pos/i22527.scala new file mode 100644 index 000000000000..055ed8acfae2 --- /dev/null +++ b/tests/pos/i22527.scala @@ -0,0 +1,65 @@ + +def f: Unit = + identity( + identity: + class X extends AnyRef, Serializable + 42 + ) + +def g: Unit = + identity( + x = + class X extends AnyRef, Serializable + 27 + ) + +def test: Unit = + assert( + identity: + true, + "ok" + ) + +def toss: Unit = + assert( + throw + null, + "ok" + ) + +def callme[A](x: => A, msg: String) = try x.toString catch case t: RuntimeException => msg + +def orElse(x: Int): Unit = + callme( + if x > 0 then + true + else + false, "fail") + +def onlyIf(x: Int): Unit = + callme( + if (x > 0) + true, "fail") // warn value discard + +def h(xs: List[Int]) = + xs.foldLeft(0) + ( + (acc, x) => + acc + + x, + ) + +def sum(x: Int, y: Int, z: Int) = x+y+z + +def k(xs: List[Int], y: Int, z: Int) = + xs.foldLeft(0) + ( + (acc, x) => + sum( + x + + y + + z, + y, + z, + ) + ) From 959e153a70c2f74696cb1a320ec72ce661b17e14 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 6 Feb 2025 09:41:27 -0800 Subject: [PATCH 2/4] Relax and restrict only colon regions --- .../dotty/tools/dotc/parsing/Scanners.scala | 4 ++-- tests/neg/i22527.scala | 13 ++++++------ tests/pos/i22527.scala | 21 ++++++++++++------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index dfa8e4a8a1bb..766ecd608995 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -747,8 +747,8 @@ object Scanners { case _ => false currentRegion match case r: Indented if isEnclosedInParens(r.outer) => - // For some region prefixes (COLONeol, EQUALS) only OUTDENT if COMMA at EOL - if canStartIndentTokens.contains(r.prefix) && !statCtdTokens.contains(r.prefix) then + // For region prefix COLONeol, only OUTDENT if COMMA at EOL + if r.prefix == COLONeol then val lookahead = LookaheadScanner() lookahead.nextToken() if lookahead.isAfterLineEnd then diff --git a/tests/neg/i22527.scala b/tests/neg/i22527.scala index b9d9166e8fc2..4b80c21ff035 100644 --- a/tests/neg/i22527.scala +++ b/tests/neg/i22527.scala @@ -6,12 +6,6 @@ def test: Unit = true, "ok" // error end of statement expected but ',' found ) -def toss: Unit = - assert( - throw - null, "ok" // error same - ) - def callme[A](x: => A, msg: String) = try x.toString catch case t: RuntimeException => msg // not all indented regions require COMMA at EOL for OUTDENT @@ -22,3 +16,10 @@ def orElse(x: Int): Unit = true // error ',' or ')' expected, but 'true' found else false, "fail") + +def g: Unit = + identity( + x = + class X extends AnyRef, Serializable // error + 27 // error + ) diff --git a/tests/pos/i22527.scala b/tests/pos/i22527.scala index 055ed8acfae2..d222c3311b86 100644 --- a/tests/pos/i22527.scala +++ b/tests/pos/i22527.scala @@ -6,13 +6,6 @@ def f: Unit = 42 ) -def g: Unit = - identity( - x = - class X extends AnyRef, Serializable - 27 - ) - def test: Unit = assert( identity: @@ -26,6 +19,11 @@ def toss: Unit = null, "ok" ) +def raise: Unit = + assert( + throw + null, "ok" // ok now + ) def callme[A](x: => A, msg: String) = try x.toString catch case t: RuntimeException => msg @@ -63,3 +61,12 @@ def k(xs: List[Int], y: Int, z: Int) = z, ) ) + +object `arrow eol`: + def f(g: Int => Int, i: Int): Int = g(i) + def g(map: Int => Int): Int => Int = map + def test = + f( + g: x => + x + 1, 42 + ) From b4a43197bdb26292f28d06abf65e3e76a593e0f6 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 8 Feb 2025 09:00:31 -0800 Subject: [PATCH 3/4] Warn about unindented args --- .../dotty/tools/dotc/parsing/Scanners.scala | 10 ++- .../tools/dotc/printing/RefinedPrinter.scala | 2 +- tests/neg/syntax-error-recovery.check | 6 +- tests/pos/i22527.scala | 9 ++- tests/warn/i22527.check | 22 +++++ tests/warn/i22527.scala | 81 +++++++++++++++++++ 6 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 tests/warn/i22527.check create mode 100644 tests/warn/i22527.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index 766ecd608995..a3d2b294fdb5 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -636,8 +636,14 @@ object Scanners { if skipping then if r.enclosing.isClosedByUndentAt(nextWidth) then 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 + val checkable = r match + case _: InBraces => true + case InParens(LPAREN, _) => true + case _ => false + if checkable && !closingRegionTokens.contains(token) then + report.warning(s"Line is indented too far to the left, or a ${showToken(r.closedBy)} is missing", + sourcePos()) else if lastWidth < nextWidth || lastWidth == nextWidth && (lastToken == MATCH || lastToken == CATCH) && token == CASE then if canStartIndentTokens.contains(lastToken) then diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 27ab73f0fe4d..86abd427bba0 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -1026,7 +1026,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { addParamssText( addParamssText(keywordStr("extension "), leadingParamss) ~~ (defKeyword ~~ valDefText(nameIdText(tree))).close, - trailingParamss) + trailingParamss) else addParamssText(defKeyword ~~ valDefText(nameIdText(tree)), tree.paramss) diff --git a/tests/neg/syntax-error-recovery.check b/tests/neg/syntax-error-recovery.check index 83b764c3062c..9f6cbaa7bf60 100644 --- a/tests/neg/syntax-error-recovery.check +++ b/tests/neg/syntax-error-recovery.check @@ -96,7 +96,5 @@ | longer explanation available when compiling with `-explain` -- Warning: tests/neg/syntax-error-recovery.scala:61:2 ----------------------------------------------------------------- 61 | println(bam) - | ^^^^^^^ - | Alphanumeric method println is not declared infix; it should not be used as infix operator. - | Instead, use method syntax .println(...) or backticked identifier `println`. - | The latter can be rewritten automatically under -rewrite -source 3.4-migration. + | ^ + | Line is indented too far to the left, or a ')' is missing diff --git a/tests/pos/i22527.scala b/tests/pos/i22527.scala index d222c3311b86..cf67d2e750fb 100644 --- a/tests/pos/i22527.scala +++ b/tests/pos/i22527.scala @@ -1,4 +1,8 @@ +//> using options -Werror + +import annotation.* + def f: Unit = identity( identity: @@ -34,6 +38,7 @@ def orElse(x: Int): Unit = else false, "fail") +@nowarn("msg=Unit") def onlyIf(x: Int): Unit = callme( if (x > 0) @@ -57,8 +62,8 @@ def k(xs: List[Int], y: Int, z: Int) = x + y + z, - y, - z, + y, + z, ) ) diff --git a/tests/warn/i22527.check b/tests/warn/i22527.check new file mode 100644 index 000000000000..b50612e12b57 --- /dev/null +++ b/tests/warn/i22527.check @@ -0,0 +1,22 @@ +-- Warning: tests/warn/i22527.scala:60:10 ------------------------------------------------------------------------------ +60 | y, // warn + | ^ + | Line is indented too far to the left, or a ')' is missing +-- Warning: tests/warn/i22527.scala:61:10 ------------------------------------------------------------------------------ +61 | z, // warn + | ^ + | Line is indented too far to the left, or a ')' is missing +-- Warning: tests/warn/i22527.scala:72:2 ------------------------------------------------------------------------------- +72 | 42 // warn + | ^ + | Line is indented too far to the left, or a ')' is missing +-- Warning: tests/warn/i22527.scala:79:0 ------------------------------------------------------------------------------- +79 |k: // warn + |^ + |Line is indented too far to the left, or a ')' is missing +-- [E190] Potential Issue Warning: tests/warn/i22527.scala:40:6 -------------------------------------------------------- +40 | true, "fail") // warn value discard + | ^^^^ + | Discarded non-Unit value of type Boolean. Add `: Unit` to discard silently. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/warn/i22527.scala b/tests/warn/i22527.scala new file mode 100644 index 000000000000..64cd32617406 --- /dev/null +++ b/tests/warn/i22527.scala @@ -0,0 +1,81 @@ + +def f: Unit = + identity( + identity: + class X extends AnyRef, Serializable + 42 + ) + +def test: Unit = + assert( + identity: + true, + "ok" + ) + +def toss: Unit = + assert( + throw + null, + "ok" + ) +def raise: Unit = + assert( + throw + null, "ok" // ok now + ) + +def callme[A](x: => A, msg: String) = try x.toString catch case t: RuntimeException => msg + +def orElse(x: Int): Unit = + callme( + if x > 0 then + true + else + false, "fail") + +def onlyIf(x: Int): Unit = + callme( + if (x > 0) + true, "fail") // warn value discard + +def h(xs: List[Int]) = + xs.foldLeft(0) + ( + (acc, x) => + acc + + x, + ) + +def sum(x: Int, y: Int, z: Int) = x+y+z + +def k(xs: List[Int], y: Int, z: Int) = + xs.foldLeft(0) + ( + (acc, x) => + sum( + x + + y + + z, + y, // warn + z, // warn + ) + ) + +object `arrow eol`: + def f(g: Int => Int, i: Int): Int = g(i) + def g(map: Int => Int): Int => Int = map + def k(i: => Int) = i + def test = + f( + g(_ + 1), + 42 // warn + ) + def test2 = + f( + g: + x => + x + 1, +k: // warn + 42 + ) From d194a4946db46488bcac3f6a9dc992cc1e8f018b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 12 Feb 2025 21:39:17 -0800 Subject: [PATCH 4/4] Clean up tests --- .../completion/CompletionExtensionSuite.scala | 3 +- .../completion/CompletionKeywordSuite.scala | 2 +- .../pc/tests/completion/CompletionSuite.scala | 2 +- .../tools/pc/tests/hover/HoverDocSuite.scala | 31 +++++++++---------- .../scaladoc/site/TemplateFileTests.scala | 20 ++++++------ 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionExtensionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionExtensionSuite.scala index e67c31329c1c..2bf05eb9d90b 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionExtensionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionExtensionSuite.scala @@ -86,7 +86,7 @@ class CompletionExtensionSuite extends BaseCompletionSuite: @Test def `filter-by-type-old` = check( - """|package example + """|package example | |object enrichments: | implicit class A(num: Int): @@ -99,7 +99,6 @@ class CompletionExtensionSuite extends BaseCompletionSuite: """|identity: String (implicit) |""".stripMargin, // identity2 won't be available filter = _.contains("(implicit)") - ) @Test def `filter-by-type-subtype` = diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionKeywordSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionKeywordSuite.scala index 5db0cf96d9ef..350e724e8df2 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionKeywordSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionKeywordSuite.scala @@ -57,7 +57,7 @@ class CompletionKeywordSuite extends BaseCompletionSuite: | **/ |} |""".stripMargin, - "", + "", includeCommitCharacter = true ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala index d270ebcc3c39..dac1124de5a9 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala @@ -1316,7 +1316,7 @@ class CompletionSuite extends BaseCompletionSuite: | val t: TT@@ |} |""".stripMargin, - "TTT[A <: Int] = List[A]" + "TTT[A <: Int] = List[A]" ) @Test def `type-lambda` = diff --git a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala index a82fa73865c8..3e9d8c1695e8 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala @@ -19,20 +19,20 @@ class HoverDocSuite extends BaseHoverSuite: @Test def `doc` = check( - """object a { - | <> - |} - |""".stripMargin, - """|**Expression type**: - |```scala - |java.util.List[Int] - |``` - |**Symbol signature**: - |```scala - |final def emptyList[T](): java.util.List[T] - |``` - |Found documentation for java/util/Collections#emptyList(). - |""".stripMargin, + """|object a { + | <> + |} + |""".stripMargin, + """|**Expression type**: + |```scala + |java.util.List[Int] + |``` + |**Symbol signature**: + |```scala + |final def emptyList[T](): java.util.List[T] + |``` + |Found documentation for java/util/Collections#emptyList(). + |""".stripMargin, ) @Test def `doc-parent` = @@ -47,7 +47,6 @@ class HoverDocSuite extends BaseHoverSuite: |``` |Found documentation for scala/collection/LinearSeqOps#headOption(). |""".stripMargin, - ) @Test def `java-method` = @@ -61,7 +60,7 @@ class HoverDocSuite extends BaseHoverSuite: |``` |Found documentation for java/lang/String#substring(). |""".stripMargin - ) + ) @Test def `object` = check( diff --git a/scaladoc/test/dotty/tools/scaladoc/site/TemplateFileTests.scala b/scaladoc/test/dotty/tools/scaladoc/site/TemplateFileTests.scala index f07868ad4f44..0a855c169275 100644 --- a/scaladoc/test/dotty/tools/scaladoc/site/TemplateFileTests.scala +++ b/scaladoc/test/dotty/tools/scaladoc/site/TemplateFileTests.scala @@ -208,26 +208,26 @@ class TemplateFileTests: testTemplate( """# Hello {{ msg }}!""", ext = "md" - ) { t => + ): t => assertEquals( """
- |

Hello there!

- |
""".stripMargin, - t.resolveInner(RenderingContext(Map("msg" -> "there"))).code.trim()) - } + |

Hello there!

+ |""".stripMargin, + t.resolveInner(RenderingContext(Map("msg" -> "there"))).code.trim() + ) @Test def mixedTemplates() : Unit = testTemplate( """# Hello {{ msg }}!""", ext = "md" - ) { t => + ): t => assertEquals( """
- |

Hello there2!

- |
""".stripMargin, - t.resolveInner(RenderingContext(Map("msg" -> "there2"))).code.trim()) - } + |

Hello there2!

+ |""".stripMargin, + t.resolveInner(RenderingContext(Map("msg" -> "there2"))).code.trim() + ) @Test def htmlOnly(): Unit =