Merge pull request #10002 from hmac/hmac/protected-methods

Ruby: Model protected methods

Author: hmac

ruby/ql/lib/change-notes/2022-08-16-protected-methods.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* `MethodBase` now has two new predicates related to visibility: `isPublic` and
+  `isProtected`. These hold, respectively, if the method is public or protected.

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

@@ -36,56 +36,119 @@ class MethodBase extends Callable, BodyStmt, Scope, TMethodBase {
     result = BodyStmt.super.getAChild(pred)
   }
 
+  /**
+   * Holds if this method is public.
+   * Methods are public by default.
+   */
+  predicate isPublic() { this.getVisibility() = "public" }
+
   /** Holds if this method is private. */
-  predicate isPrivate() { none() }
+  predicate isPrivate() { this.getVisibility() = "private" }
+
+  /** Holds if this method is protected. */
+  predicate isProtected() { this.getVisibility() = "protected" }
+
+  /**
+   * Gets a string describing the visibility of this method.
+   * This is either 'public', 'private' or 'protected'.
+   */
+  string getVisibility() {
+    result = getVisibilityModifier(this).getVisibility()
+    or
+    not exists(getVisibilityModifier(this)) and result = "public"
+  }
 }
 
 /**
- * A method call which modifies another method in some way.
- * For example, `private :foo` makes the method `foo` private.
+ * Gets the visibility modifier that explicitly sets the visibility of method
+ * `m`.
+ *
+ * Examples:
+ * ```rb
+ * def f
+ * end
+ * private :f
+ *
+ * private def g
+ * end
+ * ```
  */
