qldoc changes and renaming

Author: erik-krogh

javascript/ql/src/javascript.qll

@@ -12,6 +12,7 @@ import semmle.javascript.Base64
 import semmle.javascript.CFG
 import semmle.javascript.Classes
 import semmle.javascript.Closure
+import semmle.javascript.Collections
 import semmle.javascript.Comments
 import semmle.javascript.Concepts
 import semmle.javascript.Constants
@@ -36,7 +37,6 @@ import semmle.javascript.JsonParsers
 import semmle.javascript.JSX
 import semmle.javascript.Lines
 import semmle.javascript.Locations
-import semmle.javascript.MapAndSet
 import semmle.javascript.Modules
 import semmle.javascript.NodeJS
 import semmle.javascript.NPM

javascript/ql/src/semmle/javascript/MapAndSet.qll renamed to javascript/ql/src/semmle/javascript/Collections.qll

@@ -1,95 +1,88 @@
 /**
- * Provides predicates and classes for working with the standard library implementations of
- * [Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) and
- * [Set](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set)
+ * Provides predicates and classes for working with the standard library collection implementations.
+ * Currently [Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) and
+ * [Set](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set) are implemented.
  */
 
 import javascript
 private import semmle.javascript.dataflow.internal.StepSummary
 private import DataFlow::PseudoProperties
 
 /**
- * Common predicates and classes for type and data-flow tracking on Maps and Sets.
+ * An `AdditionalFlowStep` used to model a data-flow step related to standard library collections.
+ *
+ * The `loadStep`/`storeStep`/`loadStoreStep` methods are overloaded such that the new predicates
+ * `load`/`store`/`loadStore` can be used in the `CollectionsTypeTracking` module.
+ * (Thereby avoiding naming conflicts with a "cousin" `AdditionalFlowStep` implementation.)
  */
-private module MapsAndSets {
-  /**
-   * An `AdditionalFlowStep` used to model a data-flow step related to Maps and Sets.
-   *
-   * The `loadStep`/`storeStep`/`loadStoreStep` methods are overloaded such that the new predicates
-   * `load`/`store`/`loadStore` can be used in the `MapsAndSetsTypeTracking` module.
-   * (Thereby avoiding conflicts with a "cousin" `AdditionalFlowStep` implementation.)
-   */
-  abstract class MapOrSetFlowStep extends DataFlow::AdditionalFlowStep {
-    final override predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
-
-    final override predicate step(
-      DataFlow::Node p, DataFlow::Node s, DataFlow::FlowLabel pl, DataFlow::FlowLabel sl
-    ) {
-      none()
-    }
+private abstract class CollectionFlowStep extends DataFlow::AdditionalFlowStep {
+  final override predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
 
-    /**
-     * Holds if the property `prop` of the object `pred` should be loaded into `succ`.
-     */
-    predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
+  final override predicate step(
+    DataFlow::Node p, DataFlow::Node s, DataFlow::FlowLabel pl, DataFlow::FlowLabel sl
+  ) {
+    none()
+  }
 
-    final override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      this.load(pred, succ, prop)
-    }
+  /**
+   * Holds if the property `prop` of the object `pred` should be loaded into `succ`.
+   */
+  predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
 
-    /**
-     * Holds if `pred` should be stored in the object `succ` under the property `prop`.
-     */
-    predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
+  final override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    this.load(pred, succ, prop)
+  }
 
-    final override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      this.store(pred, succ, prop)
-    }
+  /**
+   * Holds if `pred` should be stored in the object `succ` under the property `prop`.
+   */
+  predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
 
-    /**
-     * Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
-     */
-    predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
+  final override predicate storeStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    this.store(pred, succ, prop)
+  }
 
-    final override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      this.loadStore(pred, succ, prop, prop)
-    }
+  /**
+   * Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
+   */
+  predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
 
