Repository navigation
fix: perch scan --since read unchanged methods in changed files - #439
Conversation
Only defect questions were limited to methods a hunk touched. Security, lint and custom rules, including rules shown a document, were still asked of every method in a changed file. --since now reads only the methods a hunk touched; the rest of the file stays in view as callers and callees.
![perchcode-bot[bot]](https://avatars.githubusercontent.com/in/5082622?s=60&v=4){kind=small}
There was a problem hiding this comment.
Perch found 7 issues in code this pull request changed. See the scan
| * a free one, which beats a request spent on ranking. One search need not wait on another. | ||
| */ | ||
| export async function searchUnits({ rules, scan, graph, files, tree, eligible, revision, systemOne, inScope, ignores, min = BELIEVED, | ||
| export async function searchUnits({ rules, scan, graph, files, tree, eligible, revision, systemOne, inScope, changed, ignores, min = BELIEVED, |
There was a problem hiding this comment.
Fixed: the scan of f777179 no longer reports this.
P1 Defect: wrong lookup (93%)
Perch believes searchUnits has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::searchUnits. See the scan
There was a problem hiding this comment.
The scan of f777179 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| @@ -112,7 +112,7 @@ const createWalk = (graph, candidates, inScope = () => true) => { | |||
| const visited = new Set(), stack = [], ranked = candidates.map(candidate => candidate.id); | |||
| const enqueue = ids => { | |||
There was a problem hiding this comment.
Fixed: the scan of 21a76bc no longer reports this.
P2 Defect: wrong lookup (91%)
Perch believes createWalk.enqueue has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::createWalk.enqueue. See the scan
There was a problem hiding this comment.
The scan of 21a76bc no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| && !graph.nodes.get(id).test && inScope(id)); | ||
| stack.push(...valid.sort((a, b) => score(a) - score(b))); | ||
| }; | ||
| const next = () => { |
There was a problem hiding this comment.
Fixed: the scan of 21a76bc no longer reports this.
P2 Defect: wrong lookup (78%)
Perch believes createWalk.next has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::createWalk.next. See the scan
There was a problem hiding this comment.
The scan of 21a76bc no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| const linesOf = createLineReader(root, graph), walk = createWalk(graph, candidates, inScope); | ||
| const linesOf = createLineReader(root, graph), walk = createWalk(graph, candidates, id => inScope(graph.nodes.get(id).path) && changed(id)); | ||
| const askedByMethod = new Map(); | ||
| const stepFor = async nodeId => { |
There was a problem hiding this comment.
Fixed: the scan of 21a76bc no longer reports this.
P2 Defect: inverted condition (66%)
Perch believes scanRepository.stepFor has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::scanRepository.stepFor. See the scan
There was a problem hiding this comment.
The scan of 21a76bc no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| * `mentions <text>`, which is a text match and finds the methods worth asking rather than the methods that are guilty. | ||
| */ | ||
| export function selectUnits(rule, { scan, graph, files, tree, eligible = null, inScope = () => true, ignores = () => false }) { | ||
| export function selectUnits(rule, { scan, graph, files, tree, eligible = null, inScope = () => true, changed = () => true, ignores = () => false }) { |
There was a problem hiding this comment.
Fixed: the scan of f777179 no longer reports this.
Lint: no-silent-failure (100%)
Perch believes selectUnits has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::selectUnits. See the scan
There was a problem hiding this comment.
The scan of f777179 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| // `changed` narrows a method or test in them to one a hunk touched. A unit whose document the branch changed is in it too: the | ||
| // code judged by a vocabulary has to be asked again when the vocabulary moves. | ||
| // What perch.yaml ignores stays out either way, since a document being in scope says nothing about the unit's own path. | ||
| const spared = unit => !ignores(unit.path) && !ours(unit.path) |
There was a problem hiding this comment.
Fixed: the scan of f777179 no longer reports this.
Lint: no-silent-failure (100%)
Perch believes selectUnits.spared has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::selectUnits.spared. See the scan
There was a problem hiding this comment.
The scan of f777179 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| && (inScope(unit.path) && (!unit.part || changed(unit)) || Boolean(unit.context) && inScope(unit.context.path)) | ||
| && appliesToLanguage(rule, languageOf(unit.path)) | ||
| && (!rule.except || ![rule.except].flat().some(glob => matches(glob, unit.path))); | ||
| const keep = units => units.map(shown).filter(unit => unit && spared(unit)); |
There was a problem hiding this comment.
Fixed: the scan of f777179 no longer reports this.
Lint: no-silent-failure (100%)
Perch believes selectUnits.keep has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::selectUnits.keep. See the scan
There was a problem hiding this comment.
The scan of f777179 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
|
Perch Scan: no issues in 92 methods at 2 issues fixed since the last push. |
There was a problem hiding this comment.
Perch found 2 issues in code this pull request changed. See the scan
| } | ||
| // A method is changed when a line the branch added or deleted is one of its own. A hunk's context lines are not, and neither | ||
| // are the methods inside a top-level unit's span: its `lines` are what it covers. | ||
| const owns = node => node.lines ? (own => at => own.has(at))(new Set(node.lines)) : at => at >= node.line && at <= node.end_line; |
There was a problem hiding this comment.
Fixed: the scan of f777179 no longer reports this.
P1 Defect: off by one (82%)
Perch believes createChangeReader.owns has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/change-context.js::createChangeReader.owns. See the scan
There was a problem hiding this comment.
The scan of f777179 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| * what it covers. | ||
| */ | ||
| const createWalk = (graph, candidates, inScope = () => true) => { | ||
| const createWalk = (graph, candidates, reads = () => true) => { |
There was a problem hiding this comment.
Fixed: the scan of f777179 no longer reports this.
P1 Defect: wrong lookup (63%)
Perch believes createWalk has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::createWalk. See the scan
There was a problem hiding this comment.
The scan of f777179 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
Close the retry-rule findings on methods this branch did not change.
There was a problem hiding this comment.
Perch found an issue in code this pull request changed and is still reading. See the scan
| * with callers and callees. The two ask nothing in common. A method no hunk touched is not read at all, and without a change | ||
| * there is no diff to ask the bug questions over. | ||
| */ | ||
| export function sinceQuestions(asked, changed) { |
There was a problem hiding this comment.
Fixed: the scan of cd6d817 no longer reports this.
P2 Defect: off by one (76%)
Perch believes sinceQuestions has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::sinceQuestions. See the scan
There was a problem hiding this comment.
The scan of cd6d817 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
There was a problem hiding this comment.
Perch found an issue in code this pull request changed. See the scan
| * a free one, which beats a request spent on ranking. One search need not wait on another. | ||
| */ | ||
| export async function searchUnits({ rules, scan, graph, files, tree, eligible, revision, systemOne, inScope, ignores, min = BELIEVED, | ||
| export async function searchUnits({ rules, scan, graph, files, tree, eligible, revision, systemOne, inScope, changed, ignores, min = BELIEVED, |
There was a problem hiding this comment.
Fixed: the scan of cd6d817 no longer reports this.
P1 Defect: off by one (99%)
Perch believes searchUnits has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::searchUnits. See the scan
There was a problem hiding this comment.
The scan of cd6d817 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
There was a problem hiding this comment.
Perch found 3 issues in code this pull request changed and is still reading. See the scan
| * with callers and callees. The two ask nothing in common. A method no hunk touched is not read at all, and without a change | ||
| * there is no diff to ask the bug questions over. | ||
| */ | ||
| export function sinceQuestions(asked, changed) { |
There was a problem hiding this comment.
Fixed: the scan of cd6d817 no longer reports this.
P2 Defect: other defect (76%)
Perch believes sinceQuestions has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::sinceQuestions. See the scan
There was a problem hiding this comment.
The scan of cd6d817 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| @@ -305,7 +305,7 @@ export function neighbourhood(sees, unit, { graph, files, max = MAX_SEEN }) { | |||
| const text = files.get(unit.path) ?? ''; | |||
| if (sees === 'file') return { file_source: text }; | |||
| const show = ids => ids.slice(0, max).map(id => graph.nodes.get(id)).filter(Boolean) | |||
There was a problem hiding this comment.
Fixed: the scan of cd6d817 no longer reports this.
P2 Defect: other defect (67%)
Perch believes neighbourhood.show has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::neighbourhood.show. See the scan
There was a problem hiding this comment.
The scan of cd6d817 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| * `mentions <text>`, which is a text match and finds the methods worth asking rather than the methods that are guilty. | ||
| */ | ||
| export function selectUnits(rule, { scan, graph, files, tree, eligible = null, inScope = () => true, ignores = () => false }) { | ||
| export function selectUnits(rule, { scan, graph, files, tree, eligible = null, inScope = () => true, changed = () => true, ignores = () => false }) { |
There was a problem hiding this comment.
Fixed: the scan of cd6d817 no longer reports this.
P2 Defect: other defect (62%)
Perch believes selectUnits has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::selectUnits. See the scan
There was a problem hiding this comment.
The scan of cd6d817 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
…stigation-3db22b # Conflicts: # .perch/closed.jsonl
There was a problem hiding this comment.
Perch found 2 issues in code this pull request changed and is still reading. See the scan
| const linesOf = createLineReader(root, graph), walk = createWalk(graph, candidates, inScope); | ||
| const linesOf = createLineReader(root, graph), walk = createWalk(graph, candidates, id => inScope(graph.nodes.get(id).path) && changed(id)); | ||
| const askedByMethod = new Map(); | ||
| const stepFor = async nodeId => { |
There was a problem hiding this comment.
Fixed: the scan of b1fd574 no longer reports this.
P2 Defect: other defect (70%)
Perch believes scanRepository.stepFor has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/scan.js::scanRepository.stepFor. See the scan
There was a problem hiding this comment.
The scan of b1fd574 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
| * that failed about which line failed on it. `documents` holds the text of every document a rule can be shown. | ||
| */ | ||
| export async function askUnits({ rules, scan, graph, files, tree, eligible, revision, systemOne, inScope, ignores, min = BELIEVED, | ||
| export async function askUnits({ rules, scan, graph, files, tree, eligible, revision, systemOne, inScope, changed, ignores, min = BELIEVED, |
There was a problem hiding this comment.
Fixed: the scan of b1fd574 no longer reports this.
P2 Defect: bad state change (66%)
Perch believes askUnits has this problem. It is a probability, not a located defect: the cause may sit a few lines away.
Check a fix with perch check src/units.js::askUnits. See the scan
There was a problem hiding this comment.
The scan of b1fd574 no longer reports this, so Perch marked it fixed. Resolve this thread when you are done with it. See the scan
perch scan --sincelimited only the defect questions to methods a hunk touched. Security, lint and custom rules were asked of every method in a changed file, so a one-line change in a large file paid for a reading of every method in it.--sincenow reads only the methods a hunk touched, for every question and for method rules shown a document. Unchanged methods are still sent as callers and callees, and findings an earlier scan made on them are still reported.Closes #438