Skip to content

Commit

Permalink
[Fix #6378] Add ToJSON Linter
Browse files Browse the repository at this point in the history
This fixes issue #6378

classes that override to_json need to ensure an optional argument is
available.

class MyClass
  def to_json
  end
end

While calling MyClass.new.to_json works, calling
JSON.generate(MyClass.new) results in an ArgumentError.
  • Loading branch information
Anthony Ross committed Dec 14, 2018
1 parent 3d4ba9c commit 8fd679e
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,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 @@ -3693,3 +3694,4 @@
[@tom-lord]: https://github.com/tom-lord
[@bayandin]: https://github.com/bayandin
[@nadiyaka]: https://github.com/nadiyaka
[@allcentury]: https://github.com/allcentury
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1507,6 +1507,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 @@ -314,6 +314,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 @@ -247,6 +247,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 @@ -1960,6 +1960,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 @@ -99,4 +99,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

0 comments on commit 8fd679e

Please sign in to comment.