-    /**
-     * Holds if the property `loadProp` should be copied from the object `pred` to the property `storeProp` of object `succ`.
-     */
-    predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string loadProp, string storeProp) {
-      none()
-    }
+  final override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+    this.loadStore(pred, succ, prop, prop)
+  }
 
-    final override predicate loadStoreStep(
-      DataFlow::Node pred, DataFlow::Node succ, string loadProp, string storeProp
-    ) {
-      this.loadStore(pred, succ, loadProp, storeProp)
-    }
+  /**
+   * Holds if the property `loadProp` should be copied from the object `pred` to the property `storeProp` of object `succ`.
+   */
+  predicate loadStore(DataFlow::Node pred, DataFlow::Node succ, string loadProp, string storeProp) {
+    none()
+  }
 
-    /**
-     * Holds if this is a map step that could potentially load a value where the corresponding key has a known string value.
-     */
-    predicate canLoadKnownKey() { none() }
+  final override predicate loadStoreStep(
+    DataFlow::Node pred, DataFlow::Node succ, string loadProp, string storeProp
+  ) {
+    this.loadStore(pred, succ, loadProp, storeProp)
   }
+
+  /**
+   * Holds if this step on a collection can load a value with a known key.
+   */
+  predicate canLoadValueWithKnownKey() { none() }
 }
 
 /**
- * A collection of predicates and clases for type-tracking Maps and Sets.
+ * A collection of predicates and clases for type-tracking collections.
  */