-private class MethodModifier extends MethodCall {
-  /** Gets the name of the method that this call applies to. */
-  Expr getMethodArgument() { result = this.getArgument(0) }
+private VisibilityModifier getExplicitVisibilityModifier(Method m) {
+  result.getMethodArgument() = m
+  or
+  exists(ModuleBase n, string name |
+    methodIsDeclaredIn(m, n, name) and
+    modifiesIn(result, n, name)
+  )
+}
 
-  /** Holds if this call modifies a method with name `name` in namespace `n`. */
-  pragma[noinline]
-  predicate modifiesMethod(Namespace n, string name) {
-    this = n.getAStmt() and
-    [
-      this.getMethodArgument().getConstantValue().getStringlikeValue(),
-      this.getMethodArgument().(MethodBase).getName()
-    ] = name
-  }
+/**
+ * Gets the visibility modifier that defines the visibility of method `m`, if
+ * any.
+ */
+private VisibilityModifier getVisibilityModifier(MethodBase mb) {
+  mb =
+    any(Method m |
+      result = getExplicitVisibilityModifier(m)
+      or
+      not exists(getExplicitVisibilityModifier(m)) and
+      exists(ModuleBase n, int methodPos | isDeclaredIn(m, n, methodPos) |
+        // The relevant visibility modifier is the closest call that occurs before
+        // the definition of `m` (typically this means higher up the file).
+        result =
+          max(int modifierPos, VisibilityModifier modifier |
+            modifier.modifiesAmbientVisibility() and
+            isDeclaredIn(modifier, n, modifierPos) and
+            modifierPos < methodPos
+          |
+            modifier order by modifierPos
+          )
+      )
+    )
+  or
+  mb =
+    any(SingletonMethod m |
+      result.getMethodArgument() = m
+      or
+      exists(ModuleBase n, string name |
+        methodIsDeclaredIn(m, n, name) and
+        modifiesIn(result, n, name)
+      )
+    )
 }
 
-/** A call to `private` or `private_class_method`. */
-private class Private extends MethodModifier {
-  private Namespace namespace;
-  private int position;
+/**
+ * A method call that sets the visibility of other methods.
+ * For example, `private :foo` makes the method `foo` private.
+ */
+private class VisibilityModifier extends MethodCall {
+  VisibilityModifier() {
+    this.getMethodName() =
+      ["public", "private", "protected", "public_class_method", "private_class_method"]
+  }
 
-  Private() { this.getMethodName() = "private" and namespace.getStmt(position) = this }
+  /** Gets the name of the method that this call applies to. */
+  Expr getMethodArgument() { result = this.getArgument(0) }
 
-  override predicate modifiesMethod(Namespace n, string name) {
-    n = namespace and
-    (
-      // def foo
-      // ...
-      // private :foo
-      super.modifiesMethod(n, name)
-      or
-      // private
-      // ...
-      // def foo
-      not exists(this.getMethodArgument()) and
-      exists(MethodBase m, int i | n.getStmt(i) = m and m.getName() = name and i > position)
-    )
+  /**
+   * Holds if this modifier changes the "ambient" visibility - i.e. the default
+   * visibility of any subsequent method definitions.
+   */
+  predicate modifiesAmbientVisibility() {
+    this.getMethodName() = ["public", "private", "protected"] and
+    this.getNumberOfArguments() = 0
   }
-}
 
-/** A call to `private_class_method`. */
-private class PrivateClassMethod extends MethodModifier {
-  PrivateClassMethod() { this.getMethodName() = "private_class_method" }
+  /** Gets the visibility set by this modifier. */
+  string getVisibility() {
+    this.getMethodName() = ["public", "public_class_method"] and result = "public"
+    or
+    this.getMethodName() = ["private", "private_class_method"] and
+    result = "private"
+    or
+    this.getMethodName() = "protected" and
+    result = "protected"
+  }
 }
 
 /** A normal method. */
@@ -139,29 +202,49 @@ class Method extends MethodBase, TMethod {
    * end
    * ```
    */
-  override predicate isPrivate() {
-    exists(Namespace n, string name |
-      any(Private p).modifiesMethod(n, name) and
-      isDeclaredIn(this, n, name)
-    )
-    or
-    // Top-level methods are private members of the Object class
-    this.getEnclosingModule() instanceof Toplevel
-  }
+  override predicate isPrivate() { super.isPrivate() }
 
   final override Parameter getParameter(int n) {
     toGenerated(result) = g.getParameters().getChild(n)
   }
 
   final override string toString() { result = this.getName() }
+
+  override string getVisibility() {
+    result = getVisibilityModifier(this).getVisibility()
+    or
+    this.getEnclosingModule() instanceof Toplevel and
+    not exists(getVisibilityModifier(this)) and
+    result = "private"
+    or
+    not this.getEnclosingModule() instanceof Toplevel and
+    not exists(getVisibilityModifier(this)) and
+    result = "public"
+  }
+}
+
+pragma[nomagic]
+private predicate modifiesIn(VisibilityModifier vm, ModuleBase n, string name) {
+  n = vm.getEnclosingModule() and
+  name = vm.getMethodArgument().getConstantValue().getStringlikeValue()
 }
 
 /**
- * Holds if the method `m` has name `name` and is declared in namespace `n`.
+ * Holds if statement `s` is declared in namespace `n` at position `pos`.
  */
-pragma[noinline]
-private predicate isDeclaredIn(MethodBase m, Namespace n, string name) {
-  n = m.getEnclosingModule() and name = m.getName()
+pragma[nomagic]
+private predicate isDeclaredIn(Stmt s, ModuleBase n, int pos) {
+  n = s.getEnclosingModule() and
+  n.getStmt(pos) = s
+}
+
+/**
+ * Holds if method `m` with name `name` is declared in namespace `n`.
+ */
+pragma[nomagic]
+private predicate methodIsDeclaredIn(MethodBase m, ModuleBase n, string name) {
+  isDeclaredIn(m, n, _) and
+  name = m.getName()
 }
 
 /** A singleton method. */
@@ -216,12 +299,7 @@ class SingletonMethod extends MethodBase, TSingletonMethod {
    * end
    * ```
    */
-  override predicate isPrivate() {
-    exists(Namespace n, string name |
-      any(PrivateClassMethod p).modifiesMethod(n, name) and
-      isDeclaredIn(this, n, name)
-    )
-  }
+  override predicate isPrivate() { super.isPrivate() }
 }
 
 /**

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

@@ -191,10 +191,10 @@ private.rb:
 #    1| E
 #-----| super -> Object
 
-#   42| F
+#   62| F
 
-#   62| PrivateOverride1
+#   82| PrivateOverride1
 #-----| super -> Object
 
-#   76| PrivateOverride2
+#   96| PrivateOverride2
 #-----| super -> PrivateOverride1