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 #7495] Documentation for Lint/AmbiguousBlockAssociation cop is confusing #8850

Conversation

AllanSiqueira
Copy link
Contributor

Fixes: #7495
Add an example in the documentation for Lint/AmbiguousBlockAssociation to illustrate both good solutions for this cop.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself and generates the documentation.

Copy link
Contributor

@marcandre marcandre left a comment

Choose a reason for hiding this comment

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

Good. I'd merge but there's a Changelog merge conflict, so might as well improve it and rebase 😆

@@ -15,6 +15,8 @@ module Lint
#
# # good
# # With parentheses, there's no ambiguity.
# some_method(a { |val| puts val })
# # or
Copy link
Contributor

Choose a reason for hiding this comment

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

How about # or (different meaning):

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 it : )

@marcandre
Copy link
Contributor

Also the offense message would be best with quotes, if you'd like to do that too...

Parenthesize the param a { |val| puts val } to make sure that the block will be associated with the a method call.
# to
Parenthesize the param `a { |val| puts val }` to make sure that the block will be associated with the `a` method call.

Add example on documentation for `Lint/AmbiguousBlockAssociation` to
make clear the possible resolutions
@AllanSiqueira AllanSiqueira force-pushed the add-example-documentation-lint-ambiguous-block-association branch from 9efbb04 to ef85068 Compare October 5, 2020 14:10
@AllanSiqueira
Copy link
Contributor Author

Also the offense message would be best with quotes, if you'd like to do that too...

Parenthesize the param a { |val| puts val } to make sure that the block will be associated with the a method call.
# to
Parenthesize the param `a { |val| puts val }` to make sure that the block will be associated with the `a` method call.

I search for this and seems like it already has quotes, there is somewhere who hasn't the quotes and I let pass?

@marcandre
Copy link
Contributor

Also the offense message would be best with quotes, if you'd like to do that too...

Parenthesize the param a { |val| puts val } to make sure that the block will be associated with the a method call.
# to
Parenthesize the param `a { |val| puts val }` to make sure that the block will be associated with the `a` method call.

I search for this and seems like it already has quotes, there is somewhere who hasn't the quotes and I let pass?

Oh, perfect. I got that message from the original issue only, didn't check.

@marcandre marcandre merged commit f6e0115 into rubocop:master Oct 5, 2020
@marcandre
Copy link
Contributor

Thanks for the PR! 👍

@AllanSiqueira AllanSiqueira deleted the add-example-documentation-lint-ambiguous-block-association branch October 5, 2020 14:28
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.

Documentation for Lint/AmbiguousBlockAssociation cop is confusing
2 participants