From 28556935e77bf8b4ee63d4905ce29d580abfe9ab Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 21 Mar 2022 21:15:36 +0100 Subject: [PATCH 1/5] Warn on misleading indentation in single-case catch Fixes #14721 Surprisingly, caught two instances in the compiler code base itself. --- .../src/dotty/tools/backend/jvm/BytecodeWriters.scala | 2 +- compiler/src/dotty/tools/backend/jvm/GenBCode.scala | 2 +- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 7 ++++++- tests/neg-custom-args/fatal-warnings/i14721.scala | 9 +++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i14721.scala diff --git a/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala b/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala index 06a787978a0b..327f623975fd 100644 --- a/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala +++ b/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala @@ -118,7 +118,7 @@ trait BytecodeWriters { try outfile.delete() // don't leave an empty or half-written classfile around after an interrupt catch case _: Throwable => - throw ex + throw ex finally outstream.close() report.informProgress("wrote '" + label + "' to " + outfile) } diff --git a/compiler/src/dotty/tools/backend/jvm/GenBCode.scala b/compiler/src/dotty/tools/backend/jvm/GenBCode.scala index 6a763a9c0744..751875849a22 100644 --- a/compiler/src/dotty/tools/backend/jvm/GenBCode.scala +++ b/compiler/src/dotty/tools/backend/jvm/GenBCode.scala @@ -276,7 +276,7 @@ class GenBCodePipeline(val int: DottyBackendInterface, val primitives: DottyPrim try outTastyFile.delete() // don't leave an empty or half-written tastyfile around after an interrupt catch case _: Throwable => - throw ex + throw ex finally outstream.close() val uuid = new TastyHeaderUnpickler(binary()).readHeader() diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 7a5620be0415..0464f10cf5b3 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2620,7 +2620,12 @@ object Parsers { (pattern(), guard()) } CaseDef(pat, grd, atSpan(accept(ARROW)) { - if exprOnly then expr() else block() + if exprOnly then + if in.indentSyntax && in.isAfterLineEnd && in.token != INDENT then + warning(i"""misleading indentation: this expression forms part of the preceding catch case, + |it should be indented for clarity.""") + expr() + else block() }) } diff --git a/tests/neg-custom-args/fatal-warnings/i14721.scala b/tests/neg-custom-args/fatal-warnings/i14721.scala new file mode 100644 index 000000000000..a8849f8459b8 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i14721.scala @@ -0,0 +1,9 @@ +import scala.util.chaining.given + +class C: + def op: Unit = println("op") + def handler: Unit = println("handler") + def test: Unit = + try op + catch case _: NullPointerException => + handler // error From 36300dcf1176e22b14475aea82e9fcc931f4c787 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 21 Mar 2022 21:51:59 +0100 Subject: [PATCH 2/5] Fix ShadowingTest try catch indentation --- compiler/test/dotty/tools/repl/ShadowingTests.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/repl/ShadowingTests.scala b/compiler/test/dotty/tools/repl/ShadowingTests.scala index 564ac6258633..e4b10aa8a997 100644 --- a/compiler/test/dotty/tools/repl/ShadowingTests.scala +++ b/compiler/test/dotty/tools/repl/ShadowingTests.scala @@ -32,7 +32,8 @@ object ShadowingTests: def createSubDir(name: String): Path = val subdir = dir.resolve(name) try Files.createDirectory(subdir) - catch case _: java.nio.file.FileAlreadyExistsException => + catch + case _: java.nio.file.FileAlreadyExistsException => assert(Files.isDirectory(subdir), s"failed to create shadowed subdirectory $subdir") subdir From d7fb8bf18d604e52bc0c623ca81a988d273d99fa Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 21 Mar 2022 22:22:19 +0100 Subject: [PATCH 3/5] Fix fixes in backend and improve warning message In fact the warning occurrences in the backend were also assuming an empty catch handler. Change them to do the right thing, and fix the warning message to point out this possibility. --- compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala | 5 +++-- compiler/src/dotty/tools/backend/jvm/GenBCode.scala | 5 +++-- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala b/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala index 327f623975fd..551d4f8d809e 100644 --- a/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala +++ b/compiler/src/dotty/tools/backend/jvm/BytecodeWriters.scala @@ -117,8 +117,9 @@ trait BytecodeWriters { catch case ex: ClosedByInterruptException => try outfile.delete() // don't leave an empty or half-written classfile around after an interrupt - catch case _: Throwable => - throw ex + catch + case _: Throwable => + throw ex finally outstream.close() report.informProgress("wrote '" + label + "' to " + outfile) } diff --git a/compiler/src/dotty/tools/backend/jvm/GenBCode.scala b/compiler/src/dotty/tools/backend/jvm/GenBCode.scala index 751875849a22..f00c54e584f9 100644 --- a/compiler/src/dotty/tools/backend/jvm/GenBCode.scala +++ b/compiler/src/dotty/tools/backend/jvm/GenBCode.scala @@ -275,8 +275,9 @@ class GenBCodePipeline(val int: DottyBackendInterface, val primitives: DottyPrim catch case ex: ClosedByInterruptException => try outTastyFile.delete() // don't leave an empty or half-written tastyfile around after an interrupt - catch case _: Throwable => - throw ex + catch + case _: Throwable => + throw ex finally outstream.close() val uuid = new TastyHeaderUnpickler(binary()).readHeader() diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 0464f10cf5b3..803db0ec6367 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2622,8 +2622,10 @@ object Parsers { CaseDef(pat, grd, atSpan(accept(ARROW)) { if exprOnly then if in.indentSyntax && in.isAfterLineEnd && in.token != INDENT then - warning(i"""misleading indentation: this expression forms part of the preceding catch case, - |it should be indented for clarity.""") + warning(i"""Misleading indentation: this expression forms part of the preceding catch case. + |If this is intended, it should be indented for clarity. + |Otherwise, if the handler is intended to be empty, use a multi-line catch with + |an indented case.""") expr() else block() }) From a6705180424000bbf9927f3f0cabf26317142cde Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 21 Mar 2022 22:43:42 +0100 Subject: [PATCH 4/5] Change interpretation of ShadowingTests again --- compiler/test/dotty/tools/repl/ShadowingTests.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/test/dotty/tools/repl/ShadowingTests.scala b/compiler/test/dotty/tools/repl/ShadowingTests.scala index e4b10aa8a997..1ba58a4babff 100644 --- a/compiler/test/dotty/tools/repl/ShadowingTests.scala +++ b/compiler/test/dotty/tools/repl/ShadowingTests.scala @@ -32,9 +32,8 @@ object ShadowingTests: def createSubDir(name: String): Path = val subdir = dir.resolve(name) try Files.createDirectory(subdir) - catch - case _: java.nio.file.FileAlreadyExistsException => - assert(Files.isDirectory(subdir), s"failed to create shadowed subdirectory $subdir") + catch case _: java.nio.file.FileAlreadyExistsException => + assert(Files.isDirectory(subdir), s"failed to create shadowed subdirectory $subdir") subdir // The directory on the classpath containing artifacts to be shadowed From a7ffc8ef9a0632bf08c20158798e89bde2ad8fdc Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 23 Mar 2022 11:04:01 +0100 Subject: [PATCH 5/5] Update tests/neg-custom-args/fatal-warnings/i14721.scala --- tests/neg-custom-args/fatal-warnings/i14721.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/neg-custom-args/fatal-warnings/i14721.scala b/tests/neg-custom-args/fatal-warnings/i14721.scala index a8849f8459b8..6e884cad22d7 100644 --- a/tests/neg-custom-args/fatal-warnings/i14721.scala +++ b/tests/neg-custom-args/fatal-warnings/i14721.scala @@ -1,4 +1,3 @@ -import scala.util.chaining.given class C: def op: Unit = println("op")