Merge pull request #15490 from github/koesie10/ruby-model-constructor-on-new

Ruby: Model constructors in endpoint query on new instead of initialize

Author: koesie10

ruby/ql/src/utils/modeleditor/ModelEditor.qll

@@ -63,14 +63,27 @@ class MethodEndpoint extends Endpoint instanceof DataFlow::MethodNode {
 
   DataFlow::MethodNode getNode() { result = this }
 
-  override string getName() { result = super.getMethodName() }
+  override string getName() {
+    result = super.getMethodName() and not this.isConstructor()
+    or
+    // Constructors are modeled as Type!#new rather than Type#initialize
+    result = "new" and this.isConstructor()
+  }
 
   /**
    * Gets the unbound type name of this endpoint.
    */
   override string getType() {
     result =
-      any(DataFlow::ModuleNode m | m.getOwnInstanceMethod(this.getName()) = this).getQualifiedName() or
+      any(DataFlow::ModuleNode m | m.getOwnInstanceMethod(this.getName()) = this).getQualifiedName() and
+    not this.isConstructor()
+    or
+    // Constructors are modeled on `Type!`, not on `Type`
+    result =
+      any(DataFlow::ModuleNode m | m.getOwnInstanceMethod(super.getMethodName()) = this)
+            .getQualifiedName() + "!" and
+    this.isConstructor()
+    or
     result =
       any(DataFlow::ModuleNode m | m.getOwnSingletonMethod(this.getName()) = this)
             .getQualifiedName() + "!"
@@ -136,6 +149,14 @@ class MethodEndpoint extends Endpoint instanceof DataFlow::MethodNode {
     or
     not this.isSupported() and result = ""
   }
+
+  /**
+   * Holds if this method is a constructor for a module.
+   */
+  private predicate isConstructor() {
+    super.getMethodName() = "initialize" and
+    exists(DataFlow::ModuleNode m | m.getOwnInstanceMethod(super.getMethodName()) = this)
+  }
 }
 
 string methodClassification(Call method) {

ruby/ql/test/query-tests/utils/modeleditor/FrameworkModeEndpoints.expected

@@ -1,12 +1,13 @@
 | lib/module.rb:1:1:7:3 | M1 | mylib | M1 |  |  | false | module.rb |  |
 | lib/module.rb:2:3:3:5 | foo | mylib | M1 | foo | (x,y) | false | module.rb |  |
 | lib/module.rb:5:3:6:5 | self_foo | mylib | M1! | self_foo | (x,y) | false | module.rb |  |
-| lib/mylib.rb:3:1:27:3 | A | mylib | A |  |  | false | mylib.rb |  |
-| lib/mylib.rb:4:3:5:5 | foo | mylib | A | foo | (x,y,key1:) | false | mylib.rb |  |
-| lib/mylib.rb:7:3:8:5 | bar | mylib | A | bar | (x) | false | mylib.rb |  |
-| lib/mylib.rb:10:3:11:5 | self_foo | mylib | A! | self_foo | (x,y) | false | mylib.rb |  |
-| lib/mylib.rb:18:3:26:5 | ANested | mylib | A::ANested |  |  | false | mylib.rb |  |
-| lib/mylib.rb:19:5:20:7 | foo | mylib | A::ANested | foo | (x,y) | false | mylib.rb |  |
+| lib/mylib.rb:3:1:30:3 | A | mylib | A |  |  | false | mylib.rb |  |
+| lib/mylib.rb:4:3:5:5 | initialize | mylib | A! | new | (x,y) | false | mylib.rb |  |
+| lib/mylib.rb:7:3:8:5 | foo | mylib | A | foo | (x,y,key1:) | false | mylib.rb |  |
+| lib/mylib.rb:10:3:11:5 | bar | mylib | A | bar | (x) | false | mylib.rb |  |
+| lib/mylib.rb:13:3:14:5 | self_foo | mylib | A! | self_foo | (x,y) | false | mylib.rb |  |
+| lib/mylib.rb:21:3:29:5 | ANested | mylib | A::ANested |  |  | false | mylib.rb |  |
+| lib/mylib.rb:22:5:23:7 | foo | mylib | A::ANested | foo | (x,y) | false | mylib.rb |  |
 | lib/other.rb:3:1:8:3 | B | mylib | B |  |  | false | other.rb |  |
 | lib/other.rb:6:3:7:5 | foo | mylib | B | foo | (x,y) | false | other.rb |  |
 | lib/other.rb:10:1:12:3 | C | mylib | C |  |  | false | other.rb |  |

ruby/ql/test/query-tests/utils/modeleditor/lib/mylib.rb

@@ -1,6 +1,9 @@
 require_relative "./other"
 
 class A
+  def initialize(x, y)
+  end
+
   def foo(x, y, key1:, **kwargs, &block)
   end