JS: handle accessors separately

Author: asger-semmle

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

@@ -466,6 +466,44 @@ DataFlow::SourceNode moduleMember(string path, string m) {
   )
 }
 
+/**
+ * The string `method`, `getter`, or `setter`, representing the kind of a function member
+ * in a class.
+ *
+ * Can can be used with `ClassNode.getInstanceMember` to obtain members of a specific kind.
+ */
+class MemberKind extends string {
+  MemberKind() { this = "method" or this = "getter" or this = "setter" }
+}
+
+module MemberKind {
+  /** The kind of a method, such as `m() {}` */
+  MemberKind method() { result = "method" }
+
+  /** The kind of a getter accessor, such as `get f() {}`. */
+  MemberKind getter() { result = "getter" }
+
+  /** The kind of a setter accessor, such as `set f() {}`. */
+  MemberKind setter() { result = "setter" }
+
+  /** The `getter` and `setter` kinds. */
+  MemberKind accessor() { result = getter() or result = setter() }
+
+  /**
+   * Gets the member kind of a given syntactic member declaration in a class.
+   */
+  MemberKind of(MemberDeclaration decl) {
+    decl instanceof GetterMethodDeclaration and result = getter()
+    or
+    decl instanceof SetterMethodDeclaration and result = setter()
+    or
+    decl instanceof MethodDeclaration and
+    not decl instanceof AccessorMethodDeclaration and
+    not decl instanceof ConstructorDeclaration and
+    result = method()
+  }
+}
+
 /**
  * A data flow node corresponding to a class definition or a function definition
  * acting as a class.
@@ -516,7 +554,7 @@ class ClassNode extends DataFlow::SourceNode {
    *
    * Does not include methods from superclasses.
    */
-  FunctionNode getInstanceMethod(string name) { result = impl.getInstanceMethod(name) }
+  FunctionNode getInstanceMethod(string name) { result = impl.getInstanceMember(name, MemberKind::method()) }
 
   /**
    * Gets an instance method declared in this class.
@@ -525,7 +563,28 @@ class ClassNode extends DataFlow::SourceNode {
    *
    * Does not include methods from superclasses.
    */
-  FunctionNode getAnInstanceMethod() { result = impl.getAnInstanceMethod() }
+  FunctionNode getAnInstanceMethod() { result = impl.getAnInstanceMember(MemberKind::method()) }
+
+  /**
+   * Gets the instance method, getter, or setter with the given name and kind.
+   *
+   * Does not include members from superclasses.
+   */
+  FunctionNode getInstanceMember(string name, MemberKind kind) { result = impl.getInstanceMember(name, kind) }
+
+  /**
+   * Gets an instance method, getter, or setter with the given kind.
+   *
+   * Does not include members from superclasses.
+   */
+  FunctionNode getAnInstanceMember(MemberKind kind) { result = impl.getAnInstanceMember(kind) }
+
+  /**
+   * Gets an instance method, getter, or setter declared in this class.
+   *
+   * Does not include members from superclasses.
+   */
+  FunctionNode getAnInstanceMember() { result = impl.getAnInstanceMember(_) }
 
   /**
    * Gets the static method declared in this class with the given name.
@@ -576,16 +635,14 @@ module ClassNode {
     abstract FunctionNode getConstructor();
 
     /**
-     * Gets an instance method with the given name, if any.
+     * Gets the instance member with the given name and kind.
      */
-    abstract FunctionNode getInstanceMethod(string name);
+    abstract FunctionNode getInstanceMember(string name, MemberKind kind);
 
     /**
-     * Gets an instance method of this class.
-     *
-     * The constructor is not considered an instance method.
+     * Gets an instance member with the given kind.
      */
