From 27f53efe694fe6bdb3576970f352fb28807d9005 Mon Sep 17 00:00:00 2001 From: Oliver Mannion <125105+tekumara@users.noreply.github.com> Date: Tue, 25 Apr 2023 13:02:17 +1000 Subject: [PATCH] fix: corrections larger than the misspelling would overwrite adjacent text. Now they will be correctly inserted. --- .github/workflows/ci.yml | 7 +- crates/typos-lsp/src/lsp.rs | 73 ++++++++++++++++--- package-lock.json | 28 +++++++ package.json | 3 +- src/test/fixture/diagnostics.txt | 4 +- src/test/runTest.ts | 8 +- src/test/suite/helper.ts | 6 +- ...iagnostics.test.ts => integration.test.ts} | 32 ++++++-- 8 files changed, 134 insertions(+), 27 deletions(-) rename src/test/suite/{diagnostics.test.ts => integration.test.ts} (61%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a92835d..85b9fce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,9 +23,6 @@ jobs: - run: cargo build - run: npm ci - run: npm run lint + # vscode requires an X Server - name: npm test - run: | - mkdir -p bundled - cp target/debug/typos-lsp bundled/ - # vscode requires an X Server - xvfb-run npm test + run: xvfb-run npm test diff --git a/crates/typos-lsp/src/lsp.rs b/crates/typos-lsp/src/lsp.rs index bf3d903..4626673 100644 --- a/crates/typos-lsp/src/lsp.rs +++ b/crates/typos-lsp/src/lsp.rs @@ -127,13 +127,7 @@ impl LanguageServer for Backend<'static> { changes: Some(HashMap::from([( params.text_document.uri.clone(), vec![TextEdit { - range: Range::new( - diag.range.start, - Position::new( - diag.range.start.line, - diag.range.start.character + c.len() as u32, - ), - ), + range: diag.range, new_text: c.to_string(), }], )])), @@ -321,7 +315,7 @@ mod tests { "uri": "file:///diagnostics.txt", "languageId": "plaintext", "version": 1, - "text": "this is a\ntest fo typos\n" + "text": "this is an apropriate test\nfo typos\n" } } } @@ -374,6 +368,53 @@ mod tests { } "#; + let code_action_insertion = r#" + { + "jsonrpc": "2.0", + "method": "textDocument/codeAction", + "params": { + "textDocument": { + "uri": "file:///diagnostics.txt" + }, + "range": { + "start": { + "line": 0, + "character": 11 + }, + "end": { + "line": 0, + "character": 21 + } + }, + "context": { + "diagnostics": [ + { + "range": { + "start": { + "line": 0, + "character": 11 + }, + "end": { + "line": 0, + "character": 21 + } + }, + "message": "`apropriate` should be `appropriate`", + "data": { + "corrections": ["appropriate"] + }, + "severity": 2, + "source": "typos" + } + ], + "only": ["quickfix"], + "triggerKind": 1 + } + }, + "id": 3 + } + "#; + let (mut req_client, mut resp_client) = start_server(); let mut buf = vec![0; 1024]; @@ -392,7 +433,7 @@ mod tests { similar_asserts::assert_eq!( body(&buf[..n]).unwrap(), - r#"{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"data":{"corrections":["of","for"]},"message":"`fo` should be `of`, `for`","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}},"severity":2,"source":"typos"}],"uri":"file:///diagnostics.txt","version":1}}"#, + r#"{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"data":{"corrections":["appropriate"]},"message":"`apropriate` should be `appropriate`","range":{"end":{"character":21,"line":0},"start":{"character":11,"line":0}},"severity":2,"source":"typos"},{"data":{"corrections":["of","for"]},"message":"`fo` should be `of`, `for`","range":{"end":{"character":2,"line":1},"start":{"character":0,"line":1}},"severity":2,"source":"typos"}],"uri":"file:///diagnostics.txt","version":1}}"#, ); tracing::debug!("{}", code_action); @@ -404,7 +445,19 @@ mod tests { similar_asserts::assert_eq!( body(&buf[..n]).unwrap(), - r#"{"jsonrpc":"2.0","result":[{"diagnostics":[{"data":{"corrections":["of","for"]},"message":"`fo` should be `of`, `for`","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}},"severity":2,"source":"typos"}],"edit":{"changes":{"file:///diagnostics.txt":[{"newText":"of","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}}}]}},"kind":"quickfix","title":"of"},{"diagnostics":[{"data":{"corrections":["of","for"]},"message":"`fo` should be `of`, `for`","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}},"severity":2,"source":"typos"}],"edit":{"changes":{"file:///diagnostics.txt":[{"newText":"for","range":{"end":{"character":8,"line":1},"start":{"character":5,"line":1}}}]}},"kind":"quickfix","title":"for"}],"id":2}"#, + r#"{"jsonrpc":"2.0","result":[{"diagnostics":[{"data":{"corrections":["of","for"]},"message":"`fo` should be `of`, `for`","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}},"severity":2,"source":"typos"}],"edit":{"changes":{"file:///diagnostics.txt":[{"newText":"of","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}}}]}},"kind":"quickfix","title":"of"},{"diagnostics":[{"data":{"corrections":["of","for"]},"message":"`fo` should be `of`, `for`","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}},"severity":2,"source":"typos"}],"edit":{"changes":{"file:///diagnostics.txt":[{"newText":"for","range":{"end":{"character":7,"line":1},"start":{"character":5,"line":1}}}]}},"kind":"quickfix","title":"for"}],"id":2}"#, + ); + + tracing::debug!("{}", code_action_insertion); + req_client + .write_all(req(code_action_insertion).as_bytes()) + .await + .unwrap(); + let n = resp_client.read(&mut buf).await.unwrap(); + + similar_asserts::assert_eq!( + body(&buf[..n]).unwrap(), + r#"{"jsonrpc":"2.0","result":[{"diagnostics":[{"data":{"corrections":["appropriate"]},"message":"`apropriate` should be `appropriate`","range":{"end":{"character":21,"line":0},"start":{"character":11,"line":0}},"severity":2,"source":"typos"}],"edit":{"changes":{"file:///diagnostics.txt":[{"newText":"appropriate","range":{"end":{"character":21,"line":0},"start":{"character":11,"line":0}}}]}},"isPreferred":true,"kind":"quickfix","title":"appropriate"}],"id":3}"#, ); } diff --git a/package-lock.json b/package-lock.json index a046b97..15106f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,7 @@ "@typescript-eslint/parser": "^5.53.0", "@vscode/test-electron": "^2.2.3", "@vscode/vsce": "^2.19.0", + "cross-env": "^7.0.3", "esbuild": "^0.17.18", "eslint": "^8.34.0", "glob": "^8.1.0", @@ -1336,6 +1337,24 @@ "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==", "dev": true }, + "node_modules/cross-env": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-7.0.3.tgz", + "integrity": "sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==", + "dev": true, + "dependencies": { + "cross-spawn": "^7.0.1" + }, + "bin": { + "cross-env": "src/bin/cross-env.js", + "cross-env-shell": "src/bin/cross-env-shell.js" + }, + "engines": { + "node": ">=10.14", + "npm": ">=6", + "yarn": ">=1" + } + }, "node_modules/cross-spawn": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", @@ -4725,6 +4744,15 @@ "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==", "dev": true }, + "cross-env": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-7.0.3.tgz", + "integrity": "sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==", + "dev": true, + "requires": { + "cross-spawn": "^7.0.1" + } + }, "cross-spawn": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", diff --git a/package.json b/package.json index 4bf6bf2..540e744 100644 --- a/package.json +++ b/package.json @@ -85,7 +85,7 @@ "lint": "prettier --check . && eslint src --ext ts", "fix": "prettier --write . && eslint src --ext ts --fix", "pretest": "tsc && npm run build", - "test": "node ./out/test/runTest.js" + "test": "cross-env TYPOS_LSP_PATH=$PWD/target/debug/typos-lsp node ./out/test/runTest.js" }, "devDependencies": { "@types/glob": "^8.1.0", @@ -96,6 +96,7 @@ "@typescript-eslint/parser": "^5.53.0", "@vscode/test-electron": "^2.2.3", "@vscode/vsce": "^2.19.0", + "cross-env": "^7.0.3", "esbuild": "^0.17.18", "eslint": "^8.34.0", "glob": "^8.1.0", diff --git a/src/test/fixture/diagnostics.txt b/src/test/fixture/diagnostics.txt index 8b452dd..e46b47d 100644 --- a/src/test/fixture/diagnostics.txt +++ b/src/test/fixture/diagnostics.txt @@ -1,2 +1,2 @@ -this is an apropriate -test fo typos +this is an apropriate test +fo typos diff --git a/src/test/runTest.ts b/src/test/runTest.ts index 8a6ab0e..f25a426 100644 --- a/src/test/runTest.ts +++ b/src/test/runTest.ts @@ -12,8 +12,14 @@ async function main() { // Passed to --extensionTestsPath const extensionTestsPath = path.resolve(__dirname, "./suite/index"); + const launchArgs = ["--disable-extensions"]; + // Download VS Code, unzip it and run the integration test - await runTests({ extensionDevelopmentPath, extensionTestsPath }); + await runTests({ + extensionDevelopmentPath, + extensionTestsPath, + launchArgs, + }); } catch (err) { console.error("Failed to run tests", err); process.exit(1); diff --git a/src/test/suite/helper.ts b/src/test/suite/helper.ts index 266f53c..6e20b59 100644 --- a/src/test/suite/helper.ts +++ b/src/test/suite/helper.ts @@ -9,18 +9,18 @@ export let editor: vscode.TextEditor; */ export async function activate(docUri: vscode.Uri) { const ext = vscode.extensions.getExtension("tekumara.typos-vscode")!; - // TODO: configure path + await ext.activate(); try { doc = await vscode.workspace.openTextDocument(docUri); editor = await vscode.window.showTextDocument(doc); - await sleep(2000); // Wait for server activation + await sleep(100); // Wait for server activation } catch (e) { console.error(e); } } -async function sleep(ms: number) { +export async function sleep(ms: number) { return new Promise((resolve) => setTimeout(resolve, ms)); } diff --git a/src/test/suite/diagnostics.test.ts b/src/test/suite/integration.test.ts similarity index 61% rename from src/test/suite/diagnostics.test.ts rename to src/test/suite/integration.test.ts index 1fd2f8f..029cb7a 100644 --- a/src/test/suite/diagnostics.test.ts +++ b/src/test/suite/integration.test.ts @@ -1,10 +1,14 @@ import * as vscode from "vscode"; import * as assert from "assert"; -import { getDocUri, activate } from "./helper"; +import { activate, getDocUri, sleep } from "./helper"; -suite("Should get diagnostics", () => { +suite("VS Code Integration Tests", async () => { const docUri = getDocUri("diagnostics.txt"); + suiteSetup(async () => { + await activate(docUri); + }); + test("Diagnoses typo", async () => { await testDiagnostics(docUri, [ { @@ -15,12 +19,32 @@ suite("Should get diagnostics", () => { }, { message: "`fo` should be `of`, `for`", - range: toRange(1, 5, 1, 7), + range: toRange(1, 0, 1, 2), severity: vscode.DiagnosticSeverity.Warning, source: "ex", }, ]); }); + + test("Auto fix applies corrections", async () => { + let editor = vscode.window.activeTextEditor; + + if (!editor) { + throw new Error("no active editor"); + } + + // place cursor on the spelling mistake + let position = new vscode.Position(0, 13); + let selection = new vscode.Selection(position, position); + editor.selection = selection; + + // trigger correction + await vscode.commands.executeCommand("editor.action.autoFix"); + await sleep(100); + + let contents = vscode.window.activeTextEditor?.document.getText(); + assert.equal(contents, "this is an appropriate test\nfo typos\n"); + }); }); function toRange(sLine: number, sChar: number, eLine: number, eChar: number) { @@ -33,8 +57,6 @@ async function testDiagnostics( docUri: vscode.Uri, expectedDiagnostics: vscode.Diagnostic[] ) { - await activate(docUri); - const actualDiagnostics = vscode.languages.getDiagnostics(docUri); assert.equal(actualDiagnostics.length, expectedDiagnostics.length);