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 a false positive for Lint/SuppressedException #11151

Merged
merged 1 commit into from Nov 11, 2022

Conversation

akihikodaki
Copy link
Contributor

When there is a rescue with no statement but a comment in a do block with a numbered parameter, it incorrectly registered an offense even if AllowComments is set true.

Cover the case of do block with a numbered parameter to fix the issue.


  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists). No related issue found.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

Well, running bundle exec rake changelog:fix will give a template containing [#x](https://github.com/rubocop/rubocop/pull/x) but I don't think that's what you want...

@koic
Copy link
Member

koic commented Nov 6, 2022

Can you add a changelog entry?

Well, running bundle exec rake changelog:fix will give a template containing [#x](https://github.com/rubocop/rubocop/pull/x) but I don't think that's what you want...

That template file needs to be edited. e.g.:

* [#11151](https://github.com/rubocop/rubocop/pull/11151): Fix a false positive for `Lint/SuppressedException` when empty rescue for `do` block with a numbered parameter. ([@akihikodaki][]).

And the file name must start with fix_, e.g. fix_a_false_positive_for_lint_suppressed_exception.md.

@akihikodaki
Copy link
Contributor Author

Can you add a changelog entry?

Well, running bundle exec rake changelog:fix will give a template containing [#x](https://github.com/rubocop/rubocop/pull/x) but I don't think that's what you want...

That template file needs to be edited. e.g.:

* [#11151](https://github.com/rubocop/rubocop/pull/11151): Fix a false positive for `Lint/SuppressedException` when empty rescue for `do` block with a numbered parameter. ([@akihikodaki][]).

And the file name must start with fix_, e.g. fix_a_false_positive_for_lint_suppressed_exception.md.

Added a changelog file. (Though it's a bit questionable policy as you don't get the pull request number until, well, you actually open one.)

@@ -0,0 +1 @@
* [#11151](https://github.com/rubocop/rubocop/pull/11151): Fix a false positive for `Lint/SuppressedException`. ([@akihikodaki][https://github.com/akihikodaki])
Copy link
Member

@koic koic Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is failing. Can you update it?

Suggested change
* [#11151](https://github.com/rubocop/rubocop/pull/11151): Fix a false positive for `Lint/SuppressedException`. ([@akihikodaki][https://github.com/akihikodaki])
* [#11151](https://github.com/rubocop/rubocop/pull/11151): Fix a false positive for `Lint/SuppressedException` when empty rescue for `do` block with a numbered parameter. ([@akihikodaki][]).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with commit b9c2d5e.

When there is a rescue with no statement but a comment in a `do` block
with a numbered parameter, it incorrectly registered an offense even if
AllowComments is set true.

Cover the case of `do` block with a numbered parameter to fix the issue.
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 11, 2022

Added a changelog file. (Though it's a bit questionable policy as you don't get the pull request number until, well, you actually open one.)

We know it requires a bit of extra work, but it makes the changelog more useful for the end users (it's easy to lookup changes), which is our primary aim.

@bbatsov bbatsov merged commit 8bfc4c5 into rubocop:master Nov 11, 2022
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 11, 2022

Thanks for fixing this!

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