KEMBAR78
Add new `Lint/ErbNewArguments` cop by koic · Pull Request #5845 · rubocop/rubocop · GitHub
Skip to content

Conversation

koic
Copy link
Member

@koic koic commented Apr 30, 2018

Summary

This Lint/ErbNewArguments cop emulates the following Ruby warnings in Ruby 2.6.

% cat example.rb
ERB.new('hi', nil, '-', '@output_buffer')
% ruby -vrerb example.rb
ruby 2.6.0dev (2018-04-08 trunk 63120) [x86_64-darwin17]
example.rb:1: warning: Passing safe_level with the 2nd argument of
ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
example.rb:1: warning: Passing trim_mode with the 3rd argument of
ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.
example.rb:1: warning: Passing eoutvar with the 4th argument of
ERB.new is deprecated. Use keyword argument like ERB.new(str, eoutvar: ...) instead.

The interface of ERB.new will change from Ruby 2.6.

Add :trim_mode and :eoutvar keyword arguments to ERB.new.
Now non-keyword arguments other than first one are softly deprecated
and will be removed when Ruby 2.5 becomes EOL. [Feature #14256]

https://github.com/ruby/ruby/blob/2311087b685e8dc0f21f4a89875f25c22f5c39a9/NEWS#stdlib-updates-outstanding-ones-only

Now non-keyword arguments other than first one are softly deprecated and will be removed when Ruby 2.5 becomes EOL.
ERB.new with non-keyword arguments is deprecated since ERB 2.2.0.
Use :trim_mode and :eoutvar keyword arguments to ERB.new.
This cop identifies places where ERB.new(str, trim_mode, eoutvar) can be replaced by ERB.new(str, :trim_mode: trim_mode, eoutvar: eoutvar).

This cop does not have autocorrect because uses of ERB.new depends on existing algorithm and code.

Examples

# Target codes supports Ruby 2.6 and higher only

# bad
ERB.new(str, nil, '-')

# Target codes supports Ruby 2.5 and lower only

# good
ERB.new(str, trim_mode: nil, eoutvar: '-')

# Target codes supports Ruby 2.6, 2.5 and lower
# bad
ERB.new(str, nil, '-')

# good
# Ruby standard library style
# https://github.com/ruby/ruby/commit/3406c5d
if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+
  ERB.new(str, trim_mode: nil, eoutvar: '-')
else
  ERB.new(str, nil, '-')
end

# good
# Use `RUBY_VERSION` style
if RUBY_VERSION >= '2.6'
  ERB.new(str, trim_mode: nil, eoutvar: '-')
else
  ERB.new(str, nil, '-')
end

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 rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

@koic koic force-pushed the add_new_lint_erb_new_arguments_cop branch 2 times, most recently from f1538d2 to efcb149 Compare May 1, 2018 09:48
## Summary

This `Lint/ErbNewArguments` cop emulates the following
Ruby warnings in Ruby 2.6.

```console
% cat example.rb
ERB.new('hi', nil, '-', '@output_buffer')
% ruby -vrerb example.rb
ruby 2.6.0dev (2018-04-08 trunk 63120) [x86_64-darwin17]
example.rb:1: warning: Passing safe_level with the 2nd argument of
ERB.new is deprecated. Do not use it, and specify other arguments as
keyword arguments.
example.rb:1: warning: Passing trim_mode with the 3rd argument of
ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode:
...) instead.
example.rb:1: warning: Passing eoutvar with the 4th argument of ERB.new
is deprecated. Use keyword argument like ERB.new(str, eoutvar: ...) instead.
```

The interface of `ERB.new` will change from Ruby 2.6.

> Add :trim_mode and :eoutvar keyword arguments to ERB.new.
> Now non-keyword arguments other than first one are softly deprecated
> and will be removed when Ruby 2.5 becomes EOL. [Feature rubocop#14256]

https://github.com/ruby/ruby/blob/2311087b685e8dc0f21f4a89875f25c22f5c39a9/NEWS#stdlib-updates-outstanding-ones-only

Now non-keyword arguments other than first one are softly deprecated
and will be removed when Ruby 2.5 becomes EOL.
`ERB.new` with non-keyword arguments is deprecated since ERB 2.2.0.
Use `:trim_mode` and `:eoutvar` keyword arguments to `ERB.new`.
This cop identifies places where `ERB.new(str, trim_mode, eoutvar)`
can be replaced by `ERB.new(str, :trim_mode: trim_mode, eoutvar: eoutvar)`.

This cop does not have autocorrect because uses of `ERB.new`
depends on existing algorithm and code.

## Examples

```ruby
# Target codes supports Ruby 2.6 and higher only

# bad
ERB.new(str, nil, '-')

# Target codes supports Ruby 2.5 and lower only

# good
ERB.new(str, trim_mode: nil, eoutvar: '-')

# Target codes supports Ruby 2.6, 2.5 and lower
# bad
ERB.new(str, nil, '-')

# good
# Ruby standard library style
# ruby/ruby@3406c5d
if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+
  ERB.new(str, trim_mode: nil, eoutvar: '-')
else
  ERB.new(str, nil, '-')
end

# good
# Use `RUBY_VERSION` style
if RUBY_VERSION >= '2.6'
  ERB.new(str, trim_mode: nil, eoutvar: '-')
else
  ERB.new(str, nil, '-')
end
```
@koic koic force-pushed the add_new_lint_erb_new_arguments_cop branch from efcb149 to cd117aa Compare May 1, 2018 13:02
@bbatsov bbatsov merged commit 03ddccb into rubocop:master May 2, 2018
@bbatsov
Copy link
Collaborator

bbatsov commented May 2, 2018

Great work overall, although I'm not a fan of the long messages. Thanks!

@koic koic deleted the add_new_lint_erb_new_arguments_cop branch May 2, 2018 02:40
@koic
Copy link
Member Author

koic commented May 2, 2018

Yes. I started implementing it with a short offense messages first, but eventually became a long offense messages emulating Ruby's warning. 😅 Thank you!

koic added a commit to koic/rubocop that referenced this pull request Oct 20, 2018
Parser gem has been started development for Ruby 2.6 (edge Ruby).
https://github.com/whitequark/parser/blob/master/CHANGELOG.md#v2510-2018-04-12

AFAIK, the next Ruby version is 2.7 after Ruby 2.6.
https://bugs.ruby-lang.org/issues/14256

This PR permits Ruby 2.6, the early adapters will be able to try edge
Ruby with RuboCop.

rubocop#5845 has already been finished some work to support ruby 2.6.
And this PR ends all the work related to permit to specify
TargetRubyVersion 2.6.
bbatsov pushed a commit that referenced this pull request Oct 20, 2018
Parser gem has been started development for Ruby 2.6 (edge Ruby).
https://github.com/whitequark/parser/blob/master/CHANGELOG.md#v2510-2018-04-12

AFAIK, the next Ruby version is 2.7 after Ruby 2.6.
https://bugs.ruby-lang.org/issues/14256

This PR permits Ruby 2.6, the early adapters will be able to try edge
Ruby with RuboCop.

#5845 has already been finished some work to support ruby 2.6.
And this PR ends all the work related to permit to specify
TargetRubyVersion 2.6.
@SwagDevOps
Copy link

I have the following message:

[Correctable] Lint/ErbNewArguments: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
        ERB.new(template.read, 0, trim_mode: '>').result(renderer.instance_eval { binding })

But here: https://github.com/ruby/ruby/blob/65a8d52212d3945bb5f547136a88f43022a31cf3/lib/erb.rb#L811
The signature for initialize method is:

def initialize(str, safe_level=NOT_GIVEN, legacy_trim_mode=NOT_GIVEN, legacy_eoutvar=NOT_GIVEN, trim_mode: nil, eoutvar: '_erbout')

And my code is:

ERB.new(template.read, 0, trim_mode: '>')

IMHO, cop should not be offended…

@koic
Copy link
Member Author

koic commented Jan 9, 2022

This behavior is not a false positive because it emulates the following ERB warning.

% ruby -rerb -ve "ERB.new('hi', 0, trim_mode: '>')"
ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin19]
-e:1: warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.

@SwagDevOps
Copy link

Does it mean safe_level will be completely removed?

@koic
Copy link
Member Author

koic commented Jan 11, 2022

Yes. AFAIK, it's related to the following ticket.
https://bugs.ruby-lang.org/issues/14250

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.

3 participants