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 1 commit
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
70 changes: 44 additions & 26 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 COMMON = 2;
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,14 @@ export function diff<T>(A: T[], B: T[]): Array<DiffResult<T>> {
{ length: size },
() => ({ y: -1, id: -1 }),
);
function getPoint(index: number) {
const point = fp[index];
if (!point) {
throw Error(`Missing FarthestPoint at index ${index}`);
}
return point;
}
syhol marked this conversation as resolved.
Show resolved Hide resolved
syhol marked this conversation as resolved.
Show resolved Hide resolved

/**
* INFO:
* This buffer is used to save memory and improve performance.
Expand All @@ -119,28 +127,28 @@ export function diff<T>(A: T[], B: T[]): Array<DiffResult<T>> {
}> {
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
Collaborator

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 Down Expand Up @@ -201,13 +209,13 @@ export function diff<T>(A: T[], B: T[]): Array<DiffResult<T>> {
return fp;
}

while (fp[delta + offset].y < N) {
while (getPoint(delta + offset).y < N) {
p = p + 1;
for (let k = -p; k < delta; ++k) {
fp[k + offset] = snake(
k,
fp[k - 1 + offset],
fp[k + 1 + offset],
getPoint(k - 1 + offset),
getPoint(k + 1 + offset),
offset,
A,
B,
Expand All @@ -216,17 +224,17 @@ export function diff<T>(A: T[], B: T[]): Array<DiffResult<T>> {
for (let k = delta + p; k > delta; --k) {
fp[k + offset] = snake(
k,
fp[k - 1 + offset],
fp[k + 1 + offset],
getPoint(k - 1 + offset),
getPoint(k + 1 + offset),
offset,
A,
B,
);
}
fp[delta + offset] = snake(
delta,
fp[delta - 1 + offset],
fp[delta + 1 + offset],
getPoint(delta - 1 + offset),
getPoint(delta + 1 + offset),
offset,
A,
B,
Expand All @@ -236,7 +244,7 @@ export function diff<T>(A: T[], B: T[]): Array<DiffResult<T>> {
...prefixCommon.map(
(c): DiffResult<typeof c> => ({ type: DiffType.common, value: c }),
),
...backTrace(A, B, fp[delta + offset], swapped),
...backTrace(A, B, getPoint(delta + offset), swapped),
...suffixCommon.map(
(c): DiffResult<typeof c> => ({ type: DiffType.common, value: c }),
),
Expand Down Expand Up @@ -274,31 +282,36 @@ export function diffstr(A: string, B: string) {

// 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)
) {
tokens[i] += tokens[i + 2];
tokens[i] += tokenPlusTwo;
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 +327,14 @@ export function diffstr(A: string, B: string) {
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 +370,7 @@ export function diffstr(A: string, B: string) {
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 Expand Up @@ -443,3 +457,7 @@ export function buildMessage(

return messages;
}

Deno.bench("_diff", () => {
diff(["abc", "c"], ["abc", "bcd", "c"]);
});
Loading