Add a new Style/EvalWithLocation cop

[pocke]

eval can receive a file name and a line number to display actual backtrace.

For example:

eval <<-RUBY
  raise rescue p $!.backtrace
RUBY
# => ["(eval):1:in `<main>'", "test.rb:1:in `eval'", "test.rb:1:in `<main>'"]

eval <<-RUBY, binding, __FILE__, __LINE__
  raise rescue p $!.backtrace
RUBY
# => ["test.rb:1:in `<main>'", "test.rb:1:in `eval'", "test.rb:1:in `<main>'"]

If we pass __FILE__ and __LINE__ to an eval method, an exception that occurs in the eval method has a backtrace with actual file name and line number. But sometimes we forget writing __FILE__ and __LINE__.

This cop detects this problem. This cop adds an offense for an eval method call without a location.

Note: I think we can implement auto-correction for this cop in the next step.

---

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).
• Used the same coding conventions as the rest of the project.
• 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.
• All tests(rake spec) are passing.
• The new code doesn't generate RuboCop offenses that are checked by rake internal_investigation.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Updated cop documentation with rake generate_cops_documentation (required only when you've added a new cop or changed the configuration/documentation of an existing cop).

[bbatsov]

I guess you forgot about this. :-)

[pocke]

🙀 I'll update it!

[bbatsov]

I think this should be reworded to something mentioning "source location metadata (that's used by backtraces)."

[bbatsov]

Same here.

[pocke]

I added the documentation and updated the messages.

I think this should be reworded to something mentioning "source location metadata (that's used by backtraces)."

Sorry, I'm not sure what you meant.

I updated the message to Pass `__FILE__` and `__LINE__` to `eval` method. That is used by backtraces. and Use `%<expected>s` instead of `%<actual>s`. That is used by backtraces.
I think the messages should have __FILE__ and __LINE__, because users can't understand how to fix this offense without these concrete metadata names. So I omit source location metadata from the messages.
What do you think?

[bbatsov]

I'd only replace the second sentence with , as they are used by backtraces. Generally we aim to have each message be a single sentence.

[bbatsov]

This has to be rebased, btw.

[pocke]

I'd only replace the second sentence with , as they are used by backtraces. Generally we aim to have each message be a single sentence.

I understand. Thanks!

I'll update the messages, and rebase.

[bbatsov]

👍

[brandonweiss]

@pocke Why does the example in the PR description pass __LINE__ directly but the comment in the cop does __LINE__ + 1?

[pocke]

@pocke Why does the example in the PR description pass LINE directly but the comment in the cop does LINE + 1?

It is my mistake, __LINE__ + 1 is correct if you use a haredoc.

[brandonweiss]

🤔 I don't think I understand why the offset is necessary?

Perhaps the reasoning/instructions should be documented and/or included in part of the offense message? I imagine people will frequently look at the documentation and include the offset when they don't need to and/or not include the offset when they do need to.

[marcandre]

Wow, this is the first time that RuboCop actually signals a genuine error for me, thanks to this cop 🎉
Thanks @pocke 🙇‍♂️!