diff --git a/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py b/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py index 4fd3c498a..d71327a0d 100644 --- a/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py +++ b/plugins/axiomcode/skills/axiomcode/scripts/ax_pages.py @@ -120,6 +120,22 @@ def next_path(text): a, la, b, lb = fw[0] return (f"next: no call connects them, the framework does: read {a} at {la}, where it hands over, and {b} at {lb}, " f"which the framework runs; `impact {b}` for everything else that depends on it") + # WHAT FOLLOWS "no chain" DEPENDS ON WHY THERE IS NONE (#1385). The one sentence below used to close every + # no-chain answer, and it named "the unresolved sites above" under answers that had printed none: under an + # independent pair, and under one joined only by a library call. + if 'which is the key it is registered under' in text: + return ("next: the start writes the key the other is registered under (named above), so a framework connects " + "them and no call does; the `impact … --tests` command printed there follows that hop") + if 'NOT shown to be independent' in text: + return ("next: no chain of calls; the library calls named above are where one could continue: read the body " + "that makes them — one that publishes, schedules or registers what the entered method handles connects " + "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" + 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 " + "each writes and reads") if re.search(r'no (chain|route|path)', text, re.I): return ("next: no resolved chain — the graph loses the call at one of the unresolved sites named above, so the " "code may still connect them; read the start's body from those sites on") diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact index b516ac191..123fc43ed 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact @@ -2031,6 +2031,62 @@ def main(argv): if libs: print(" into libraries (not traversed): " + ', '.join(str(x['lab']).split('(')[0] for x in libs[:6]) + (f" … +{len(libs) - 6}" if len(libs) > 6 else '')) if unres and seen_d: print(f" bound: {unres} unresolved call(s) in its body — what those reach is not here") + # an override of a type the graph does not contain is called BY that library / framework: "nothing reaches it" would be a lie. + # Computed ahead of the delete verdict and `next:`, which both have to read it (#1388): each used to print its own + # "nothing" / "the change is local" one line under the `outside the graph:` note that said otherwise. + # A MARKER INTERFACE DECLARES NO METHOD, so no library calls a method THROUGH it, which is what the note says: an + # accessor on an entity that is Serializable was told a library calls it, and with `next:` now reading this list + # that became "not local" for every accessor of every entity. Sorted, because a set's order made the note differ + # between two runs of the same query. + MARKER = ('Serializable', 'Cloneable', 'RandomAccess', 'Remote', 'EventListener') + # WHICH SEEDS A LIBRARY SUPERTYPE CAN CALL AT ALL. A private, static or constructor body is never entered through + # the supertype, so a private helper in a servlet is as local as one in any class; a method that overrides a + # library method (an override row, or @Override) is certainly entered by it; any other instance method is entered + # only if it happens to override one, which the graph cannot tell while the supertype's members are not in it. + # 'yes' is said as a fact in `next:`, 'maybe' as the condition it is. + def via_super(m): + s = g.sym[m] + if not s.get('method_id'): return 'maybe' if s.get('type_id') else None + 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 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' + how = {} + for m in seeds: + if m not in g.sym: continue + own = g.sym[m].get('owner') or (g.sym[m]['display'] if g.sym[m].get('type_id') and not g.sym[m].get('method_id') else None) + v = via_super(m) if own else None + 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] + 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 + # file defines is a binding: something loads the declaration by that name, so it counts against a delete and + # `next:` names it. Anything else is a lead, as a [by name] row is: a bare name ("names the type": the simple name + # as a whole token in any text file) is as often a JSON fixture, a SQL column or a template spelling the same word, + # and a method "in full" is only `Owner.method`, which a document or a test resource writes as readily as a + # binding does. On the test cases themselves both matched each case's own case.json. A key is defined only in a + # settings file; the same key spelled in any other text file is a mention, which the row's wording does not tell apart. + ext_strong = [x for x in ext if x[3] == 'names the type in full' + or (x[3] == 'defines or overrides the key' and os.path.splitext(x[0])[1].lower() in Impact.KEYFILE_EXT)] + ext_weak = [x for x in ext if x not in ext_strong] + # WHAT "NOTHING HERE CALLS IT" IS MEASURED AGAINST: production code bound to it by an exact edge -- a strong direct + # user, or a method the closure reaches through exact edges. It used to be `not reached`, and a test or a [by name] + # row in the reach switched the library-supertype note off: a Validator's validate(), reached only by its test, + # was then called local with no note at all. And a method reached through the base it overrides (an abstract + # template method, whose caller is the base class's own body) printed that caller in the reach and was still + # called local, because `next:` read only the direct rows. + # `remote` (#1468): the client that calls it across a process is a dependent the engine joined at both ends, so it + # is a place to read, not a by-name lead, and it makes the declaration as non-local as a resolved caller does + STRONG_DIRECT = ('resolved', 'one of a set', 'registered', 'capped set', 'remote', 'framework') + prod_users = {c for c, role, why, cert, loc, n in D if cert in STRONG_DIRECT and not _is_t(c)} + prod_reach = sorted((m for m in reached if m in sure_set and not _is_t(m)), key=lambda m: (reached[m], g.loc(m))) + lone = not prod_users and not prod_reach # a green line over an empty set would be printed exactly where the answer is least trustworthy: say what was checked if want_delete: # is it safe to delete? every reason it is not, and — when there is none — what the graph cannot vouch for @@ -2091,6 +2147,10 @@ def main(argv): why.append("it implements the abstract " + ', '.join(sorted(set(abstract_over))[:3]) + ", so deleting it leaves the type without an implementation and the build fails") elif ov: why.append("it overrides " + ', '.join(sorted(set(ov))[:3]) + ", so the base's callers reach this body with no call site of its own") + # A FILE THE INDEX DOES NOT READ AS SOURCE CAN STILL LOAD IT (#1388). A spring.factories entry, a services file, + # 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") if why: print(" NOT SAFE — " + '; '.join(why) + ". They are listed above.") 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," @@ -2101,11 +2161,14 @@ def main(argv): " — deleting it COMPILES, and the inherited body runs instead: a behaviour change, not a build break") if exported and why: bits.append("it is declared " + '/'.join(exported) + " — exported surface, so consumers outside this graph may call it too") if lits: bits.append(f"{len(lits)} string literal(s) equal to the name (reflection? a bean name? a config key?): " + ', '.join(f"{f}:{l}" for f, l in lits[:4]) + (' …' if len(lits) > 4 else '')) + if outside and lone: bits.append("it extends / implements " + ', '.join(sorted({a for _o, a in outside})[:3]) + ", which the graph does not contain — that library or framework can instantiate or call it with no call site here") + if ext_weak: bits.append(f"{len(ext_weak)} mention(s) of the name in {len({f for f, *_ in ext_weak})} file(s) outside the source ([text] above): " + + ', '.join(f"{f}:{l}" for f, l, *_ in ext_weak[:4]) + (' …' if len(ext_weak) > 4 else '') + " — each may be the same word meaning something else") if decs: bits.append("decorations on it: @" + ', @'.join(decs[:5]) + " — a framework may call it with no call site, and a decorator can hand the wrapper to the caller instead of this declaration") 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: 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: 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 @@ -2150,13 +2213,9 @@ def main(argv): print("bound: the closure could not start — no dependent above is an edge it can walk back from, so what " "reaches THEM is not in this answer") if u: print(f"bound: {u} unresolved call(s) inside the impacted set — what those reach is unknown, so the set is a lower bound") - # an override of a type the graph does not contain is called BY that library / framework: "nothing reaches it" would be a lie - outside = [] - for own in {g.sym[m]['owner'] for m in seeds if m in g.sym and g.sym[m].get('owner')}: - 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'", own): outside.append((own, r[0])) - 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 '')}) if dunder and not reached: print(f"outside the graph: {', '.join(dunder)} is a protocol method — the language runtime calls it (an operator, a conversion, a lifecycle hook), which is no call site, so nothing reaching it here does NOT mean nothing does") - if outside and not reached: print("outside the graph: " + ', '.join(f"{o} extends / implements {a}, which the graph does not contain" for o, a in outside[:3]) + " — a library or framework can call this method, so nothing reaching it here does NOT mean nothing does") + if outside and lone: print("outside the graph: " + ', '.join(f"{o} extends / implements {a}, which the graph does not contain" for o, a in outside[:3]) + " — a library or framework can call this method, so " + + ("what reaches it above is not all that does" if reached else "nothing reaching it here does NOT mean nothing does")) if byname_seeds: print(f"through a call the engine could not resolve: {len(byname_seeds)} caller(s) write this method's name on an untyped receiver — counted in the reach above, so that part of it MAY not be real: " + ', '.join(f"{g.disp(c)} {g.loc(c)}" for c in sorted(byname_seeds)[:4])) byname = [x for x in D if x[3] in ('one of a set', 'by name', 'text')] # a label the answer USED must be explained here; a label it did not use must not be, or the note @@ -2184,7 +2243,6 @@ def main(argv): # to do with them, and what NOT to spend reads on. # `framework` (#1509): the producer or publisher the engine joined to this through the framework's own # registration is a place bound to it, not a same-named lead, so it is named here rather than counted as one - STRONG_DIRECT = ('resolved', 'one of a set', 'registered', 'capped set', 'framework') 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'}) @@ -2192,7 +2250,62 @@ def main(argv): 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 '' - if not places: + # "LOCAL" IS A CLAIM ABOUT EVERYTHING ABOVE IT, NOT ABOUT `D` (#1388). Two things this answer prints are not rows of + # D and were never read here: a name in a file the index does not read as source ([text] under "bound from outside + # the source"), and a supertype the graph does not contain (the `outside the graph:` note). Each means something + # calls or loads the declaration with no call site in the graph, so neither answer is local: the [text] sites are + # named as places to read, and the library supertype as the caller whose signature the declaration must keep. + ext_locs = list(dict.fromkeys(f"{f}:{l}" for f, l, *_ in ext_strong)) + weak_ext = len({(f, l) for f, l, *_ in ext_weak}) + other = ' other' if ext_strong else '' + if weak_ext: lead += (f"; the {weak_ext}{other} [text] mention(s) outside the source are leads too" if weak_n else + f"; the {weak_ext}{other} [text] mention(s) outside the source are leads — open one only if the change is to the name") + # the callers the closure reached through the base this overrides, when no direct row names one: the nearest + # production ones are the places bound to it + if not places and prod_reach: + near = min(reached[m] for m in prod_reach) + places = [x for x in dict.fromkeys(g.loc(m) for m in prod_reach if reached[m] == near) if x not in here] + foreign = sorted({a for a, h in outside_how.items() if h == 'yes'}) if outside and lone else [] + maybe = sorted({a for a, h in outside_how.items() if h == 'maybe'}) if outside and lone and not foreign else [] + if dunder and not reached: foreign.append('the language runtime') + keep = (f"; {', '.join(foreign[:3])} calls it from outside the graph, so keep the signature it expects" if foreign else + f"; if it overrides a method of {', '.join(maybe[:3])} (not in the graph, so this cannot tell), that library calls it: keep that signature" if maybe else '') + nobody = 'nothing in this graph calls it' if not reached else 'nothing but tests and leads reaches it in this graph' + # THREE MORE THINGS THE ANSWER PRINTS THAT ARE NOT A RESOLVED PRODUCTION CALLER, AND EACH WAS CALLED "LOCAL" + # (#1467, #1573, #1552): an entry point the answer itself said the outside world calls; [by name] / [in scope] / + # [text] rows, whose callers may well be real (an untyped receiver is not an absent one); and tests the answer + # counted as reaching the change. "Local" is left for the answer that holds none of them. + ep_why = sorted(ep_by_reason) + weak_locs = list(dict.fromkeys(loc for c, role, why, cert, loc, n in sorted(D, key=lambda x: (CERT.get(x[3], 99), x[4] or '')) + if cert not in STRONG_DIRECT and cert != 'alongside' and loc and loc not in here)) + nt = len(tests) + tpart = (f"; {nt} test(s) reach it — run them after the edit (`test-impact`)" if nt else '') + if places: places = list(dict.fromkeys(places + [x for x in ext_locs if x not in here])) + if not places and ext_locs: + print(f"next: nothing resolved in the source depends on it, but {len(ext_locs)} site(s) outside the source {'define' if all(x[3].startswith('defines') for x in ext_strong) else 'name'} it and " + f"break at RUN time on a rename, a move or a delete — read {', '.join(here) or 'the declaration'}, then " + f"{', '.join(ext_locs[:6])}{' …' if len(ext_locs) > 6 else ''}{keep}{lead}") + elif not places and foreign: + print(f"next: {nobody}, but it is not local — {', '.join(foreign[:3])} calls it from outside the graph: " + f"read {', '.join(here) or 'the declaration'}; a body change is safe there, a change to its name or signature " + f"stops that caller reaching it{lead}") + elif not places and ep_why: + 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 maybe: + print(f"next: {nobody} — read {', '.join(here) or 'the declaration'}; a body change stays there, but " + 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}") + 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, " + f"then read {', '.join(here) or 'the declaration'}{tpart}") + elif not places and nt: + print(f"next: no production code depends on it, but {nt} test(s) reach it — read {', '.join(here) or 'the declaration'} " + f"and edit there, then run those tests (`test-impact` after the edit){lead}") + elif not places: print(f"next: nothing resolved depends on it, so the change is local — read {', '.join(here) or 'the declaration'} and edit there{lead}") elif len(places) <= 6: print(f"next: read {', '.join(here) or 'the declaration'}, then only these {len(places)} place(s) bound to it: {', '.join(places)}{lead}{ti}") diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path index 4e00c03c6..9bdad8a34 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path @@ -517,15 +517,22 @@ class G: def library_calls(self, ids, cap=10): """the library / platform methods these methods call — the ends of the client graph, listed, never traversed""" - if not ids: return [] - q = ','.join('?' * len(ids)); t = ','.join('?' * len(self.TERMINAL_TIERS)) - rows = self.q(f"""SELECT COALESCE(m.qualified_name, e.callee_label, s.callee_name) AS lib, count(*) n, - (m.qualified_name IS NULL AND e.callee_label IS NULL) AS from_site - FROM call_edges e LEFT JOIN methods m ON m.id = e.callee_method_id - LEFT JOIN call_sites s ON s.id = e.call_site_id - WHERE e.caller_id IN ({q}) AND e.tier IN ({t}) - AND COALESCE(m.qualified_name, e.callee_label, s.callee_name) IS NOT NULL - GROUP BY lib ORDER BY n DESC""", *ids, *self.TERMINAL_TIERS) + ids = list(ids) + if not ids: return [], [] + t = ','.join('?' * len(self.TERMINAL_TIERS)) + # in slices: a closure handed in whole can hold more ids than SQLite takes as parameters + tot = collections.Counter() + for i in range(0, len(ids), 900): + part = ids[i:i + 900]; q = ','.join('?' * len(part)) + for r in self.q(f"""SELECT COALESCE(m.qualified_name, e.callee_label, s.callee_name) AS lib, count(*) n, + (m.qualified_name IS NULL AND e.callee_label IS NULL) AS from_site + FROM call_edges e LEFT JOIN methods m ON m.id = e.callee_method_id + LEFT JOIN call_sites s ON s.id = e.call_site_id + WHERE e.caller_id IN ({q}) AND e.tier IN ({t}) + AND COALESCE(m.qualified_name, e.callee_label, s.callee_name) IS NOT NULL + GROUP BY lib""", *part, *self.TERMINAL_TIERS): + tot[(r['lib'], r['from_site'])] += r['n'] + rows = [{'lib': l, 'from_site': f, 'n': n} for (l, f), n in sorted(tot.items(), key=lambda x: (-x[1], x[0][0]))] # TWO LISTS, because they are two different facts and only one of them is "a library". # A named callee IS the dependency: the engine resolved the call to a declaration outside the # client and stopped there on purpose. A site-named one is only the word written at the call — @@ -1052,6 +1059,33 @@ def closure(g, sel, upstream, limit=40, depth=40): print(f" (chain from any one of them: path {sel}" + (")" if upstream else " — or the reverse)")) return 0 +# entry-point reasons whose caller is a request, a runner, a timer, a process start or the container building its beans +# at startup: nothing the client calls INTO a library lands there, so they do not make a no-chain pair "not shown to be +# independent" (#1385). Every other reason does. The container kinds were measured, not assumed: counted as reachable, +# a @Bean factory upstream of almost any method flipped 12 of 64 independent pairs on two real projects, every one +# of them through a chain from a configuration class, none through a listener or a callback. +NOT_FROM_A_LIBRARY_CALL = {'test', 'fixture', 'http', 'url', 'grpc_service', 'hub', 'cli', 'main', 'web_filter', 'web_listener', + 'scheduled', 'factory', 'bean_ctor', 'di_provider', 'lifecycle', + 'exported_from_entry_module', 'unimported_module'} + +def entered_in_process(g, ids): + """the methods among `ids` that code outside the graph enters in-process: an entry point of a kind a library call can + trigger, or an override of a method the graph does not contain. [(symbol id, why)], nearest to the endpoint first.""" + mid = {g.sym[i]['method_id']: i for i in ids if i in g.sym and g.sym[i].get('method_id')} + out = {} + keys = list(mid) + for k in range(0, len(keys), 900): + part = keys[k:k + 900]; q = ','.join('?' * len(part)) + if g.has('entry_points'): + for r in g.q(f"SELECT method_id, reason FROM entry_points WHERE method_id IN ({q})", *part): + if r['reason'] not in NOT_FROM_A_LIBRARY_CALL: out.setdefault(mid[r['method_id']], f"{r['reason']} entry point") + if g.has('overrides'): + for r in g.q(f"""SELECT o.overriding_method_id m, b.qualified_name b FROM overrides o JOIN methods b ON b.id = o.method_id + WHERE o.overriding_method_id IN ({q}) AND b.provenance <> 'client'""", *part): + out.setdefault(mid[r['m']], f"overrides {r['b']}") + order = {i: n for n, i in enumerate(ids)} + return sorted(out.items(), key=lambda x: order.get(x[0], 0)) + # ── the query ──────────────────────────────────────────────────────────────────────────────────────────────────── def path(g, a, b, show_all=False, limit=10, every=False, max_paths=20): g.export() @@ -1138,13 +1172,59 @@ def path(g, a, b, show_all=False, limit=10, every=False, max_paths=20): sites = g.q("SELECT s.file_path f, s.start_line ln FROM call_sites s JOIN unresolved_sites u ON u.call_site_id = s.id WHERE s.caller_id = ? AND s.callee_name = ?", c, g.sym[m2]['name']) print(f" `{g.sym[m2]['name']}` called in {g.disp(c)} at " + ', '.join(f"{g.site_file(s['f'])}:{s['ln']}" for s in sites[:4]) + f" — receiver not typed; if it is {g.disp(m2)}, the chain is real") return 1 - # "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 - else "the two are independent in this graph")) + # A LIBRARY CALL IS WHERE THE GRAPH STOPS, NOT WHERE THE CODE DOES (#1385). A start that publishes an event + # through a framework's publisher reaches the listener at run time, and the call that does it is a resolved + # `boundary_lib` edge -- not an unresolved site, so the by-name search above never sees it. "Independent" was + # printed for it word for word as for a start that calls nothing. + # + # Two facts together withhold the word, and neither alone does. One side (or what it reaches) calls into a + # library; and the OTHER side (or something that reaches it) is entered by code outside the graph running in the + # same process -- a listener, a lifecycle or ORM hook, a factory, a task, an override of a library method. A + # library call can land only on such a method. A route, a test or a main is entered by a request, a runner or + # a process start, never by a call the client makes into a library, so a controller that calls String.substring + # stays independent of another controller. With the library call alone deciding, 73 of 116 sampled independent + # pairs on three real Spring projects stopped being independent; with both facts, 2 did. A reason not in this + # list counts as reachable, so a new kind of entry point errs toward "not shown", never toward a false + # "independent". + lib_side = False + if not remote_found: + radj = collections.defaultdict(set) + for x, ys in adj.items(): + for y in ys: radj[y].add(x) + def closure_of(ids, nbr): + own = [i for i in ids if i in g.sym and g.sym[i]['kind'] not in ('library', 'written')] + seen = set(own); fr = list(own) + while fr: + fr = list(dict.fromkeys(y for x in fr for y in nbr.get(x, ()) + if y not in seen and y in g.sym and g.sym[y]['kind'] not in ('library', 'written'))) + seen.update(fr) + return own, sorted(seen - set(own)) + for (xs, lx), (ys, ly) in (((A_, la), (B_, lb)), ((B_, lb), (A_, la))): + own, rest = closure_of(xs, adj) + n0, u0 = g.library_calls(own); n1, u1 = g.library_calls(rest) + if not (n0 or u0 or n1 or u1): continue + entered = entered_in_process(g, sum(closure_of(ys, radj), [])) + if not entered: continue + lib_side = True + print_boundary(g, own, f"{lx} calls into libraries", f"{lx} makes") + print_boundary(g, rest, f"the {len(rest)} method(s) {lx} reaches call into libraries", "they make") + print(f" {ly} is entered from outside the graph in the same process, as such a call can do: " + + ', '.join(f"{g.disp(m)} ({why}) {g.loc(m)}" for m, why in entered[:3]) + + (f" … +{len(entered) - 3}" if len(entered) > 3 else '')) # …but a framework may still connect them, and the graph holds the evidence for it: the target is registered # under a key, and the source writes that key. That is not a chain of calls, so it is reported and not walked. - for line in framework_note(g, B_, set(A_)) + framework_note(g, A_, set(B_)): print(line) + # 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) + # "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 + else ": no CALL connects them, but one writes the key the other is registered under (below), so a framework" + " 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 ": the two are independent in this graph")) + for line in fw: print(line) return 1 # ── self-test: the engine's expected edges, through the whole tool ─────────────────────────────────────────────── diff --git a/tests/cases/csharp/next-step-reads-the-whole-answer/App.cs b/tests/cases/csharp/next-step-reads-the-whole-answer/App.cs new file mode 100644 index 000000000..b6e7009dd --- /dev/null +++ b/tests/cases/csharp/next-step-reads-the-whole-answer/App.cs @@ -0,0 +1,21 @@ +using System; +using Microsoft.AspNetCore.Mvc; + +namespace Shop +{ + public class WidgetController : ControllerBase + { + [HttpGet("widgets")] + public IActionResult Get() { return Ok(); } + } + + public class Orphan { public void X() { } } + + public class Service + { + public void Place(string id) { Console.WriteLine(id); } + public void Lonely() { } + } + + public class Listener { public void On(string id) { } } +} diff --git a/tests/cases/csharp/next-step-reads-the-whole-answer/Shop.csproj b/tests/cases/csharp/next-step-reads-the-whole-answer/Shop.csproj new file mode 100644 index 000000000..ef8279905 --- /dev/null +++ b/tests/cases/csharp/next-step-reads-the-whole-answer/Shop.csproj @@ -0,0 +1,3 @@ + + net8.0 + diff --git a/tests/cases/csharp/next-step-reads-the-whole-answer/appsettings.json b/tests/cases/csharp/next-step-reads-the-whole-answer/appsettings.json new file mode 100644 index 000000000..8ec1cc008 --- /dev/null +++ b/tests/cases/csharp/next-step-reads-the-whole-answer/appsettings.json @@ -0,0 +1 @@ +{"Handlers": {"Type": "Shop.Orphan"}} diff --git a/tests/cases/csharp/next-step-reads-the-whole-answer/case.json b/tests/cases/csharp/next-step-reads-the-whole-answer/case.json new file mode 100644 index 000000000..a4be63faa --- /dev/null +++ b/tests/cases/csharp/next-step-reads-the-whole-answer/case.json @@ -0,0 +1,75 @@ +{ + "lang": "csharp", + "checks": [ + { + "why": "a type named in appsettings.json is not safe to delete, and next: names that file instead of calling the change local", + "run": [ + "impact", + "Orphan", + "--delete" + ], + "want": [ + "NOT SAFE", + "outside the source name it", + "then appsettings.json:1" + ], + "avoid": [ + "the change is local", + "\u00b7 nothing" + ] + }, + { + "why": "control: a type nothing names outside the source keeps the local answer", + "run": [ + "impact", + "Listener" + ], + "want": [ + "the change is local" + ] + }, + { + "why": "an HTTP action the answer calls an entry point is not local: its callers are outside the graph (#1467)", + "run": [ + "impact", + "WidgetController.Get" + ], + "want": [ + "entry point, so its callers are outside the graph" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "control: a plain method nothing calls in the same file keeps the local answer", + "run": [ + "impact", + "Service.Lonely" + ], + "want": [ + "the change is local" + ], + "avoid": [ + "entry point" + ] + }, + { + "why": "control: a console write cannot land on a method nothing outside the graph enters, so the pair stays independent", + "run": [ + "path", + "Service.Place", + "Listener.On" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph", + "nothing in this graph connects them" + ], + "avoid": [ + "NOT shown", + "unresolved sites named above" + ] + } + ] +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/case.json b/tests/cases/java/next-step-reads-the-whole-answer/case.json new file mode 100644 index 000000000..5d439f594 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/case.json @@ -0,0 +1,283 @@ +{ + "lang": "java", + "src": "src", + "checks": [ + { + "why": "a type named in spring.factories is not safe to delete, and next: names that file instead of calling the change local", + "run": [ + "impact", + "GreeterInitializer", + "--delete" + ], + "want": [ + "NOT SAFE", + "outside the source name it", + "then resources/META-INF/spring.factories:3" + ], + "avoid": [ + "the change is local", + "· nothing" + ] + }, + { + "why": "control: a type named in full nowhere keeps the local answer, and a bare mention of its name (in this case.json) stays a lead", + "run": [ + "impact", + "Orphan", + "--delete" + ], + "want": [ + "the change is local", + "NOT SAFE TO ASSUME" + ], + "avoid": [ + "outside the source name it" + ] + }, + { + "why": "a class whose supertype the graph does not contain is called by that library through its @Override, so next: does not call it local", + "run": [ + "impact", + "FooServlet" + ], + "want": [ + "outside the graph: FooServlet extends / implements HttpServlet", + "HttpServlet calls it from outside the graph" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "the overriding method itself: the library calls it, said as a fact", + "run": [ + "impact", + "FooServlet.doGet" + ], + "want": [ + "outside the graph: FooServlet extends / implements HttpServlet", + "HttpServlet calls it from outside the graph" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a public method of that class with no @Override may or may not override the library's: said as the condition, not as local and not as a fact", + "run": [ + "impact", + "FooServlet.helper" + ], + "want": [ + "if it overrides one of its methods" + ], + "avoid": [ + "the change is local", + "HttpServlet calls it from outside the graph" + ] + }, + { + "why": "control: a private method of the same class is never entered through the supertype, so it is still local and gets no outside-the-graph note", + "run": [ + "impact", + "FooServlet.inner" + ], + "want": [ + "the change is local" + ], + "avoid": [ + "outside the graph:" + ] + }, + { + "why": "control: a method of a plain class with no caller is still local", + "run": [ + "impact", + "src/com/example/Orphan.java:2", + "--kind", + "method" + ], + "want": [ + "the change is local" + ] + }, + { + "why": "a configuration key defined in a properties file is named in next:, beside the binding", + "run": [ + "impact", + "greet.prefix" + ], + "want": [ + "then resources/application.properties:1" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a method reached only through the generic abstract base it overrides (the base's own body calls the abstract method) names that caller in next:, the caller the reach above printed, instead of calling the change local", + "run": [ + "impact", + "RowExtractor.add" + ], + "want": [ + "how each is reached", + "Extractor.extractData", + "place(s) bound to it: src/com/example/Extractor.java:14" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a library interface's method that only a test calls is still called by that library: the test does not switch the outside-the-graph note off", + "run": [ + "impact", + "NameFormatter.parse" + ], + "want": [ + "outside the graph: NameFormatter extends / implements Formatter", + "what reaches it above is not all that does", + "nothing but tests and leads reaches it in this graph, but it is not local — Formatter calls it from outside the graph" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a plain method that only a test calls is not called local: next: names the tests that reach it, and there is no outside-the-graph note", + "run": [ + "impact", + "Doubler.twice" + ], + "want": [ + "no production code depends on it, but 1 test(s) reach it" + ], + "avoid": [ + "outside the graph:", + "the change is local" + ] + }, + { + "why": "a publisher and a listener joined only through a library call are not called independent, and the call is named", + "run": [ + "path", + "OrderService.place", + "Audit.on" + ], + "expect_error": true, + "want": [ + "ApplicationEventPublisher.publishEvent", + "Audit.on (queue entry point)", + "NOT shown to be independent", + "the library calls named above" + ], + "avoid": [ + "the two are independent in this graph", + "unresolved sites named above" + ] + }, + { + "why": "the same pair asked the other way round gets the same answer", + "run": [ + "path", + "Audit.on", + "OrderService.place" + ], + "expect_error": true, + "want": [ + "NOT shown to be independent" + ], + "avoid": [ + "the two are independent in this graph" + ] + }, + { + "why": "control: a start that calls nothing is still independent of the listener, and next: does not point at unresolved sites that were never printed", + "run": [ + "path", + "Lonely.alone", + "Audit.on" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph", + "nothing in this graph connects them" + ], + "avoid": [ + "NOT shown", + "unresolved sites named above" + ] + }, + { + "why": "control: a resolved chain is printed as before", + "run": [ + "path", + "OrderService.direct", + "Helper.run" + ], + "want": [ + "the chain is verified" + ], + "avoid": [ + "NOT shown" + ] + }, + { + "why": "an entry point the answer says the outside world calls is not local (#1467)", + "run": [ + "impact", + "WidgetController.count" + ], + "want": [ + "is a http entry point", + "it is not local", + "http entry point, so its callers are outside the graph" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "control: a method of a plain class nothing calls, next to it, keeps the local answer", + "run": [ + "impact", + "Widgets.spare" + ], + "want": [ + "the change is local" + ], + "avoid": [ + "entry point" + ] + }, + { + "why": "a gRPC handler whose client is a [remote] row names that client as a place to read, not as a by-name lead (#1468)", + "run": [ + "impact", + "WidgetHandler.placeWidget" + ], + "want": [ + "[remote] WidgetClient.placeBlocking", + "WidgetClient.java:5" + ], + "avoid": [ + "the change is local", + "[by name]/[text] row(s) are leads" + ] + }, + { + "why": "control: a method with one resolved caller names that caller", + "run": [ + "impact", + "WidgetStore.save" + ], + "want": [ + "then only these 1 place(s) bound to it: src/com/example/WidgetHandler.java:6" + ], + "avoid": [ + "the change is local" + ] + } + ] +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/resources/META-INF/spring.factories b/tests/cases/java/next-step-reads-the-whole-answer/resources/META-INF/spring.factories new file mode 100644 index 000000000..d13765c2a --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/resources/META-INF/spring.factories @@ -0,0 +1,3 @@ +org.springframework.context.ApplicationContextInitializer=\ + com.example.Other,\ + com.example.GreeterInitializer diff --git a/tests/cases/java/next-step-reads-the-whole-answer/resources/application.properties b/tests/cases/java/next-step-reads-the-whole-answer/resources/application.properties new file mode 100644 index 000000000..3ca4894cd --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/resources/application.properties @@ -0,0 +1 @@ +greet.prefix=Hello diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Audit.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Audit.java new file mode 100644 index 000000000..16b12b579 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Audit.java @@ -0,0 +1,4 @@ +package com.example; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; +@Component public class Audit { @EventListener public void on(OrderPlaced e) { System.out.println(e.id); } } diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Doubler.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Doubler.java new file mode 100644 index 000000000..18735961c --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Doubler.java @@ -0,0 +1,5 @@ +package com.example; + +public class Doubler { + public int twice(int x) { return x * 2; } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Extractor.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Extractor.java new file mode 100644 index 000000000..d4b8ca660 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Extractor.java @@ -0,0 +1,25 @@ +package com.example; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.springframework.jdbc.core.ResultSetExtractor; +import org.springframework.jdbc.core.RowMapper; + +public abstract class Extractor implements ResultSetExtractor> { + private final RowMapper rootMapper; + private final RowMapper childMapper; + protected Extractor(RowMapper rootMapper, RowMapper childMapper) { this.rootMapper = rootMapper; this.childMapper = childMapper; } + public List extractData(ResultSet rs) throws SQLException { + List out = new ArrayList<>(); + int row = 0; + while (rs.next()) { + R root = rootMapper.mapRow(rs, row); + add(root, childMapper.mapRow(rs, row++)); + out.add(root); + } + return out; + } + protected abstract void add(R root, C child); +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/FooServlet.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/FooServlet.java new file mode 100644 index 000000000..6985fbb0f --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/FooServlet.java @@ -0,0 +1,14 @@ +package com.example; + +import jakarta.servlet.annotation.WebServlet; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +@WebServlet("/foo") +public class FooServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { } + public void helper() { } + private void inner() { } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/FormattingTest.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/FormattingTest.java new file mode 100644 index 000000000..dc25cb972 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/FormattingTest.java @@ -0,0 +1,11 @@ +package com.example; + +import java.util.Locale; +import org.junit.jupiter.api.Test; + +class FormattingTest { + @Test + void parsesAName() { new NameFormatter().parse("widget", Locale.ROOT); } + @Test + void doublesANumber() { new Doubler().twice(3); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/GreeterInitializer.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/GreeterInitializer.java new file mode 100644 index 000000000..c1027337e --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/GreeterInitializer.java @@ -0,0 +1,6 @@ +package com.example; +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; +public class GreeterInitializer implements ApplicationContextInitializer { + public void initialize(ConfigurableApplicationContext ctx) { } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Greeting.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Greeting.java new file mode 100644 index 000000000..47d453044 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Greeting.java @@ -0,0 +1,4 @@ +package com.example; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; +@Component public class Greeting { @Value("${greet.prefix}") private String prefix; public String hi() { return prefix; } } diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Helper.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Helper.java new file mode 100644 index 000000000..96c139dc0 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Helper.java @@ -0,0 +1,3 @@ +package com.example; +import org.springframework.stereotype.Component; +@Component public class Helper { public void run(String id) { System.out.println(id); } } diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Lonely.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Lonely.java new file mode 100644 index 000000000..b0310618e --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Lonely.java @@ -0,0 +1,2 @@ +package com.example; +public class Lonely { public void alone() { } } diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/NameFormatter.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/NameFormatter.java new file mode 100644 index 000000000..66db2ec81 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/NameFormatter.java @@ -0,0 +1,11 @@ +package com.example; + +import java.util.Locale; +import org.springframework.format.Formatter; + +public class NameFormatter implements Formatter { + @Override + public String print(String name, Locale locale) { return name.trim(); } + @Override + public String parse(String text, Locale locale) { return text.toUpperCase(); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Order.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Order.java new file mode 100644 index 000000000..73938dbc9 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Order.java @@ -0,0 +1,8 @@ +package com.example; + +import java.util.ArrayList; +import java.util.List; + +public class Order { + public final List lines = new ArrayList<>(); +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderLine.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderLine.java new file mode 100644 index 000000000..1eda7f59c --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderLine.java @@ -0,0 +1,6 @@ +package com.example; + +public class OrderLine { + public final String sku; + public OrderLine(String sku) { this.sku = sku; } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderPlaced.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderPlaced.java new file mode 100644 index 000000000..f75f34500 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderPlaced.java @@ -0,0 +1,2 @@ +package com.example; +public class OrderPlaced { public final String id; public OrderPlaced(String id) { this.id = id; } } diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderService.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderService.java new file mode 100644 index 000000000..f1567e34a --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/OrderService.java @@ -0,0 +1,11 @@ +package com.example; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.stereotype.Service; +@Service +public class OrderService { + private final ApplicationEventPublisher publisher; + private final Helper helper; + public OrderService(ApplicationEventPublisher publisher, Helper helper) { this.publisher = publisher; this.helper = helper; } + public void place(String id) { publisher.publishEvent(new OrderPlaced(id)); } + public void direct(String id) { helper.run(id); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Orphan.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Orphan.java new file mode 100644 index 000000000..878cddea2 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Orphan.java @@ -0,0 +1,2 @@ +package com.example; +public class Orphan { public void x() { } } diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Report.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Report.java new file mode 100644 index 000000000..1a6ca2e35 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Report.java @@ -0,0 +1,9 @@ +package com.example; + +import org.springframework.jdbc.core.JdbcTemplate; + +public class Report { + private final JdbcTemplate jdbc; + public Report(JdbcTemplate jdbc) { this.jdbc = jdbc; } + public String build() { return jdbc.query("select name from widgets", new RowExtractor()).toString(); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/RowExtractor.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/RowExtractor.java new file mode 100644 index 000000000..94d39e285 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/RowExtractor.java @@ -0,0 +1,7 @@ +package com.example; + +public class RowExtractor extends Extractor { + public RowExtractor() { super((rs, i) -> new Order(), (rs, i) -> new OrderLine(rs.getString(1))); } + @Override + protected void add(Order root, OrderLine child) { root.lines.add(child); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetClient.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetClient.java new file mode 100644 index 000000000..5345c3480 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetClient.java @@ -0,0 +1,6 @@ +package com.example; + +public class WidgetClient { + private final WidgetServiceGrpc.WidgetServiceBlockingStub stub = null; + public String placeBlocking(String req) { return stub.placeWidget(req); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetController.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetController.java new file mode 100644 index 000000000..9e1b18886 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetController.java @@ -0,0 +1,10 @@ +package com.example; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class WidgetController { + @GetMapping("/widgets/count") + public int count() { return Widgets.count(); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetHandler.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetHandler.java new file mode 100644 index 000000000..4fefa50a8 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetHandler.java @@ -0,0 +1,7 @@ +package com.example; + +public class WidgetHandler extends WidgetServiceGrpc.WidgetServiceImplBase { + private final WidgetStore store = new WidgetStore(); + @Override + public void placeWidget(String req) { store.save(req); } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetServiceGrpc.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetServiceGrpc.java new file mode 100644 index 000000000..841024827 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetServiceGrpc.java @@ -0,0 +1,10 @@ +package com.example; + +public final class WidgetServiceGrpc { + public static abstract class WidgetServiceImplBase { + public void placeWidget(String req) { } + } + public static class WidgetServiceBlockingStub { + public String placeWidget(String req) { return null; } + } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetStore.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetStore.java new file mode 100644 index 000000000..3a9d3fe97 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/WidgetStore.java @@ -0,0 +1,5 @@ +package com.example; + +public class WidgetStore { + public void save(String req) { } +} diff --git a/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Widgets.java b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Widgets.java new file mode 100644 index 000000000..74bb91a83 --- /dev/null +++ b/tests/cases/java/next-step-reads-the-whole-answer/src/com/example/Widgets.java @@ -0,0 +1,6 @@ +package com.example; + +public class Widgets { + public static int count() { return 1; } + public static int spare() { return 2; } +} diff --git a/tests/cases/javascript/next-step-reads-the-whole-answer/case.json b/tests/cases/javascript/next-step-reads-the-whole-answer/case.json new file mode 100644 index 000000000..3ec5353f5 --- /dev/null +++ b/tests/cases/javascript/next-step-reads-the-whole-answer/case.json @@ -0,0 +1,63 @@ +{ + "lang": "javascript", + "src": "src", + "checks": [ + { + "why": "a method whose one caller is untyped ([by name]) names that caller to check instead of calling the change local (#1573)", + "run": [ + "impact", + "Store.tally" + ], + "want": [ + "[by name] use", + "check src/store.js:7" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a method only a test calls is not called local: next: names the tests that reach it (#1552)", + "run": [ + "impact", + "Store.counted" + ], + "want": [ + "no production code depends on it, but 1 test(s) reach it" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "control: a method nothing calls, by any name, keeps the local answer", + "run": [ + "impact", + "Store.spare" + ], + "want": [ + "the change is local" + ], + "avoid": [ + "[by name]", + "test(s) reach it" + ] + }, + { + "why": "control: two functions that call nothing are independent", + "run": [ + "path", + "lonely", + "onEvent" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph" + ], + "avoid": [ + "NOT shown", + "NOT independent" + ] + } + ] +} diff --git a/tests/cases/javascript/next-step-reads-the-whole-answer/src/store.js b/tests/cases/javascript/next-step-reads-the-whole-answer/src/store.js new file mode 100644 index 000000000..f9375dfdd --- /dev/null +++ b/tests/cases/javascript/next-step-reads-the-whole-answer/src/store.js @@ -0,0 +1,12 @@ +class Store { + tally() { return 1; } + spare() { return 2; } + counted() { return 3; } +} + +function use(s) { return s.tally(); } + +function lonely() { return 1; } + +function onEvent(e) { return e; } +module.exports = { Store, use, lonely, onEvent }; diff --git a/tests/cases/javascript/next-step-reads-the-whole-answer/src/store.test.js b/tests/cases/javascript/next-step-reads-the-whole-answer/src/store.test.js new file mode 100644 index 000000000..eb5fe10e3 --- /dev/null +++ b/tests/cases/javascript/next-step-reads-the-whole-answer/src/store.test.js @@ -0,0 +1,5 @@ +const { Store } = require('./store'); + +test('counts', () => { + expect(new Store().counted()).toBe(3); +}); diff --git a/tests/cases/python/next-step-reads-the-whole-answer/app.py b/tests/cases/python/next-step-reads-the-whole-answer/app.py new file mode 100644 index 000000000..f92790775 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/app.py @@ -0,0 +1,26 @@ +import logging +from http.server import BaseHTTPRequestHandler + +log = logging.getLogger(__name__) + + +class Handler(BaseHTTPRequestHandler): + def do_GET(self): + pass + + +class Orphan: + def x(self): + pass + + +def notify(evt): + log.info(evt) + + +def on_event(evt): + return evt + + +def lonely(): + return 1 diff --git a/tests/cases/python/next-step-reads-the-whole-answer/case.json b/tests/cases/python/next-step-reads-the-whole-answer/case.json new file mode 100644 index 000000000..7e75b0cf8 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/case.json @@ -0,0 +1,90 @@ +{ + "lang": "python", + "src": ".", + "checks": [ + { + "why": "a class named by its dotted path in a YAML file is not safe to delete, and next: names that file instead of calling the change local", + "run": [ + "impact", + "Handler", + "--delete" + ], + "want": [ + "NOT SAFE", + "outside the source name it", + "then config.yaml:1" + ], + "avoid": [ + "the change is local", + "· nothing" + ] + }, + { + "why": "control: a class nothing names outside the source keeps the local answer", + "run": [ + "impact", + "Orphan" + ], + "want": [ + "the change is local" + ] + }, + { + "why": "control: a function that calls nothing is independent, and next: does not point at unresolved sites that were never printed", + "run": [ + "path", + "lonely", + "on_event" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph", + "nothing in this graph connects them" + ], + "avoid": [ + "NOT shown", + "unresolved sites named above" + ] + }, + { + "why": "a fixture the tests take is not local: next: says the tests the answer counted reach it (#1552)", + "run": [ + "impact", + "tests/conftest.py:7" + ], + "want": [ + "test(s) reach it" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a function whose only callers are a fixture and the tests behind it is not local either (#1552)", + "run": [ + "impact", + "client.py:5" + ], + "want": [ + "test(s) reach it" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a method whose one caller is untyped ([by name]) names that caller to check instead of calling the change local (#1573)", + "run": [ + "impact", + "Store.tally" + ], + "want": [ + "[by name]", + "check store.py:7" + ], + "avoid": [ + "the change is local" + ] + } + ] +} diff --git a/tests/cases/python/next-step-reads-the-whole-answer/client.py b/tests/cases/python/next-step-reads-the-whole-answer/client.py new file mode 100644 index 000000000..08718c131 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/client.py @@ -0,0 +1,6 @@ +class Client: + pass + + +def make_client(): + return Client() diff --git a/tests/cases/python/next-step-reads-the-whole-answer/config.yaml b/tests/cases/python/next-step-reads-the-whole-answer/config.yaml new file mode 100644 index 000000000..1b6655ba3 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/config.yaml @@ -0,0 +1 @@ +handler: app.Handler diff --git a/tests/cases/python/next-step-reads-the-whole-answer/store.py b/tests/cases/python/next-step-reads-the-whole-answer/store.py new file mode 100644 index 000000000..9f6780042 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/store.py @@ -0,0 +1,7 @@ +class Store: + def tally(self): + return 1 + + +def use(s): + return s.tally() diff --git a/tests/cases/python/next-step-reads-the-whole-answer/tests/conftest.py b/tests/cases/python/next-step-reads-the-whole-answer/tests/conftest.py new file mode 100644 index 000000000..fc5d22f00 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/tests/conftest.py @@ -0,0 +1,8 @@ +import pytest + +from client import make_client + + +@pytest.fixture +def plain_client(): + return make_client() diff --git a/tests/cases/python/next-step-reads-the-whole-answer/tests/test_a.py b/tests/cases/python/next-step-reads-the-whole-answer/tests/test_a.py new file mode 100644 index 000000000..c032e7311 --- /dev/null +++ b/tests/cases/python/next-step-reads-the-whole-answer/tests/test_a.py @@ -0,0 +1,6 @@ +def test_one(plain_client): + assert plain_client + + +def test_two(plain_client): + assert plain_client diff --git a/tests/cases/typescript/next-step-reads-the-whole-answer/case.json b/tests/cases/typescript/next-step-reads-the-whole-answer/case.json new file mode 100644 index 000000000..62b982757 --- /dev/null +++ b/tests/cases/typescript/next-step-reads-the-whole-answer/case.json @@ -0,0 +1,63 @@ +{ + "lang": "typescript", + "src": "src", + "checks": [ + { + "why": "a method whose one caller is untyped ([by name]) names that caller to check instead of calling the change local (#1573)", + "run": [ + "impact", + "Store.tally" + ], + "want": [ + "[by name] use", + "check src/store.ts:7" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "a method only a test calls is not called local: next: names the tests that reach it (#1552)", + "run": [ + "impact", + "Store.counted" + ], + "want": [ + "no production code depends on it, but 1 test(s) reach it" + ], + "avoid": [ + "the change is local" + ] + }, + { + "why": "control: a method nothing calls, by any name, keeps the local answer", + "run": [ + "impact", + "Store.spare" + ], + "want": [ + "the change is local" + ], + "avoid": [ + "[by name]", + "test(s) reach it" + ] + }, + { + "why": "control: two functions that call nothing are independent", + "run": [ + "path", + "lonely", + "onEvent" + ], + "expect_error": true, + "want": [ + "the two are independent in this graph" + ], + "avoid": [ + "NOT shown", + "NOT independent" + ] + } + ] +} diff --git a/tests/cases/typescript/next-step-reads-the-whole-answer/src/store.test.ts b/tests/cases/typescript/next-step-reads-the-whole-answer/src/store.test.ts new file mode 100644 index 000000000..740fd638b --- /dev/null +++ b/tests/cases/typescript/next-step-reads-the-whole-answer/src/store.test.ts @@ -0,0 +1,5 @@ +import { Store } from './store'; + +test('counts', () => { + expect(new Store().counted()).toBe(3); +}); diff --git a/tests/cases/typescript/next-step-reads-the-whole-answer/src/store.ts b/tests/cases/typescript/next-step-reads-the-whole-answer/src/store.ts new file mode 100644 index 000000000..46344950a --- /dev/null +++ b/tests/cases/typescript/next-step-reads-the-whole-answer/src/store.ts @@ -0,0 +1,11 @@ +export class Store { + tally(): number { return 1; } + spare(): number { return 2; } + counted(): number { return 3; } +} + +export function use(s: any): number { return s.tally(); } + +export function lonely(): number { return 1; } + +export function onEvent(e: string): string { return e; }