Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(assert): prepare for noUncheckedIndexedAccess #4045

Merged
merged 15 commits into from
Jan 10, 2024
Merged

refactor(assert): prepare for noUncheckedIndexedAccess #4045

Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8d4acbd
refactor(assert): prepare for noUncheckedIndexedAccess
syhol Dec 31, 2023
30298f8
remove bad bench
syhol Dec 31, 2023
bac997f
use more undefined where expected
syhol Dec 31, 2023
ac23b92
no deno.json change
syhol Dec 31, 2023
0b6ea87
better error message
syhol Dec 31, 2023
3fe1090
tweak
iuioiua Jan 1, 2024
b9fdee7
Merge branch 'main' into assert-no-unchecked-indexed-access
iuioiua Jan 1, 2024
99dce77
One line per const declaration
syhol Jan 3, 2024
99893d0
Replace undefined type with optional parameter
syhol Jan 3, 2024
94df1b6
Replace undefined type with optional parameter again
syhol Jan 3, 2024
f852d18
Pass type to ensureDefined
syhol Jan 3, 2024
da21e49
ensureDefined moved to top level
syhol Jan 3, 2024
8aedb7c
slide/down FarthestPoint changed the optional param back to or undefined
syhol Jan 3, 2024
9ce47fc
Merge branch 'main' into assert-no-unchecked-indexed-access
syhol Jan 3, 2024
a929c01
Merge branch 'main' into assert-no-unchecked-indexed-access
iuioiua Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 46 additions & 29 deletions assert/_diff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@
const ADDED = 3;

