Merge pull request github#15663 from asgerf/js/endpoint-naming2

asgerf · web-flow · commit db10c229de4f · 2024-02-21T19:36:57.000+01:00
JS: Improvements to endpoint naming
diff --git a/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll b/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll
diff --git a/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected b/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.expected
@@ -1,7 +1,8 @@
 testFailures
 ambiguousPreferredPredecessor
+| pack2/lib.js:1:1:3:1 | def moduleImport("pack2").getMember("exports").getMember("lib").getMember("LibClass").getInstance() |
+| pack2/lib.js:8:22:8:34 | def moduleImport("pack2").getMember("exports").getMember("lib").getMember("LibClass").getMember("foo") |
+| pack2/main.js:1:1:3:1 | def moduleImport("pack2").getMember("exports").getMember("MainClass").getInstance() |
 ambiguousSinkName
-ambiguousClassObjectName
-ambiguousClassInstanceName
 ambiguousFunctionName
 failures
diff --git a/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.ql b/javascript/ql/test/library-tests/EndpointNaming/EndpointNaming.ql
@@ -5,28 +5,21 @@ import semmle.javascript.endpoints.EndpointNaming as EndpointNaming
 import testUtilities.InlineExpectationsTest
 import EndpointNaming::Debug
 
+private predicate isIgnored(DataFlow::FunctionNode function) {
+  function.getFunction() = any(ConstructorDeclaration decl | decl.isSynthetic()).getBody()
+}
+
 module TestConfig implements TestSig {
-  string getARelevantTag() { result = ["instance", "class", "method", "alias"] }
+  string getARelevantTag() { result = ["name", "alias"] }
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
-    exists(string package, string name |
-      element = "" and
+    element = "" and
+    tag = "name" and
+    exists(DataFlow::SourceNode function, string package, string name |
+      EndpointNaming::functionHasPrimaryName(function, package, name) and
+      not isIgnored(function) and
+      location = function.getAstNode().getLocation() and
       value = EndpointNaming::renderName(package, name)
-    |
-      exists(DataFlow::ClassNode cls | location = cls.getAstNode().getLocation() |
-        tag = "class" and
-        EndpointNaming::classObjectHasPrimaryName(cls, package, name)
-        or
-        tag = "instance" and
-        EndpointNaming::classInstanceHasPrimaryName(cls, package, name)
-      )
-      or
-      exists(DataFlow::FunctionNode function |
-        not function.getFunction() = any(ConstructorDeclaration decl | decl.isSynthetic()).getBody() and
-        location = function.getFunction().getLocation() and
-        tag = "method" and
-        EndpointNaming::functionHasPrimaryName(function, package, name)
-      )
     )
     or
     element = "" and
@@ -35,7 +28,7 @@ module TestConfig implements TestSig {
       API::Node aliasDef, string primaryPackage, string primaryName, string aliasPackage,
       string aliasName
     |
-      EndpointNaming::aliasDefinition(primaryPackage, primaryName, aliasPackage, aliasName, aliasDef) and
+      EndpointNaming::aliasDefinition(aliasPackage, aliasName, primaryPackage, primaryName, aliasDef) and
       value =
         EndpointNaming::renderName(aliasPackage, aliasName) + "==" +
           EndpointNaming::renderName(primaryPackage, primaryName) and
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack1/main.js b/javascript/ql/test/library-tests/EndpointNaming/pack1/main.js
@@ -1,13 +1,15 @@
-export class PublicClass {} // $ class=(pack1).PublicClass instance=(pack1).PublicClass.prototype
+export class PublicClass {} // $ name=(pack1).PublicClass
 
 class PrivateClass {}
 
-export const ExportedConst = class ExportedConstClass {} // $ class=(pack1).ExportedConst instance=(pack1).ExportedConst.prototype
+export const ExportedConst = class ExportedConstClass {} // $ name=(pack1).ExportedConst
 
-class ClassWithEscapingInstance {} // $ instance=(pack1).ClassWithEscapingInstance.prototype
+class ClassWithEscapingInstance {
+    m() {} // $ name=(pack1).ClassWithEscapingInstance.prototype.m
+}
 
 export function getEscapingInstance() {
     return new ClassWithEscapingInstance();
-} // $ method=(pack1).getEscapingInstance
+} // $ name=(pack1).getEscapingInstance
 
-export function publicFunction() {} // $ method=(pack1).publicFunction
+export function publicFunction() {} // $ name=(pack1).publicFunction
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack10/foo.js b/javascript/ql/test/library-tests/EndpointNaming/pack10/foo.js
@@ -1 +1 @@
-export default class FooClass {} // $ class=(pack10).Foo instance=(pack10).Foo.prototype
+export default class FooClass {} // $ name=(pack10).Foo
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack11/index.ts b/javascript/ql/test/library-tests/EndpointNaming/pack11/index.ts
@@ -0,0 +1,59 @@
+const f1 = {
+    m() {} // $ name=(pack11).C1.publicField.really.long.name.m
+};
+
+export class C1 {
+    private static privateField = f1;
+
+    public static publicField = {
+        really: {
+            long: {
+                name: f1
+            }
+        }
+    }
+} // $ name=(pack11).C1
+
+const f2 = {
+    m() {} // $ name=(pack11).C2.publicField.really.long.name.m
+}
+
+export class C2 {
+    static #privateField = f2;
+
+    static publicField = {
+        really: {
+            long: {
+                name: f2
+            }
+        }
+    }
+} // $ name=(pack11).C2
+
+function f3() {} // $ name=(pack11).C3.publicField.really.long.name
+
+export class C3 {
+    private static privateField = f3;
+
+    public static publicField = {
+        really: {
+            long: {
+                name: f3
+            }
+        }
+    }
+} // $ name=(pack11).C3
+
+
+const f4 = {
+    m() {} // $ name=(pack11).C4.really.long.name.m
+};
+
+export const C4 = {
+    [Math.random()]: f4,
+    really: {
+        long: {
+            name: f4
+        }
+    }
+}
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack11/package.json b/javascript/ql/test/library-tests/EndpointNaming/pack11/package.json
@@ -0,0 +1,4 @@
+{
+    "name": "pack11",
+    "main": "./index.js"
+}
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack12/index.js b/javascript/ql/test/library-tests/EndpointNaming/pack12/index.js
@@ -0,0 +1,10 @@
+function wrap(fn) {
+    return x => fn(x);
+}
+
+function f() {}
+export const f1 = wrap(f); // $ name=(pack12).f1
+export const f2 = wrap(f); // $ name=(pack12).f2
+
+function g() {}
+export const g1 = wrap(g); // $ name=(pack12).g1
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack12/package.json b/javascript/ql/test/library-tests/EndpointNaming/pack12/package.json
@@ -0,0 +1,4 @@
+{
+    "name": "pack12",
+    "main": "./index.js"
+}
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack2/lib.js b/javascript/ql/test/library-tests/EndpointNaming/pack2/lib.js
@@ -1,6 +1,8 @@
 class AmbiguousClass {
-    instanceMethod(foo) {} // $ method=(pack2).lib.LibClass.prototype.instanceMethod
-} // $ class=(pack2).lib.LibClass instance=(pack2).lib.LibClass.prototype
+    instanceMethod(foo) {} // $ name=(pack2).lib.LibClass.prototype.instanceMethod
+} // $ name=(pack2).lib.LibClass
 
 export default AmbiguousClass; // $ alias=(pack2).lib.default==(pack2).lib.LibClass
 export { AmbiguousClass as LibClass }
+
+AmbiguousClass.foo = function() {} // $ name=(pack2).lib.LibClass.foo
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack2/main.js b/javascript/ql/test/library-tests/EndpointNaming/pack2/main.js
@@ -1,6 +1,6 @@
 class AmbiguousClass {
-    instanceMethod() {} // $ method=(pack2).MainClass.prototype.instanceMethod
-} // $ class=(pack2).MainClass instance=(pack2).MainClass.prototype
+    instanceMethod() {} // $ name=(pack2).MainClass.prototype.instanceMethod
+} // $ name=(pack2).MainClass
 
 export default AmbiguousClass; // $ alias=(pack2).default==(pack2).MainClass
 export { AmbiguousClass as MainClass }
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack3/lib.js b/javascript/ql/test/library-tests/EndpointNaming/pack3/lib.js
@@ -1 +1 @@
-export default function(x,y,z) {} // $ method=(pack3).libFunction alias=(pack3).libFunction.default==(pack3).libFunction
+export default function(x,y,z) {} // $ name=(pack3).libFunction alias=(pack3).libFunction.default==(pack3).libFunction
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack3/main.js b/javascript/ql/test/library-tests/EndpointNaming/pack3/main.js
@@ -1,4 +1,4 @@
-function ambiguousFunction(x, y, z) {} // $ method=(pack3).namedFunction
+function ambiguousFunction(x, y, z) {} // $ name=(pack3).namedFunction
 
 export default ambiguousFunction; // $ alias=(pack3).default==(pack3).namedFunction
 export { ambiguousFunction as namedFunction };
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack4/index.js b/javascript/ql/test/library-tests/EndpointNaming/pack4/index.js
@@ -1 +1 @@
-export default class C {} // $ class=(pack4) instance=(pack4).prototype
+export default class C {} // $ name=(pack4)
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack5/src/index.js b/javascript/ql/test/library-tests/EndpointNaming/pack5/src/index.js
@@ -1 +1 @@
-export default class C {} // $ class=(pack5) instance=(pack5).prototype
+export default class C {} // $ name=(pack5)
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack6/index.js b/javascript/ql/test/library-tests/EndpointNaming/pack6/index.js
@@ -1,6 +1,6 @@
 class C {
-    instanceMethod() {} // $ method=(pack6).instanceMethod
+    instanceMethod() {} // $ name=(pack6).instanceMethod
     static staticMethod() {} // not accessible
-} // $ instance=(pack6)
+}
 
 export default new C();
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack7/index.js b/javascript/ql/test/library-tests/EndpointNaming/pack7/index.js
@@ -1,6 +1,6 @@
-export class D {} // $ class=(pack7).D instance=(pack7).D.prototype
+export class D {} // $ name=(pack7).D
 
 // In this case we are forced to include ".default" to avoid ambiguity with class D above.
 export default {
-    D: class {} // $ class=(pack7).default.D instance=(pack7).default.D.prototype
+    D: class {} // $ name=(pack7).default.D
 };
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack8/foo.js b/javascript/ql/test/library-tests/EndpointNaming/pack8/foo.js
@@ -1,4 +1,4 @@
-class Foo {} // $ class=(pack8).Foo instance=(pack8).Foo.prototype
+class Foo {} // $ name=(pack8).Foo
 
 module.exports = Foo;
 module.exports.default = Foo; // $ alias=(pack8).Foo.default==(pack8).Foo
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack8/index.js b/javascript/ql/test/library-tests/EndpointNaming/pack8/index.js
@@ -1,4 +1,4 @@
-class Main {} // $ class=(pack8) instance=(pack8).prototype
+class Main {} // $ name=(pack8)
 
 Main.Foo = require('./foo');
 
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack9/foo.js b/javascript/ql/test/library-tests/EndpointNaming/pack9/foo.js
@@ -1 +1 @@
-export class Foo {} // $ instance=(pack9/foo).Foo.prototype
+export class Foo {}
diff --git a/javascript/ql/test/library-tests/EndpointNaming/pack9/index.ts b/javascript/ql/test/library-tests/EndpointNaming/pack9/index.ts
@@ -6,4 +6,4 @@ import * as foo from "./foo";
 
 export function expose() {
     return new foo.Foo(); // expose an instance of Foo but not the class
-} // $ method=(pack9).expose
+} // $ name=(pack9).expose

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-export default class FooClass {} // $ class=(pack10).Foo instance=(pack10).Foo.prototype`
	`1`	`+export default class FooClass {} // $ name=(pack10).Foo`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +    "name": "pack11",
 +    "main": "./index.js"
 +}