mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
test: давать послабление переноса блоку, а не строке (#592)
Находка M1 код-ревью r1, воспроизведена: первая редакция сопоставляла одиночные строки по всему диффу, и этого хватало для обхода. Несвязанная уборка удаляет где-то строку с `any`, новый код добавляет свою — текстуально такую же, — и гейт молчит. Совпадение здесь не экзотика: в базе 887 явных `any`, типовые однострочники повторяются буквально, и две такие строки встретились в самом коммите переноса. Теперь перенесённым признаётся только непрерывный кусок не короче пяти строк, встречающийся подряд и целиком среди удалённых строк ОДНОГО файла. Случайно совпасть пятью строками подряд практически невозможно, а настоящее извлечение подсистемы из таких кусков и состоит: на этом диффе признано 1296 строк из 1395 — на одну меньше, чем при построчном сопоставлении, и эта одна была ровно случайным совпадением. Каждый удалённый кусок оплачивает ровно одно добавление: повторная вставка того же блока остаётся новым кодом. Мутант заменён на `no-new-any-forgives-a-single-matching-line` — он опускает порог до одной строки, то есть открывает ровно найденную дыру; тест обхода на нём краснеет. Тестов пять: перенос куска, обход одиночной строкой, кусок короче порога, бюджет на повторную вставку, смена отступа. Issue: #592 User-Visible: no
This commit is contained in:
@@ -10136,15 +10136,16 @@ const MUTANT_DEFINITIONS = [
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'no-new-any-counts-every-added-line-as-moved',
|
||||
id: 'no-new-any-forgives-a-single-matching-line',
|
||||
guard: 'node --test test/no-new-any.test.mjs',
|
||||
because: '#592: послабление для переноса держится на точном совпадении текста и бюджете '
|
||||
+ 'удалений. Если признать перенесённой любую добавленную строку, гейт #342 перестанет '
|
||||
+ 'ловить новый any вовсе — и сделает это молча, оставшись зелёным',
|
||||
because: '#592 (ревью r1, M1): послабление для переноса держится на длине куска. Стоит '
|
||||
+ 'признать переносом одиночное совпадение — и гейт #342 обходится тривиально: '
|
||||
+ 'несвязанная уборка удаляет типовую строку с any, новый код добавляет такую же. '
|
||||
+ 'В базе 887 явных any, однострочники повторяются буквально',
|
||||
patches: [{
|
||||
file: 'scripts/no-new-any.mjs',
|
||||
find: ' const budget = removed.get(item.text) || 0;\n if (!budget) continue;',
|
||||
replace: ' const budget = removed.get(item.text) || Number(item.line >= 0);\n if (!budget) continue;',
|
||||
find: 'export const MOVED_BLOCK_MIN = 5;',
|
||||
replace: 'export const MOVED_BLOCK_MIN = 1;',
|
||||
}],
|
||||
},
|
||||
];
|
||||
|
||||
+95
-26
@@ -28,13 +28,27 @@
|
||||
* «надо») гейт не проходят. Формулировка вида «внешний контракт HA не
|
||||
* типизирован» проходит.
|
||||
*
|
||||
* Второе исключение — перенос (#592). Строка, которая в этом же диапазоне
|
||||
* удалена из одного файла и добавлена в другой дословно, новым кодом не
|
||||
* является: ответственность за её тип не менялась, и долг в `src/**` не вырос.
|
||||
* Гейт считает такие строки перенесёнными и не судит их, но бюджет ведёт
|
||||
* мультимножеством: два добавления при одном удалении дают одну находку.
|
||||
* Без этого любое извлечение подсистемы — то самое, чем долг и снимается по
|
||||
* замыслу #342, — краснит гейт ровно за то, что ничего не изменило.
|
||||
* Второе исключение — перенос (#592). Код, который в этом же диапазоне удалён
|
||||
* из одного файла и дословно добавлен в другой, новым не является:
|
||||
* ответственность за его типы не менялась, и долг в `src/**` не вырос. Без
|
||||
* этого послабления любое извлечение подсистемы — то самое, чем долг и
|
||||
* снимается по замыслу #342, — краснит гейт ровно за то, что ничего не
|
||||
* изменило.
|
||||
*
|
||||
* Послабление даётся БЛОКУ, а не строке (ревью кода #592, M1). Первая редакция
|
||||
* сопоставляла одиночные строки по всему диффу, и этого хватало для обхода:
|
||||
* несвязанная уборка удаляет где-то строку с `any`, а новый код добавляет свою,
|
||||
* текстуально совпадающую, — и гейт молчит. Совпадение тут не экзотика: в этой
|
||||
* базе 887 явных `any`, и типовые однострочники вроде
|
||||
* `<span>${...(k as any)}</span>` повторяются буквально (две таких строки
|
||||
* встретились в самом коммите переноса).
|
||||
*
|
||||
* Поэтому перенесённым признаётся только непрерывный кусок длиной не меньше
|
||||
* MOVED_BLOCK_MIN строк, встречающийся подряд и целиком среди удалённых строк
|
||||
* ОДНОГО файла. Случайно совпасть пятью строками подряд в двух местах одного
|
||||
* диффа практически невозможно, а настоящее извлечение подсистемы состоит из
|
||||
* таких кусков по определению. Каждый удалённый кусок оплачивает ровно одно
|
||||
* добавление: повторная вставка того же блока остаётся новым кодом.
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
@@ -135,22 +149,30 @@ export function addedLinesByFile(diff) {
|
||||
return files;
|
||||
}
|
||||
|
||||
/** Минимальная длина непрерывного куска, который считается переносом. */
|
||||
export const MOVED_BLOCK_MIN = 5;
|
||||
|
||||
/**
|
||||
* Строки, добавленные одним файлом и дословно удалённые другим (#592).
|
||||
* Строки, которые приехали в файл переносом непрерывного куска (#592).
|
||||
*
|
||||
* Возвращает по файлу номера таких строк. Сравнение точное, без обрезки
|
||||
* пробелов: перенос с изменением отступа — уже правка, и судить её гейт обязан.
|
||||
* Бюджет ведётся мультимножеством: одно удаление покрывает одно добавление.
|
||||
* пробелов: перенос с изменённым отступом — уже правка, и судить её гейт обязан.
|
||||
*/
|
||||
export function movedLinesByFile(diff) {
|
||||
const removed = new Map();
|
||||
const added = [];
|
||||
let current = null;
|
||||
export function movedLinesByFile(diff, { minBlock = MOVED_BLOCK_MIN } = {}) {
|
||||
const removedByFile = new Map();
|
||||
const addedByFile = new Map();
|
||||
let target = null;
|
||||
let sourcePath = null;
|
||||
let next = 0;
|
||||
for (const raw of String(diff).split('\n')) {
|
||||
if (raw.startsWith('--- ')) {
|
||||
const path = raw.slice(4).replace(/^a\//, '');
|
||||
sourcePath = path === '/dev/null' ? null : path;
|
||||
continue;
|
||||
}
|
||||
if (raw.startsWith('+++ ')) {
|
||||
const path = raw.slice(4).replace(/^b\//, '');
|
||||
current = path === '/dev/null' ? null : path;
|
||||
target = path === '/dev/null' ? null : path;
|
||||
continue;
|
||||
}
|
||||
if (raw.startsWith('@@')) {
|
||||
@@ -158,24 +180,71 @@ export function movedLinesByFile(diff) {
|
||||
next = match ? Number(match[1]) : 0;
|
||||
continue;
|
||||
}
|
||||
if (raw.startsWith('---') || raw.startsWith('diff --git')) continue;
|
||||
if (raw.startsWith('diff --git')) continue;
|
||||
if (raw.startsWith('-')) {
|
||||
const text = raw.slice(1);
|
||||
removed.set(text, (removed.get(text) || 0) + 1);
|
||||
const key = sourcePath || target || '';
|
||||
if (!removedByFile.has(key)) removedByFile.set(key, []);
|
||||
removedByFile.get(key).push(raw.slice(1));
|
||||
continue;
|
||||
}
|
||||
if (!target || !next) continue;
|
||||
if (raw.startsWith('+')) {
|
||||
if (!addedByFile.has(target)) addedByFile.set(target, []);
|
||||
addedByFile.get(target).push({ line: next, text: raw.slice(1) });
|
||||
next += 1;
|
||||
continue;
|
||||
}
|
||||
if (!current || !next) continue;
|
||||
if (raw.startsWith('+')) { added.push({ path: current, line: next, text: raw.slice(1) }); next += 1; continue; }
|
||||
if (raw.startsWith('\\')) continue;
|
||||
next += 1;
|
||||
}
|
||||
|
||||
// Позиции удалённых строк по тексту — чтобы искать начало куска за один шаг.
|
||||
const index = new Map();
|
||||
for (const [path, lines] of removedByFile) {
|
||||
lines.forEach((text, at) => {
|
||||
if (!index.has(text)) index.set(text, []);
|
||||
index.get(text).push({ path, at });
|
||||
});
|
||||
}
|
||||
const spent = new Map();
|
||||
const isSpent = (path, at) => spent.get(path)?.has(at) === true;
|
||||
const spend = (path, from, length) => {
|
||||
if (!spent.has(path)) spent.set(path, new Set());
|
||||
for (let step = 0; step < length; step += 1) spent.get(path).add(from + step);
|
||||
};
|
||||
|
||||
const moved = new Map();
|
||||
for (const item of added) {
|
||||
const budget = removed.get(item.text) || 0;
|
||||
if (!budget) continue;
|
||||
removed.set(item.text, budget - 1);
|
||||
if (!moved.has(item.path)) moved.set(item.path, new Set());
|
||||
moved.get(item.path).add(item.line);
|
||||
for (const [path, added] of addedByFile) {
|
||||
// Куски считаются по непрерывным номерам строк: разрыв — конец куска.
|
||||
const runs = [];
|
||||
for (const item of added) {
|
||||
const last = runs[runs.length - 1];
|
||||
if (last && last[last.length - 1].line + 1 === item.line) last.push(item);
|
||||
else runs.push([item]);
|
||||
}
|
||||
for (const run of runs) {
|
||||
let at = 0;
|
||||
while (at < run.length) {
|
||||
let best = null;
|
||||
for (const start of index.get(run[at].text) || []) {
|
||||
if (isSpent(start.path, start.at)) continue;
|
||||
const source = removedByFile.get(start.path) || [];
|
||||
let length = 0;
|
||||
while (at + length < run.length
|
||||
&& start.at + length < source.length
|
||||
&& !isSpent(start.path, start.at + length)
|
||||
&& source[start.at + length] === run[at + length].text) length += 1;
|
||||
if (length >= minBlock && (!best || length > best.length)) {
|
||||
best = { path: start.path, at: start.at, length };
|
||||
}
|
||||
}
|
||||
if (!best) { at += 1; continue; }
|
||||
spend(best.path, best.at, best.length);
|
||||
if (!moved.has(path)) moved.set(path, new Set());
|
||||
for (let step = 0; step < best.length; step += 1) moved.get(path).add(run[at + step].line);
|
||||
at += best.length;
|
||||
}
|
||||
}
|
||||
}
|
||||
return moved;
|
||||
}
|
||||
|
||||
+85
-39
@@ -2,6 +2,7 @@ import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
import {
|
||||
MOVED_BLOCK_MIN,
|
||||
addedLinesByFile, anyKeywordLines, blameLine, findNewAnyViolations, formatViolation,
|
||||
movedLinesByFile, parseAnyOk,
|
||||
} from '../scripts/no-new-any.mjs';
|
||||
@@ -137,61 +138,106 @@ test('недоступный blame не выдумывает источник и
|
||||
// выглядит для диффа как тысяча добавленных строк. Судить по ним «новый код»
|
||||
// значит требовать типизации ровно там, где ничего не изменилось, и заодно
|
||||
// ломать доказательство переноса: тело обязано совпадать побайтово.
|
||||
test('#592 строка, перенесённая дословно, новым кодом не считается', () => {
|
||||
//
|
||||
// Послабление даётся блоку, а не строке (ревью кода #592, M1): одиночное
|
||||
// совпадение текста подделывается слишком легко.
|
||||
|
||||
const block = (n, prefix = 'line') => Array.from({ length: n }, (_, i) => ` const ${prefix}${i} = ${i};`);
|
||||
|
||||
test('#592 непрерывный перенесённый кусок новым кодом не считается', () => {
|
||||
const body = [...block(3), ' const handler = (e: any) => e;', ...block(3, 'tail')];
|
||||
const diff = [
|
||||
'diff --git a/src/big.ts b/src/big.ts',
|
||||
'--- a/src/big.ts',
|
||||
'+++ b/src/big.ts',
|
||||
'@@ -10,1 +10,0 @@',
|
||||
'- const handler = (e: any) => e;',
|
||||
`@@ -10,${body.length} +10,0 @@`,
|
||||
...body.map((line) => `-${line}`),
|
||||
'diff --git a/src/editors/part.ts b/src/editors/part.ts',
|
||||
'--- /dev/null',
|
||||
'+++ b/src/editors/part.ts',
|
||||
'@@ -0,0 +1,2 @@',
|
||||
'+ const handler = (e: any) => e;',
|
||||
'+ const fresh = (e: any) => e;',
|
||||
`@@ -0,0 +1,${body.length} @@`,
|
||||
...body.map((line) => `+${line}`),
|
||||
].join('\n');
|
||||
const moved = movedLinesByFile(diff);
|
||||
assert.deepEqual([...(moved.get('src/editors/part.ts') || [])], [1],
|
||||
'перенесена первая строка; вторая такого удаления не имеет');
|
||||
const moved = movedLinesByFile(diff).get('src/editors/part.ts');
|
||||
assert.equal(moved?.size, body.length, 'перенесён весь кусок целиком');
|
||||
|
||||
const text = ' const handler = (e: any) => e;\n const fresh = (e: any) => e;\n';
|
||||
const violations = findNewAnyViolations({ files: [{
|
||||
path: 'src/editors/part.ts', text,
|
||||
addedLines: new Set([1, 2]),
|
||||
movedLines: moved.get('src/editors/part.ts'),
|
||||
path: 'src/editors/part.ts', text: `${body.join('\n')}\n`,
|
||||
addedLines: new Set(body.map((_, i) => i + 1)),
|
||||
movedLines: moved,
|
||||
}] });
|
||||
assert.equal(violations.length, 1, 'новый any по-прежнему находка');
|
||||
assert.equal(violations[0].line, 2);
|
||||
assert.deepEqual(violations, [], 'внутри перенесённого куска новых any нет');
|
||||
});
|
||||
|
||||
test('#592 бюджет переноса ведётся мультимножеством, а не признаком', () => {
|
||||
test('#592 одиночное совпадение переносом не считается (M1)', () => {
|
||||
// Обход первой редакции: несвязанная уборка удаляет типовую строку с any,
|
||||
// а новый файл добавляет свою — текстуально такую же. Это не перенос.
|
||||
const line = ' <span>${this.host._t(k as any)}</span>';
|
||||
const diff = [
|
||||
'--- a/src/big.ts',
|
||||
'+++ b/src/big.ts',
|
||||
'@@ -10,1 +10,0 @@',
|
||||
'- const cast = v as any;',
|
||||
'diff --git a/src/unrelated.ts b/src/unrelated.ts',
|
||||
'--- a/src/unrelated.ts',
|
||||
'+++ b/src/unrelated.ts',
|
||||
'@@ -40,1 +40,0 @@',
|
||||
`-${line}`,
|
||||
'diff --git a/src/fresh.ts b/src/fresh.ts',
|
||||
'--- /dev/null',
|
||||
'+++ b/src/editors/part.ts',
|
||||
'@@ -0,0 +1,2 @@',
|
||||
'+ const cast = v as any;',
|
||||
'+ const cast = v as any;',
|
||||
].join('\n');
|
||||
const moved = movedLinesByFile(diff);
|
||||
assert.deepEqual([...(moved.get('src/editors/part.ts') || [])], [1],
|
||||
'одно удаление покрывает одно добавление, второе остаётся новым');
|
||||
});
|
||||
|
||||
test('#592 перенос с изменённым отступом переносом не считается', () => {
|
||||
const diff = [
|
||||
'--- a/src/big.ts',
|
||||
'+++ b/src/big.ts',
|
||||
'@@ -10,1 +10,0 @@',
|
||||
'- const cast = v as any;',
|
||||
'--- /dev/null',
|
||||
'+++ b/src/editors/part.ts',
|
||||
'+++ b/src/fresh.ts',
|
||||
'@@ -0,0 +1,1 @@',
|
||||
'+ const cast = v as any;',
|
||||
`+${line}`,
|
||||
].join('\n');
|
||||
assert.equal(movedLinesByFile(diff).size, 0, 'одна строка переносом не признаётся');
|
||||
|
||||
const violations = findNewAnyViolations({ files: [{
|
||||
path: 'src/fresh.ts', text: `${line}\n`,
|
||||
addedLines: new Set([1]),
|
||||
movedLines: movedLinesByFile(diff).get('src/fresh.ts'),
|
||||
}] });
|
||||
assert.equal(violations.length, 1, 'новый any остаётся находкой');
|
||||
});
|
||||
|
||||
test('#592 кусок короче порога переносом не считается', () => {
|
||||
const body = block(MOVED_BLOCK_MIN - 1);
|
||||
const diff = [
|
||||
'--- a/src/big.ts',
|
||||
'+++ b/src/big.ts',
|
||||
`@@ -10,${body.length} +10,0 @@`,
|
||||
...body.map((line) => `-${line}`),
|
||||
'--- /dev/null',
|
||||
'+++ b/src/editors/part.ts',
|
||||
`@@ -0,0 +1,${body.length} @@`,
|
||||
...body.map((line) => `+${line}`),
|
||||
].join('\n');
|
||||
assert.equal(movedLinesByFile(diff).size, 0);
|
||||
});
|
||||
|
||||
test('#592 один удалённый кусок оплачивает ровно одну вставку', () => {
|
||||
const body = block(MOVED_BLOCK_MIN);
|
||||
const diff = [
|
||||
'--- a/src/big.ts',
|
||||
'+++ b/src/big.ts',
|
||||
`@@ -10,${body.length} +10,0 @@`,
|
||||
...body.map((line) => `-${line}`),
|
||||
'--- /dev/null',
|
||||
'+++ b/src/editors/part.ts',
|
||||
`@@ -0,0 +1,${body.length * 2} @@`,
|
||||
...body.map((line) => `+${line}`),
|
||||
...body.map((line) => `+${line}`),
|
||||
].join('\n');
|
||||
const moved = [...(movedLinesByFile(diff).get('src/editors/part.ts') || [])].sort((a, b) => a - b);
|
||||
assert.deepEqual(moved, body.map((_, i) => i + 1), 'вторая копия остаётся новым кодом');
|
||||
});
|
||||
|
||||
test('#592 перенос с изменённым отступом переносом не считается', () => {
|
||||
const body = block(MOVED_BLOCK_MIN + 2);
|
||||
const diff = [
|
||||
'--- a/src/big.ts',
|
||||
'+++ b/src/big.ts',
|
||||
`@@ -10,${body.length} +10,0 @@`,
|
||||
...body.map((line) => `- ${line.trim()}`),
|
||||
'--- /dev/null',
|
||||
'+++ b/src/editors/part.ts',
|
||||
`@@ -0,0 +1,${body.length} @@`,
|
||||
...body.map((line) => `+ ${line.trim()}`),
|
||||
].join('\n');
|
||||
assert.equal(movedLinesByFile(diff).size, 0);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user