Skip to content

Commit c224262

Browse files
changed: classify an edit by the declaration's place and its parameters, not by its name
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 (`<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>
1 parent 378e775 commit c224262

23 files changed

Lines changed: 719 additions & 27 deletions

File tree

‎plugins/axiomcode/skills/axiomcode/scripts/axiomcode-changed‎

Lines changed: 171 additions & 21 deletions
Large diffs are not rendered by default.

‎tests/cases/csharp/edit-targets-the-declaration-edited/case.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@
2222
"avoid": ["PlainTests", "CountedTests"]},
2323
{"why": "a renamed overload still reports what the old name's callers lose, and only that overload's",
2424
"run": ["changed", "{repo}", "--old", "{repo}/old.txt", "--new", "{repo}/new-rename.txt", "--file", "src/App/Job.cs", "--impact"],
25-
"want": ["Job.Run src/App/Job.cs:10", "→ impact src/App/Job.cs:10", "Callers.Counted"],
26-
"avoid": ["Callers.Plain", "PlainTests"]}]}
25+
"want": ["Job.Run src/App/Job.cs:10", "→ impact src/App/Job.cs:10", "Callers.Counted", "renamed Run → Execute"],
26+
"avoid": ["Callers.Plain", "PlainTests", "added ", "removed "]}]}

‎tests/cases/java/edit-targets-the-declaration-edited/case.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@
2222
"avoid": ["PlainTest", "CountedTest"]},
2323
{"why": "a renamed overload still reports what the old name's callers lose, and only that overload's",
2424
"run": ["changed", "{repo}", "--old", "{repo}/old.txt", "--new", "{repo}/new-rename.txt", "--file", "src/app/Job.java", "--impact"],
25-
"want": ["Job.run src/app/Job.java:8", "→ impact src/app/Job.java:8", "Callers.counted"],
26-
"avoid": ["Callers.plain", "PlainTest"]}]}
25+
"want": ["Job.run src/app/Job.java:8", "→ impact src/app/Job.java:8", "Callers.counted", "renamed run → execute"],
26+
"avoid": ["Callers.plain", "PlainTest", "added ", "removed "]}]}
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
{
2+
"lang": "javascript",
3+
"src": "src",
4+
"checks": [
5+
{
6+
"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'",
7+
"run": [
8+
"changed",
9+
"{repo}",
10+
"--old",
11+
"{repo}/old.js",
12+
"--new",
13+
"{repo}/new-rename.js",
14+
"--file",
15+
"src/a.js",
16+
"--impact"
17+
],
18+
"want": [
19+
"signature S.oldName",
20+
"renamed oldName → newName",
21+
"twice",
22+
"use"
23+
],
24+
"avoid": [
25+
"removed ",
26+
"added S.newName",
27+
"signature S.newName"
28+
]
29+
},
30+
{
31+
"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",
32+
"run": [
33+
"changed",
34+
"{repo}",
35+
"--old",
36+
"{repo}/old.js",
37+
"--new",
38+
"{repo}/new-ctor.js",
39+
"--file",
40+
"src/a.js"
41+
],
42+
"want": [
43+
"signature S.<constructor>",
44+
"-b",
45+
"+c"
46+
],
47+
"avoid": [
48+
"removed S.<constructor>",
49+
"added S.constructor",
50+
"-a,",
51+
"placed by name"
52+
]
53+
},
54+
{
55+
"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)",
56+
"run": [
57+
"changed",
58+
"{repo}",
59+
"--old",
60+
"{repo}/old.js",
61+
"--new",
62+
"{repo}/new-destructure.js",
63+
"--file",
64+
"src/a.js"
65+
],
66+
"want": [
67+
"signature S.<constructor>",
68+
"+c",
69+
"impact src/a.js:2"
70+
],
71+
"avoid": [
72+
"-a",
73+
"-b",
74+
"src/a.js:2(c)"
75+
]
76+
},
77+
{
78+
"why": "a multi-line destructured parameter with one field replaced is reported as that one field",
79+
"run": [
80+
"changed",
81+
"{repo}",
82+
"--old",
83+
"{repo}/old-c.js",
84+
"--new",
85+
"{repo}/new-c.js",
86+
"--file",
87+
"src/c.js"
88+
],
89+
"want": [
90+
"signature Auth.<constructor>",
91+
"-jwt",
92+
"+tokens"
93+
],
94+
"avoid": [
95+
"-users",
96+
"-clock",
97+
"removed Auth"
98+
]
99+
},
100+
{
101+
"why": "an edit inside a default value is the function's own parameter change, not a change to the arrow written inside the default",
102+
"run": [
103+
"changed",
104+
"{repo}",
105+
"--old",
106+
"{repo}/old.js",
107+
"--new",
108+
"{repo}/new-default.js",
109+
"--file",
110+
"src/a.js",
111+
"--impact"
112+
],
113+
"want": [
114+
"signature stamp",
115+
"clock: default",
116+
"twice"
117+
],
118+
"avoid": [
119+
"signature millis",
120+
"parameter clock of millis"
121+
]
122+
},
123+
{
124+
"why": "a field whose assignment moved and changed (`this.n = 2` now first in the constructor) is still assigned, so it is not removed",
125+
"run": [
126+
"changed",
127+
"{repo}",
128+
"--old",
129+
"{repo}/old.js",
130+
"--new",
131+
"{repo}/new-reorder.js",
132+
"--file",
133+
"src/a.js"
134+
],
135+
"want": [
136+
"field S.n",
137+
"still assigned"
138+
],
139+
"avoid": [
140+
"removed S.n"
141+
]
142+
},
143+
{
144+
"why": "a graph that already holds the new name reads the old header as that name renamed, never as a 'return type' change",
145+
"run": [
146+
"changed",
147+
"{repo}",
148+
"--old",
149+
"{repo}/old-d.js",
150+
"--new",
151+
"{repo}/new-d.js",
152+
"--file",
153+
"src/d.js"
154+
],
155+
"want": [
156+
"renamed stale → fresh"
157+
],
158+
"avoid": [
159+
"return type"
160+
]
161+
},
162+
{
163+
"why": "control: a body edit is a body change of that method, and no field of the constructor is touched",
164+
"run": [
165+
"changed",
166+
"{repo}",
167+
"--old",
168+
"{repo}/old.js",
169+
"--new",
170+
"{repo}/new-body.js",
171+
"--file",
172+
"src/a.js"
173+
],
174+
"want": [
175+
"body S.oldName"
176+
],
177+
"avoid": [
178+
"signature ",
179+
"removed ",
180+
"field "
181+
]
182+
},
183+
{
184+
"why": "control: a statement added to a constructor body leaves its header and its untouched field this.name alone",
185+
"run": [
186+
"changed",
187+
"{repo}",
188+
"--old",
189+
"{repo}/old.js",
190+
"--new",
191+
"{repo}/new-errbody.js",
192+
"--file",
193+
"src/a.js"
194+
],
195+
"want": [
196+
"body Err.<constructor>"
197+
],
198+
"avoid": [
199+
"Err.name",
200+
"signature ",
201+
"removed "
202+
]
203+
}
204+
]
205+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
export class S {
2+
constructor({ a, b }) {
3+
this.a = a;
4+
this.b = b;
5+
this.n = 1;
6+
}
7+
8+
async oldName(x, y) {
9+
const s = x + y;
10+
return s * 2;
11+
}
12+
13+
keep(v) {
14+
return v;
15+
}
16+
}
17+
18+
export function stamp(v, clock = { millis: () => Date.now() }) {
19+
return { v, at: clock.millis() };
20+
}
21+
22+
export class Err extends Error {
23+
constructor(msg) {
24+
super(msg);
25+
this.name = 'Err';
26+
}
27+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export class Auth {
2+
constructor({
3+
users,
4+
tokens,
5+
clock,
6+
}) {
7+
this.users = users;
8+
this.tokens = tokens;
9+
this.clock = clock;
10+
}
11+
12+
check(token) {
13+
return this.tokens.verify(token);
14+
}
15+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
export class S {
2+
constructor({ a, c }) {
3+
this.a = a;
4+
this.c = c;
5+
this.n = 1;
6+
}
7+
8+
async oldName(x, y) {
9+
return x + y;
10+
}
11+
12+
keep(v) {
13+
return v;
14+
}
15+
}
16+
17+
export function stamp(v, clock = { millis: () => Date.now() }) {
18+
return { v, at: clock.millis() };
19+
}
20+
21+
export class Err extends Error {
22+
constructor(msg) {
23+
super(msg);
24+
this.name = 'Err';
25+
}
26+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export class R {
2+
async fresh(x) {
3+
return x + 1;
4+
}
5+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
export class S {
2+
constructor({ a, b }) {
3+
this.a = a;
4+
this.b = b;
5+
this.n = 1;
6+
}
7+
8+
async oldName(x, y) {
9+
return x + y;
10+
}
11+
12+
keep(v) {
13+
return v;
14+
}
15+
}
16+
17+
export function stamp(v, clock = { millis: () => Date.now() + 1 }) {
18+
return { v, at: clock.millis() };
19+
}
20+
21+
export class Err extends Error {
22+
constructor(msg) {
23+
super(msg);
24+
this.name = 'Err';
25+
}
26+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
export class S {
2+
constructor({ a, b, c }) {
3+
this.a = a;
4+
this.b = b;
5+
this.n = 1;
6+
}
7+
8+
async oldName(x, y) {
9+
return x + y;
10+
}
11+
12+
keep(v) {
13+
return v;
14+
}
15+
}
16+
17+
export function stamp(v, clock = { millis: () => Date.now() }) {
18+
return { v, at: clock.millis() };
19+
}
20+
21+
export class Err extends Error {
22+
constructor(msg) {
23+
super(msg);
24+
this.name = 'Err';
25+
}
26+
}

0 commit comments

Comments
 (0)