From 7217619c037244144db28576cc99026115e594f7 Mon Sep 17 00:00:00 2001 From: swapnil <78632212+swapnilpaliwal-sd@users.noreply.github.com> Date: Tue, 29 Sep 2026 23:57:51 -0700 Subject: [PATCH] changed: classify an edit by the declaration's place and its parameters, not by its name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-declaration diff behind `changed`, test-impact and the edit hooks found a declaration's new header by searching for its name, and read a parameter list with a regex. So: - a method renamed in place was "removed old" + "added new"; - a JS/TS constructor (`` in the graph, `constructor(` in the text) was never found again: every header edit came back as removed + "added S.constructor"; - a destructured parameter's `{` ended the header, and an arrow inside a default value took the edit ("signature millis"); - a `this.x = ...` field whose line moved was "removed" while still assigned; - a graph holding the new name printed the old header's words as a "return type". Now a header that became another name with the same parameters (and no overload keeps them) is "renamed old → new", targeted at the old declaration; constructors are looked for as written; parameters are parsed by bracket depth, destructured fields are one row each, defaults compared; a function written inside another's parameter list is charged to that one; `this.x =` declares a field; a header found word for word further down is a move, not a signature change. A span whose header line was rewritten into a header of another name with the same parameters is kept there when the graph already holds the new name (a refresh that raced the edit), and reported as the rename. Co-authored-by: axiomcode-bot[bot] <334110751+axiomcode-bot[bot]@users.noreply.github.com> --- .../axiomcode/scripts/axiomcode-changed | 192 ++++++++++++++-- .../case.json | 4 +- .../case.json | 4 +- .../cases/javascript/edits-in-place/case.json | 205 ++++++++++++++++++ .../javascript/edits-in-place/new-body.js | 27 +++ .../cases/javascript/edits-in-place/new-c.js | 15 ++ .../javascript/edits-in-place/new-ctor.js | 26 +++ .../cases/javascript/edits-in-place/new-d.js | 5 + .../javascript/edits-in-place/new-default.js | 26 +++ .../edits-in-place/new-destructure.js | 26 +++ .../javascript/edits-in-place/new-errbody.js | 27 +++ .../javascript/edits-in-place/new-rename.js | 26 +++ .../javascript/edits-in-place/new-reorder.js | 26 +++ .../cases/javascript/edits-in-place/old-c.js | 15 ++ .../cases/javascript/edits-in-place/old-d.js | 5 + tests/cases/javascript/edits-in-place/old.js | 26 +++ .../cases/javascript/edits-in-place/src/a.js | 26 +++ .../cases/javascript/edits-in-place/src/b.js | 9 + .../cases/javascript/edits-in-place/src/c.js | 15 ++ .../cases/javascript/edits-in-place/src/d.js | 5 + .../cases/javascript/edits-in-place/src/e.js | 5 + .../case.json | 7 +- tests/edit_stale_spans.py | 24 ++ 23 files changed, 719 insertions(+), 27 deletions(-) create mode 100644 tests/cases/javascript/edits-in-place/case.json create mode 100644 tests/cases/javascript/edits-in-place/new-body.js create mode 100644 tests/cases/javascript/edits-in-place/new-c.js create mode 100644 tests/cases/javascript/edits-in-place/new-ctor.js create mode 100644 tests/cases/javascript/edits-in-place/new-d.js create mode 100644 tests/cases/javascript/edits-in-place/new-default.js create mode 100644 tests/cases/javascript/edits-in-place/new-destructure.js create mode 100644 tests/cases/javascript/edits-in-place/new-errbody.js create mode 100644 tests/cases/javascript/edits-in-place/new-rename.js create mode 100644 tests/cases/javascript/edits-in-place/new-reorder.js create mode 100644 tests/cases/javascript/edits-in-place/old-c.js create mode 100644 tests/cases/javascript/edits-in-place/old-d.js create mode 100644 tests/cases/javascript/edits-in-place/old.js create mode 100644 tests/cases/javascript/edits-in-place/src/a.js create mode 100644 tests/cases/javascript/edits-in-place/src/b.js create mode 100644 tests/cases/javascript/edits-in-place/src/c.js create mode 100644 tests/cases/javascript/edits-in-place/src/d.js create mode 100644 tests/cases/javascript/edits-in-place/src/e.js diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-changed b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-changed index 72e740285..c97b818d3 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-changed +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-changed @@ -262,11 +262,28 @@ class Changed: if k in ('field', 'const', 'enum_member', 'variable'): if is_py: return bool(re.search(rf'(^\s*|\bself\.|\bcls\.){e}\s*(:[^=]*)?=(?!=)|^\s*{e}\s*:', text)) if k == 'enum_member' and re.match(rf'^\s*{e}\s*(\(|,|;|$|=|\{{)', text): return True + # a JavaScript / TypeScript field is declared where it is assigned, `this.x = …` in the constructor: read as + # "no line declares it", a field whose line moved was reported removed while it was still assigned + if k == 'field' and re.search(rf'\bthis\.{e}\s*=(?![=>])', text): return True return bool(re.search(rf'^\s*(?:[\w<>\[\],.?@]+\s+)+{e}\s*(=(?![=>])|;|\{{|=>|$)', text)) and not re.match(r'^\s*(return|throw|new|await|using|yield)\b', text) if is_py: return bool(re.search(rf'^\s*(async\s+)?def\s+{e}\s*[(\[]', text)) m = re.search(rf'^\s*((?:[\w<>\[\],.?@]+\s+)*){e}\s*(<[^()]*>)?\s*\(', text) return bool(m) and not re.match(r'^\s*(return|throw|new|await|if|while|for|foreach|switch|else|using|lock|catch|yield)\b', text) \ and (bool(m.group(1).strip()) or not text.rstrip().endswith(';')) + @staticmethod + def declared_name(h, is_py): + """the name a callable header declares (`async save(a) {`, `def save(self):`, `public int Save(int a)`, + `save = async (a) =>`, `function save(a)`); None when the text declares no callable""" + h = strip_code(h, hash_comments=is_py) + h = re.sub(r'^\s*(@[\w.]+\s*(\([^)]*\))?\s*)+', '', h) + if is_py: + m = re.match(r'\s*(?:async\s+)?def\s+([A-Za-z_]\w*)\s*[(\[]', h); return m.group(1) if m else None + m = re.match(r'\s*(?:(?:export|default|declare|public|private|protected|static|readonly|override|async|const|let|var)\s+)*' + r'([A-Za-z_$#][\w$]*)\s*[?!]?\s*(?::[^=()]*)?=\s*(?:async\s+)?(?:function\b\s*\*?\s*[\w$]*\s*)?(?:<[^()]*>)?\s*\(', h) + if m: return m.group(1) + m = re.search(r'([A-Za-z_$#][\w$]*)\s*(?:<[^()]*>)?\s*\(', h) + if not m or m.group(1) in ('if', 'for', 'while', 'switch', 'catch', 'return', 'new', 'function', 'super', 'this', 'await', 'typeof', 'yield'): return None + return m.group(1) if Changed.declares(h, m.group(1), 'method', False) else None def anchor(self, rel, spans, G, O): """THE GRAPH'S LINES ARE IN THE TEXT IT WAS INDEXED FROM, and the text an edit is judged against can be a later one: an edit made since the index (the background refresh has not run yet), or a baseline behind a refresh. Read @@ -311,7 +328,7 @@ class Changed: # texts: 3 of 393 files fall under 0.8 this way, and every file does with its text shifted by two lines tstarts = {a for a, b, i, k, d, n in spans if k in ('class', 'interface', 'enum', 'type', 'namespace')} GS = strip_code(G, hash_comments=is_py).split('\n') - chk = [(a, n) for a, b, i, k, d, n in spans if shaped(k, n) and re.fullmatch(r'[A-Za-z_$][\w$]*', n) + chk = [(a, self.header_name(n, d, rel)) for a, b, i, k, d, n in spans if shaped(k, n) and re.fullmatch(r'[A-Za-z_$][\w$]*', self.header_name(n, d, rel)) and not (rel.endswith('.cs') and re.match(r'(get|set|add|remove|init)_', n)) and (k in ('class', 'interface', 'enum', 'type', 'namespace') or a not in tstarts)] hit = sum(1 for a, n in chk if starts(GS, a, n)) @@ -326,11 +343,23 @@ class Changed: elif tag == 'replace': near[j1 + k_ + 1] = i1 + min(k_ + 1, i2 - i1) # a rewritten line: the line in its position else: near[j1 + k_ + 1] = max(1, i1) # a line this text lacks: the one before it taken = {exact[a] for a, b, i, k, d, n in spans if a in exact} + def renamed_line(a, d, n, k): + """A HEADER THE GRAPH HOLDS UNDER A NEW NAME is the same declaration when this text's line in its place declares + another name with the same parameters (a rename the refresh already indexed). Dropped as "not declared here", + the old header's line fell to the module ("body a.") and the new name was "added".""" + j = near.get(a) + if k not in ('method', 'function', 'constructor') or not j or j in taken or j in exact.values() or a > len(GL) or j > len(OL): return None + gh, oh = ' '.join(GS[a - 1:self.header_end(GS, a)]), ' '.join(OS[j - 1:self.header_end(OS, j)]) + on = self.declared_name(oh, is_py) + if self.declared_name(gh, is_py) != self.header_name(n, d, rel) or not on or on == n: return None + pl = lambda h: [p for p, _, _ in self.param_list(self.before_block_body(self.before_expression_body(h)))] + return j if pl(gh) == pl(oh) else None for a, b, i, k, d, n in spans: a2 = exact.get(a) if a2 is None: if not shaped(k, n): continue # a lambda or a module whose first line is gone a2, many = found(n, k, near.get(a) or a, taken, d) + if a2 is None: a2, many = renamed_line(a, d, n, k), False if a2 is None: continue # this text does not declare it if many: self.unsure[rel].add(i) taken.add(a2) @@ -403,31 +432,74 @@ class Changed: """the last line of a header starting at `start`: up to the first line holding `{`, ending with `;`, or (Python) `:`. Read on stripped text: a route template, a cache key or an authorization expression inside an annotation holds a brace, and taking that for the end of the header classified an annotation edit as body-only, its exact inverse""" + # a `{` inside the parameter list (a destructured parameter, an object default) is not the body's: counted by + # bracket depth, `constructor({` ended its header on its first line and the fields below read as body + depth = 0 for i in range(start - 1, min(len(L), start + 15)): t = strip_code(L[i]) - if '{' in t or t.rstrip().endswith(';') or (t.rstrip().endswith(':') and not t.strip().startswith(('case', 'default'))): return i + 1 + for ch in t: + if ch in '([': depth += 1 + elif ch in ')]': depth = max(0, depth - 1) + elif ch == '{' and depth == 0: return i + 1 + if depth: continue + if t.rstrip().endswith(';') or (t.rstrip().endswith(':') and not t.strip().startswith(('case', 'default'))): return i + 1 return start @staticmethod - def params(header): - m = re.search(r'\(([^()]*(?:\([^()]*\)[^()]*)*)\)', header) - if not m: return [] + def split_top(s, sep=','): + """`s` cut at each `sep` outside brackets; an arrow's `>` (`=>`, `->`) closes nothing""" out = []; depth = 0; cur = '' - for ch in m.group(1): - depth += ch in '<[('; depth -= ch in '>])' - if ch == ',' and depth == 0: out.append(cur); cur = '' + for x, ch in enumerate(s): + if ch in '<[({': depth += 1 + elif ch in '>])}' and not (ch == '>' and x and s[x - 1] in '=-'): depth = max(0, depth - 1) + if ch == sep and depth == 0: out.append(cur); cur = '' else: cur += ch if cur.strip(): out.append(cur) - res = [] - for p in out: - p = re.sub(r'=.*$', '', p).strip(); p = re.sub(r'@\w+(\([^)]*\))?\s*', '', p) - p = re.sub(r'^((public|private|protected|readonly|override)\s+)+(?=[A-Za-z_$][\w$]*\s*[?!]?\s*:)', '', p) # a TypeScript parameter property - if not p: continue - if ':' in p: name, typ = p.split(':', 1)[0].strip(), p.split(':', 1)[1].strip() # TS / Python: name: Type + return out + @staticmethod + def param_list(header): + """[(name, type, default)] of a header's parameter list. The list is the text between the first `(` and the `)` + that closes it, however deep its default values nest (`clock = { now: () => Date.now() }`). A destructured + parameter (`{ users, jwt }`, `[a, b]`) is its fields, one row each: those are the names a caller passes, and + read as one parameter a change to one field was every field removed and added.""" + s = header.find('(') + if s < 0: return _Params() + depth = 0; e = None + for x in range(s, len(header)): + if header[x] in '([{': depth += 1 + elif header[x] in ')]}': + depth -= 1 + if depth == 0: e = x; break + body = header[s + 1:e] if e is not None else header[s + 1:] + res = _Params() + def one(p, into): + p = re.sub(r'@\w+(\([^)]*\))?\s*', '', p).strip() + eq = re.search(r'(?])=(?![=>])', p) + head, dflt = (p[:eq.start()].strip(), re.sub(r'\s+', ' ', p[eq.end():].strip())) if eq and not p.startswith(('{', '[')) else (p, '') + if head.startswith(('{', '[')): + close = {'{': '}', '[': ']'}[head[0]]; depth = 0; end = len(head) + for x, ch in enumerate(head): + depth += ch in '{['; depth -= ch in '}]' + if depth == 0 and ch == close: end = x; break + for f in Changed.split_top(head[1:end]): + f = f.strip() + if not f: continue + eqf = re.search(r'(?])=(?![=>])', f) + fd = re.sub(r'\s+', ' ', f[eqf.end():].strip()) if eqf else '' + key = (f[:eqf.start()] if eqf else f).split(':', 1)[0].strip().lstrip('.') + if key: into.append((key, '', fd)); into.fields.add(key) + return + head = re.sub(r'^((public|private|protected|readonly|override)\s+)+(?=[A-Za-z_$][\w$]*\s*[?!]?\s*:)', '', head) # a TypeScript parameter property + if not head: return + if ':' in head: name, typ = head.split(':', 1)[0].strip(), head.split(':', 1)[1].strip() # TS / Python: name: Type else: - toks = re.findall(r'[A-Za-z_$][\w$]*', p); name = toks[-1] if toks else p; typ = p[:p.rfind(name)].strip() if toks else '' - res.append((name.lstrip('*.'), re.sub(r'\s+', ' ', typ))) + toks = re.findall(r'[A-Za-z_$][\w$]*', head); name = toks[-1] if toks else head; typ = head[:head.rfind(name)].strip() if toks else '' + into.append((name.lstrip('*.').rstrip('?'), re.sub(r'\s+', ' ', typ), dflt)) + for p in Changed.split_top(body): one(p, res) return res @staticmethod + def params(header): + return [(n, t) for n, t, _ in Changed.param_list(header)] + @staticmethod def before_expression_body(h): """a header's text up to an expression body's `=>` written after its parameter list, at bracket depth 0 (a lambda passed as a default value inside the parameters stays in them)""" @@ -717,6 +789,13 @@ class Changed: elif not f: f = next(((a, b, i, k, d, n) for a, b, i, k, d, n in decls if k in ('field', 'const', 'enum_member', 'variable') and a < ln and any(is_lam(x) and x[0] <= ln <= x[1] and a <= x[0] <= max(b, a) for x in decls)), None) m = narrowest(ln, {'method', 'function', 'constructor', 'module'}) + # A FUNCTION WRITTEN IN ANOTHER'S PARAMETER LIST IS PART OF THAT HEADER. A default value + # (`clock = { millis: () => Date.now() }`) holds a callable the graph records on its own; an edit inside the + # default was charged to it ("signature millis") instead of to the function whose parameter changed + if m and m[3] in ('method', 'function', 'constructor'): + host = [x for x in decls if x[3] in ('method', 'function', 'constructor') and x[2] != m[2] and not is_lam(x) + and x[0] <= m[0] and m[1] <= x[1] and m[1] <= self.header_end(OL, x[0]) and x[0] <= ln <= self.header_end(OL, x[0])] + if host: m = min(host, key=lambda x: x[1] - x[0]) t = narrowest(ln, {'class', 'interface', 'enum', 'type', 'namespace'}) lam = None if f else lambda_decl(ln, m) if lam: hits.setdefault(('lambda', lam), set()).add(ln); continue @@ -751,8 +830,30 @@ class Changed: overrun_note = ('', f"{rel}: the graph's spans run past the end of this file's committed version ({len(OL)} lines): it was " "indexed from a working tree with uncommitted edits, so positions here may be off; re-index " "(`axiomcode index`) for exact answers", None) if overrun else None + sig_of = lambda h: re.sub(r'^\s*(@\w[\w.]*\s*(\([^)]*\))?\s*)+', '', h).strip() + def renamed_at(a, n, k): + """A DECLARATION RENAMED IN PLACE is the same declaration: its header line became one that declares another + name, with the same parameters, the old name is declared nowhere in the new text and the new name nowhere in + the old. Read as "removed " plus "added " (or as a signature change of the NEW name, 0 callers), the + edit's dependents were never the old name's callers. (new name, new line), or None""" + if k not in ('method', 'function', 'constructor') or n in P.LAMBDA_NAMES: return None + j = counterpart(a) + if not j or j > len(NL): return None + oh = ' '.join(x.strip() for x in OK[a - 1:self.header_end(OL, a)]); nh = ' '.join(x.strip() for x in NK[j - 1:self.header_end(NL, j)]) + if self.declared_name(oh, is_py) != n: return None + nn = self.declared_name(nh, is_py) + if not nn or nn == n: return None + plist = lambda L, x: [p for p, _, _ in self.param_list(self.before_block_body(self.before_expression_body(sig_of(' '.join(y.strip() for y in L[x - 1:self.header_end(L, x)])))))] + po, pn = plist(OK, a), plist(NK, j) + if po != pn: return None + # the old name may stay declared by an OVERLOAD (another parameter list); declared with this one, it moved + if any(self.declares(NS[x - 1], n, k, is_py) and plist(NK, x) == po for x in range(1, len(NS) + 1)): return None + if any(self.declares(OS[x - 1], nn, k, is_py) and plist(OK, x) == po for x in range(1, len(OS) + 1)): return None + return (nn, j, oh, nh) + renamed_new = set() for (kind, (a, b, i, k, d, n)), lines in hits.items(): decs = decorated.get((a, b, i, k, d, n)) + n = self.header_name(n, d, rel) if a > len(OL): continue # starts past the committed file: nothing of it is there entry = dict(kind=kind, symbol=d, id=i, file=rel, line=a, end=b, old_lines=sorted(x for x in lines if x > 0), target_kind=('param' if kind == 'signature' else KIND(k))) if kind == 'lambda': @@ -814,10 +915,21 @@ class Changed: # parameter read as removed ("signature f -self, -rel"). Declared nowhere in the new text, it is removed; # declared elsewhere, what changed cannot be read from here if not nh: + ren = renamed_at(a, n, k) + if ren: + nn_, j_, oh_r, nh_r = ren + entry.update(detail=f"renamed {n} → {nn_} (same parameters; its callers still name {n})", old_header=oh_r, new_header=nh_r, target=d, target_kind='method') + renamed_new.add((d.rsplit('.', 1)[0] + '.' + nn_) if '.' in d else nn_) + out.append(entry); continue again = still_declared(hn, k, na or a) if not again: if not any(e['kind'] == 'removed' and e['id'] == i for e in out): entry.update(kind='removed', target=self.target(kind, d, k, n)); out.append(entry) continue + # the same header, word for word, further along: code inserted above it moved it, and its header did not change + if re.sub(r'\s', '', ' '.join(OS[a - 1:self.header_end(OS, a)])) == re.sub(r'\s', '', ' '.join(NS[again - 1:self.header_end(NS, again)])): + entry.update(kind='body', detail=f"moved: the same header is now at line {again}", target=d, target_kind='method') + if not any(e['id'] == i for e in out): out.append(entry) + continue entry.update(detail=f"may have changed: its header is no longer at its line; a declaration of {n} is at line {again}", target=d, target_kind='method') out.append(entry); continue # A BLOCK BODY ON THE HEADER'S LINE IS NOT HEADER EITHER. `public int total() { return 42; }` is one line, @@ -828,11 +940,24 @@ class Changed: oh, oh_raw, nh, nh_raw = body_brace(oh), body_brace(oh_raw), body_brace(nh), body_brace(nh_raw) # the header may start on an annotation line: its arguments are not a parameter list, and its text is # not a return type. Read the signature from the first line that is not a decoration - sig = lambda h: re.sub(r'^\s*(@\w[\w.]*\s*(\([^)]*\))?\s*)+', '', h).strip() + sig = sig_of oh_s, nh_s = sig(oh), sig(nh) - op, np_ = self.params(oh_s), self.params(nh_s) if nh_s else [] + opl, npl = self.param_list(oh_s), self.param_list(nh_s) if nh_s else [] + op, np_ = [(x, t) for x, t, _ in opl], [(x, t) for x, t, _ in npl] on, nn = [x for x, _ in op], [x for x, _ in np_] detail = [] + # THE OLD HEADER MUST BE THIS DECLARATION'S. A graph from another text of the file (a refresh that already + # holds the edit) can place a declaration on a line that declares something else, or on a body line; its + # words were then printed as a "return type" (`const iat = Math.floor → export function`). A header that + # declares another name is that name renamed to this one; a line that declares nothing says so + names_n = bool(re.search(rf'(?])=(?![=>])|;', pre_o)): + entry.update(detail=(f"renamed {named_o} → {n} (the graph already holds the new name)" if named_o and nh else + f"may have changed: the graph places it at line {a}, which is not its header (the graph is from another text of this file)"), + old_header=oh_raw, new_header=nh_raw, target=d, target_kind='method') + out.append(entry); continue if nh and not re.search(rf'\b{re.escape(hn)}\b', nh): detail.append('renamed') for x in on: if x not in nn: detail.append(f'-{x}') @@ -840,6 +965,9 @@ class Changed: if x not in on: detail.append(f'+{x}') for (x, t1), (y, t2) in zip(op, np_): if x == y and t1 != t2 and t1 and t2: detail.append(f'{x}: {t1} → {t2}') + od = {x: v for x, _, v in opl} + for x, _, v in npl: + if x in od and re.sub(r'\s', '', od[x]) != re.sub(r'\s', '', v): detail.append(f'{x}: default {od[x] or "(none)"} → {v or "(none)"}') pre_o = re.sub(r'\(.*$', '', oh_s); pre_n = re.sub(r'\(.*$', '', nh_s) if nh_s else '' if nh and pre_o.split() != pre_n.split(): ro = [w for w in pre_o.split() if w != hn]; rn = [w for w in pre_n.split() if w != hn] @@ -850,7 +978,7 @@ class Changed: if decs: detail.append('decoration changed: ' + ', '.join(dict.fromkeys(decs)) + ' — what the framework does with it (a proxy, a transaction, a cache, a route) is NOT in the graph; only the code that names it is') if not detail: entry['kind'] = 'body' # whitespace or a comment in the header entry['detail'] = ', '.join(detail); entry['old_header'] = oh_raw; entry['new_header'] = nh_raw - changed_params = [x for x, _ in op if any(dd.startswith(f'{x}:') or dd == f'-{x}' for dd in detail)] + changed_params = [x for x, _ in op if x not in opl.fields and any(dd.startswith(f'{x}:') or dd == f'-{x}' for dd in detail)] entry['target'] = (f"{d}({changed_params[0]})" if len(changed_params) == 1 and entry['kind'] == 'signature' else d) entry['target_kind'] = 'param' if '(' in entry['target'] else 'method' elif kind == 'field': @@ -912,6 +1040,14 @@ class Changed: # a declaration found removed is not also listed by a body line of it that the diff paired with other text gone_ids = {e['id'] for e in out if e['kind'] == 'removed'} out = [e for e in out if e['kind'] == 'removed' or e['id'] not in gone_ids] + # one body row per declaration (a header line found moved and a body line are two keys of it) + seen_body = set(); keep = [] + for e in out: + if e['kind'] == 'body' and e['id'] is not None: + if e['id'] in seen_body: continue + seen_body.add(e['id']) + keep.append(e) + out = keep adds = [] SL = strip_code(new, hash_comments=is_py).split('\n') # the new text, strings and comments blanked ndepth = [0] * (len(NL) + 2) # brace depth at the START of each new line @@ -1016,7 +1152,7 @@ class Changed: def old_has(nm, hdr, t=t): ht = types_of(hdr) if '(' in hdr else None for a, b, i, k, d, n in decls: - if n != nm or (t and not d.startswith(t[4] + '.') and d != t[4] + '.' + nm and k not in ('class', 'interface', 'enum')): continue + if self.header_name(n, d, rel) != nm or (t and not d.startswith(t[4] + '.') and d != t[4] + '.' + nm and k not in ('class', 'interface', 'enum')): continue if ht is not None and k in ('field', 'const', 'enum_member', 'variable'): continue # a field of that name is not this method if ht is None or k in ('class', 'interface', 'enum', 'field', 'const', 'enum_member'): return True sig = self.g.sym.get(i, {}).get('signature') or '' @@ -1073,6 +1209,8 @@ class Changed: renamed = {(e['file'], e['symbol'].rsplit('.', 1)[0] + '.' + str(e.get('detail'))[len('renamed → '):] if '.' in e['symbol'] else str(e.get('detail'))[len('renamed → '):]) for e in out if e['kind'] == 'field' and str(e.get('detail', '')).startswith('renamed → ')} out = [e for e in out if not (e['kind'] == 'added' and (e['file'], e['symbol']) in renamed)] + # and a callable renamed in place is not also a new callable of the new name (renamed_at) + out = [e for e in out if not (e['kind'] == 'added' and e['symbol'] in renamed_new)] if overrun_note: adds.append(overrun_note) # THE TARGET NAMES THIS DECLARATION, NOT EVERY DECLARATION OF ITS NAME. `symbol` stays the short display; the # target impact is asked is where the declaration is (at_line), and `shown_target` keeps its name for reading. @@ -1095,7 +1233,15 @@ class Changed: if isinstance(i, str) and i.startswith('f:'): row = self.g.q("SELECT file, line FROM symbols WHERE rowid = ?", int(i[2:])) if i[2:].isdigit() else None at = f"{row[0][0]}:{row[0][1]}" if row and row[0][0] and row[0][1] else None - else: at = self.g.lambda_target(i) + else: + at = self.g.lambda_target(i) + # file:line names the NARROWEST callable spanning the line: a function whose line also holds another one (an + # arrow in a default value, `f(clock = { now: () => 0 })`) is not named by it, and asked that way impact + # answered for the arrow. Its qualified name names it + r = self.g.all_sym.get(i) or self.g.sym.get(i) or {} + if at and not self.g.is_lambda(i) and r.get('end_line') and self.g.q( + "SELECT 1 FROM symbols WHERE file = ? AND line <= ? AND end_line >= ? AND method_id IS NOT NULL AND kind <> 'module' AND id <> ? AND end_line - line < ? LIMIT 1", + r['file'], r['line'], r['line'], i, r['end_line'] - r['line']) and self.qualified(i, 'method'): return None return at + sfx if at else None def new_file(self, rel, new, decls, is_py): """A FILE THE BASE DOES NOT HAVE is one change: `added (N declarations)`. Read line by line as an insertion, @@ -1165,6 +1311,10 @@ class Changed: def target(kind, d, k, n): return d +class _Params(list): + """a parameter list; `fields` are the names that are fields of a destructured parameter, not parameters of their own""" + def __init__(self): super().__init__(); self.fields = set() + NO_GIT = ("no git base: {repo} is not a git checkout (a copy without .git), so there is no earlier version to diff the " "working tree against, and nothing can say what changed. Name the files you edited: `axiomcode changed …` " "or `axiomcode test-impact …` (MCP files=[…]) counts every declaration in each named file as changed.") diff --git a/tests/cases/csharp/edit-targets-the-declaration-edited/case.json b/tests/cases/csharp/edit-targets-the-declaration-edited/case.json index f923fe2aa..ab76ede01 100644 --- a/tests/cases/csharp/edit-targets-the-declaration-edited/case.json +++ b/tests/cases/csharp/edit-targets-the-declaration-edited/case.json @@ -22,5 +22,5 @@ "avoid": ["PlainTests", "CountedTests"]}, {"why": "a renamed overload still reports what the old name's callers lose, and only that overload's", "run": ["changed", "{repo}", "--old", "{repo}/old.txt", "--new", "{repo}/new-rename.txt", "--file", "src/App/Job.cs", "--impact"], - "want": ["Job.Run src/App/Job.cs:10", "→ impact src/App/Job.cs:10", "Callers.Counted"], - "avoid": ["Callers.Plain", "PlainTests"]}]} + "want": ["Job.Run src/App/Job.cs:10", "→ impact src/App/Job.cs:10", "Callers.Counted", "renamed Run → Execute"], + "avoid": ["Callers.Plain", "PlainTests", "added ", "removed "]}]} diff --git a/tests/cases/java/edit-targets-the-declaration-edited/case.json b/tests/cases/java/edit-targets-the-declaration-edited/case.json index a91bbe58c..af1aa5f3b 100644 --- a/tests/cases/java/edit-targets-the-declaration-edited/case.json +++ b/tests/cases/java/edit-targets-the-declaration-edited/case.json @@ -22,5 +22,5 @@ "avoid": ["PlainTest", "CountedTest"]}, {"why": "a renamed overload still reports what the old name's callers lose, and only that overload's", "run": ["changed", "{repo}", "--old", "{repo}/old.txt", "--new", "{repo}/new-rename.txt", "--file", "src/app/Job.java", "--impact"], - "want": ["Job.run src/app/Job.java:8", "→ impact src/app/Job.java:8", "Callers.counted"], - "avoid": ["Callers.plain", "PlainTest"]}]} + "want": ["Job.run src/app/Job.java:8", "→ impact src/app/Job.java:8", "Callers.counted", "renamed run → execute"], + "avoid": ["Callers.plain", "PlainTest", "added ", "removed "]}]} diff --git a/tests/cases/javascript/edits-in-place/case.json b/tests/cases/javascript/edits-in-place/case.json new file mode 100644 index 000000000..b595e6e80 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/case.json @@ -0,0 +1,205 @@ +{ + "lang": "javascript", + "src": "src", + "checks": [ + { + "why": "a method renamed in place (same position, same parameters) is one declaration renamed, answered from the OLD name's callers; never 'removed' plus 'added'", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-rename.js", + "--file", + "src/a.js", + "--impact" + ], + "want": [ + "signature S.oldName", + "renamed oldName → newName", + "twice", + "use" + ], + "avoid": [ + "removed ", + "added S.newName", + "signature S.newName" + ] + }, + { + "why": "a constructor header edit is a signature change of the constructor, found under the name it is written with ('constructor'), not removed and re-added", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-ctor.js", + "--file", + "src/a.js" + ], + "want": [ + "signature S.", + "-b", + "+c" + ], + "avoid": [ + "removed S.", + "added S.constructor", + "-a,", + "placed by name" + ] + }, + { + "why": "a field added to a destructured parameter names only that field, and targets the constructor (a destructured field is no parameter of its own)", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-destructure.js", + "--file", + "src/a.js" + ], + "want": [ + "signature S.", + "+c", + "impact src/a.js:2" + ], + "avoid": [ + "-a", + "-b", + "src/a.js:2(c)" + ] + }, + { + "why": "a multi-line destructured parameter with one field replaced is reported as that one field", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old-c.js", + "--new", + "{repo}/new-c.js", + "--file", + "src/c.js" + ], + "want": [ + "signature Auth.", + "-jwt", + "+tokens" + ], + "avoid": [ + "-users", + "-clock", + "removed Auth" + ] + }, + { + "why": "an edit inside a default value is the function's own parameter change, not a change to the arrow written inside the default", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-default.js", + "--file", + "src/a.js", + "--impact" + ], + "want": [ + "signature stamp", + "clock: default", + "twice" + ], + "avoid": [ + "signature millis", + "parameter clock of millis" + ] + }, + { + "why": "a field whose assignment moved and changed (`this.n = 2` now first in the constructor) is still assigned, so it is not removed", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-reorder.js", + "--file", + "src/a.js" + ], + "want": [ + "field S.n", + "still assigned" + ], + "avoid": [ + "removed S.n" + ] + }, + { + "why": "a graph that already holds the new name reads the old header as that name renamed, never as a 'return type' change", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old-d.js", + "--new", + "{repo}/new-d.js", + "--file", + "src/d.js" + ], + "want": [ + "renamed stale → fresh" + ], + "avoid": [ + "return type" + ] + }, + { + "why": "control: a body edit is a body change of that method, and no field of the constructor is touched", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-body.js", + "--file", + "src/a.js" + ], + "want": [ + "body S.oldName" + ], + "avoid": [ + "signature ", + "removed ", + "field " + ] + }, + { + "why": "control: a statement added to a constructor body leaves its header and its untouched field this.name alone", + "run": [ + "changed", + "{repo}", + "--old", + "{repo}/old.js", + "--new", + "{repo}/new-errbody.js", + "--file", + "src/a.js" + ], + "want": [ + "body Err." + ], + "avoid": [ + "Err.name", + "signature ", + "removed " + ] + } + ] +} \ No newline at end of file diff --git a/tests/cases/javascript/edits-in-place/new-body.js b/tests/cases/javascript/edits-in-place/new-body.js new file mode 100644 index 000000000..404393886 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-body.js @@ -0,0 +1,27 @@ +export class S { + constructor({ a, b }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async oldName(x, y) { + const s = x + y; + return s * 2; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-c.js b/tests/cases/javascript/edits-in-place/new-c.js new file mode 100644 index 000000000..4347c45ed --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-c.js @@ -0,0 +1,15 @@ +export class Auth { + constructor({ + users, + tokens, + clock, + }) { + this.users = users; + this.tokens = tokens; + this.clock = clock; + } + + check(token) { + return this.tokens.verify(token); + } +} diff --git a/tests/cases/javascript/edits-in-place/new-ctor.js b/tests/cases/javascript/edits-in-place/new-ctor.js new file mode 100644 index 000000000..78e2bd92f --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-ctor.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, c }) { + this.a = a; + this.c = c; + this.n = 1; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-d.js b/tests/cases/javascript/edits-in-place/new-d.js new file mode 100644 index 000000000..512949b4a --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-d.js @@ -0,0 +1,5 @@ +export class R { + async fresh(x) { + return x + 1; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-default.js b/tests/cases/javascript/edits-in-place/new-default.js new file mode 100644 index 000000000..4b6f4a520 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-default.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, b }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() + 1 }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-destructure.js b/tests/cases/javascript/edits-in-place/new-destructure.js new file mode 100644 index 000000000..271374c52 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-destructure.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, b, c }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-errbody.js b/tests/cases/javascript/edits-in-place/new-errbody.js new file mode 100644 index 000000000..37b4b7573 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-errbody.js @@ -0,0 +1,27 @@ +export class S { + constructor({ a, b }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.code = 7; + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-rename.js b/tests/cases/javascript/edits-in-place/new-rename.js new file mode 100644 index 000000000..3b8d98050 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-rename.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, b }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async newName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/new-reorder.js b/tests/cases/javascript/edits-in-place/new-reorder.js new file mode 100644 index 000000000..75aeddfcf --- /dev/null +++ b/tests/cases/javascript/edits-in-place/new-reorder.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, b }) { + this.n = 2; + this.a = a; + this.b = b; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/old-c.js b/tests/cases/javascript/edits-in-place/old-c.js new file mode 100644 index 000000000..790996dd4 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/old-c.js @@ -0,0 +1,15 @@ +export class Auth { + constructor({ + users, + jwt, + clock, + }) { + this.users = users; + this.jwt = jwt; + this.clock = clock; + } + + check(token) { + return this.jwt.verify(token); + } +} diff --git a/tests/cases/javascript/edits-in-place/old-d.js b/tests/cases/javascript/edits-in-place/old-d.js new file mode 100644 index 000000000..9fef5b22a --- /dev/null +++ b/tests/cases/javascript/edits-in-place/old-d.js @@ -0,0 +1,5 @@ +export class R { + async stale(x) { + return x; + } +} diff --git a/tests/cases/javascript/edits-in-place/old.js b/tests/cases/javascript/edits-in-place/old.js new file mode 100644 index 000000000..50c66c63b --- /dev/null +++ b/tests/cases/javascript/edits-in-place/old.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, b }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/src/a.js b/tests/cases/javascript/edits-in-place/src/a.js new file mode 100644 index 000000000..50c66c63b --- /dev/null +++ b/tests/cases/javascript/edits-in-place/src/a.js @@ -0,0 +1,26 @@ +export class S { + constructor({ a, b }) { + this.a = a; + this.b = b; + this.n = 1; + } + + async oldName(x, y) { + return x + y; + } + + keep(v) { + return v; + } +} + +export function stamp(v, clock = { millis: () => Date.now() }) { + return { v, at: clock.millis() }; +} + +export class Err extends Error { + constructor(msg) { + super(msg); + this.name = 'Err'; + } +} diff --git a/tests/cases/javascript/edits-in-place/src/b.js b/tests/cases/javascript/edits-in-place/src/b.js new file mode 100644 index 000000000..bfe91357a --- /dev/null +++ b/tests/cases/javascript/edits-in-place/src/b.js @@ -0,0 +1,9 @@ +import { S, stamp } from './a.js'; + +export function use() { + return new S({ a: 1, b: 2 }).oldName(1, 2); +} + +export function twice() { + return new S({}).oldName(2, 2) + stamp(1).at; +} diff --git a/tests/cases/javascript/edits-in-place/src/c.js b/tests/cases/javascript/edits-in-place/src/c.js new file mode 100644 index 000000000..790996dd4 --- /dev/null +++ b/tests/cases/javascript/edits-in-place/src/c.js @@ -0,0 +1,15 @@ +export class Auth { + constructor({ + users, + jwt, + clock, + }) { + this.users = users; + this.jwt = jwt; + this.clock = clock; + } + + check(token) { + return this.jwt.verify(token); + } +} diff --git a/tests/cases/javascript/edits-in-place/src/d.js b/tests/cases/javascript/edits-in-place/src/d.js new file mode 100644 index 000000000..c2ccd38fa --- /dev/null +++ b/tests/cases/javascript/edits-in-place/src/d.js @@ -0,0 +1,5 @@ +export class R { + async fresh(x) { + return x; + } +} diff --git a/tests/cases/javascript/edits-in-place/src/e.js b/tests/cases/javascript/edits-in-place/src/e.js new file mode 100644 index 000000000..1e65abf6f --- /dev/null +++ b/tests/cases/javascript/edits-in-place/src/e.js @@ -0,0 +1,5 @@ +import { Auth } from './c.js'; + +export function login(t) { + return new Auth({ users: [], jwt: null, clock: null }).check(t); +} diff --git a/tests/cases/python/edit-targets-the-declaration-edited/case.json b/tests/cases/python/edit-targets-the-declaration-edited/case.json index 070660ce2..3cc8d65b2 100644 --- a/tests/cases/python/edit-targets-the-declaration-edited/case.json +++ b/tests/cases/python/edit-targets-the-declaration-edited/case.json @@ -124,11 +124,14 @@ "want": [ "main alpha/cli.py:5", "→ impact alpha/cli.py:5", - "go_a" + "go_a", + "renamed main → entry" ], "avoid": [ "go_b", - "test_beta" + "test_beta", + "added ", + "removed " ] }, { diff --git a/tests/edit_stale_spans.py b/tests/edit_stale_spans.py index aac559c8e..7cb4b6c0b 100644 --- a/tests/edit_stale_spans.py +++ b/tests/edit_stale_spans.py @@ -16,6 +16,8 @@ control: a real parameter added to that method is still a signature change; · a method moved below its neighbour: not removed; · removing a function whose name another file also declares: only this file's callers; + · a method renamed in place (same parameters): renamed, with the old name's callers; and so when the graph already + holds the new name; control: another parameter list in its place is no rename; · (Python) an edited import line: never a signature of the module; · the graph's rows and the tree it records for them disagree (a refresh raced an edit): declarations are placed by name and the answer says so; control: a faithful recorded tree says nothing of the kind; @@ -178,6 +180,28 @@ def project(lang, files): check(f'{lang}: removing a function lists this file\'s caller', c['mine'] in out and 'removed' in out, out) check(f'{lang}: ... and not the caller of the same-named function in another file', c['other'] not in out, out) + # a rename in place (same line, same parameters) is one declaration renamed, answered from the OLD name's callers + cnt = {'python': 'def count(self)', 'java': 'public int count()', 'csharp': 'public int Count()'}[lang] + ren = cnt.replace('ount(', 'ountAll(') + out = fire(repo, c['stale'], cnt, ren) + check(f'{lang}: a method renamed in place is renamed, with its callers', 'renamed' in out and 'removed' not in out and c['mine'].split('.')[-1] in out, out) + # the graph already holds the new name (a refresh indexed the rename): the old header's line is still that declaration + f = os.path.join(repo, c['stale']); t0 = open(f).read() + tf = tempfile.NamedTemporaryFile('w', suffix=os.path.splitext(f)[1], delete=False); tf.write(t0.replace(cnt, ren, 1)); tf.close() + r = subprocess.run([sys.executable, AX + '-changed', repo, '--old', tf.name, '--new', f, '--file', c['stale'], '--json'], capture_output=True, text=True, timeout=120) + try: kinds = [f"{e['kind']} {e['symbol']} {e.get('detail', '')}" for e in json.loads(r.stdout).get('changed', [])] + except ValueError: kinds = [r.stderr[-300:]] + check(f'{lang}: a rename the graph already holds is that declaration renamed, not a module body edit plus an added method', + any(k.startswith('signature') and 'renamed' in k for k in kinds) and not any(k.startswith('added') or '' in k for k in kinds), kinds) + # control: the same method with another parameter list in its place is not a rename + tf2 = tempfile.NamedTemporaryFile('w', suffix=os.path.splitext(f)[1], delete=False) + tf2.write(t0.replace(cnt, ren.replace('()', '(int k)').replace('(self)', '(self, k)'), 1)); tf2.close() + r = subprocess.run([sys.executable, AX + '-changed', repo, '--old', tf2.name, '--new', f, '--file', c['stale'], '--json'], capture_output=True, text=True, timeout=120) + try: kinds = [f"{e['kind']} {e['symbol']} {e.get('detail', '')}" for e in json.loads(r.stdout).get('changed', [])] + except ValueError: kinds = [r.stderr[-300:]] + check(f'{lang}: control: a header with other parameters in its place is not called a rename', not any('renamed' in k for k in kinds), kinds) + os.unlink(tf.name); os.unlink(tf2.name) + if lang == 'python': out = fire(repo, c['stale'], 'import os\n', 'import os, http\n') check('python: an edited import line is never a signature of the module', 'signature' not in out, out)