diff --git a/graph/javascript/engine/call-edge-generation/calls.dl b/graph/javascript/engine/call-edge-generation/calls.dl index f80d83e46..34fbd6b81 100644 --- a/graph/javascript/engine/call-edge-generation/calls.dl +++ b/graph/javascript/engine/call-edge-generation/calls.dl @@ -78,9 +78,10 @@ unresolved_receiver(ce, cn, ck, "other_form") :- call_site("client", ck, cn, _, // name narrows what it calls, so any function that escapes as a value may be the // callee, and a question that finds no chain from its caller must answer "unknown", // never "independent" (#1649). Binding: parameter | loop_variable | local | -// computed | dynamic_code | module_variable | field | parameter_member | expression | -// imported_variable. An imported binding is one only when it names a variable its -// module reassigns (below); otherwise it names a module member. +// computed | dynamic_code | module_variable | field | instance_member | +// parameter_member | expression | imported_variable. An imported binding is one only +// when it names a variable its module reassigns (below); otherwise it names a module +// member. value_callee_open(ce, c) :- call_site("client", ck, _, _, _, _, ce, _, _), !call_kind_is_dynamic(ck), !call_resolved(ce), !callee_value_known(ce), expr_child(_, ce, "CALLEE", _, c). unresolved_value_callee(ce, "parameter") :- value_callee_open(ce, c), expr_param(_, _, c). @@ -124,6 +125,18 @@ unresolved_value_callee(ce, "field") :- field_call(ce, k, t, n), !call_resolved( // `this.cb = cb || function () {}`: the call resolves to the fallback, but the // parameter it falls back from is a function no caller in the graph supplies. unresolved_value_callee(ce, "field") :- field_call(ce, k, t, n), field_open_fallback(k, t, n). +// `this.hook()` where neither the class, an ancestor, nor any write the graph can +// follow gives it a `hook`: the function is stored on the instance from outside, +// through another reference (`e.hook = fn`) or a copy (`Object.assign(this, opts)`) +// (#1709). A class with a base the graph could not resolve is left out: that base +// may declare `hook`, and then the call names a method, not a value. +unresolved_value_callee(ce, "instance_member") :- field_call(ce, "inst", t, n), !call_resolved(ce), + !callee_value_known(ce), !ambient_site(ce), !this_field_named("inst", t, n), !this_type_open(t), + expr_child(_, ce, "RECEIVER", _, r), expr_kind(_, "THIS", _, r). +this_type_open(t) :- heritage_unresolved(t, _, _). +this_type_open(t) :- heritage_dynamic(t, _). +this_type_open(t) :- type_ancestor(t, a), heritage_unresolved(a, _, _). +this_type_open(t) :- type_ancestor(t, a), heritage_dynamic(a, _). field_call(ce, k, t, n) :- call_site("client", ck, n, "SYNTACTIC", _, _, ce, _, _), (call_kind_is_member_form(ck) ; ck = "CONSTRUCTOR_CALL"), !call_kind_is_dynamic(ck), expr_child(_, ce, "RECEIVER", _, r), instance_or_class_value(r, k, t). diff --git a/graph/javascript/souffle/decls_all.dl b/graph/javascript/souffle/decls_all.dl index 8c46f9e0a..4e08b8c0f 100644 --- a/graph/javascript/souffle/decls_all.dl +++ b/graph/javascript/souffle/decls_all.dl @@ -87,6 +87,7 @@ .decl variable_reassigned(c0:symbol) .decl call_passes_function(c0:symbol) .decl live_export_variable(c0:symbol, c1:symbol) +.decl this_type_open(c0:symbol) // ── containment/ownership.dl ── .decl call_caller_method(c0:symbol, c1:symbol) diff --git a/graph/test/javascript/cases/68-instance-member-callee/src/index.js b/graph/test/javascript/cases/68-instance-member-callee/src/index.js new file mode 100644 index 000000000..e749e0504 --- /dev/null +++ b/graph/test/javascript/cases/68-instance-member-callee/src/index.js @@ -0,0 +1,23 @@ +// `this.x()` where no class declares `x`: the function is stored on the instance +// from outside, so the site is a value callee (#1709). +function alpha() { return 'a'; } +class External { run() { return this.hook(); } } +function wire(e, fn) { e.hook = fn; } +class Assigned { + constructor(opts) { Object.assign(this, opts); } + run() { return this.onReady(); } +} +class Stored { + constructor(x) { this.x = x; } + run() { return this.x(); } +} +// controls: a declared method resolves; a base the graph cannot see may declare it +class Declared { + own() { return 1; } + run() { return this.own(); } +} +const Base = require('some-unstaged-base'); +class Sub extends Base { run() { return this.emit('x'); } } +// control: an Object.prototype member is the platform's, not a stored value +class Proto { run(k) { return this.hasOwnProperty(k); } } +module.exports = { alpha, External, wire, Assigned, Stored, Declared, Sub, Proto }; diff --git a/graph/test/javascript/expected/68-instance-member-callee.diag b/graph/test/javascript/expected/68-instance-member-callee.diag new file mode 100644 index 000000000..d6ed769f8 --- /dev/null +++ b/graph/test/javascript/expected/68-instance-member-callee.diag @@ -0,0 +1,11 @@ +import_cause index.js:19:14 some-unstaged-base not_staged +package_entry @axiomcode/code-graph . [] MAIN dist/reason.js NOT_STAGED -> - +unresolved index.js:12:18 METHOD_CALL x member_absent +unresolved index.js:20:41 METHOD_CALL emit member_absent +unresolved index.js:22:31 METHOD_CALL hasOwnProperty member_absent +unresolved index.js:4:33 METHOD_CALL hook member_absent +unresolved index.js:7:23 METHOD_CALL assign no_target +unresolved index.js:8:18 METHOD_CALL onReady member_absent +value_callee index.js:12:18 this.x field +value_callee index.js:4:33 this.hook instance_member +value_callee index.js:8:18 this.onReady instance_member diff --git a/graph/test/javascript/expected/68-instance-member-callee.edges b/graph/test/javascript/expected/68-instance-member-callee.edges new file mode 100644 index 000000000..68cc3a334 --- /dev/null +++ b/graph/test/javascript/expected/68-instance-member-callee.edges @@ -0,0 +1,7 @@ +index.js:12:18 METHOD_CALL this.x -> ambiguous_unknown - +index.js:17:18 METHOD_CALL this.own -> known_edge index.js:16:3 own +index.js:20:41 METHOD_CALL this.emit -> ambiguous_unknown - +index.js:22:31 METHOD_CALL this.hasOwnProperty -> ambient_terminal - +index.js:4:33 METHOD_CALL this.hook -> ambiguous_unknown - +index.js:7:23 METHOD_CALL Object.assign -> ambient_terminal - +index.js:8:18 METHOD_CALL this.onReady -> ambiguous_unknown - diff --git a/graph/test/javascript/expected/68-instance-member-callee.oracle b/graph/test/javascript/expected/68-instance-member-callee.oracle new file mode 100644 index 000000000..9512748e8 --- /dev/null +++ b/graph/test/javascript/expected/68-instance-member-callee.oracle @@ -0,0 +1,4 @@ +index.js:17:18 METHOD_CALL own EXACT index.js:16:3 +index.js:22:31 METHOD_CALL hasOwnProperty LIB_AMBIENT_OK +index.js:7:23 METHOD_CALL assign LIB_AMBIENT_OK +# defects: 0 diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path index 1a9ebafe0..d4462a815 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path @@ -1553,8 +1553,9 @@ def path(g, a, b, show_all=False, limit=10, every=False, max_paths=20): else ": the two are independent in this graph")) WHY = {'parameter': 'a parameter', 'loop_variable': 'a loop variable', 'local': 'a local', 'computed': 'a computed member', 'dynamic_code': 'code built at run time', 'module_variable': 'a module-level variable', - 'field': 'a function stored in a field', 'parameter_member': 'a member of a parameter', - 'expression': 'computed by an expression', + 'field': 'a function stored in a field', + 'instance_member': 'a function stored on the instance from outside the class', + 'parameter_member': 'a member of a parameter', 'expression': 'computed by an expression', 'imported_variable': "another module's exported variable, which that module reassigns"} for lx, rows in opaque: print(f" {lx} reaches {len(rows)} call(s) through a value:") diff --git a/tests/cases/javascript/instance-member-callee-is-unknown/case.json b/tests/cases/javascript/instance-member-callee-is-unknown/case.json new file mode 100644 index 000000000..9b2689955 --- /dev/null +++ b/tests/cases/javascript/instance-member-callee-is-unknown/case.json @@ -0,0 +1,105 @@ +{ + "lang": "javascript", + "src": "src", + "checks": [ + { + "why": "External.run calls this.hook(), stored on the instance by another function: path says unknown and names the site (#1709)", + "run": [ + "path", + "External.run", + "alpha" + ], + "expect_error": true, + "want": [ + "connection is UNKNOWN, not absent", + "`hook()` in External.run at src/index.js:4 — the callee is a function stored on the instance from outside the class", + "not shown to be independent" + ], + "avoid": [ + "the two are independent in this graph" + ] + }, + { + "why": "Assigned.run calls this.onReady(), stored by Object.assign(this, opts): path says unknown and names the site (#1709)", + "run": [ + "path", + "Assigned.run", + "alpha" + ], + "expect_error": true, + "want": [ + "connection is UNKNOWN, not absent", + "`onReady()` in Assigned.run at src/index.js:8 — the callee is a function stored on the instance from outside the class", + "not shown to be independent" + ], + "avoid": [ + "the two are independent in this graph" + ] + }, + { + "why": "control: this.x = x in the constructor is a field the graph sees, so it keeps its field reading (#1675)", + "run": [ + "path", + "Stored.run", + "alpha" + ], + "expect_error": true, + "want": [ + "connection is UNKNOWN, not absent", + "`x()` in Stored.run at src/index.js:12 — the callee is a function stored in a field", + "not shown to be independent" + ], + "avoid": [ + "the two are independent in this graph" + ] + }, + { + "why": "control: this.own() resolves to a declared method, so the answer stays independent", + "run": [ + "path", + "Declared.run", + "alpha" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph" + ], + "avoid": [ + "connection is UNKNOWN, not absent", + "through a value" + ] + }, + { + "why": "control: a base the graph cannot see may declare emit, so the call is left to the by-name leads", + "run": [ + "path", + "Sub.run", + "alpha" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph" + ], + "avoid": [ + "connection is UNKNOWN, not absent", + "through a value" + ] + }, + { + "why": "control: this.hasOwnProperty() is inherited from Object.prototype, not a stored value", + "run": [ + "path", + "Proto.run", + "alpha" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph" + ], + "avoid": [ + "connection is UNKNOWN, not absent", + "through a value" + ] + } + ] +} diff --git a/tests/cases/javascript/instance-member-callee-is-unknown/src/index.js b/tests/cases/javascript/instance-member-callee-is-unknown/src/index.js new file mode 100644 index 000000000..e749e0504 --- /dev/null +++ b/tests/cases/javascript/instance-member-callee-is-unknown/src/index.js @@ -0,0 +1,23 @@ +// `this.x()` where no class declares `x`: the function is stored on the instance +// from outside, so the site is a value callee (#1709). +function alpha() { return 'a'; } +class External { run() { return this.hook(); } } +function wire(e, fn) { e.hook = fn; } +class Assigned { + constructor(opts) { Object.assign(this, opts); } + run() { return this.onReady(); } +} +class Stored { + constructor(x) { this.x = x; } + run() { return this.x(); } +} +// controls: a declared method resolves; a base the graph cannot see may declare it +class Declared { + own() { return 1; } + run() { return this.own(); } +} +const Base = require('some-unstaged-base'); +class Sub extends Base { run() { return this.emit('x'); } } +// control: an Object.prototype member is the platform's, not a stored value +class Proto { run(k) { return this.hasOwnProperty(k); } } +module.exports = { alpha, External, wire, Assigned, Stored, Declared, Sub, Proto };