function createCommon<T>(A: T[], B: T[], reverse?: boolean): T[] {
const common = [];
const common: T[] = [];
if (A.length === 0 || B.length === 0) return [];
for (let i = 0; i < Math.min(A.length, B.length); i += 1) {
if (
A[reverse ? A.length - i - 1 : i] === B[reverse ? B.length - i - 1 : i]
) {
common.push(A[reverse ? A.length - i - 1 : i]);
common.push(A[reverse ? A.length - i - 1 : i]!);
iuioiua marked this conversation as resolved.
Show resolved Hide resolved
} else {
return common;
}
Expand Down Expand Up @@ -95,6 +95,7 @@
{ length: size },
() => ({ y: -1, id: -1 }),
);

/**
* INFO:
* This buffer is used to save memory and improve performance.
Expand All @@ -119,28 +120,28 @@
}> {
const M = A.length;
const N = B.length;
const result = [];
const result: { type: DiffType; value: T }[] = [];
let a = M - 1;
let b = N - 1;
let j = routes[current.id];
let type = routes[current.id + diffTypesPtrOffset];
while (true) {
if (!j && !type) break;
const prev = j;
const prev = j!;
if (type === REMOVED) {
result.unshift({
type: swapped ? DiffType.removed : DiffType.added,
value: B[b],
value: B[b]!,
});
b -= 1;
} else if (type === ADDED) {
result.unshift({
type: swapped ? DiffType.added : DiffType.removed,
value: A[a],
value: A[a]!,
});
a -= 1;
} else {
result.unshift({ type: DiffType.common, value: A[a] });
result.unshift({ type: DiffType.common, value: A[a]! });
Comment on lines +141 to +151
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel a bit unsure about this one. We can't say for certain if these indices are correct as it's all dependent on if routes was built correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure what to do here too. @kt3k, WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff is hugely used across Deno ecosystem (via assertEquals). I don't think this includes such basic bugs. I think it's fine to use non null assertion here.

a -= 1;
b -= 1;
}
Expand All @@ -151,37 +152,38 @@
}

function createFP(
slide: FarthestPoint,
down: FarthestPoint,
slide: FarthestPoint | undefined,
down: FarthestPoint | undefined,
iuioiua marked this conversation as resolved.
Show resolved Hide resolved
k: number,
M: number,
): FarthestPoint {
if (slide && slide.y === -1 && down && down.y === -1) {
return { y: 0, id: 0 };
}
if (
(down && down.y === -1) ||
const isAdding = (down?.y === -1) ||
k === M ||
(slide && slide.y) > (down && down.y) + 1
) {
(slide?.y || 0) > (down?.y || 0) + 1;
if (slide && isAdding) {
const prev = slide.id;
ptr++;
routes[ptr] = prev;
routes[ptr + diffTypesPtrOffset] = ADDED;
return { y: slide.y, id: ptr };
} else {
} else if (down && !isAdding) {
const prev = down.id;
ptr++;
routes[ptr] = prev;
routes[ptr + diffTypesPtrOffset] = REMOVED;
return { y: down.y + 1, id: ptr };
} else {
throw new Error("Unexpected missing FarthestPoint");

Check warning on line 179 in assert/_diff.ts

View check run for this annotation

Codecov / codecov/patch

assert/_diff.ts#L179 

Added line #L179 was not covered by tests
}
}

function snake<T>(
k: number,
slide: FarthestPoint,
down: FarthestPoint,
slide: FarthestPoint | undefined,
down: FarthestPoint | undefined,
syhol marked this conversation as resolved.
Show resolved Hide resolved
_offset: number,
A: T[],
B: T[],
Expand All @@ -201,7 +203,15 @@
return fp;
}

while (fp[delta + offset].y < N) {
function ensureDefined<T>(item: T | undefined): T {
if (!item) {
throw Error("Unexpected missing FarthestPoint");
}

Check warning on line 209 in assert/_diff.ts

View check run for this annotation

Codecov / codecov/patch

assert/_diff.ts#L208-L209 

Added lines #L208 - L209 were not covered by tests
return item;
}
syhol marked this conversation as resolved.
Show resolved Hide resolved

let currentFP: FarthestPoint = ensureDefined(fp[delta + offset]);
syhol marked this conversation as resolved.
Show resolved Hide resolved
while (currentFP && currentFP.y < N) {
p = p + 1;
for (let k = -p; k < delta; ++k) {
fp[k + offset] = snake(
Expand Down Expand Up @@ -231,12 +241,13 @@
A,
B,
);
currentFP = ensureDefined(fp[delta + offset]);
}
return [
...prefixCommon.map(
(c): DiffResult<typeof c> => ({ type: DiffType.common, value: c }),
),
...backTrace(A, B, fp[delta + offset], swapped),
...backTrace(A, B, currentFP, swapped),
...suffixCommon.map(
(c): DiffResult<typeof c> => ({ type: DiffType.common, value: c }),
),
Expand Down Expand Up @@ -274,31 +285,36 @@

// Join boundary splits that we do not consider to be boundaries and merge empty strings surrounded by word chars
for (let i = 0; i < tokens.length - 1; i++) {
const token = tokens[i];
const tokenPlusTwo = tokens[i + 2];
if (
!tokens[i + 1] && tokens[i + 2] && words.test(tokens[i]) &&
words.test(tokens[i + 2])
!tokens[i + 1] &&
token &&
tokenPlusTwo &&
words.test(token) &&
words.test(tokenPlusTwo)

Check warning on line 295 in assert/_diff.ts

View check run for this annotation

Codecov / codecov/patch

assert/_diff.ts#L291-L295 

Added lines #L291 - L295 were not covered by tests
) {
tokens[i] += tokens[i + 2];
tokens[i] += tokenPlusTwo;

Check warning on line 297 in assert/_diff.ts

View check run for this annotation

Codecov / codecov/patch

assert/_diff.ts#L297 

Added line #L297 was not covered by tests
tokens.splice(i + 1, 2);
i--;
}
}
return tokens.filter((token) => token);
} else {
// Split string on new lines symbols
const tokens = [], lines = string.split(/(\n|\r\n)/);
const tokens: string[] = [], lines = string.split(/(\n|\r\n)/);
syhol marked this conversation as resolved.
Show resolved Hide resolved

// Ignore final empty token when text ends with a newline
if (!lines[lines.length - 1]) {
lines.pop();
}

// Merge the content and line separators into single tokens
for (let i = 0; i < lines.length; i++) {
for (const [i, line] of lines.entries()) {
if (i % 2) {
tokens[tokens.length - 1] += lines[i];
tokens[tokens.length - 1] += line;
} else {
tokens.push(lines[i]);
tokens.push(line);
}
}
return tokens;
Expand All @@ -314,13 +330,14 @@
return tokens.filter(({ type }) =>
type === line.type || type === DiffType.common
).map((result, i, t) => {
const token = t[i - 1];
if (
(result.type === DiffType.common) && (t[i - 1]) &&
(t[i - 1]?.type === t[i + 1]?.type) && /\s+/.test(result.value)
(result.type === DiffType.common) && token &&
(token.type === t[i + 1]?.type) && /\s+/.test(result.value)
) {
return {
...result,
type: t[i - 1].type,
type: token.type,
};
}
return result;
Expand Down Expand Up @@ -356,7 +373,7 @@
const tokenized = [
tokenize(a.value, { wordDiff: true }),
tokenize(b?.value ?? "", { wordDiff: true }),
] as string[][];
] as [string[], string[]];
if (hasMoreRemovedLines) tokenized.reverse();
tokens = diff(tokenized[0], tokenized[1]);
if (
Expand Down