Skip to content

TypeScript live suite: six accessors are verified only in their refusal paths because superset-frontend has no decorators or implements edges #359

Description

@rahlk

Problem

The TypeScript live suite has only ever run against superset-frontend, which declares no decorators and no interface implementations. Two accessors were therefore verified only in their refusal and empty paths, and four more tests carry premises that a richer corpus falsifies.

Run against a codeanalyzer-typescript 1.3.0 projection of microsoft/vscode's src/ tree — 9,005 files, 2.45M nodes, 11,449 TS_DECORATED_BY, 3,374 TS_IMPLEMENTS, 7,888 TS_EXTENDS:

493 passed, 6 failed, 50 skipped (1h 46m).

The two that were passing vacuously

Both say so themselves, and both fired exactly as their authors intended:

def test_decorated_callables_are_empty_because_the_graph_has_no_decorator_edges(...):
    assert cypher("MATCH ()-[r:TS_DECORATED_BY]->() RETURN count(r) AS n")[0]["n"] == 0, \
        "this graph grew decorator edges; tighten this test"
    assert analysis.get_decorated_callables(["Component", "Injectable"]) == []

AssertionError: this graph grew decorator edges; tighten this test — assert 11449 == 0

def test_extends_comes_from_the_relationship_and_implements_names_the_absent_type(...):
    assert "TS_IMPLEMENTS" not in {r["relationshipType"] for r in cypher("CALL db.relationshipTypes()")}
    with pytest.raises(CodeanalyzerExecutionException, match="TS_IMPLEMENTS"):
        analysis.get_implemented_interfaces(row["sig"])

→ the relationship now exists, so the refusal no longer applies.

So get_decorated_callables, get_methods_with_decorators and get_implemented_interfaces have never been exercised on a graph that can answer them. That is the gap to close, and it is the substance of this issue.

The four premises the corpus falsifies

test premise vscode
test_callables_overview_covers_every_callable_with_a_verified_path owner_kind ⊆ {class, interface, None} also yields field
test_declaration_merged_nodes_are_one_facet_each_and_never_a_namespace every merged node has a declaring :TSModule 16 of 683 do not; the test dereferences an OPTIONAL MATCH result and raises AttributeError: 'NoneType' object has no attribute 'types'
test_an_anonymous_callable_is_addressed_by_its_signature_never_by_its_name an anonymous callable is named (anonymous) one is named construct (a construct signature)
test_reaches_is_true_over_a_real_edge_and_false_at_zero_useful_depth #356; the unbounded query does not terminate here

superset-frontend has no TSField-merged nodes at all, so the merged-node else branch never executed. vscode has 683, because a method written as a class property (foo = () => {}) is idiomatic there and absent from superset.

One ruling needed

Is field a correct owner_kind for a callable declared as a class property, or should it normalise to class? The value is defensible — the callable really is owned by a field — but it is undocumented, and any consumer switching on owner_kind will meet it for the first time on a corpus like this one. Decide, then either widen the test or normalise the value; do not widen the test silently.

Scope boundary

In scope: replace the two vacuous assertions with real ones measured against a decorator- and implements-bearing graph; fix the OPTIONAL MATCH dereference; settle and document the owner_kind question; correct the anonymous-callable premise.

Out of scope: #356 (unbounded reaches), which this run also surfaced and which has its own fix.

Goals

  • get_decorated_callables and get_methods_with_decorators are asserted against a graph with decorator edges, including a decorator name that matches nothing
  • get_implemented_interfaces is asserted in its answering path, against the graph's own TS_IMPLEMENTS edges, not only in its refusal path
  • The merged-node test handles a node with no declaring module instead of raising on it, and states what such a node is
  • owner_kind's full domain is documented and pinned
  • The suite is corpus-agnostic enough to run against either graph, since it already derives every expectation from the graph at run time

Caveats and known risks

  • The vscode graph is expensive: the suite takes 1h 46m and the pytest process holds a steady 5 GB. Whatever corpus is adopted for CI, it must be sized deliberately — a smaller decorator-bearing project may prove these accessors just as well.
  • microsoft/vscode cannot be analysed whole by 1.3.0 (codeanalyzer-typescript#180); src/ is the largest workable subtree.
  • The two vacuous tests must not simply be deleted. Their tripwire form is what made this discoverable; the replacement should keep an assertion that fails if the corpus loses the edges again.

Definition of done

  • The six accessors that superset could not exercise are asserted against a graph that answers them, and a reviewer can point at the assertion for each.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions