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.

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

fix: use correct ErrorMessageID for EmptyCatchOrFinallyBlock #14786

Merged
merged 1 commit into from Mar 28, 2022

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Mar 26, 2022

Previously we were always assigning this to EmptyCatchOrFinallyBlockID
but both EmptyCatchBlock and EmptyCatchAndFinallyBlock both use this
class and pass in their corresponding ErrorMessageID. This caused the
following two code samples to give the same ErrorMessageID when they
should have been different:

try {}
try {} catch {} finally {}

The second one gave this as an error:

scala> try {} catch {} finally {}
-- [E000] Syntax Error: --------------------------------------------------------
1 |try {} catch {} finally {}
  |       ^^^^^^^^
  |       The catch block does not contain a valid expression, try
  |       adding a case like - case e: Exception => to the block
  |
  | longer explanation available when compiling with `-explain`

When the ID should be E001, EmptyCatchBlockId. Now this correctly
returns:

scala> try {} catch{} finally {}
-- [E001] Syntax Error: --------------------------------------------------------
1 |try {} catch{} finally {}
  |       ^^^^^^^
  |       The catch block does not contain a valid expression, try
  |       adding a case like - case e: Exception => to the block
  |
  | longer explanation available when compiling with `-explain`

The first example should actually be using E002 since it's missing both the
catch and finally blocks.

@som-snytt
Copy link
Contributor

TIL the first awesome error message.

Previously we were always assigning this to `EmptyCatchOrFinallyBlockID`
but both `EmptyCatchBlock` and `EmptyCatchAndFinallyBlock` both use this
class and pass in their corresponding `ErrorMessageID`. This caused the
following two code samples to give the same `ErrorMessageID` when they
should have been different than they currently are:

```scala
try {}
```

```scala
try {} catch {} finally {}
```

The second one gave this as an error:

```
scala> try {} catch {} finally {}
-- [E000] Syntax Error: --------------------------------------------------------
1 |try {} catch {} finally {}
  |       ^^^^^^^^
  |       The catch block does not contain a valid expression, try
  |       adding a case like - case e: Exception => to the block
  |
  | longer explanation available when compiling with `-explain`

```

When the ID should `E001`, `EmptyCatchBlockId`. Now this correctly
returns:

```
scala> try {} catch{} finally {}
-- [E001] Syntax Error: --------------------------------------------------------
1 |try {} catch{} finally {}
  |       ^^^^^^^
  |       The catch block does not contain a valid expression, try
  |       adding a case like - case e: Exception => to the block
  |
  | longer explanation available when compiling with `-explain`
```

The first example with the missing catch and finally block should
actually be `[E002]`.
@Kordyjan Kordyjan merged commit 1c033a7 into scala:main Mar 28, 2022
@ckipp01 ckipp01 deleted the correctErrorId branch March 28, 2022 12:54
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants