Merge pull request #16308 from asgerf/js/model-generation-quote

JS: Fix naming issue in generated models

Author: RasmusWL

javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll

@@ -108,7 +108,11 @@ module ModelExport<ModelExportSig S> {
     }
 
     predicate exposedName(API::Node node, string type, string path) {
-      node = API::moduleExport(type) and path = ""
+      exists(string moduleName |
+        node = API::moduleExport(moduleName) and
+        path = "" and
+        type = "(" + moduleName + ")"
+      )
     }
 
     predicate suggestedName(API::Node node, string type) {

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll

@@ -34,6 +34,11 @@ class Location = JS::Location;
  *
  * Type names have form `package.type` or just `package` if referring to the package export
  * object. If `package` contains a `.` character it must be enclosed in single quotes, such as `'package'.type`.
+ *
+ * A type name of form `(package)` may also be used when refering to the package export object.
+ * We allow this syntax as an alternative to the above, so models generated based on `EndpointNaming` look more consistent.
+ * However, access paths are deliberately not parsed here, as we can not handle aliasing at this stage.
+ * The model generator must explicitly generate the step between `(package)` and `(package).foo`, for example.
  */
 bindingset[rawType]
 predicate parseTypeString(string rawType, string package, string qualifiedName) {
@@ -42,6 +47,9 @@ predicate parseTypeString(string rawType, string package, string qualifiedName)
     package = rawType.regexpCapture(regexp, 1).regexpReplaceAll("^'|'$", "") and
     qualifiedName = rawType.regexpCapture(regexp, 2).regexpReplaceAll("^\\.", "")
   )
+  or
+  package = rawType.regexpCapture("[(]([^)]+)[)]", 1) and
+  qualifiedName = ""
 }
 
 /**

javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected

@@ -1,17 +1,17 @@
 typeModel
-| (aliases).Alias1 | aliases | Member[Alias1] |
-| (aliases).Alias1 | aliases | Member[Alias2] |
-| (aliases).Alias1 | aliases | Member[Alias3].Member[x] |
-| (aliases).Alias1 | aliases | Member[Alias4].Member[x].Member[x] |
-| (aliases).Alias1 | aliases | Member[AliasedClass] |
+| (aliases).Alias1 | (aliases) | Member[Alias1] |
+| (aliases).Alias1 | (aliases) | Member[Alias2] |
+| (aliases).Alias1 | (aliases) | Member[Alias3].Member[x] |
+| (aliases).Alias1 | (aliases) | Member[Alias4].Member[x].Member[x] |
+| (aliases).Alias1 | (aliases) | Member[AliasedClass] |
 | (aliases).Alias1.prototype | (aliases).Alias1 | Instance |
 | (aliases).Alias1.prototype | (aliases).Alias1.prototype.foo | ReturnValue |
 | (aliases).Alias1.prototype.foo | (aliases).Alias1.prototype | Member[foo] |
-| (long-access-path).a.shortcut.d | long-access-path | Member[a].Member[b].Member[c].Member[d] |
-| (long-access-path).a.shortcut.d | long-access-path | Member[a].Member[shortcut].Member[d] |
+| (long-access-path).a.shortcut.d | (long-access-path) | Member[a].Member[b].Member[c].Member[d] |
+| (long-access-path).a.shortcut.d | (long-access-path) | Member[a].Member[shortcut].Member[d] |
 | (long-access-path).a.shortcut.d.e | (long-access-path).a.shortcut.d | Member[e] |
-| (reexport).func | reexport | Member[func] |
-| (return-this).FluentInterface | return-this | Member[FluentInterface] |
+| (reexport).func | (reexport) | Member[func] |
+| (return-this).FluentInterface | (return-this) | Member[FluentInterface] |
 | (return-this).FluentInterface.prototype | (return-this).FluentInterface | Instance |
 | (return-this).FluentInterface.prototype | (return-this).FluentInterface.prototype.bar | ReturnValue |
 | (return-this).FluentInterface.prototype | (return-this).FluentInterface.prototype.baz | ReturnValue |
@@ -21,45 +21,45 @@ typeModel
 | (return-this).FluentInterface.prototype.foo | (return-this).FluentInterface.prototype | Member[foo] |
 | (return-this).FluentInterface.prototype.notFluent | (return-this).FluentInterface.prototype | Member[notFluent] |
 | (return-this).FluentInterface.prototype.notFluent2 | (return-this).FluentInterface.prototype | Member[notFluent2] |
-| (root-function).PublicClass | root-function | Member[PublicClass] |
+| (root-function).PublicClass | (root-function) | Member[PublicClass] |
+| (root-function).PublicClass.prototype | (root-function) | ReturnValue |
 | (root-function).PublicClass.prototype | (root-function).PublicClass | Instance |
-| (root-function).PublicClass.prototype | root-function | ReturnValue |
 | (root-function).PublicClass.prototype.method | (root-function).PublicClass.prototype | Member[method] |
-| (semi-internal-class).PublicClass | semi-internal-class | Member[PublicClass] |
+| (semi-internal-class).PublicClass | (semi-internal-class) | Member[PublicClass] |
 | (semi-internal-class).PublicClass.prototype | (semi-internal-class).PublicClass | Instance |
 | (semi-internal-class).PublicClass.prototype | (semi-internal-class).SemiInternalClass.prototype.method | ReturnValue |
 | (semi-internal-class).PublicClass.prototype | (semi-internal-class).getAnonymous~expr2 | ReturnValue |
 | (semi-internal-class).PublicClass.prototype.publicMethod | (semi-internal-class).PublicClass.prototype | Member[publicMethod] |
 | (semi-internal-class).SemiInternalClass.prototype | (semi-internal-class).get | ReturnValue |
 | (semi-internal-class).SemiInternalClass.prototype.method | (semi-internal-class).SemiInternalClass.prototype | Member[method] |
-| (semi-internal-class).get | semi-internal-class | Member[get] |
-| (semi-internal-class).getAnonymous | semi-internal-class | Member[getAnonymous] |
+| (semi-internal-class).get | (semi-internal-class) | Member[get] |
+| (semi-internal-class).getAnonymous | (semi-internal-class) | Member[getAnonymous] |
 | (semi-internal-class).getAnonymous~expr1 | (semi-internal-class).getAnonymous | ReturnValue |
 | (semi-internal-class).getAnonymous~expr2 | (semi-internal-class).getAnonymous~expr1 | Member[method] |
-| (subclass).A | subclass | Member[A] |
+| (subclass).A | (subclass) | Member[A] |
 | (subclass).A.prototype | (subclass).A | Instance |
 | (subclass).A.prototype | (subclass).B.prototype |  |
 | (subclass).A.prototype | (subclass).ExposedMidSubClass.prototype~expr1 |  |
 | (subclass).A.prototype.a | (subclass).A.prototype | Member[a] |
-| (subclass).B | subclass | Member[B] |
+| (subclass).B | (subclass) | Member[B] |
 | (subclass).B.prototype | (subclass).B | Instance |
 | (subclass).B.prototype | (subclass).C.prototype |  |
 | (subclass).B.prototype.b | (subclass).B.prototype | Member[b] |
-| (subclass).C | subclass | Member[C] |
+| (subclass).C | (subclass) | Member[C] |
 | (subclass).C.prototype | (subclass).C | Instance |
 | (subclass).C.prototype.c | (subclass).C.prototype | Member[c] |
-| (subclass).D | subclass | Member[D] |
+| (subclass).D | (subclass) | Member[D] |
 | (subclass).D.prototype | (subclass).D | Instance |
 | (subclass).D.prototype.d | (subclass).D.prototype | Member[d] |
-| (subclass).ExposedMidSubClass | subclass | Member[ExposedMidSubClass] |
+| (subclass).ExposedMidSubClass | (subclass) | Member[ExposedMidSubClass] |
 | (subclass).ExposedMidSubClass.prototype | (subclass).ExposedMidSubClass | Instance |
 | (subclass).ExposedMidSubClass.prototype.m | (subclass).ExposedMidSubClass.prototype | Member[m] |
 | (subclass).ExposedMidSubClass.prototype~expr1 | (subclass).ExposedMidSubClass.prototype |  |
+| upstream-lib | (reexport) | Member[lib] |
 | upstream-lib | (reexport).func | ReturnValue |
-| upstream-lib | reexport | Member[lib] |
 | upstream-lib.Type | (subclass).D.prototype |  |
-| upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] |
-| upstream-lib.XYZ | reexport | Member[xy].Member[z] |
+| upstream-lib.XYZ | (reexport) | Member[x].Member[y].Member[z] |
+| upstream-lib.XYZ | (reexport) | Member[xy].Member[z] |
 summaryModel
 | (aliases).Alias1.prototype |  |  | Member[foo].ReturnValue | type |
 | (return-this).FluentInterface.prototype |  |  | Member[bar].ReturnValue | type |

javascript/ql/test/library-tests/frameworks/data/test.expected

@@ -78,6 +78,7 @@ taintFlow
 | test.js:265:6:265:39 | new MyS ... ource() | test.js:265:6:265:39 | new MyS ... ource() |
 | test.js:269:10:269:31 | this.ba ... ource() | test.js:269:10:269:31 | this.ba ... ource() |
 | test.js:272:6:272:40 | new MyS ... ource() | test.js:272:6:272:40 | new MyS ... ource() |
+| test.js:274:6:274:39 | testlib ... eName() | test.js:274:6:274:39 | testlib ... eName() |
 isSink
 | test.js:54:18:54:25 | source() | test-sink |
 | test.js:55:22:55:29 | source() | test-sink |

javascript/ql/test/library-tests/frameworks/data/test.ext.yml

@@ -10,6 +10,7 @@ extensions:
       - ['testlib', 'Member[MethodDecorator].DecoratedMember.Parameter[0]', 'test-source']
       - ['testlib', 'Member[ParamDecoratorSource].DecoratedParameter', 'test-source']
       - ['testlib', 'Member[getSource].ReturnValue', 'test-source']
+      - ['(testlib)', 'Member[parenthesizedPackageName].ReturnValue', 'test-source']
 
   - addsTo:
       pack: codeql/javascript-all
@@ -73,4 +74,4 @@ extensions:
     data:
       - ['ABC', 'Member[a].Member[b].WithArity[0].ReturnValue.Member[c]']
       - ['LeftRight', 'Member[left].TypeVar[LeftRight].Member[right]']
-      - ['LeftRight', 'Member[x]']
+      - ['LeftRight', 'Member[x]']

javascript/ql/test/library-tests/frameworks/data/test.js

@@ -270,3 +270,5 @@ class MySubclass2 extends MySubclass {
   }
 }
 sink(new MySubclass2().baseclassSource()); // NOT OK
+
+sink(testlib.parenthesizedPackageName()); // NOT OK