changes based on review

Author: erik-krogh

ruby/ql/lib/codeql/ruby/security/KernelOpenQuery.qll

@@ -1,5 +1,5 @@
 /**
- * Provides a utility classes and predicates for queries reasoning about Kernel.open and related methods.
+ * Provides utility classes and predicates for reasoning about `Kernel.open` and related methods.
  */
 
 private import codeql.ruby.AST
@@ -9,36 +9,28 @@ private import codeql.ruby.ApiGraphs
 private import codeql.ruby.frameworks.core.Kernel::Kernel
 
 /** A call to a method that might access a file or start a process. */
-abstract class AmbiguousPathCall extends DataFlow::CallNode {
-  /** Gets the name for the method being called. */
-  abstract string getName();
-
-  /** Gets the name for a safer method that can be used instead. */
-  abstract string getReplacement();
-
-  /** Gets the argument that specifies the path to be accessed. */
-  abstract DataFlow::Node getPathArgument();
-}
-
-private class KernelOpenCall extends KernelMethodCall, AmbiguousPathCall {
-  KernelOpenCall() { this.getMethodName() = "open" }
-
-  override string getName() { result = "Kernel.open" }
+class AmbiguousPathCall extends DataFlow::CallNode {
+  string name;
 
-  override string getReplacement() { result = "File.open" }
-
-  override DataFlow::Node getPathArgument() { result = this.getArgument(0) }
-}
-
-private class IOReadCall extends DataFlow::CallNode, AmbiguousPathCall {
-  IOReadCall() {
+  AmbiguousPathCall() {
+    this.(KernelMethodCall).getMethodName() = "open" and
+    name = "Kernel.open"
+    or
     this = API::getTopLevelMember("IO").getAMethodCall("read") and
-    not this = API::getTopLevelMember("File").getAMethodCall("read") // needed in e.g. opal/opal, where some calls have both paths, but I'm not sure why
+    not this = API::getTopLevelMember("File").getAMethodCall("read") and // needed in e.g. opal/opal, where some calls have both paths, but I'm not sure why
+    name = "IO.read"
   }
 
-  override string getName() { result = "IO.read" }
+  /** Gets the name for the method being called. */
+  string getName() { result = name }
 
-  override string getReplacement() { result = "File.read" }
+  /** Gets the name for a safer method that can be used instead. */
+  string getReplacement() {
+    result = "File.read" and name = "IO.read"
+    or
+    result = "File.open" and name = "Kernel.open"
+  }
 
-  override DataFlow::Node getPathArgument() { result = this.getArgument(0) }
+  /** Gets the argument that specifies the path to be accessed. */
+  DataFlow::Node getPathArgument() { result = this.getArgument(0) }
 }