From 491a601ad27d196b2b095227f49719d885978a92 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 23 Aug 2025 19:25:52 +0100 Subject: [PATCH] Lint --- .github/workflows/check.yml | 31 +--------- .../views/cursors/remote-cursors-plugin.ts | 9 +-- .../safe-filesystem-operations.ts | 21 ++++--- .../src/sync-operations/cursor-tracker.ts | 6 +- .../sync-client/src/utils/create-promise.ts | 8 ++- frontend/sync-client/src/utils/locks.test.ts | 62 ++++++++++++------- frontend/sync-client/src/utils/locks.ts | 12 ++-- frontend/test-client/src/agent/mock-client.ts | 2 +- scripts/check.sh | 27 ++++++++ 9 files changed, 101 insertions(+), 77 deletions(-) create mode 100755 scripts/check.sh diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index cb54ca89..0f0d18e1 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -30,32 +30,5 @@ jobs: sqlx database create --database-url sqlite://db.sqlite3 sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3 - - name: Lint sync-server - run: | - cd sync-server - cargo clippy --all-targets --all-features - cargo fmt --all -- --check - cargo machete - - - name: Test sync-server - run: | - cd sync-server - cargo test --verbose - - - name: Lint frontend - run: | - cd frontend - npm ci - npm run build - npm run lint - if [[ $(git status --porcelain) ]]; then - git status --porcelain - echo "Failing CI because the working directory is not clean after linting" - exit 1 - fi - - - name: Test frontend - run: | - cd frontend - npm ci - npm run test + - name: Lint & test + run: scripts/check.sh diff --git a/frontend/obsidian-plugin/src/views/cursors/remote-cursors-plugin.ts b/frontend/obsidian-plugin/src/views/cursors/remote-cursors-plugin.ts index 7676dc08..5dff2c59 100644 --- a/frontend/obsidian-plugin/src/views/cursors/remote-cursors-plugin.ts +++ b/frontend/obsidian-plugin/src/views/cursors/remote-cursors-plugin.ts @@ -15,7 +15,8 @@ import { MarkdownView } from "obsidian"; import { StateEffect } from "@codemirror/state"; import { getRandomColor } from "src/utils/get-random-color"; -import { reconcileWithHistory, SpanWithHistory } from "reconcile-text"; +import type { SpanWithHistory } from "reconcile-text"; +import { reconcileWithHistory } from "reconcile-text"; function findWhereToMoveCursor( cursor: number, @@ -39,8 +40,6 @@ function findWhereToMoveCursor( const forceUpdate = StateEffect.define(); export class RemoteCursorsPluginValue implements PluginValue { - public decorations: DecorationSet = RangeSet.of([]); - private static cursors: { name: string; path: string; @@ -49,6 +48,8 @@ export class RemoteCursorsPluginValue implements PluginValue { isOutdated: boolean; }[] = []; + public decorations: DecorationSet = RangeSet.of([]); + public static setCursors( clients: MaybeOutdatedClientCursors[], app: App @@ -101,7 +102,7 @@ export class RemoteCursorsPluginValue implements PluginValue { const original = update.startState.doc.toString(); const edited = update.state.doc.toString(); - let updatedPositions: number[] = []; + const updatedPositions: number[] = []; const reconciled = reconcileWithHistory( original, { diff --git a/frontend/sync-client/src/file-operations/safe-filesystem-operations.ts b/frontend/sync-client/src/file-operations/safe-filesystem-operations.ts index 339401af..2b1f908a 100644 --- a/frontend/sync-client/src/file-operations/safe-filesystem-operations.ts +++ b/frontend/sync-client/src/file-operations/safe-filesystem-operations.ts @@ -31,14 +31,17 @@ export class SafeFileSystemOperations implements FileSystemOperations { this.logger.debug(`Reading file '${path}'`); return this.safeOperation( path, - async () => this.locks.withLock(path, () => this.fs.read(path)), + async () => + this.locks.withLock(path, async () => this.fs.read(path)), "read" ); } public async write(path: RelativePath, content: Uint8Array): Promise { this.logger.debug(`Writing to file '${path}'`); - return this.locks.withLock(path, () => this.fs.write(path, content)); + return this.locks.withLock(path, async () => + this.fs.write(path, content) + ); } public async atomicUpdateText( @@ -49,7 +52,7 @@ export class SafeFileSystemOperations implements FileSystemOperations { return this.safeOperation( path, async () => - this.locks.withLock(path, () => + this.locks.withLock(path, async () => this.fs.atomicUpdateText(path, updater) ), "atomicUpdateText" @@ -61,19 +64,23 @@ export class SafeFileSystemOperations implements FileSystemOperations { return this.safeOperation( path, async () => - this.locks.withLock(path, () => this.fs.getFileSize(path)), + this.locks.withLock(path, async () => + this.fs.getFileSize(path) + ), "getFileSize" ); } public async exists(path: RelativePath): Promise { this.logger.debug(`Checking if file '${path}' exists`); - return this.locks.withLock(path, () => this.fs.exists(path)); + return this.locks.withLock(path, async () => this.fs.exists(path)); } public async createDirectory(path: RelativePath): Promise { this.logger.debug(`Creating directory '${path}'`); - return this.locks.withLock(path, () => this.fs.createDirectory(path)); + return this.locks.withLock(path, async () => + this.fs.createDirectory(path) + ); } public async delete(path: RelativePath): Promise { @@ -89,7 +96,7 @@ export class SafeFileSystemOperations implements FileSystemOperations { return this.safeOperation( oldPath, async () => - this.locks.withLock([oldPath, newPath], () => + this.locks.withLock([oldPath, newPath], async () => this.fs.rename(oldPath, newPath) ), "rename" diff --git a/frontend/sync-client/src/sync-operations/cursor-tracker.ts b/frontend/sync-client/src/sync-operations/cursor-tracker.ts index e50c9e1f..28ca8b33 100644 --- a/frontend/sync-client/src/sync-operations/cursor-tracker.ts +++ b/frontend/sync-client/src/sync-operations/cursor-tracker.ts @@ -61,7 +61,7 @@ export class CursorTracker { } ); - this.fileChangeNotifier.addFileChangeListener(async (relativePath) => { + this.fileChangeNotifier.addFileChangeListener(async (relativePath) => this.updateLock.withLock(async () => { for (const clientCursor of this.knownRemoteCursors) { if ( @@ -74,8 +74,8 @@ export class CursorTracker { await this.getDocumentsUpToDateness(clientCursor); } } - }); - }); + }) + ); } /// Update the local cursors for the given documents. diff --git a/frontend/sync-client/src/utils/create-promise.ts b/frontend/sync-client/src/utils/create-promise.ts index 9d6c41a2..3099f0da 100644 --- a/frontend/sync-client/src/utils/create-promise.ts +++ b/frontend/sync-client/src/utils/create-promise.ts @@ -15,9 +15,11 @@ export function createPromise(): [ let reject: undefined | ((error: unknown) => unknown) = undefined; const creationPromise = new Promise( - (resolve_, reject_) => ( - (resolve = resolve_ as ResolveFunction), (reject = reject_) - ) + (resolve_, reject_) => + ( + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + (resolve = resolve_ as ResolveFunction), (reject = reject_) + ) ); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion diff --git a/frontend/sync-client/src/utils/locks.test.ts b/frontend/sync-client/src/utils/locks.test.ts index a721d2aa..1e6bd38b 100644 --- a/frontend/sync-client/src/utils/locks.test.ts +++ b/frontend/sync-client/src/utils/locks.test.ts @@ -29,7 +29,7 @@ describe("withLock", () => { let executionCount = 0; const result = await locks.withLock(testPath, async () => { executionCount++; - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); return "async-success"; }); @@ -54,14 +54,14 @@ describe("withLock", () => { // Start two concurrent operations with keys in different orders const promise1 = locks.withLock([testPath2, testPath], async () => { executionOrder.push("operation1-start"); - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, 50)); executionOrder.push("operation1-end"); return "result1"; }); const promise2 = locks.withLock([testPath, testPath2], async () => { executionOrder.push("operation2-start"); - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, 50)); executionOrder.push("operation2-end"); return "result2"; }); @@ -84,14 +84,14 @@ describe("withLock", () => { const promise1 = locks.withLock(testPath, async () => { executionOrder.push("operation1-start"); - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, 50)); executionOrder.push("operation1-end"); return "result1"; }); const promise2 = locks.withLock(testPath, async () => { executionOrder.push("operation2-start"); - await new Promise(resolve => setTimeout(resolve, 30)); + await new Promise((resolve) => setTimeout(resolve, 30)); executionOrder.push("operation2-end"); return "result2"; }); @@ -113,14 +113,14 @@ describe("withLock", () => { const promise1 = locks.withLock(testPath, async () => { executionOrder.push("operation1-start"); - await new Promise(resolve => setTimeout(resolve, 50)); + await new Promise((resolve) => setTimeout(resolve, 50)); executionOrder.push("operation1-end"); return "result1"; }); const promise2 = locks.withLock(testPath2, async () => { executionOrder.push("operation2-start"); - await new Promise(resolve => setTimeout(resolve, 30)); + await new Promise((resolve) => setTimeout(resolve, 30)); executionOrder.push("operation2-end"); return "result2"; }); @@ -136,26 +136,36 @@ describe("withLock", () => { test("should release locks even if function throws", async () => { const error = new Error("test error"); - - await expect(locks.withLock(testPath, () => { - throw error; - })).rejects.toThrow("test error"); + + await expect( + locks.withLock(testPath, () => { + throw error; + }) + ).rejects.toThrow("test error"); // Lock should be released, allowing another operation - const result = await locks.withLock(testPath, () => "success-after-error"); + const result = await locks.withLock( + testPath, + () => "success-after-error" + ); expect(result).toBe("success-after-error"); }); test("should release locks even if async function throws", async () => { const error = new Error("async test error"); - - await expect(locks.withLock(testPath, async () => { - await new Promise(resolve => setTimeout(resolve, 10)); - throw error; - })).rejects.toThrow("async test error"); + + await expect( + locks.withLock(testPath, async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + throw error; + }) + ).rejects.toThrow("async test error"); // Lock should be released, allowing another operation - const result = await locks.withLock(testPath, () => "success-after-async-error"); + const result = await locks.withLock( + testPath, + () => "success-after-async-error" + ); expect(result).toBe("success-after-async-error"); }); @@ -170,30 +180,34 @@ describe("withLock", () => { // Start first operation that holds the lock const firstPromise = locks.withLock(testPath, async () => { executionOrder.push("first-start"); - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise((resolve) => setTimeout(resolve, 100)); executionOrder.push("first-end"); return "first"; }); // Small delay to ensure first operation starts - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Queue second and third operations const secondPromise = locks.withLock(testPath, async () => { executionOrder.push("second-start"); - await new Promise(resolve => setTimeout(resolve, 30)); + await new Promise((resolve) => setTimeout(resolve, 30)); executionOrder.push("second-end"); return "second"; }); const thirdPromise = locks.withLock(testPath, async () => { executionOrder.push("third-start"); - await new Promise(resolve => setTimeout(resolve, 20)); + await new Promise((resolve) => setTimeout(resolve, 20)); executionOrder.push("third-end"); return "third"; }); - const [first, second, third] = await Promise.all([firstPromise, secondPromise, thirdPromise]); + const [first, second, third] = await Promise.all([ + firstPromise, + secondPromise, + thirdPromise + ]); expect(first).toBe("first"); expect(second).toBe("second"); @@ -207,4 +221,4 @@ describe("withLock", () => { "third-end" ]); }); -}); \ No newline at end of file +}); diff --git a/frontend/sync-client/src/utils/locks.ts b/frontend/sync-client/src/utils/locks.ts index fa513de1..e09da236 100644 --- a/frontend/sync-client/src/utils/locks.ts +++ b/frontend/sync-client/src/utils/locks.ts @@ -17,16 +17,16 @@ export class Locks { /** * Executes a function while holding exclusive locks on one or more keys. - * + * * This method ensures that the provided function runs with exclusive access to the * specified key(s). Multiple keys are sorted to prevent deadlocks when different * operations request the same keys in different orders. - * + * * @template R The return type of the function to execute * @param keyOrKeys A single key or array of keys to lock during function execution * @param fn The function to execute while holding the lock(s). Can be sync or async. * @returns A Promise that resolves to the return value of the executed function - * + * * @example * ```typescript * // Lock a single key @@ -34,14 +34,14 @@ export class Locks { * // Critical section - only one operation can access 'file1' at a time * return processFile('file1'); * }); - * + * * // Lock multiple keys (prevents deadlocks through consistent ordering) * await locks.withLock(['file1', 'file2'], async () => { * // Critical section - exclusive access to both files * await moveFile('file1', 'file2'); * }); * ``` - * + * * @throws Any error thrown by the provided function will be propagated after locks are released */ public async withLock( @@ -49,7 +49,7 @@ export class Locks { fn: () => R | Promise ): Promise { const keys = Array.isArray(keyOrKeys) ? keyOrKeys : [keyOrKeys]; - keys.sort(); // Ensure consistent order to prevent deadlocks + keys.sort((a, b) => String(a).localeCompare(String(b))); // Ensure consistent order to prevent deadlocks await Promise.all(keys.map(async (key) => this.waitForLock(key))); diff --git a/frontend/test-client/src/agent/mock-client.ts b/frontend/test-client/src/agent/mock-client.ts index 2833ba29..3ef55c8f 100644 --- a/frontend/test-client/src/agent/mock-client.ts +++ b/frontend/test-client/src/agent/mock-client.ts @@ -37,7 +37,7 @@ export class MockClient implements FileSystemOperations { fs: this, persistence: { load: async () => this.data, - save: async (data) => (this.data = data) + save: async (data) => void (this.data = data) }, fetch: fetchImplementation, webSocket: webSocketImplementation diff --git a/scripts/check.sh b/scripts/check.sh new file mode 100755 index 00000000..c75e10c5 --- /dev/null +++ b/scripts/check.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash + +set -e + +echo "Running checks in sync-server" +cd sync-server +cargo clippy --all-targets --all-features +cargo fmt --all -- --check +cargo machete +cargo test --verbose + + +echo "Running checks in frontend" +cd ../frontend +npm ci +npm run build +npm run lint +if [[ $(git status --porcelain) ]]; then + git status --porcelain + echo "Failing CI because the working directory is not clean after linting" + exit 1 +fi +npm run test + +echo "Finished" + +cd ..