-module MapsAndSetsTypeTracking {
-  private import MapsAndSets
-
+module CollectionsTypeTracking {
   /**
-   * Gets the result from a single step through a Map or Set, from `pred` to `result` summarized by `summary`.
+   * Gets the result from a single step through a collection, from `pred` to `result` summarized by `summary`.
    */
   pragma[inline]
-  DataFlow::SourceNode mapOrSetStep(DataFlow::Node pred, StepSummary summary) {
-    exists(MapOrSetFlowStep step, string field |
+  DataFlow::SourceNode collectionStep(DataFlow::Node pred, StepSummary summary) {
+    exists(CollectionFlowStep step, string field |
       summary = LoadStep(field) and
       step.load(pred, result, field) and
-      (not step.canLoadKnownKey() or not field = mapValueUnknownKey()) // for a step that could load a known key, we prune the steps where the key is unknown.
+      (not step.canLoadValueWithKnownKey() or not field = mapValueUnknownKey()) // for a step that could load a known key, we prune the steps where the key is unknown.
       or
       summary = StoreStep(field) and
       step.store(pred, result, field)
@@ -104,42 +97,40 @@ module MapsAndSetsTypeTracking {
   }
 
   /**
-   * Gets the result from a single step through a Map or set, from `pred` with tracker `t2` to `result` with tracker `t`.
+   * Gets the result from a single step through a collection, from `pred` with tracker `t2` to `result` with tracker `t`.
    */
   pragma[inline]
-  DataFlow::SourceNode mapOrSetStep(
+  DataFlow::SourceNode collectionStep(
     DataFlow::SourceNode pred, DataFlow::TypeTracker t, DataFlow::TypeTracker t2
   ) {
     exists(DataFlow::Node mid, StepSummary summary | pred.flowsTo(mid) and t = t2.append(summary) |
-      result = mapOrSetStep(mid, summary)
+      result = collectionStep(mid, summary)
     )
   }
 
   /**
-   * A class enabling the use of the Map and Set related pseudo-properties as a pseudo-property in type-tracking predicates.
+   * A class enabling the use of the collection related pseudo-properties in type-tracking predicates.
    */
   private class MapRelatedPseudoFieldAsTypeTrackingProperty extends TypeTrackingPseudoProperty {
     MapRelatedPseudoFieldAsTypeTrackingProperty() {
       this = [setElement(), iteratorElement()] or
-      any(MapOrSetFlowStep step).store(_, _, this)
+      any(CollectionFlowStep step).store(_, _, this)
     }
 
     override string getLoadStoreToProp() {
-      exists(MapOrSetFlowStep step | step.loadStore(_, _, this, result))
+      exists(CollectionFlowStep step | step.loadStore(_, _, this, result))
     }
   }
 }
 
 /**
- * A module for data-flow steps related to `Set` and `Map`.
+ * A module for data-flow steps related standard library collection implementations.
  */
-private module MapAndSetDataFlow {
-  private import MapsAndSets
-
+private module CollectionDataFlow {
   /**
-   * A step for an `add` method, which adds an element to a Set.
+   * A step for `Set.add()` method, which adds an element to a Set.
    */
-  private class SetAdd extends MapOrSetFlowStep, DataFlow::MethodCallNode {
+  private class SetAdd extends CollectionFlowStep, DataFlow::MethodCallNode {
     SetAdd() { this.getMethodName() = "add" }
 
     override predicate store(DataFlow::Node element, DataFlow::Node obj, string prop) {
@@ -152,7 +143,7 @@ private module MapAndSetDataFlow {
   /**
    * A step for the `Set` constructor, which copies any elements from the first argument into the resulting set.
    */
-  private class SetConstructor extends MapOrSetFlowStep, DataFlow::NewNode {
+  private class SetConstructor extends CollectionFlowStep, DataFlow::NewNode {
     SetConstructor() { this = DataFlow::globalVarRef("Set").getAnInstantiation() }
 
     override predicate loadStore(
@@ -171,9 +162,9 @@ private module MapAndSetDataFlow {
    * For Maps the l-value is a tuple containing a key and a value.
    *
    * This is partially duplicated behavior with the `for of` step for Arrays (in Arrays.qll).
-   * This duplication is required for the type-tracking steps defined in `MapsAndSetsTypeTracking`.
+   * This duplication is required for the type-tracking steps defined in `CollectionsTypeTracking`.
    */
-  private class ForOfStep extends MapOrSetFlowStep, DataFlow::ValueNode {
+  private class ForOfStep extends CollectionFlowStep, DataFlow::ValueNode {
     ForOfStmt forOf;
     DataFlow::Node element;
 
@@ -201,7 +192,7 @@ private module MapAndSetDataFlow {
   /**
    * A step for a call to `forEach` on a Set or Map.
    */
-  private class SetMapForEach extends MapOrSetFlowStep, DataFlow::MethodCallNode {
+  private class SetMapForEach extends CollectionFlowStep, DataFlow::MethodCallNode {
     SetMapForEach() { this.getMethodName() = "forEach" }
 
     override predicate load(DataFlow::Node obj, DataFlow::Node element, string prop) {
@@ -215,7 +206,7 @@ private module MapAndSetDataFlow {
    * A call to the `get` method on a Map.
    * If the key of the call to `get` has a known string value, then only the value corresponding to that key will be retrieved.
    */
-  private class MapGet extends MapOrSetFlowStep, DataFlow::MethodCallNode {
+  private class MapGet extends CollectionFlowStep, DataFlow::MethodCallNode {
     MapGet() { this.getMethodName() = "get" }
 
     override predicate load(DataFlow::Node obj, DataFlow::Node element, string prop) {
@@ -224,7 +215,7 @@ private module MapAndSetDataFlow {
       prop = mapValue(this.getArgument(0))
     }
 
-    override predicate canLoadKnownKey() { any() }
+    override predicate canLoadValueWithKnownKey() { any() }
   }
 
   /**
@@ -234,7 +225,7 @@ private module MapAndSetDataFlow {
    * then the value will be saved into a pseudo-property corresponding to the known string value.
    * The value will additionally be saved into a pseudo-property corresponding to values with unknown keys.
    */
-  private class MapSet extends MapOrSetFlowStep, DataFlow::MethodCallNode {
+  private class MapSet extends CollectionFlowStep, DataFlow::MethodCallNode {
     MapSet() { this.getMethodName() = "set" }
 
     override predicate store(DataFlow::Node element, DataFlow::Node obj, string prop) {
@@ -248,7 +239,7 @@ private module MapAndSetDataFlow {
   /**
    * A step for a call to `values` on a Map or a Set.
    */
-  private class MapAndSetValues extends MapOrSetFlowStep, DataFlow::MethodCallNode {
+  private class MapAndSetValues extends CollectionFlowStep, DataFlow::MethodCallNode {
     MapAndSetValues() { this.getMethodName() = "values" }
 
     override predicate loadStore(
@@ -264,7 +255,7 @@ private module MapAndSetDataFlow {
   /**
    * A step for a call to `keys` on a Set.
    */
-  private class SetKeys extends MapOrSetFlowStep, DataFlow::MethodCallNode {
+  private class SetKeys extends CollectionFlowStep, DataFlow::MethodCallNode {
     SetKeys() { this.getMethodName() = "keys" }
 
     override predicate loadStore(

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

@@ -576,36 +576,39 @@ abstract class AdditionalFlowStep extends DataFlow::Node {
 
 /**
  * A collection of pseudo-properties that are used in multiple files.
+ * 
+ * A pseudo-property represents the location where some value is stored in an object. 
+ * 
  * For use with load/store steps in `DataFlow::AdditionalFlowStep` and TypeTracking.
  */
 module PseudoProperties {
   /**
-   * Gets a pseudo-property representing elements inside a `Set`
+   * Gets a pseudo-property for the location of elements in a `Set`
    */
   string setElement() { result = "$setElement$" }
 
   /**
-   * Gets a pseudo-property representing elements inside a JavaScript iterator.
+   * Gets a pseudo-property for the location of elements in a JavaScript iterator.
    */
   string iteratorElement() { result = "$iteratorElement$" }
 
   /**
-   * Gets a pseudo-field representing an elements inside an `Array`.
+   * Gets a pseudo-property for the location of elements in an `Array`.
    */
   string arrayElement() { result = "$arrayElement$" }
 
   /**
-   * Gets a pseudo-property representing elements inside some array-like object. (Set, Array, or Iterator).
+   * Gets a pseudo-property for the location of elements in some array-like object. (Set, Array, or Iterator).
    */
   string arrayLikeElement() { result = [setElement(), iteratorElement(), arrayElement()] }
 
   /**
-   * Gets a pseudo-property representing the values of a Map, where the key is unknown.
+   * Gets a pseudo-property for the location of map values, where the key is unknown.
    */
   string mapValueUnknownKey() { result = "$UnknownMapValue$" }
 
   /**
-   * Gets a pseudo property for a Map value where the key is `key`.
+   * Gets a pseudo-property for the location of a map value where the key is `key`.
    * The string value of the `key` is encoded in the result, and there is only a result if the string value of `key` is known.
    */
   pragma[inline]
@@ -614,7 +617,7 @@ module PseudoProperties {
   }
 
   /**
-   * Gets a psuedo property for a map value where the key is `key`.
+   * Gets a pseudo-property for the location of a map value where the key is `key`.
    */
   pragma[inline]
   string mapValue(DataFlow::Node key) {

javascript/ql/test/library-tests/frameworks/Sets/test.expected renamed to javascript/ql/test/library-tests/frameworks/Collections/test.expected

@@ -25,7 +25,7 @@ DataFlow::SourceNode trackSource(DataFlow::TypeTracker t, DataFlow::SourceNode s
   exists(DataFlow::TypeTracker t2 | t = t2.step(trackSource(t2, start), result))
   or
   exists(DataFlow::TypeTracker t2 |
-    result = MapsAndSetsTypeTracking::mapOrSetStep(trackSource(t2, start), t, t2)
+    result = CollectionsTypeTracking::collectionStep(trackSource(t2, start), t, t2)
   )
 }