diff --git a/changelog/change_add_more_detection_patterns_on.md b/changelog/change_add_more_detection_patterns_on.md new file mode 100644 index 0000000000..8128c2e9ad --- /dev/null +++ b/changelog/change_add_more_detection_patterns_on.md @@ -0,0 +1 @@ +* [#1571](https://github.com/rubocop/rubocop-rails/pull/1571): Add more detection patterns on `Rails/ResponseParsedBody`. ([@r7kamura][]) diff --git a/lib/rubocop/cop/rails/response_parsed_body.rb b/lib/rubocop/cop/rails/response_parsed_body.rb index 184ddcb3a7..583416998b 100644 --- a/lib/rubocop/cop/rails/response_parsed_body.rb +++ b/lib/rubocop/cop/rails/response_parsed_body.rb @@ -14,12 +14,15 @@ module Rails # @example # # bad # JSON.parse(response.body) - # - # # bad + # Nokogiri::HTML(response.body) + # Nokogiri::HTML4(response.body) + # Nokogiri::HTML5(response.body) # Nokogiri::HTML.parse(response.body) - # - # # bad + # Nokogiri::HTML4.parse(response.body) # Nokogiri::HTML5.parse(response.body) + # Nokogiri::HTML::Document.parse(response.body) + # Nokogiri::HTML4::Document.parse(response.body) + # Nokogiri::HTML5::Document.parse(response.body) # # # good # response.parsed_body @@ -27,71 +30,77 @@ class ResponseParsedBody < Base extend AutoCorrector extend TargetRailsVersion - RESTRICT_ON_SEND = %i[parse].freeze + MSG = 'Prefer `response.parsed_body`.' + + HTML = %i[HTML HTML4 HTML5].to_set.freeze + + RESTRICT_ON_SEND = [:parse, *HTML].freeze minimum_target_rails_version 5.0 # @!method json_parse_response_body?(node) def_node_matcher :json_parse_response_body?, <<~PATTERN - (send - (const {nil? cbase} :JSON) - :parse - (send - (send nil? :response) - :body - ) - ) + (send #json? :parse #response_body?) PATTERN - # @!method nokogiri_html_parse_response_body(node) - def_node_matcher :nokogiri_html_parse_response_body, <<~PATTERN - (send - (const - (const {nil? cbase} :Nokogiri) - ${:HTML :HTML5} - ) - :parse - (send - (send nil? :response) - :body - ) - ) + # @!method nokogiri_html_response_body?(node) + def_node_matcher :nokogiri_html_response_body?, <<~PATTERN + (send #nokogiri? HTML #response_body?) PATTERN - def on_send(node) - check_json_parse_response_body(node) + # @!method nokogiri_html_parse_response_body?(node) + def_node_matcher :nokogiri_html_parse_response_body?, <<~PATTERN + (send #nokogiri_html? :parse #response_body?) + PATTERN - return unless target_rails_version >= 7.1 + # @!method nokogiri_html_document_parse_response_body?(node) + def_node_matcher :nokogiri_html_document_parse_response_body?, <<~PATTERN + (send (const #nokogiri_html? :Document) :parse #response_body?) + PATTERN - check_nokogiri_html_parse_response_body(node) - end + # @!method json?(node) + def_node_matcher :json?, <<~PATTERN + (const {nil? cbase} :JSON) + PATTERN - private + # @!method nokogiri?(node) + def_node_matcher :nokogiri?, <<~PATTERN + (const {nil? cbase} :Nokogiri) + PATTERN - def autocorrect(corrector, node) - corrector.replace(node, 'response.parsed_body') - end + # @!method nokogiri_html?(node) + def_node_matcher :nokogiri_html?, <<~PATTERN + (const #nokogiri? HTML) + PATTERN - def check_json_parse_response_body(node) - return unless json_parse_response_body?(node) + # @!method response_body?(node) + def_node_matcher :response_body?, <<~PATTERN + (send (send nil? :response) :body) + PATTERN + + def on_send(node) + return unless html_offense?(node) || json_offense?(node) - add_offense( - node, - message: 'Prefer `response.parsed_body` to `JSON.parse(response.body)`.' - ) do |corrector| - autocorrect(corrector, node) + add_offense(node) do |corrector| + corrector.replace(node, 'response.parsed_body') end end - def check_nokogiri_html_parse_response_body(node) - return unless (const = nokogiri_html_parse_response_body(node)) + private - add_offense( - node, - message: "Prefer `response.parsed_body` to `Nokogiri::#{const}.parse(response.body)`." - ) do |corrector| - autocorrect(corrector, node) - end + def html_offense?(node) + support_response_parsed_body_for_html? && + (nokogiri_html_response_body?(node) || + nokogiri_html_parse_response_body?(node) || + nokogiri_html_document_parse_response_body?(node)) + end + + def json_offense?(node) + json_parse_response_body?(node) + end + + def support_response_parsed_body_for_html? + target_rails_version >= 7.1 end end end diff --git a/spec/rubocop/cop/rails/response_parsed_body_spec.rb b/spec/rubocop/cop/rails/response_parsed_body_spec.rb index fac0c61c19..f874a54fa1 100644 --- a/spec/rubocop/cop/rails/response_parsed_body_spec.rb +++ b/spec/rubocop/cop/rails/response_parsed_body_spec.rb @@ -21,7 +21,7 @@ it 'registers offense' do expect_offense(<<~RUBY) expect(JSON.parse(response.body)).to eq('foo' => 'bar') - ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body` to `JSON.parse(response.body)`. + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. RUBY expect_correction(<<~RUBY) @@ -34,7 +34,7 @@ it 'registers offense' do expect_offense(<<~RUBY) expect(::JSON.parse(response.body)).to eq('foo' => 'bar') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body` to `JSON.parse(response.body)`. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. RUBY expect_correction(<<~RUBY) @@ -43,19 +43,58 @@ end end - context 'when `Nokogiri::HTML.parse(response.body)` is used on Rails 7.0', :rails70 do + context 'when `Nokogiri::HTML(response.body)` is used on Rails 7.0', :rails70 do it 'registers no offense' do expect_no_offenses(<<~RUBY) - Nokogiri::HTML.parse(response.body) + Nokogiri::HTML(response.body) + RUBY + end + end + + context 'when `Nokogiri::HTML(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML4(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML4(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML5(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML5(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body RUBY end end - context 'when `Nokogiri::HTML.parse(response.body)` is used on Rails 7.1', :rails71 do + context 'when `Nokogiri::HTML.parse(response.body)`', :rails71 do it 'registers offense' do expect_offense(<<~RUBY) Nokogiri::HTML.parse(response.body) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body` to `Nokogiri::HTML.parse(response.body)`. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. RUBY expect_correction(<<~RUBY) @@ -64,11 +103,63 @@ end end - context 'when `Nokogiri::HTML5.parse(response.body)` is used on Rails 7.1', :rails71 do + context 'when `Nokogiri::HTML4.parse(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML4.parse(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML5.parse(response.body)`', :rails71 do it 'registers offense' do expect_offense(<<~RUBY) Nokogiri::HTML5.parse(response.body) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body` to `Nokogiri::HTML5.parse(response.body)`. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML::Document.parse(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML::Document.parse(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML4::Document.parse(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML4::Document.parse(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML5::Document.parse(response.body)`', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML5::Document.parse(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body`. RUBY expect_correction(<<~RUBY)