ruby: Filter out loops over constants

Author: yoff

ruby/ql/src/queries/performance/CouldBeHoisted.ql

@@ -16,6 +16,7 @@
 // If you already did article includes book, there should be no problem.
 import ruby
 private import codeql.ruby.AST
+import codeql.ruby.ast.internal.Constant
 import codeql.ruby.Concepts
 import codeql.ruby.frameworks.ActiveRecord
 private import codeql.ruby.TaintTracking
@@ -84,12 +85,14 @@ DataFlow::Node guardForLoopControl(ConditionalExpr cond, Stmt control) {
 
 from LoopingCall loop, DataFlow::CallNode call
 where
-  // TODO: Filter loops over constants
+  // Filter loops over constants
+  not isArrayConstant(loop.getReceiver().asExpr(), _) and
+  // Filter tests
   not call.getLocation().getFile().getAbsolutePath().matches("%test%") and
-  // not call = any(PluckCall p).chaines() and
+  not call = any(PluckCall p).chaines() and
   not usedInLoopControlGuard(call, _) and
   happensInInnermostLoop(loop, call) and
-  call instanceof ActiveRecordModelFinderCall and
-  not call.getMethodName() in ["new", "create"] //and
-// call.getLocation().getFile().getAbsolutePath().matches("%app/models/stafftools/%")
+  (call instanceof ActiveRecordModelFinderCall or call instanceof PluckCall) and
+  not call.getMethodName() in ["new", "create"] and
+  call.getLocation().getFile().getAbsolutePath().matches(["%app/models/s%", "%app/models/e%"])
 select call, "This call happens inside $@, and could be hoisted.", loop, "this loop"

ruby/ql/src/queries/performance/issues.md

@@ -0,0 +1,43 @@
+In this snippet
+```ruby
+users = []
+app_integration = authentication_token.authenticatable.integration
+app_owner = app_integration.owner
+if app_owner.user?
+    users << app_owner.id
+else
+    potential_users = []
+    Permissions::Enumerator.actor_ids_with_permission(
+    action: :manage_app,
+    subject_id: app_integration.id,
+    context: { owner_id: app_owner.id },
+    ).map do |actor_id|
+    potential_users << actor_id
+    end
+
+    Permissions::Enumerator.actor_ids_with_permission(
+    action: :manage_all_apps,
+    subject_id: app_owner.id
+    ).map do |actor_id|
+    potential_users << actor_id
+    end
+
+    app_owner.members(actor_ids: potential_users).each do |member|
+    users << member.id
+    end
+end
+
+users.each do |user_id|
+    if !is_one_click_revoke
+    notify_user_of_revoked_access(
+        target: target,
+        user: User.find(user_id),
+        token: token,
+        app_name: app_integration.name,
+        revoked_by: revoked_by,
+        is_expired: authentication_token.expires_at_timestamp.to_i < Time.now.to_i,
+    )
+    end
+end
+```
+`users` is identified as an array constant.