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

Handle mismatched end when using rescue without begin #83

Merged

Conversation

mauro-oto
Copy link
Contributor

Handles cases where mismatched ends happens on methods using
rescue without begin:

unexpected `rescue', expecting `end'

Handles cases where mismatched ends happens on methods using
rescue without `begin`:
```
unexpected `rescue', expecting `end'
```
@mauro-oto
Copy link
Contributor Author

I was just reading #68, and it looks like this is related.. so probably not the best way to handle it, it fixes this case, but might make it confusing for other cases.

@schneems should we scope this to just rescue, or do you prefer I close it for the time being?

@schneems
Copy link
Collaborator

Before:

$ dead_end /tmp/scratch.rb

file: /tmp/scratch.rb
simplified:

    ❯ 1  def foo
    ❯ 2    if bar
    ❯ 4    else
    ❯ 6  rescue FooBar
    ❯ 7    nil
    ❯ 8  end

After:

$ exe/dead_end /tmp/scratch.rb

DeadEnd: Unmatched `end` detected

This code has an unmatched `end`. Ensure that all `end` lines
in your code have a matching syntax keyword  (`def`,  `do`, etc.)
and that you don't have any extra `end` lines.

file: /tmp/scratch.rb
simplified:

    ❯ 1  def foo
    ❯ 2    if bar
    ❯ 4    else
    ❯ 6  rescue FooBar
    ❯ 7    nil
    ❯ 8  end

That is improved, as you mention #68 though it causes some somewhat unexpected side effects, using the code from #68:

Before:

file: /tmp/scratch.rb
simplified:

      1  def foo
    ❯ 2    1,2,3,4]
      3  end

After:

DeadEnd: Unmatched `end` detected

This code has an unmatched `end`. Ensure that all `end` lines
in your code have a matching syntax keyword  (`def`,  `do`, etc.)
and that you don't have any extra `end` lines.

file: /tmp/scratch.rb
simplified:

      1  def foo
    ❯ 2    1,2,3,4]
      3  end

Which isn't correct. I'm not totally sure how to move forwards. I think the change is good, the behavior to not parse the error message when there's a backtick in it was more-or-less accidental.

I'm thinking that we merge this, and then consider this new behavior of identifying a missing bracket as a "missing end" as a separate bug.

What do you think?

Then regarding that bug I'm wondering if maybe it's good enough assumption that "unexpected comma expecting end" is an edge case and we can use that to filter or work around it?

@schneems
Copy link
Collaborator

Also, I filed some other label issues #84 and #85.

@schneems schneems merged commit 619eeef into main Oct 13, 2021
@schneems schneems deleted the mauro-oto/handle-mismatched-end-on-rescue-without-begin branch October 13, 2021 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants