Ruby: Model private class methods

`Module#private_class_method` takes a symbol representing the name of a
method in the current module scope and makes that module private. This
is similar to `private`, but applies only to class (singleton) methods.
Unlike `private`, it must be called with an argument, and does not
change the ambient visibility for any subsequent method definitions.

    class Foo
      def public
      end

      def private1
      end
      private_class_method :private1

      # This alternate form works because method definition
      # returns its name as a symbol:

      private_class_method def private2
      end
    end

ruby/ql/lib/codeql/ruby/ast/Method.qll

@@ -35,27 +35,37 @@ class MethodBase extends Callable, BodyStmt, Scope, TMethodBase {
     or
     result = BodyStmt.super.getAChild(pred)
   }
+
+  /** Holds if this method is private. */
+  predicate isPrivate() { none() }
 }
 
-/** A call to `private`. */
-private class Private extends MethodCall {
+/**
+ * A method call which modifies another method in some way.
+ * For example, `private :foo` makes the method `foo` private.
+ */
+private class MethodModifier extends MethodCall {
+  /** Gets the name of the method that this call applies to. */
+  Expr getMethodArgument() { result = this.getArgument(0) }
+
+  /** Gets the method that this call applies to. */
+  MethodBase getMethod() {
+    result = this.getMethodArgument()
+    or
+    exists(Namespace n |
+      n.getAStmt() = this and
+      n.getAStmt() = result and
+      result.getName() = this.getMethodArgument().(StringlikeLiteral).getValueText()
+    )
+  }
+}
+
+/** A call to `private` or `private_class_method`. */
+private class Private extends MethodModifier {
   Private() { this.getMethodName() = "private" }
 
-  /** Gets the method that this `private` call applies to, if any */
-  Expr getMethod() { result = this.getArgument(0) }
-
   /**
-   * Holds if this `private` call happens inside `c`, and refers to a
-   * method named `name`.
-   */
-  pragma[noinline]
-  predicate isRef(Namespace c, string name) {
-    this = c.getAStmt() and
-    name = this.getMethod().(SymbolLiteral).getValueText()
-  }
-
-  /**
-   * Holds if this `private` call happens at position `i` inside `c`,
+   * Holds if this call happens at position `i` inside `c`,
    * and the call has no arguments.
    */
   pragma[noinline]
@@ -65,6 +75,11 @@ private class Private extends MethodCall {
   }
 }
 
+/** A call to `private_class_method`. */
+private class PrivateClassMethod extends MethodModifier {
+  PrivateClassMethod() { this.getMethodName() = "private_class_method" }
+}
+
 /** A normal method. */
 class Method extends MethodBase, TMethod {
   private Ruby::Method g;
@@ -90,12 +105,6 @@ class Method extends MethodBase, TMethod {
    */
   final predicate isSetter() { g.getName() instanceof Ruby::Setter }
 
-  pragma[noinline]
-  private predicate isDeclaredIn(Namespace c, string name) {
-    this = c.getAStmt() and
-    name = this.getName()
-  }
-
   /**
    * Holds if this method is private. All methods with the name prefix
    * `private` are private below:
@@ -122,14 +131,9 @@ class Method extends MethodBase, TMethod {
    * end
    * ```
    */
-  predicate isPrivate() {
+  override predicate isPrivate() {
     this = any(Private p).getMethod()
     or
-    exists(Namespace c, Private p, string name |
-      this.isDeclaredIn(c, name) and
-      p.isRef(c, name)
-    )
-    or
     exists(Namespace c, Private p, int i, int j |
       p.hasNoArg(c, i) and
       this = c.getStmt(j) and
@@ -175,6 +179,31 @@ class SingletonMethod extends MethodBase, TSingletonMethod {
     or
     pred = "getObject" and result = this.getObject()
   }
+
+  /**
+   * Holds if this method is private. All methods with the name prefix
+   * `private` are private below:
+   *
+   * ```rb
+   * class C
+   *   private_class_method def self.private1
+   *   end
+   *
+   *   def self.public
+   *   end
+   *
+   *   def self.private2
+   *   end
+   *   private_class_method :private2
+   *
+   *   private # this has no effect on singleton methods
+   *
+   *   def self.public2
+   *   end
+   * end
+   * ```
+   */
+  override predicate isPrivate() { this = any(PrivateClassMethod p).getMethod() }
 }
 
 /**

ruby/ql/test/library-tests/modules/callgraph.expected

@@ -78,16 +78,16 @@ getTarget
 | private.rb:2:3:3:5 | call to private | calls.rb:94:5:94:20 | private |
 | private.rb:10:3:10:19 | call to private | calls.rb:94:5:94:20 | private |
 | private.rb:12:3:12:9 | call to private | calls.rb:94:5:94:20 | private |
-| private.rb:24:1:24:5 | call to new | calls.rb:99:5:99:16 | new |
-| private.rb:25:1:25:5 | call to new | calls.rb:99:5:99:16 | new |
-| private.rb:26:1:26:5 | call to new | calls.rb:99:5:99:16 | new |
-| private.rb:27:1:27:5 | call to new | calls.rb:99:5:99:16 | new |
-| private.rb:28:1:28:5 | call to new | calls.rb:99:5:99:16 | new |
-| private.rb:28:1:28:12 | call to public | private.rb:5:3:6:5 | public |
-| private.rb:30:1:30:15 | call to private_on_main | private.rb:21:1:22:3 | private_on_main |
-| private.rb:33:3:34:5 | call to private | calls.rb:94:5:94:20 | private |
-| private.rb:41:3:41:19 | call to private | calls.rb:94:5:94:20 | private |
-| private.rb:43:3:43:9 | call to private | calls.rb:94:5:94:20 | private |
+| private.rb:34:1:34:5 | call to new | calls.rb:99:5:99:16 | new |
+| private.rb:35:1:35:5 | call to new | calls.rb:99:5:99:16 | new |
+| private.rb:36:1:36:5 | call to new | calls.rb:99:5:99:16 | new |
+| private.rb:37:1:37:5 | call to new | calls.rb:99:5:99:16 | new |
+| private.rb:38:1:38:5 | call to new | calls.rb:99:5:99:16 | new |
+| private.rb:38:1:38:12 | call to public | private.rb:5:3:6:5 | public |
+| private.rb:40:1:40:15 | call to private_on_main | private.rb:31:1:32:3 | private_on_main |
+| private.rb:43:3:44:5 | call to private | calls.rb:94:5:94:20 | private |
+| private.rb:51:3:51:19 | call to private | calls.rb:94:5:94:20 | private |
+| private.rb:53:3:53:9 | call to private | calls.rb:94:5:94:20 | private |
 unresolvedCall
 | calls.rb:19:5:19:14 | call to instance_m |
 | calls.rb:20:5:20:19 | call to instance_m |
@@ -113,10 +113,12 @@ unresolvedCall
 | hello.rb:20:16:20:26 | ... + ... |
 | hello.rb:20:16:20:34 | ... + ... |
 | hello.rb:20:16:20:40 | ... + ... |
-| private.rb:24:1:24:14 | call to private1 |
-| private.rb:25:1:25:14 | call to private2 |
-| private.rb:26:1:26:14 | call to private3 |
-| private.rb:27:1:27:14 | call to private4 |
+| private.rb:23:3:24:5 | call to private_class_method |
+| private.rb:28:3:28:32 | call to private_class_method |
+| private.rb:34:1:34:14 | call to private1 |
+| private.rb:35:1:35:14 | call to private2 |
+| private.rb:36:1:36:14 | call to private3 |
+| private.rb:37:1:37:14 | call to private4 |
 privateMethod
 | calls.rb:1:1:3:3 | foo |
 | calls.rb:62:1:65:3 | optional_arg |
@@ -129,8 +131,10 @@ privateMethod
 | private.rb:8:3:9:5 | private2 |
 | private.rb:14:3:15:5 | private3 |
 | private.rb:17:3:18:5 | private4 |
-| private.rb:21:1:22:3 | private_on_main |
-| private.rb:33:11:34:5 | private1 |
-| private.rb:39:3:40:5 | private2 |
-| private.rb:45:3:46:5 | private3 |
-| private.rb:48:3:49:5 | private4 |
+| private.rb:23:24:24:5 | private5 |
+| private.rb:26:3:27:5 | private6 |
+| private.rb:31:1:32:3 | private_on_main |
+| private.rb:43:11:44:5 | private1 |
+| private.rb:49:3:50:5 | private2 |
+| private.rb:55:3:56:5 | private3 |
+| private.rb:58:3:59:5 | private4 |

ruby/ql/test/library-tests/modules/callgraph.ql

@@ -4,4 +4,4 @@ query Callable getTarget(Call call) { result = call.getATarget() }
 
 query predicate unresolvedCall(Call call) { not exists(call.getATarget()) }
 
-query predicate privateMethod(Method m) { m.isPrivate() }
+query predicate privateMethod(MethodBase m) { m.isPrivate() }

ruby/ql/test/library-tests/modules/private.rb

@@ -16,6 +16,16 @@ class C
 
   def private4
   end
+  
+  def self.public2
+  end
+  
+  private_class_method def self.private5
+  end
+  
+  def self.private6
+  end
+  private_class_method :private6
 end
 
 def private_on_main