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

Add ToJSON Linter #6379

Merged
merged 2 commits into from
Mar 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@
* [#6386](https://github.com/rubocop-hq/rubocop/pull/6386): Add `VersionAdded` meta data to config/default.yml when running `rake new_cop`. ([@koic][])
* [#6395](https://github.com/rubocop-hq/rubocop/pull/6395): Permit to specify TargetRubyVersion 2.6. ([@koic][])
* [#6392](https://github.com/rubocop-hq/rubocop/pull/6392): Add `Whitelist` config to `Rails/SkipsModelValidations` rule. ([@DiscoStarslayer][])
* [#6378](https://github.com/rubocop-hq/rubocop/issues/6378): Added ToJSON cop to enforce an argument when overriding #to_json. ([@allcentury][])

### Bug fixes

Expand Down Expand Up @@ -3838,6 +3839,7 @@
[@tom-lord]: https://github.com/tom-lord
[@bayandin]: https://github.com/bayandin
[@nadiyaka]: https://github.com/nadiyaka
[@allcentury]: https://github.com/allcentury
[@antonzaytsev]: https://github.com/antonzaytsev
[@amatsuda]: https://github.com/amatsuda
[@Intrepidd]: https://github.com/Intrepidd
Expand All @@ -3850,4 +3852,4 @@
[@mhelmetag]: https://github.com/mhelmetag
[@Bhacaz]: https://github.com/bhacaz
[@enkessler]: https://github.com/enkessler
[@tagliala]: https://github.com/tagliala
[@tagliala]: https://github.com/tagliala
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,10 @@ Lint/Syntax:
VersionAdded: '0.9'


Lint/ToJSON:
Description: 'Ensure #to_json includes an optional argument.'
Enabled: true

Lint/UnderscorePrefixedVariableName:
Description: 'Do not use prefix `_` for a variable that is used.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@
require_relative 'rubocop/cop/lint/shadowing_outer_local_variable'
require_relative 'rubocop/cop/lint/string_conversion_in_interpolation'
require_relative 'rubocop/cop/lint/syntax'
require_relative 'rubocop/cop/lint/to_json'
require_relative 'rubocop/cop/lint/underscore_prefixed_variable_name'
require_relative 'rubocop/cop/lint/unified_integer'
require_relative 'rubocop/cop/lint/unneeded_cop_disable_directive'
Expand Down
38 changes: 38 additions & 0 deletions lib/rubocop/cop/lint/to_json.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks to make sure `#to_json` includes an optional argument.
# When overriding `#to_json`, callers may invoke JSON
# generation via `JSON.generate(your_obj)`. Since `JSON#generate` allows
# for an optional argument, your method should too.
#
# @example
# # bad
# def to_json
# end
#
# # good
# def to_json(_opts)
# end
#
class ToJSON < Cop
MSG = ' `#to_json` requires an optional argument to be parsable ' \
'via JSON.generate(obj).'.freeze

def on_def(node)
return unless node.method?(:to_json) && node.arguments.empty?

add_offense(node)
end

def autocorrect(node)
lambda do |corrector|
corrector.insert_after(node.loc.name, '(_opts)')
end
end
end
end
end
end
5 changes: 5 additions & 0 deletions lib/rubocop/cop/style/option_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class OptionHash < Cop

def on_args(node)
return if super_used?(node)
return if whitelist.include?(node.parent.method_name.to_s)

option_hash(node) do |options|
add_offense(options)
Expand All @@ -36,6 +37,10 @@ def on_args(node)

private

def whitelist
cop_config['Whitelist'] || []
end

def suspicious_name?(arg_name)
cop_config.key?('SuspiciousParamNames') &&
cop_config['SuspiciousParamNames'].include?(arg_name.to_s)
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ In the following section you find all available cops:
* [Lint/ShadowingOuterLocalVariable](cops_lint.md#lintshadowingouterlocalvariable)
* [Lint/StringConversionInInterpolation](cops_lint.md#lintstringconversionininterpolation)
* [Lint/Syntax](cops_lint.md#lintsyntax)
* [Lint/ToJSON](cops_lint.md#linttojson)
* [Lint/UnderscorePrefixedVariableName](cops_lint.md#lintunderscoreprefixedvariablename)
* [Lint/UnifiedInteger](cops_lint.md#lintunifiedinteger)
* [Lint/UnneededCopDisableDirective](cops_lint.md#lintunneededcopdisabledirective)
Expand Down
23 changes: 23 additions & 0 deletions manual/cops_lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,29 @@ This is not actually a cop. It does not inspect anything. It just
provides methods to repack Parser's diagnostics/errors
into RuboCop's offenses.

## Lint/ToJSON

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | Yes | - | -

This cop checks to make sure `#to_json` includes an optional argument.
When overriding `#to_json`, callers may invoke JSON
generation via `JSON.generate(your_obj)`. Since `JSON#generate` allows
for an optional argument, your method should too.

### Examples

```ruby
# bad
def to_json
end

# good
def to_json(_opts)
end
```

## Lint/UnderscorePrefixedVariableName

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
33 changes: 33 additions & 0 deletions spec/rubocop/cop/lint/to_json_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::ToJSON do
subject(:cop) { described_class.new(config) }

let(:config) { RuboCop::Config.new }

it 'registers an offense when using `#to_json` without arguments' do
expect_offense(<<-RUBY.strip_indent)
def to_json
^^^^^^^^^^^ `#to_json` requires an optional argument to be parsable via JSON.generate(obj).
end
RUBY
end

it 'does not register an offense when using `#to_json` with arguments' do
expect_no_offenses(<<-RUBY.strip_indent)
def to_json(opts)
end
RUBY
end

it 'autocorrects' do
corrected = autocorrect_source(<<-RUBY.strip_indent)
def to_json
end
RUBY
expect(corrected).to eq(<<-RUBY.strip_indent)
def to_json(_opts)
end
RUBY
end
end
11 changes: 11 additions & 0 deletions spec/rubocop/cop/style/option_hash_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,15 @@ def allowed(foo, options = {})
RUBY
end
end

context 'whitelist' do
let(:cop_config) { { 'Whitelist' => %w[to_json] } }

it 'ignores if the method is whitelisted' do
expect_no_offenses(<<-RUBY.strip_indent)
def to_json(options = {})
end
RUBY
end
end
end