-    abstract FunctionNode getAnInstanceMethod();
+    abstract FunctionNode getAnInstanceMember(MemberKind kind);
 
     /**
      * Gets the static method of this class with the given name.
@@ -618,20 +675,20 @@ module ClassNode {
 
     override FunctionNode getConstructor() { result = astNode.getConstructor().getBody().flow() }
 
-    override FunctionNode getInstanceMethod(string name) {
+    override FunctionNode getInstanceMember(string name, MemberKind kind) {
       exists(MethodDeclaration method |
         method = astNode.getMethod(name) and
         not method.isStatic() and
-        not method instanceof ConstructorDeclaration and
+        kind = MemberKind::of(method) and
         result = method.getBody().flow()
       )
     }
 
-    override FunctionNode getAnInstanceMethod() {
+    override FunctionNode getAnInstanceMember(MemberKind kind) {
       exists(MethodDeclaration method |
         method = astNode.getAMethod() and
         not method.isStatic() and
-        not method instanceof ConstructorDeclaration and
+        kind = MemberKind::of(method) and
         result = method.getBody().flow()
       )
     }
@@ -671,12 +728,36 @@ module ClassNode {
 
     override FunctionNode getConstructor() { result = this }
 
-    override FunctionNode getInstanceMethod(string name) {
+    private PropertyAccessor getAnAccessor(MemberKind kind) {
+      result.getObjectExpr() = getAPrototypeReference().asExpr() and
+      (
+        kind = MemberKind::getter() and
+        result instanceof PropertyGetter
+        or
+        kind = MemberKind::setter() and
+        result instanceof PropertySetter
+      )
+    }
+
+    override FunctionNode getInstanceMember(string name, MemberKind kind) {
+      kind = MemberKind::method() and
       result = getAPrototypeReference().getAPropertySource(name)
+      or
+      exists (PropertyAccessor accessor |
+        accessor = getAnAccessor(kind) and
+        accessor.getName() = name and
+        result = accessor.getInit().flow()
+      )
     }
 
-    override FunctionNode getAnInstanceMethod() {
+    override FunctionNode getAnInstanceMember(MemberKind kind) {
+      kind = MemberKind::method() and
       result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource()
+      or
+      exists (PropertyAccessor accessor |
+        accessor = getAnAccessor(kind) and
+        result = accessor.getInit().flow()
+      )
     }
 
     override FunctionNode getStaticMethod(string name) {

javascript/ql/test/library-tests/ClassNode/InstanceMember.expected

@@ -0,0 +1,8 @@
+| tst.js:4:17:4:21 | () {} | A.instanceMethod | method |
+| tst.js:7:6:7:10 | () {} | A.bar | method |
+| tst.js:9:10:9:14 | () {} | A.baz | getter |
+| tst.js:17:19:17:31 | function() {} | B.foo | method |
+| tst.js:21:19:21:31 | function() {} | C.bar | method |
+| tst.js:25:13:25:17 | () {} | D.getter | getter |
+| tst.js:26:13:26:18 | (x) {} | D.setter | setter |
+| tst.js:27:4:27:8 | () {} | D.m | method |

javascript/ql/test/library-tests/ClassNode/InstanceMember.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from DataFlow::ClassNode cls, string name, string kind
+select cls.getInstanceMember(name, kind), cls.getName() + "." + name, kind

javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected

@@ -1,4 +1,5 @@
 | tst.js:4:17:4:21 | () {} | A.instanceMethod |
 | tst.js:7:6:7:10 | () {} | A.bar |
-| tst.js:15:19:15:31 | function() {} | B.foo |
-| tst.js:19:19:19:31 | function() {} | C.bar |
+| tst.js:17:19:17:31 | function() {} | B.foo |
+| tst.js:21:19:21:31 | function() {} | C.bar |
+| tst.js:27:4:27:8 | () {} | D.m |

javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql

@@ -1,4 +1,4 @@
 import javascript
 
 from DataFlow::ClassNode cls, string name
-select cls.getAnInstanceMethod(name), cls.getName() + "." + name
+select cls.getInstanceMethod(name), cls.getName() + "." + name

javascript/ql/test/library-tests/ClassNode/SuperClass.expected

@@ -1,3 +1,3 @@
-| tst.js:11:1:11:21 | class A ... ds A {} | A2 | tst.js:3:1:8:1 | class A ... () {}\\n} | A |
-| tst.js:13:1:13:15 | function B() {} | B | tst.js:3:1:8:1 | class A ... () {}\\n} | A |
-| tst.js:17:1:17:15 | function C() {} | C | tst.js:13:1:13:15 | function B() {} | B |
+| tst.js:13:1:13:21 | class A ... ds A {} | A2 | tst.js:3:1:10:1 | class A ... () {}\\n} | A |
+| tst.js:15:1:15:15 | function B() {} | B | tst.js:3:1:10:1 | class A ... () {}\\n} | A |
+| tst.js:19:1:19:15 | function C() {} | C | tst.js:15:1:15:15 | function B() {} | B |

javascript/ql/test/library-tests/ClassNode/tst.js

@@ -5,6 +5,8 @@ class A {
   static staticMethod() {}
 
   bar() {}
+  
+  get baz() {}
 }
 
 
@@ -17,3 +19,10 @@ B.prototype.foo = function() {}
 function C() {}
 C.prototype = new B();
 C.prototype.bar = function() {}
+
+function D() {}
+D.prototype = {
+  get getter() {},
+  set setter(x) {},
+  m() {}
+}

javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected

@@ -1,4 +1,3 @@
-| points.js:1:1:18:1 | class P ... ;\\n  }\\n} | dist | points.js:7:11:9:3 | () {\\n   ... y);\\n  } |
 | points.js:1:1:18:1 | class P ... ;\\n  }\\n} | toString | points.js:11:11:13:3 | () {\\n   ... )";\\n  } |
 | points.js:20:1:33:1 | class C ... ;\\n  }\\n} | toString | points.js:26:11:28:3 | () {\\n   ... ur;\\n  } |
 | tst.js:1:9:4:1 | class { ...  */ }\\n} | constructor | tst.js:3:18:3:56 | () { /* ... r. */ } |