From 4fe8b5278cfa2cb9fce91f4f4b2398f21ff98871 Mon Sep 17 00:00:00 2001 From: Swapnil <78632212+swapnilpaliwal-sd@users.noreply.github.com> Date: Mon, 28 Sep 2026 01:24:02 -0700 Subject: [PATCH] impact, path: an unmodelled framework entry is "not checked", never local (#1446, #1486) When nothing in the graph calls a declaration, impact said "the change is local", tests said "0" and path said "independent", even when the declaration or its owner carries a signal that code outside the graph calls it: a decoration the rules do not model, or a library base the graph does not contain. One general rule now reads those signals (every decoration except a short inert list, and every base type no client type carries, including the unresolved ones Python and C# keep only in type_refs) and says what was not checked, names the signal and prints the grep to run. A private member is never entered through its owner's signals. --delete no longer prints the dead-code verdict for an entry point or an unmodelled entry (#1446). A field target's next: reads the field's own line, not its owner's (#1486). A section with more than 5 [by name] rows shows the 3 nearest and one count with the grep for the rest; --limit, --page all and --json still list every row. --- .../skills/axiomcode/scripts/ax_pages.py | 8 ++ .../skills/axiomcode/scripts/axiomcode-impact | 80 ++++++++++++- .../skills/axiomcode/scripts/axiomcode-path | 112 +++++++++++++++++- .../unmodelled-entry-not-local/case.json | 35 ++++++ .../src/App/App.csproj | 4 + .../src/App/Program.cs | 3 + .../src/App/Workers.cs | 21 ++++ .../java/unmodelled-entry-not-local/case.json | 49 ++++++++ .../src/com/example/Inbox.java | 19 +++ .../src/com/example/Registry.java | 14 +++ .../unmodelled-entry-not-local/case.json | 102 ++++++++++++++++ .../shop/__init__.py | 0 .../unmodelled-entry-not-local/shop/cart.py | 7 ++ .../shop/far1/__init__.py | 0 .../shop/far1/use.py | 2 + .../shop/far2/__init__.py | 0 .../shop/far2/use.py | 2 + .../shop/far3/__init__.py | 0 .../shop/far3/use.py | 2 + .../shop/far4/__init__.py | 0 .../shop/far4/use.py | 2 + .../shop/far5/__init__.py | 0 .../shop/far5/use.py | 2 + .../shop/far6/__init__.py | 0 .../shop/far6/use.py | 2 + .../shop/far7/__init__.py | 0 .../shop/far7/use.py | 2 + .../unmodelled-entry-not-local/shop/hooks.py | 31 +++++ .../shop/settings.py | 10 ++ 29 files changed, 502 insertions(+), 7 deletions(-) create mode 100644 tests/cases/csharp/unmodelled-entry-not-local/case.json create mode 100644 tests/cases/csharp/unmodelled-entry-not-local/src/App/App.csproj create mode 100644 tests/cases/csharp/unmodelled-entry-not-local/src/App/Program.cs create mode 100644 tests/cases/csharp/unmodelled-entry-not-local/src/App/Workers.cs create mode 100644 tests/cases/java/unmodelled-entry-not-local/case.json create mode 100644 tests/cases/java/unmodelled-entry-not-local/src/com/example/Inbox.java create mode 100644 tests/cases/java/unmodelled-entry-not-local/src/com/example/Registry.java create mode 100644 tests/cases/python/unmodelled-entry-not-local/case.json create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/cart.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far1/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far1/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far2/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far2/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far3/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far3/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far4/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far4/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far5/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far5/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far6/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far6/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far7/__init__.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/far7/use.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/hooks.py create mode 100644 tests/cases/python/unmodelled-entry-not-local/shop/settings.py diff --git a/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py b/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py index 7fa0f46da..2ae8e4773 100644 --- a/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py +++ b/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py @@ -132,6 +132,11 @@ def next_path(text): "the two at run time") if 'the hop above is the only connection' in text: return "next: the cross-process hop above is the only connection; `impact ` lists it as a [remote] dependent" + # an endpoint a framework enters in a way the graph does not model: the check is the grep the verdict printed + m = re.search(r'NOT CHECKED: (.+?) — a framework calls it.*?run: (grep [^\n]+)', text) + if m: + return (f"next: not shown to be independent — {m.group(1)}, and the graph does not model that framework's call; " + f"run {m.group(2)} and read the site that registers or triggers it before treating the two as unconnected") if 'independent in this graph' in text: return ("next: nothing in this graph connects them — no call, no unresolved site that could, and no library call " "that could land on the other; for a connection through data (a table, a file, a message) look at what " @@ -188,6 +193,7 @@ def next_test_impact(text): # a verb that already has a --budget of its own keeps it, and its page size is --page-budget: `context --budget N` is # how many FILES to list, and taking it here turned `--budget 5` into a 5-token page of the default 12 files OWN_BUDGET = {'context'} +PAGE = 1 def install(verb): @@ -197,6 +203,8 @@ def install(verb): page, budget = 1, PAGE_BUDGET # taken out of argv in every mode, --json included: if '--page' in argv: # the verb itself does not know these flags i = argv.index('--page'); v = argv[i + 1]; page = 'all' if v == 'all' else int(v); del argv[i:i + 2] + global PAGE + PAGE = page # a verb that shortens its default view reads this (impact collapses a [by name] flood) if flag in argv: i = argv.index(flag); budget = int(argv[i + 1]); del argv[i:i + 2] if '--json' in argv: diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact index 478c93256..467ec300e 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact @@ -1445,6 +1445,8 @@ def main(argv): # cut short and so lose every route that passes through code outside the path) to test files containing it tests_only = '--tests-only' in args; want_tests = '--tests' in args or tests_only; want_why = '--why' in args want_delete = '--delete' in args; want_warm = '--warm' in args; want_depends = '--depends' in args + # the default view collapses a flood of [by name] rows; an explicit --limit or --page all asks for every row + list_all = '--limit' in args or getattr(sys.modules.get('ax_pages'), 'PAGE', None) == 'all' args = [a for a in args if a not in ('--tests', '--tests-only', '--why', '--json', '--delete', '--warm', '--depends')] tests_in = None if '--tests-in' in args: @@ -1821,6 +1823,30 @@ def main(argv): n_ep_test = len({m for m, _ in ep_rows if _is_t(m)}) if n_ep_test: print(f"entry point: {n_ep_test} test entry point(s) — a runner invokes them, so no client call site reaches them either; they are not a surface the outside world calls. The tests section below is what names them.") + # THE DECLARATIONS ASKED ABOUT, as the question named them: a field target seeds with its OWNER (seed_of in + # dl/impact.dl, right for its dependents), so a place computed from the seeds sent the reader to the owner's line + # -- line 1 of the module for a module-level constant (#1486). Where to read is the declaration's own line. + asked = []; field_locs = [] + for k, _lab, pay in targets: + if k not in ('method', 'type', 'field'): continue + for t in (pay if isinstance(pay, list) else []): + # a field row is the symbols row itself, and a module-level constant's row may carry no id: its own line + # is still in it + i = t.get('id') if isinstance(t, dict) else t + if k == 'field' and isinstance(t, dict) and t.get('file') and t.get('line'): field_locs.append(f"{t['file']}:{t['line']}") + if isinstance(i, str) and (i in g.sym or i in getattr(g, 'all_sym', {})): asked.append(i) + asked = list(dict.fromkeys(asked)) + # AN UNMODELLED ENTRY (P.unmodelled_entry): a decoration the rules do not model, or a library base, on the + # declaration or its owner. With no caller bound to it, "local", "tests: 0" and "independent" would be claims about + # the model, not the code, and they were false on exactly these declarations: say it was not checked, and how to. + unmod = [(i, sig) for i, sig in P.unmodelled_entry(g, asked)] + unmod_name = next((((getattr(g, 'all_sym', {}).get(i) or g.sym.get(i) or {}).get('name')) for i, _ in unmod), None) + unmod_grep = P.grep_for(g, unmod_name) if unmod_name else '' + # a decoration on the declaration itself says a framework enters THIS member; one on its owner, or the owner's + # library base, says only that a framework enters the type -- the member may be a plain helper, and where a + # library base is the only signal `next:` states it as the condition it is (the `maybe` case below) + unmod_own = [x for x in unmod if not (' on its type ' in x[1] or x[1].endswith('which the graph does not contain'))] + unmod_sig = '; '.join(sig for _i, sig in unmod[:3]) + (f" (+{len(unmod) - 3} more)" if len(unmod) > 3 else '') if not D: print("directly touches it: nothing the graph can see" + (" — the declaration is used only where it is declared, or through something the parser did not record" if not contract else '')) # a throws clause is a contract: say what adding one to this method would cost, and who already handles the ones it has n_callers = len({c for c, role, why, cert, loc, n in D if cert == 'resolved' and 'calls it' in why}) @@ -1881,6 +1907,24 @@ def main(argv): by = collections.Counter(cert for _, _, _, cert, _, _ in rows) print(f"{title} ({len({c for c, *_ in rows})} callable(s): " + ', '.join(f"{n} {c}" for c, n in sorted(by.items(), key=lambda x: CERT[x[0]])) + (f"; {len({c for c, *_ in tst})} of them test" if tst else '') + "):") + # A [BY NAME] FLOOD BURIES THE ANSWER. A common method name on untyped receivers matched dozens of unrelated + # calls, and they filled the window above the rows that were edges. They are leads, not dependents (the + # verdict already treats them so), so the default view keeps the 3 nearest to the target -- its own file, + # then its directory, then the longest shared path -- and prints the rest as one count with the search that + # lists them. --limit N and --page all list every row; --json is unchanged; the header above counts them all. + collapsed = [] + bn = [r for r in body if r[3] == 'by name'] + if len(bn) > 5 and not list_all: + tf = {g.sym[t]['file'] for t in seeds if t in g.sym and g.sym[t].get('file')} + td = {os.path.dirname(f) for f in tf} + def _near(r): + f = (r[4] or '').rsplit(':', 1)[0] + return (0 if f in tf else 1 if os.path.dirname(f) in td else 2, + -max((len(os.path.commonprefix([f, x])) for x in tf), default=0), r[4] or '', g.disp(r[0])) + kept = sorted(bn, key=_near)[:3]; keep = set(kept) + collapsed = [r for r in bn if r not in keep] + at = next(n for n, r in enumerate(body) if r[3] == 'by name') # the nearest first, where the by-name rows stood + rest = [r for r in body if r[3] != 'by name']; body = rest[:at] + kept + rest[at:] # and no single file may fill the window: one test file with 21 anonymous callbacks used to hide every other file cap = max(3, limit // 5); per = collections.Counter(); shown = []; hidden = [] for r in body: @@ -1891,6 +1935,11 @@ def main(argv): if hidden: byf = collections.Counter((r[4] or '').rsplit(':', 1)[0] for r in hidden) print(f" … +{len(hidden)} more in {len(byf)} file(s): " + ', '.join(f"{f} ({n})" for f, n in byf.most_common(6)) + (' …' if len(byf) > 6 else '') + " (--limit N for all of them)") + if collapsed: + nm = sorted({g.sym[t]['name'] for t in seeds if t in g.sym and g.sym[t].get('method_id') and g.sym[t].get('name')}) + srch = (f'grep -rn "\\.{nm[0]}(" {P.grep_roots(g)}' if len(nm) == 1 else P.grep_for(g, nm[0]) if nm else '') + print(f" +{len({c for c, *_ in collapsed})} [by name] (the same method name on an untyped receiver; may be another method)" + + (f" — {srch} lists them" if srch else '') + "; --page all or --limit N prints each row") if tst and not want_tests: # the count names its files: a pointer that says only "+31" leaves the reader not knowing what was cut or # where, which is what the crowding rule exists to prevent (tests/cases/typescript/anonymous-crowding) @@ -1952,6 +2001,9 @@ def main(argv): weak_tests = [m for m in tests if m not in sure_set] sys.stdout = _stdout print(f"tests: {len(tests)} of {universe} test method(s) reach the change" + (" (their own body, or a fixture that runs before them: constructor, static initializer, @Before*/setUp)" if tests else '')) + if not tests and unmod: + print(f" NOT CHECKED: {unmod_sig} — a framework calls it and the graph does not model how, so a test that enters " + f"through that framework (a request, a started context, an event) reaches it with no route counted here: {unmod_grep}") if weak_tests: print(f" {len(weak_tests)} of them only through a dependent that is not an exact edge") if tests: byc = collections.Counter(test_cert.values()) @@ -2050,6 +2102,7 @@ def main(argv): row = g.q("SELECT kind, visibility FROM methods WHERE id = ?", s['method_id']) if g.has('methods') else [] if row and (any(k in (row[0][0] or '').upper() for k in ('STATIC', 'CONSTRUCTOR', 'INITIALIZER')) or (row[0][1] or '').upper() == 'PRIVATE'): return None + if row and not row[0][1] and re.match(r'_(?!_\w*__$)', s.get('name') or ''): return None # private by convention if g.has('overrides') and g.q("SELECT 1 FROM overrides o JOIN methods b ON b.id = o.method_id WHERE o.overriding_method_id = ? AND b.provenance <> 'client' LIMIT 1", s['method_id']): return 'yes' if g.has('decorations') and g.q("SELECT 1 FROM decorations WHERE owner_id = ? AND name = 'Override' LIMIT 1", s['method_id']): return 'yes' return 'maybe' @@ -2061,8 +2114,11 @@ def main(argv): if v: how[own] = 'yes' if 'yes' in (how.get(own), v) else v outside = []; outside_how = {} for own in sorted(how): - for r in g.q("SELECT DISTINCT x.name FROM symbols s JOIN type_ancestors a ON a.type_id = s.type_id JOIN types x ON x.id = a.ancestor_type_id WHERE s.display = ? AND s.method_id IS NULL AND x.provenance <> 'client' ORDER BY x.name", own): - if r[0] not in MARKER: outside.append((own, r[0])); outside_how[r[0]] = 'yes' if 'yes' in (outside_how.get(r[0]), how[own]) else how[own] + # a library ancestor the engine resolved, or a written base no client type carries (Python and C# leave an + # unresolvable library base out of type_ancestors, so a subclass of one read as having no outside caller) + for a in sorted({b for (sid,) in g.q("SELECT id FROM symbols WHERE display = ? AND method_id IS NULL AND type_id IS NOT NULL", own) + for b in P._library_bases(g, sid)}): + if a.split('.')[-1] not in MARKER: outside.append((own, a)); outside_how[a] = 'yes' if 'yes' in (outside_how.get(a), how[own]) else how[own] dunder = sorted({g.sym[m]['name'] for m in seeds if m in g.sym and re.fullmatch(r'__\w+__', g.sym[m]['name'] or '')}) # A NAME IN A FILE THE INDEX DOES NOT READ AS SOURCE IS TWO KINDS OF EVIDENCE (#1388). A type written in full # (a qualified class name in spring.factories, a services file, a mapper XML, a settings file) or a key a settings @@ -2151,7 +2207,13 @@ def main(argv): # a mapper XML: the rows were printed under "bound from outside the source" and then left out of the verdict, # which read "nothing" for a class Spring instantiates at startup only because such a file names it. if ext_strong: why.append(f"{len(ext_strong)} site(s) in {len({f for f, *_ in ext_strong})} file(s) outside the source {'define' if all(x[3].startswith('defines') for x in ext_strong) else 'name'} it ([text] above), so deleting it breaks them at RUN time") + # AN ENTRY POINT IS CALLED BY A FRAMEWORK, AND SO IS AN UNMODELLED ONE (#1446). The entry-point line above said + # a framework invokes it, and this verdict then printed the one for a class nothing registers, word for word. + fw_why = []; epw = sorted(ep_by_reason) + if epw: fw_why.append(f"it is {'an' if epw[0][:1] in 'aeiou' else 'a'} {'/'.join(epw[:3])} entry point (above) — a framework calls it, so no call site in the graph is expected") + if unmod: fw_why.append(f"NOT CHECKED: {unmod_sig} — a framework may call it and the graph does not model how; before deleting run: {unmod_grep}") if why: print(" NOT SAFE — " + '; '.join(why) + ". They are listed above.") + elif fw_why: print(" NOT SAFE TO ASSUME — no dependent in this graph, but " + '; '.join(fw_why) + ". Check what else the graph cannot see:") elif exported: print(" NOT SAFE TO ASSUME — no dependent in this graph, but it is declared " + '/'.join(exported) + ": exported surface. For a library, no in-repo caller is the normal state of the public API," " and a consumer outside this graph is not visible here. Check what else the graph cannot see:") @@ -2168,7 +2230,7 @@ def main(argv): if u: bits.append(f"{u} unresolved call(s) inside the impacted set") if tests: bits.append(f"{len(tests)} test(s) reach it") for b in bits: print(f" · {b}") - if not bits and not calls and not contract and not weak and not ov and not ext: print(" · nothing — no by-name match, no literal, no decoration, no unresolved call inside") + if not bits and not calls and not contract and not weak and not ov and not ext and not fw_why: print(" · nothing — no by-name match, no literal, no decoration, no unresolved call inside") if not vis and g.has('methods'): print(" · visibility is not recorded in this graph, so exported surface cannot be told from internal") # WHAT hops == 0 MEANS IS "NO ROW CLAIMS A CALL EDGE", NOT "NO ROW IS RESOLVED". The old parenthetical — # "every entry is by name, in scope, text, or one of a target set" — is a claim about the rows, and it is @@ -2246,7 +2308,10 @@ def main(argv): must = [g.loc(c) for c, _ in contract] users = sorted({loc for c, role, why, cert, loc, n in D if cert in STRONG_DIRECT and not _is_t(c) and loc}) weak_n = len({c for c, role, why, cert, loc, n in D if cert not in STRONG_DIRECT and cert != 'alongside'}) - here = sorted({g.loc(t) for t in seeds if t in g.sym})[:2] + # a field target seeds with its owner (seed_of in dl/impact.dl), so its place to read is its own line (#1486); a + # method or type target seeds with itself and what it is bound to, which is where its reading starts + here = ((sorted(set(x for x in field_locs if x))[:2] if field_locs else []) + or sorted({g.loc(t) for t in seeds if t in g.sym})[:2]) places = [x for x in dict.fromkeys(must + users) if x not in here] ti = '; `test-impact` after the edit for the tests' if tests else '' lead = f"; the {weak_n} [by name]/[text] row(s) are leads — open one only if the change is to the name or the signature" if weak_n else '' @@ -2293,11 +2358,14 @@ def main(argv): print(f"next: nothing in this graph calls it, but it is not local — it is {'an' if ep_why[0][:1] in 'aeiou' else 'a'} {'/'.join(ep_why[:3])} entry point, so its " f"callers are outside the graph (the framework, a request, a client, a runner, a scheduler): read {', '.join(here) or 'the declaration'}; a body change " f"is safe there, a change to its name, route, parameters or result breaks those callers{tpart}{lead}") + elif not places and unmod and (unmod_own or not maybe): + print(f"next: {nobody}, but NOT CHECKED: {unmod_sig} — a framework calls it and the graph does not model how; " + f"before treating the change as local run: {unmod_grep}{' — then read ' + ', '.join(here) if here else ''}{tpart}{lead}") elif not places and maybe: - print(f"next: {nobody} — read {', '.join(here) or 'the declaration'}; a body change stays there, but " + print(f"next: {nobody} — read {', '.join(here) or 'the declaration'}; a body change stays there, but NOT CHECKED: " f"it is declared on a subtype of {', '.join(maybe[:3])}, which the graph does not contain: if it overrides one of " f"its methods (for a type: wherever one of its methods does), that library calls it with no call site here, so a " - f"change to that name or signature is not local{lead}") + f"change to that name or signature is not local{'; to check: ' + unmod_grep if unmod_grep else ''}{lead}") elif not places and weak_n: print(f"next: no resolved caller, but {weak_n} caller(s) above reach it by name or text, which the graph could not confirm — " f"check {', '.join(weak_locs[:4]) or 'them'}{' …' if len(weak_locs) > 4 else ''} before treating the change as contained, " diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path index d6c1d0155..fa67aad7a 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path @@ -1086,7 +1086,101 @@ def entered_in_process(g, ids): order = {i: n for n, i in enumerate(ids)} return sorted(out.items(), key=lambda x: order.get(x[0], 0)) -# ── the query ──────────────────────────────────────────────────────────────────────────────────────────────────── +# AN UNMODELLED ENTRY: A SIGNAL THAT SOMETHING OUTSIDE THE CODE CALLS A DECLARATION, WHICH THE ENGINE DID NOT TURN INTO AN +# EDGE OR AN ENTRY POINT. A decoration the rules do not know (a listener, a hook, a validator, a registration), or a +# base type the graph does not contain (a library class the declaration's owner extends, which the parser could not +# resolve). "No caller" is then a gap in the model, not a fact about the code, and every verdict that turns "no caller" +# into "local", "tests: 0" or "independent" was false on exactly these declarations. The rule is one rule, with no +# framework's names in it: every decoration counts except the short list below, which change nothing about who calls +# the declaration (a compiler check, a nullness mark, a generated accessor, a wrapper around the same call). +INERT_DECORATIONS = { + # compiler checks and documentation + 'Override', 'SuppressWarnings', 'Deprecated', 'Serial', 'FunctionalInterface', 'SafeVarargs', 'Documented', + 'Retention', 'Target', 'Inherited', 'Obsolete', 'Serializable', 'Flags', 'Pure', 'MethodImpl', 'AttributeUsage', + 'Generated', 'GeneratedCode', 'CompilerGenerated', 'ExcludeFromCodeCoverage', 'DebuggerStepThrough', + 'DebuggerNonUserCode', 'DebuggerDisplay', 'DebuggerHidden', 'CallerMemberName', 'CallerFilePath', 'CallerLineNumber', + 'override', 'overload', 'final', 'abstractmethod', 'deprecated', + # nullness + 'Nullable', 'NonNull', 'Nonnull', 'NotNull', 'CheckForNull', 'NotNullWhen', 'MaybeNullWhen', 'AllowNull', 'DisallowNull', + # members the compiler writes, and wrappers the caller still calls by name + 'staticmethod', 'classmethod', 'property', 'cached_property', 'hybrid_property', 'wraps', 'lru_cache', 'cache', + 'cached', 'memoize', 'dataclass', + 'total_ordering', 'contextmanager', 'asynccontextmanager', + 'Getter', 'Setter', 'Data', 'Builder', 'ToString', 'EqualsAndHashCode', 'AllArgsConstructor', 'NoArgsConstructor', + 'RequiredArgsConstructor', 'Slf4j', 'Log4j2', +} +BASE_CONTEXTS = ('BASE_LIST', 'BASE_CLASS', 'SUPER_TYPE', 'IMPLEMENTS_INTERFACE') +_NOT_A_BASE = {'object', 'Object', 'string', 'String', 'int', 'bool', 'long', 'double', 'float', 'decimal', 'byte', 'char', 'short'} + +def _dec_name(n): + n = (n or '').lstrip('@').split('(')[0].split('.')[-1] + return n[:-len('Attribute')] if n.endswith('Attribute') and len(n) > len('Attribute') else n + +def _library_bases(g, tid): + """the base types written on type symbol `tid` that are not declared in the graph: a library ancestor the engine + resolved (types.provenance <> 'client'), or a written base name no client type carries (the parser could not + resolve it). Only the type's OWN header: a base written on a nested type belongs to the nested type.""" + s = g.all_sym.get(tid) if hasattr(g, 'all_sym') else g.sym.get(tid) + if not s: return [] + out = [] + if s.get('type_id') and g.has('type_ancestors') and g.has('types'): + out += [r[0] for r in g.q("""SELECT DISTINCT x.name FROM type_ancestors a JOIN types x ON x.id = a.ancestor_type_id + WHERE a.type_id = ? AND x.provenance <> 'client' ORDER BY x.name""", s['type_id'])] + if s.get('file') and s.get('line') and g.has('type_refs'): + end = s.get('end_line') or s['line'] + nested = [(r[0], r[1] or r[0]) for r in g.q("""SELECT line, end_line FROM symbols WHERE file = ? AND method_id IS NULL + AND type_id IS NOT NULL AND line > ? AND line <= ?""", s['file'], s['line'], end)] + ph = ','.join('?' * len(BASE_CONTEXTS)) + for name, line in g.q(f"SELECT name, line FROM type_refs WHERE file = ? AND line BETWEEN ? AND ? AND context IN ({ph})", + s['file'], s['line'], end, *BASE_CONTEXTS): + if any(a <= line <= b for a, b in nested): continue + simple = re.split(r'[<(\[]', name or '')[0].split('.')[-1] + if not simple or simple in _NOT_A_BASE: continue + if g.has('types') and g.q("SELECT 1 FROM types WHERE name = ? AND provenance = 'client' LIMIT 1", simple): continue + out.append(name) + return list(dict.fromkeys(out)) + +def unmodelled_entry(g, ids): + """[(symbol id, signal)]: for each declaration in `ids` (a method, a field or a type) that is not a test, what says + something outside the code may call it with no edge in this graph -- a decoration outside INERT_DECORATIONS on it + or on its owner type, or a base type of its owner (of itself, for a type) the graph does not contain. A private or + static member is never entered through its owner's decoration or base, so only its own decorations count.""" + allsym = g.all_sym if hasattr(g, 'all_sym') else g.sym + decs = lambda i: [n for n in dict.fromkeys(_dec_name(r[0]) for r in g.q("SELECT name FROM decorations WHERE owner_id = ?", i)) + if n and n not in INERT_DECORATIONS] if g.has('decorations') else [] + out = [] + for i in ids: + s = allsym.get(i) + if not s or s.get('is_test'): continue + for n in decs(i): out.append((i, f"@{n} on {s['display']}")) + is_type = s.get('type_id') and not s.get('method_id') + if is_type: + for b in _library_bases(g, i): out.append((i, f"{s['display']} extends {b}, which the graph does not contain")) + continue + if s.get('method_id') and g.has('methods'): + row = g.q("SELECT kind, visibility FROM methods WHERE id = ?", s['method_id']) + if row and (any(k in (row[0][0] or '').upper() for k in ('STATIC', 'INITIALIZER')) or (row[0][1] or '').upper() == 'PRIVATE'): continue + # where the language records no visibility, a leading underscore is how private is written + if row and not row[0][1] and re.match(r'_(?!_\w*__$)', s.get('name') or ''): continue + own = s.get('owner') + if not own: continue + for r in g.q("SELECT id FROM symbols WHERE display = ? AND method_id IS NULL AND type_id IS NOT NULL", own): + for n in decs(r[0]): out.append((i, f"@{n} on its type {own}")) + if s.get('method_id') and s.get('kind') != 'constructor': + for b in _library_bases(g, r[0]): out.append((i, f"{own} extends {b}, which the graph does not contain")) + return list(dict.fromkeys(out)) + +def grep_roots(g): + """the directories a by-hand check should search: the top-level directories holding the graph's declarations, + or '.' when there are more than three of them or a declaration sits at the root""" + tops = {(f.split('/', 1)[0] if '/' in f else '.') for (f,) in g.q("SELECT DISTINCT file FROM symbols WHERE file IS NOT NULL AND file <> ''")} + return '.' if not tops or '.' in tops or len(tops) > 3 else ' '.join(sorted(tops)) + +def grep_for(g, name): + """the exact search that finds what the graph could not: the name as a whole word in the source roots""" + return f'grep -rnw "{name}" {grep_roots(g)}' + +# ── the query──────────────────────────────────────────────────────────────────────────────────────────────────── def path(g, a, b, show_all=False, limit=10, every=False, max_paths=20): g.export() la, A = g.resolve(a); lb, B = g.resolve(b) @@ -1216,6 +1310,19 @@ def path(g, a, b, show_all=False, limit=10, every=False, max_paths=20): # Computed BEFORE the verdict line, which used to say "independent" one line above the note that joined them. fw = framework_note(g, B_, set(A_)) + framework_note(g, A_, set(B_)) keyed = any('which is the key it is registered under' in l for l in fw) + # AN ENDPOINT NOTHING IN THE GRAPH CALLS, CARRYING A SIGNAL THAT SOMETHING OUTSIDE THE CODE DOES (a decoration the + # rules do not model, a library base), is entered by a mechanism the graph does not contain, so "independent" is + # a claim about the model and not the code. An endpoint the engine already made an entry point is modelled, and + # the library-call rule above decides it. + unmod = [] + if not (remote_found or keyed or lib_side): + called = {y for ys in adj.values() for y in ys} + eps = set() + ends = [i for i in list(B_) + list(A_) if i in g.sym and g.sym[i]['kind'] not in ('library', 'written') and i not in called] + mids = [g.sym[i]['method_id'] for i in ends if g.sym[i].get('method_id')] + if mids and g.has('entry_points'): + eps = {r[0] for r in g.q("SELECT method_id FROM entry_points WHERE method_id IN (%s)" % ','.join('?' * len(mids)), *mids)} + unmod = unmodelled_entry(g, [i for i in ends if g.sym[i].get('method_id') not in eps]) # "independent" would contradict the cross-process hop printed above, so say the narrower true thing print(" and no unresolved site would connect them by name either" + (": no CALL connects them — the hop above is the only connection" if remote_found @@ -1223,6 +1330,9 @@ def path(g, a, b, show_all=False, limit=10, every=False, max_paths=20): " does: NOT independent" if keyed else " — but a library called above can reach the other at run time (an event, a callback, a hook) with no" " call edge to record it: NOT shown to be independent" if lib_side + else f" — but NOT CHECKED: {'; '.join(s_ for _i, s_ in unmod[:3])} — a framework calls it and the graph does not" + f" model how, so the other may reach it through that framework. Before treating them as independent run: " + f"{grep_for(g, g.sym[unmod[0][0]]['name'])}" if unmod else ": the two are independent in this graph")) for line in fw: print(line) return 1 diff --git a/tests/cases/csharp/unmodelled-entry-not-local/case.json b/tests/cases/csharp/unmodelled-entry-not-local/case.json new file mode 100644 index 000000000..20324c61d --- /dev/null +++ b/tests/cases/csharp/unmodelled-entry-not-local/case.json @@ -0,0 +1,35 @@ +{ + "lang": "csharp", + "checks": [ + { + "why": "--delete on a hosted service says a framework calls it, not the dead-code verdict (#1446)", + "run": ["impact", "SweepWorker", "--kind", "type", "--delete"], + "want": ["NOT SAFE TO ASSUME", "lifecycle entry point (above) — a framework calls it"], + "avoid": ["· nothing", "no dependent at any certainty in this graph. Before deleting"] + }, + { + "why": "--delete on the hosted service's method says the same (#1446)", + "run": ["impact", "SweepWorker.ExecuteAsync", "--delete"], + "want": ["NOT SAFE TO ASSUME", "a framework calls it"], + "avoid": ["· nothing"] + }, + { + "why": "control: a type nothing registers, with no library base, keeps the dead-code verdict", + "run": ["impact", "Unused", "--kind", "type", "--delete"], + "want": ["no dependent at any certainty in this graph. Before deleting"], + "avoid": ["NOT SAFE TO ASSUME", "NOT CHECKED"] + }, + { + "why": "a method carrying an attribute the rules do not model is not called local", + "run": ["impact", "Handlers.OnOrder"], + "want": ["NOT CHECKED: @Subscribe on Handlers.OnOrder", "grep -rnw \"OnOrder\""], + "avoid": ["the change is local"] + }, + { + "why": "near-miss control: [Obsolete] is on the inert list, so the method stays local", + "run": ["impact", "Handlers.Legacy"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + } + ] +} diff --git a/tests/cases/csharp/unmodelled-entry-not-local/src/App/App.csproj b/tests/cases/csharp/unmodelled-entry-not-local/src/App/App.csproj new file mode 100644 index 000000000..68aee9d2b --- /dev/null +++ b/tests/cases/csharp/unmodelled-entry-not-local/src/App/App.csproj @@ -0,0 +1,4 @@ + + net8.0enable + + diff --git a/tests/cases/csharp/unmodelled-entry-not-local/src/App/Program.cs b/tests/cases/csharp/unmodelled-entry-not-local/src/App/Program.cs new file mode 100644 index 000000000..15a1e3e4a --- /dev/null +++ b/tests/cases/csharp/unmodelled-entry-not-local/src/App/Program.cs @@ -0,0 +1,3 @@ +var builder = Host.CreateApplicationBuilder(args); +builder.Services.AddHostedService(); +builder.Build().Run(); diff --git a/tests/cases/csharp/unmodelled-entry-not-local/src/App/Workers.cs b/tests/cases/csharp/unmodelled-entry-not-local/src/App/Workers.cs new file mode 100644 index 000000000..ef53495ea --- /dev/null +++ b/tests/cases/csharp/unmodelled-entry-not-local/src/App/Workers.cs @@ -0,0 +1,21 @@ +namespace App; + +public sealed class SweepWorker : BackgroundService +{ + protected override Task ExecuteAsync(CancellationToken t) => Task.CompletedTask; +} + +// control: not registered, not a BackgroundService +public sealed class Unused +{ + public Task ExecuteAsync(CancellationToken t) => Task.CompletedTask; +} + +public sealed class Handlers +{ + [Subscribe("orders")] + public void OnOrder(string body) { } + + [Obsolete] + public void Legacy() { } +} diff --git a/tests/cases/java/unmodelled-entry-not-local/case.json b/tests/cases/java/unmodelled-entry-not-local/case.json new file mode 100644 index 000000000..637a47498 --- /dev/null +++ b/tests/cases/java/unmodelled-entry-not-local/case.json @@ -0,0 +1,49 @@ +{ + "lang": "java", + "src": "src", + "checks": [ + { + "why": "a method carrying an annotation the rules do not model is not called local: next: names the annotation and prints the grep", + "run": ["impact", "Inbox.receive"], + "want": ["NOT CHECKED: @OnMessage on Inbox.receive", "before treating the change as local run: grep -rnw \"receive\" src"], + "avoid": ["the change is local"] + }, + { + "why": "near-miss control: @Deprecated is on the inert list, so a method nothing calls stays local", + "run": ["impact", "Inbox.legacy"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "control: an unannotated method in the same class stays local", + "run": ["impact", "Inbox.plain"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "an annotation on the owner type counts for its public methods", + "run": ["impact", "Registry.refresh"], + "want": ["NOT CHECKED: @Registered on its type Registry"], + "avoid": ["the change is local"] + }, + { + "why": "near-miss control: a private method is never entered through its type's annotation", + "run": ["impact", "Registry.tidy"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "path does not call a plain method independent of an annotated one nothing in the graph calls", + "run": ["path", "Inbox.plain", "Inbox.receive"], + "expect_error": true, + "want": ["NOT CHECKED: @OnMessage on Inbox.receive"], + "avoid": ["the two are independent in this graph"] + }, + { + "why": "control: two plain methods stay independent", + "run": ["path", "Inbox.plain", "Registry.tidy"], + "expect_error": true, + "want": ["the two are independent in this graph"] + } + ] +} diff --git a/tests/cases/java/unmodelled-entry-not-local/src/com/example/Inbox.java b/tests/cases/java/unmodelled-entry-not-local/src/com/example/Inbox.java new file mode 100644 index 000000000..1df9dd0c8 --- /dev/null +++ b/tests/cases/java/unmodelled-entry-not-local/src/com/example/Inbox.java @@ -0,0 +1,19 @@ +package com.example; + +import com.acme.bus.OnMessage; + +public class Inbox { + @OnMessage("orders") + public void receive(String body) { + System.out.println(body); + } + + @Deprecated + public void legacy() { + System.out.println("old"); + } + + public void plain() { + System.out.println("plain"); + } +} diff --git a/tests/cases/java/unmodelled-entry-not-local/src/com/example/Registry.java b/tests/cases/java/unmodelled-entry-not-local/src/com/example/Registry.java new file mode 100644 index 000000000..e5b494673 --- /dev/null +++ b/tests/cases/java/unmodelled-entry-not-local/src/com/example/Registry.java @@ -0,0 +1,14 @@ +package com.example; + +import com.acme.bus.Registered; + +@Registered +public class Registry { + public void refresh() { + System.out.println("refresh"); + } + + private void tidy() { + System.out.println("tidy"); + } +} diff --git a/tests/cases/python/unmodelled-entry-not-local/case.json b/tests/cases/python/unmodelled-entry-not-local/case.json new file mode 100644 index 000000000..fe297fca2 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/case.json @@ -0,0 +1,102 @@ +{ + "lang": "python", + "src": ".", + "checks": [ + { + "why": "a function registered by a decorator the rules do not model is not called local: next: says what was not checked and prints the grep", + "run": ["impact", "warm_caches"], + "want": ["NOT CHECKED: @on_boot on warm_caches", "before treating the change as local run: grep -rnw \"warm_caches\" shop"], + "avoid": ["the change is local"] + }, + { + "why": "the tests line does not let tests: 0 stand alone for a framework-entered function", + "run": ["impact", "warm_caches"], + "want": ["tests: 0 of", " NOT CHECKED: @on_boot on warm_caches — a framework calls it"] + }, + { + "why": "near-miss control: a wrapper decorator on the inert list changes nothing about who calls it, so the change stays local", + "run": ["impact", "cached_helper"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "control: an undecorated function nothing calls stays local", + "run": ["impact", "plain_helper"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "a method of a class whose base the graph does not contain is not local", + "run": ["impact", "Nightly.run"], + "want": ["NOT CHECKED: it is declared on a subtype of Worker", "grep -rnw \"run\" shop", " NOT CHECKED: Nightly extends Worker, which the graph does not contain"], + "avoid": ["the change is local"] + }, + { + "why": "near-miss control: a private method of the same class is never entered through the base, so it stays local", + "run": ["impact", "Nightly._private_step"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "control: a method of a class with no outside base stays local", + "run": ["impact", "Ledger.total"], + "want": ["the change is local"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "--delete on a decorated function says a framework may call it, not 'nothing'", + "run": ["impact", "warm_caches", "--delete"], + "want": ["NOT SAFE TO ASSUME", "NOT CHECKED: @on_boot on warm_caches"], + "avoid": ["· nothing", "no dependent at any certainty in this graph. Before deleting"] + }, + { + "why": "path does not call two functions independent when one is entered by an unmodelled framework", + "run": ["path", "plain_helper", "warm_caches"], + "expect_error": true, + "want": ["NOT CHECKED: @on_boot on warm_caches", "grep -rnw \"warm_caches\" shop", "next: not shown to be independent"], + "avoid": ["the two are independent in this graph"] + }, + { + "why": "control: two plain functions stay independent", + "run": ["path", "plain_helper", "Ledger.total"], + "expect_error": true, + "want": ["the two are independent in this graph"], + "avoid": ["NOT CHECKED"] + }, + { + "why": "a module-level constant's next: reads the assignment's own line, not line 1 of the module (#1486)", + "run": ["impact", "ORDER_LIMIT"], + "want": ["read shop/settings.py:6"], + "avoid": ["shop/settings.py:1,", "shop/settings.py:1 "] + }, + { + "why": "control: a function in the same file is still read at its own line", + "run": ["impact", "order_limit"], + "want": ["read shop/settings.py:9"] + }, + { + "why": "a [by name] flood shows the 3 nearest rows (same file first) and one count with the grep for the rest", + "run": ["impact", "Cart.checkout"], + "want": ["[by name] near shop/cart.py:7", "+5 [by name] (the same method name on an untyped receiver", "grep -rn \"\\.checkout(\" shop", "8 by name"], + "avoid": ["[by name] go3", "[by name] go7"] + }, + { + "why": "--page all still lists every [by name] row", + "run": ["impact", "Cart.checkout", "--page", "all"], + "want": ["[by name] go3", "[by name] go7", "[by name] near"], + "avoid": ["+5 [by name]"] + }, + { + "why": "--limit N still lists every [by name] row", + "run": ["impact", "Cart.checkout", "--limit", "20"], + "want": ["[by name] go3", "[by name] go7"], + "avoid": ["+5 [by name]"] + }, + { + "why": "--json is unchanged: every by-name caller is in the document", + "run": ["impact", "Cart.checkout", "--json"], + "stdout_json": true, + "want": ["go3", "go7"] + } + ] +} diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/cart.py b/tests/cases/python/unmodelled-entry-not-local/shop/cart.py new file mode 100644 index 000000000..309169aee --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/cart.py @@ -0,0 +1,7 @@ +class Cart: + def checkout(self): + return 1 + + +def near(c): + return c.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far1/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far1/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far1/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far1/use.py new file mode 100644 index 000000000..daa4be9a3 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far1/use.py @@ -0,0 +1,2 @@ +def go1(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far2/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far2/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far2/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far2/use.py new file mode 100644 index 000000000..2172f149c --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far2/use.py @@ -0,0 +1,2 @@ +def go2(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far3/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far3/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far3/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far3/use.py new file mode 100644 index 000000000..f8c242b64 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far3/use.py @@ -0,0 +1,2 @@ +def go3(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far4/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far4/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far4/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far4/use.py new file mode 100644 index 000000000..df376fcf7 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far4/use.py @@ -0,0 +1,2 @@ +def go4(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far5/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far5/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far5/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far5/use.py new file mode 100644 index 000000000..92e408d05 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far5/use.py @@ -0,0 +1,2 @@ +def go5(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far6/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far6/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far6/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far6/use.py new file mode 100644 index 000000000..1ff0aee58 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far6/use.py @@ -0,0 +1,2 @@ +def go6(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far7/__init__.py b/tests/cases/python/unmodelled-entry-not-local/shop/far7/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/far7/use.py b/tests/cases/python/unmodelled-entry-not-local/shop/far7/use.py new file mode 100644 index 000000000..828629603 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/far7/use.py @@ -0,0 +1,2 @@ +def go7(x): + return x.checkout() diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/hooks.py b/tests/cases/python/unmodelled-entry-not-local/shop/hooks.py new file mode 100644 index 000000000..dba8bffdd --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/hooks.py @@ -0,0 +1,31 @@ +import functools + +from plugin_host import hooks +from plugin_host.workers import Worker + + +@hooks.on_boot +def warm_caches(app): + return app + + +@functools.lru_cache(maxsize=None) +def cached_helper(): + return 2 + + +def plain_helper(): + return 1 + + +class Nightly(Worker): + def run(self): + return 3 + + def _private_step(self): + return 4 + + +class Ledger: + def total(self): + return 5 diff --git a/tests/cases/python/unmodelled-entry-not-local/shop/settings.py b/tests/cases/python/unmodelled-entry-not-local/shop/settings.py new file mode 100644 index 000000000..7979188d3 --- /dev/null +++ b/tests/cases/python/unmodelled-entry-not-local/shop/settings.py @@ -0,0 +1,10 @@ +INSTALLED_WIDGETS = [ + "widgets", +] + + +ORDER_LIMIT = ["orders"] + + +def order_limit(): + return ORDER_LIMIT