Improve documentation for Style/EvalWithLocation cop

[taichi-ishitani]

This PR is to improve documentation for Style/EvalWithLocation cop.
There are no description and example for the case that a string variable is given as a code string like below.

code = <<-RUBY
def do_something
end
RUBY

eval code

I added description and example to make this case clear.

---

Before submitting the PR make sure the following are checked:

• 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).
• 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.

[bbatsov]

Hmm, I wonder why we'd treat this case differently. Perhaps an oversight on our part and we should treat a variable param just a string literal. @dvandersluis what do you think?

[taichi-ishitani]

I think we cannot know where the given string variable is defined so the cop does not check this case.

[bbatsov]

This is true about a string that came externally, but for any other string parameter it's pretty much the same as for an explicit string literal IMO. E.g. in your example you can supply the location metadata.

[taichi-ishitani]

Ah, I understand. You're thinking the cop should check as much as possible.
Do following examples match your thought?

Examples which the cop should check:

def eval_string
  code = <<~CODE
    def do_something
    end
  CODE
  eval(code) # report offense
end

def eval_code_from_file(file)
  code = File.read(file)
  eval(code, file, 1)
end

Examples which the cop does not check:

def eval_string(s)
  eval(s)
end

def eval_code
  eval(@code)
end

[dvandersluis]

@taichi-ishitani I think what @bbatsov is getting at is that your examples are still calls to eval without a file and location given.

In other words, your eval_string method would be better done as:

def eval_string
  code = <<~CODE
    def do_something
    end
  CODE
  eval(code, binding, __FILE__, __LINE__) # this line is changed
end

We don't need to know what the code is doing for this cop, just that eval is called without the proper arguments.

However, I'm not sure if we should autocorrect when not given a string, because the cop sets a value relative to __LINE__ and as you said we don't know where code comes from (unless we always just set it to __LINE__ with no offset when not given a string).

[bbatsov]

Yeah, that's exactly what I was referring to. Thanks for explaining it much better than me! :D

[taichi-ishitani]

def eval_string
  code = <<~S


    foo
  S
  instance_eval(code, __FILE__, __LINE__)
end
eval_string

We will get following back trace when we invoke the above code:

$ ruby with_location_info.rb
with_location_info.rb:9:in `eval_string': undefined local variable or method `foo' for main:Object (NameError)
        from with_location_info.rb:7:in `instance_eval'
        from with_location_info.rb:7:in `eval_string'
        from with_location_info.rb:9:in `<main>'

The 1st line of the back trace shows incorrect location.
Therefore, I think we should not set the location info if we cannot know where the given string variable is defined correctly.

[bbatsov]

Fair enough. I think updating the docs is the simplest thing we can do right now. Thanks!