Add new Rails/LinkToBlank cop
#6580
Conversation
| def on_send(node) | ||
| return unless node.method?(:link_to) | ||
|
|
||
| option_nodes = [node.children.last, node.children[3]].compact |
There was a problem hiding this comment.
Not sure about the [3] here, any better idea ?
There was a problem hiding this comment.
I would probably go with something like:
node.arguments.each_child_node(:hash)There was a problem hiding this comment.
Thanks for the tip ! I managed to make it work with node.each_child_node(:hash)
|
|
||
| option_nodes = [node.children.last, node.children[3]].compact | ||
|
|
||
| option_nodes.map(&:children).each do |options| |
There was a problem hiding this comment.
You can go with #each_pair here, instead of map(&:children).
There was a problem hiding this comment.
Unfortunately at this point option_nodes is an Enumerator of Hash nodes
|
Hi @Drenmi ! Can I do anything to help get this PR merged ? Thanks a lot ! |
CHANGELOG.md
Outdated
| @@ -1,6 +1,7 @@ | |||
| # Change log | |||
|
|
|||
| ## master (unreleased) | |||
| * New cop `Rails/LinkToBlank` checks for `link_to` calls with `target: '_blank'` and no `rel: 'noopener'`. ([@Intrepidd][]) | |||
There was a problem hiding this comment.
You have to put this under "New features".
| module Rails | ||
| # This cop checks for calls to `link_to` 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 |
There was a problem hiding this comment.
This line seems misaligned.
Intrepidd
force-pushed
the
linktoblank-cop
branch
from
460a445
to
5138cd2
Compare
|
Thanks @bbatsov ! I applied your comments an rebased |
Intrepidd
force-pushed
the
linktoblank-cop
branch
from
abeb693
to
fa1497a
Compare
|
Thanks! |
|
FYI, although it has not been merged yet, there is a PR opened in rails/rails that adds |
This cop checks for calls to
link_tothat contain atarget: '_blank'but norel: '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.Demo resource for reference : https://mathiasbynens.github.io/rel-noopener/