WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Betterment/UnscopedFind Improvement: Flag unscoped #52

@6f6d6172

Description

@6f6d6172

UnscopedFind has made the assumption that if you're doing a find off of an object (e.g. current_user), your query will be scoped to whatever belongs to that object. However, it turns out there's an unscoped method which when called at any point will remove any scopes, effectively letting you find any object regardless of who owns it.

For example, current_user.cats.find(params[:id]) will not raise an offense because the find call on cats is scoped to the current user. This same logic is erroneously extended to current_user.cats.unscoped.find(params[:id]), which does the same query but without the where clause that scopes it to the current user.

This cop should be modified to flag any uses of unscoped.

I was able to put together this diff that is the bare minimum needed to start detecting unscoped calls, but I haven't taken the time to sit down and list out test cases or write any specs.

diff --git a/lib/rubocop/cop/betterment/unscoped_find.rb b/lib/rubocop/cop/betterment/unscoped_find.rb
index fe3aa8e..bc6a71d 100644
--- a/lib/rubocop/cop/betterment/unscoped_find.rb
+++ b/lib/rubocop/cop/betterment/unscoped_find.rb
@@ -31,6 +31,10 @@ module RuboCop
           (send (const ... _) {#{FINDS.map(&:inspect).join(' ')}} ...)
         PATTERN
 
+        def_node_matcher :unscoped?, <<-PATTERN
+          (send (send ... :unscoped) {#{FINDS.map(&:inspect).join(' ')}} ...)
+        PATTERN
+
         def_node_search :find_graphql_namespace_nodes, <<~PATTERN, name: GRAPHQL_PATTERN
           (const _ %name)
         PATTERN
@@ -49,6 +53,7 @@ module RuboCop
           _, _, *arg_nodes = *node # rubocop:disable InternalAffairs/NodeDestructuring
           return unless
             (
+              unscoped?(node) ||
               find?(node) ||
               custom_scope_find?(node) ||
               static_method_name(node.method_name)

tl;dr todo

  • flag all unscoped
  • list out test cases
  • write tests

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions