From e8d60f3bf6a91946cc8a72c647ca0b09b277e6c1 Mon Sep 17 00:00:00 2001 From: Felix Wolfsteller Date: Thu, 30 May 2024 08:07:15 +0200 Subject: [PATCH] add link_to_if and link_to_unless to matched calls of the LinkToBlank cop. Add changelog entry and add specs within three contexts for the three different methods Co-authored-by: Koichi ITO --- ...add_link_to_if_and_unless_to_link_blank.md | 1 + lib/rubocop/cop/rails/link_to_blank.rb | 4 +- spec/rubocop/cop/rails/link_to_blank_spec.rb | 484 ++++++++++++++---- 3 files changed, 396 insertions(+), 93 deletions(-) create mode 100644 changelog/change_add_link_to_if_and_unless_to_link_blank.md diff --git a/changelog/change_add_link_to_if_and_unless_to_link_blank.md b/changelog/change_add_link_to_if_and_unless_to_link_blank.md new file mode 100644 index 0000000000..2f3090c6ba --- /dev/null +++ b/changelog/change_add_link_to_if_and_unless_to_link_blank.md @@ -0,0 +1 @@ +* [#1288](https://github.com/rubocop/rubocop-rails/issues/1288): Let `Rails/LinkToBlank` look into `link_to_if` and `link_to_unless`, too. ([@fwolfst][]) diff --git a/lib/rubocop/cop/rails/link_to_blank.rb b/lib/rubocop/cop/rails/link_to_blank.rb index 61484050d2..c28f277b3e 100644 --- a/lib/rubocop/cop/rails/link_to_blank.rb +++ b/lib/rubocop/cop/rails/link_to_blank.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Rails - # Checks for calls to `link_to` that contain a + # Checks for calls to `link_to`, `link_to_if`, and `link_to_unless` methods that contain a # `target: '_blank'` but no `rel: 'noopener'`. This can be a security # risk as the loaded page will have control over the previous page # and could change its location for phishing purposes. @@ -24,7 +24,7 @@ class LinkToBlank < Base extend AutoCorrector MSG = 'Specify a `:rel` option containing noopener.' - RESTRICT_ON_SEND = %i[link_to].freeze + RESTRICT_ON_SEND = %i[link_to link_to_if link_to_unless].freeze def_node_matcher :blank_target?, <<~PATTERN (pair {(sym :target) (str "target")} {(str "_blank") (sym :_blank)}) diff --git a/spec/rubocop/cop/rails/link_to_blank_spec.rb b/spec/rubocop/cop/rails/link_to_blank_spec.rb index 83e71c8f59..0e111d5d41 100644 --- a/spec/rubocop/cop/rails/link_to_blank_spec.rb +++ b/spec/rubocop/cop/rails/link_to_blank_spec.rb @@ -1,149 +1,451 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Rails::LinkToBlank, :config do - context 'when not using target _blank' do - it 'does not register an offense' do - expect_no_offenses(<<~RUBY) - link_to 'Click here', 'https://www.example.com' - RUBY - end + context 'when using link_to' do + context 'when not using target _blank' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + link_to 'Click here', 'https://www.example.com' + RUBY + end + + it 'does not register an offense when passing options' do + expect_no_offenses(<<~RUBY) + link_to 'Click here', 'https://www.example.com', class: 'big' + RUBY + end - it 'does not register an offense when passing options' do - expect_no_offenses(<<~RUBY) - link_to 'Click here', 'https://www.example.com', class: 'big' - RUBY + it 'does not register an offense when using the block syntax' do + expect_no_offenses(<<~RUBY) + link_to 'https://www.example.com', class: 'big' do + "Click Here" + end + RUBY + end end - it 'does not register an offense when using the block syntax' do - expect_no_offenses(<<~RUBY) - link_to 'https://www.example.com', class: 'big' do - "Click Here" + context 'when using target_blank' do + context 'when using no rel' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: '_blank' + ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener' + RUBY + end + + it 'registers an offense when using a string for the target key' do + expect_offense(<<~RUBY) + link_to 'Click here', 'https://www.example.com', "target" => '_blank' + ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + end + + it 'registers an offense when using a symbol for the target value' do + expect_offense(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: :_blank + ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + end + + it 'registers an offense and autocorrects when using the block syntax' do + expect_offense(<<~RUBY) + link_to 'https://www.example.com', target: '_blank' do + ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + "Click here" + end + RUBY + + expect_correction(<<~RUBY) + link_to 'https://www.example.com', target: '_blank', rel: 'noopener' do + "Click here" + end + RUBY + end + + it 'autocorrects with a new rel when using the block syntax with parenthesis' do + new_source = autocorrect_source(<<~RUBY) + link_to('https://www.example.com', target: '_blank') do + "Click here" + end + RUBY + + expect(new_source).to eq(<<~RUBY) + link_to('https://www.example.com', target: '_blank', rel: 'noopener') do + "Click here" + end + RUBY + end + + it 'autocorrects with a new rel when using a symbol for the target value' do + new_source = autocorrect_source(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: :_blank + RUBY + + expect(new_source).to eq(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + RUBY + end + + it 'registers and corrects an offense when using hash brackets for the option' do + expect_offense(<<~RUBY) + link_to 'Click here', 'https://www.example.com', { target: :_blank } + ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to 'Click here', 'https://www.example.com', { target: :_blank, rel: :noopener } + RUBY + end + end + + context 'when using rel' do + context 'when the rel does not contain noopener' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + link_to 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated' + ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated noopener' + RUBY + end + end + + context 'when the rel contains noopener' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener unrelated' + RUBY + end + end + + context 'when the rel contains noreferrer' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'unrelated noreferrer' + RUBY + end end - RUBY + + context 'when the rel contains noopener and noreferrer' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener noreferrer' + RUBY + end + end + + context 'when the rel is symbol noopener' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + RUBY + end + end + end end end - context 'when using target_blank' do - context 'when using no rel' do - it 'registers and corrects an offense' do - expect_offense(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: '_blank' - ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + context 'when using link_to_if' do + context 'when not using target _blank' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com' RUBY + end - expect_correction(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener' + it 'does not register an offense when passing options' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', class: 'big' RUBY end - it 'registers an offense when using a string for the target key' do - expect_offense(<<~RUBY) - link_to 'Click here', 'https://www.example.com', "target" => '_blank' - ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + it 'does not register an offense when using the block syntax' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'https://www.example.com', class: 'big' do + "Click Here" + end RUBY end + end - it 'registers an offense when using a symbol for the target value' do - expect_offense(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: :_blank - ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. - RUBY + context 'when using target_blank' do + context 'when using no rel' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: '_blank' + ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener' + RUBY + end + + it 'registers an offense when using a string for the target key' do + expect_offense(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', "target" => '_blank' + ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + end + + it 'registers an offense when using a symbol for the target value' do + expect_offense(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: :_blank + ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + end + + it 'registers an offense and autocorrects when using the block syntax' do + expect_offense(<<~RUBY) + link_to_if condition?, 'https://www.example.com', target: '_blank' do + ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + "Click here" + end + RUBY + + expect_correction(<<~RUBY) + link_to_if condition?, 'https://www.example.com', target: '_blank', rel: 'noopener' do + "Click here" + end + RUBY + end + + it 'autocorrects with a new rel when using the block syntax with parenthesis' do + new_source = autocorrect_source(<<~RUBY) + link_to_if(condition?, 'https://www.example.com', target: '_blank') do + "Click here" + end + RUBY + + expect(new_source).to eq(<<~RUBY) + link_to_if(condition?, 'https://www.example.com', target: '_blank', rel: 'noopener') do + "Click here" + end + RUBY + end + + it 'autocorrects with a new rel when using a symbol for the target value' do + new_source = autocorrect_source(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: :_blank + RUBY + + expect(new_source).to eq(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + RUBY + end + + it 'registers and corrects an offense when using hash brackets for the option' do + expect_offense(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', { target: :_blank } + ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', { target: :_blank, rel: :noopener } + RUBY + end end - it 'registers an offense and autocorrects when using the block syntax' do - expect_offense(<<~RUBY) - link_to 'https://www.example.com', target: '_blank' do - ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. - "Click here" + context 'when using rel' do + context 'when the rel does not contain noopener' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated' + ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated noopener' + RUBY end - RUBY + end - expect_correction(<<~RUBY) - link_to 'https://www.example.com', target: '_blank', rel: 'noopener' do - "Click here" + context 'when the rel contains noopener' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener unrelated' + RUBY end - RUBY - end + end - it 'autocorrects with a new rel when using the block syntax with parenthesis' do - new_source = autocorrect_source(<<~RUBY) - link_to('https://www.example.com', target: '_blank') do - "Click here" + context 'when the rel contains noreferrer' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'unrelated noreferrer' + RUBY end - RUBY + end - expect(new_source).to eq(<<~RUBY) - link_to('https://www.example.com', target: '_blank', rel: 'noopener') do - "Click here" + context 'when the rel contains noopener and noreferrer' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener noreferrer' + RUBY end - RUBY - end + end - it 'autocorrects with a new rel when using a symbol for the target value' do - new_source = autocorrect_source(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: :_blank - RUBY + context 'when the rel is symbol noopener' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_if condition?, 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + RUBY + end + end + end + end + end - expect(new_source).to eq(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + context 'when using link_to_unless' do + context 'when not using target _blank' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com' RUBY end - it 'registers and corrects an offense when using hash brackets for the option' do - expect_offense(<<~RUBY) - link_to 'Click here', 'https://www.example.com', { target: :_blank } - ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + it 'does not register an offense when passing options' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', class: 'big' RUBY + end - expect_correction(<<~RUBY) - link_to 'Click here', 'https://www.example.com', { target: :_blank, rel: :noopener } + it 'does not register an offense when using the block syntax' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'https://www.example.com', class: 'big' do + "Click Here" + end RUBY end end - context 'when using rel' do - context 'when the rel does not contain noopener' do - it 'registers an offense and corrects' do + context 'when using target_blank' do + context 'when using no rel' do + it 'registers and corrects an offense' do expect_offense(<<~RUBY) - link_to 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated' - ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + link_to_unless condition?, 'Click here', 'https://www.example.com', target: '_blank' + ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. RUBY expect_correction(<<~RUBY) - link_to 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated noopener' + link_to_unless condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener' RUBY end - end - context 'when the rel contains noopener' do - it 'register no offense' do - expect_no_offenses(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener unrelated' + it 'registers an offense when using a string for the target key' do + expect_offense(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', "target" => '_blank' + ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. RUBY end - end - context 'when the rel contains noreferrer' do - it 'register no offense' do - expect_no_offenses(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'unrelated noreferrer' + it 'registers an offense when using a symbol for the target value' do + expect_offense(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: :_blank + ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. RUBY end - end - context 'when the rel contains noopener and noreferrer' do - it 'register no offense' do - expect_no_offenses(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener noreferrer' + it 'registers an offense and autocorrects when using the block syntax' do + expect_offense(<<~RUBY) + link_to_unless condition?, 'https://www.example.com', target: '_blank' do + ^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + "Click here" + end + RUBY + + expect_correction(<<~RUBY) + link_to_unless condition?, 'https://www.example.com', target: '_blank', rel: 'noopener' do + "Click here" + end RUBY end - end - context 'when the rel is symbol noopener' do - it 'register no offense' do - expect_no_offenses(<<~RUBY) - link_to 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + it 'autocorrects with a new rel when using the block syntax with parenthesis' do + new_source = autocorrect_source(<<~RUBY) + link_to_unless(condition?, 'https://www.example.com', target: '_blank') do + "Click here" + end + RUBY + + expect(new_source).to eq(<<~RUBY) + link_to_unless(condition?, 'https://www.example.com', target: '_blank', rel: 'noopener') do + "Click here" + end + RUBY + end + + it 'autocorrects with a new rel when using a symbol for the target value' do + new_source = autocorrect_source(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: :_blank RUBY + + expect(new_source).to eq(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + RUBY + end + + it 'registers and corrects an offense when using hash brackets for the option' do + expect_offense(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', { target: :_blank } + ^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', { target: :_blank, rel: :noopener } + RUBY + end + end + + context 'when using rel' do + context 'when the rel does not contain noopener' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated' + ^^^^^^^^^^^^^^^^^^^^ Specify a `:rel` option containing noopener. + RUBY + + expect_correction(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', "target" => '_blank', rel: 'unrelated noopener' + RUBY + end + end + + context 'when the rel contains noopener' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener unrelated' + RUBY + end + end + + context 'when the rel contains noreferrer' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'unrelated noreferrer' + RUBY + end + end + + context 'when the rel contains noopener and noreferrer' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: '_blank', rel: 'noopener noreferrer' + RUBY + end + end + + context 'when the rel is symbol noopener' do + it 'register no offense' do + expect_no_offenses(<<~RUBY) + link_to_unless condition?, 'Click here', 'https://www.example.com', target: :_blank, rel: :noopener + RUBY + end end end end