diff --git a/integ/spec/function_call1.f90.spec.ts b/integ/spec/function_call1.f90.spec.ts index 12dfd14..b361ff9 100644 --- a/integ/spec/function_call1.f90.spec.ts +++ b/integ/spec/function_call1.f90.spec.ts @@ -110,6 +110,66 @@ async function triggerHoverAndGetText() : Promise { return text; } +async function renameSymbol(newName: string): Promise { + // await editor.sendKeys(Key.F2); + await driver.actions().clear(); // necessary for the key-down event + await driver.actions().sendKeys(Key.F2).perform(); + const renameInput: WebElement = + driver.wait( + until.elementLocated( + By.css('input.rename-input')), + timeout); + let oldName: string = await renameInput.getAttribute("value"); + for (let i = 0, k = oldName.length; + (oldName.length > 0) && (i < k); + i++) { + await renameInput.sendKeys(Key.BACK_SPACE); + oldName = await renameInput.getAttribute("value"); + } + oldName = await renameInput.getAttribute("value"); + assert.isEmpty( + oldName, + "Failed to clear all characters from .rename-input"); + await renameInput.sendKeys(newName); + await renameInput.sendKeys(Key.ENTER); +} + +async function getErrorAlert(): Promise { + await driver.actions().clear(); + await driver.actions().sendKeys(Key.F8).perform(); + // NOTE: k=10 might be excessive, but I don't like failing due to a lack + // of retries ... + // --------------------------------------------------------------------- + for (let i = 0, k = 10; i < k; i++) { + try { + const outerElement: WebElement = + await driver.wait( + until.elementLocated( + By.css('div.message[role="alert"][aria-label^="error"] div')), + timeout); + const innerElement: WebElement = + await outerElement.findElement( + By.css(':first-child')); + const outerMessage: string = await outerElement.getText(); + const innerMessage: string = await innerElement.getText(); + // NOTE: Drop the inner message from the error message since it contains + // the name of the plugin: + // --------------------------------------------------------------------- + const errorMessage: string = + outerMessage.substring(0, outerMessage.length - innerMessage.length); + return errorMessage; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } catch (error: any) { + const retryMessage: string = + "stale element reference: stale element not found in the current frame"; + if (!error.message.startsWith(retryMessage) || ((i + 1) == k)) { + throw error; + } + } + } + throw new Error("This should not be reached!"); +} + // initialize the browser and webdriver before(async () => { browser = VSBrowser.instance; @@ -143,6 +203,7 @@ describe(fileName, () => { afterEach(async () => { await workbench.executeCommand("revert file"); await editorView.closeEditor(fileName); + await driver.actions().clear(); }); describe('When I type "m"', () => { @@ -251,4 +312,46 @@ describe(fileName, () => { assert.equal(column, 5); }); }); + + describe('When I rename eval_1d to foo', () => { + it('should rename all the respective symbols', async () => { + await editor.setCursor(18, 22); // hover over "eval_1d" + await renameSymbol("foo"); + const text: string = await editor.getText(); + assert.equal(text, [ + "module module_function_call1", + " type :: softmax", + " contains", + " procedure :: foo", + " end type softmax", + " contains", + " ", + " pure function foo(self, x) result(res)", + " class(softmax), intent(in) :: self", + " real, intent(in) :: x(:)", + " real :: res(size(x))", + " end function foo", + " ", + " pure function eval_1d_prime(self, x) result(res)", + " class(softmax), intent(in) :: self", + " real, intent(in) :: x(:)", + " real :: res(size(x))", + " res = self%foo(x)", + " end function eval_1d_prime", + "end module module_function_call1", + "", + ].join("\n")); + }); + }); + + describe('When I introduce an error', () => { + it('should notify me of the issue.', async () => { + await editor.setCursor(21, 1); + await editor.typeText("error"); + const errorMessage: string = await getErrorAlert(); + assert.equal( + errorMessage, + "Statement or Declaration expected inside program, found Variable name"); + }); + }); }); diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 1f041c3..fc2c791 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -75,7 +75,8 @@ function activate-conda() { pandoc \ gcc \ gxx \ - libcxx + libcxx \ + rapidjson RETURN_CODE=$? if (( RETURN_CODE != EXIT_SUCCESS )); then echo "`conda create -n $CONDA_ENV` failed with status $RETURN_CODE" 1>&2 @@ -143,7 +144,12 @@ function build-lfortran() { return $EXIT_BUILD_FAILED fi - cmake --fresh -DCMAKE_BUILD_TYPE=Debug -DWITH_LSP=yes -DWITH_LLVM=yes -DCMAKE_INSTALL_PREFIX=`pwd`/inst . + cmake --fresh \ + -DCMAKE_BUILD_TYPE=Debug \ + -DWITH_LSP=yes \ + -DWITH_LLVM=yes \ + -DWITH_JSON=yes \ + -DCMAKE_INSTALL_PREFIX=`pwd`/inst . RETURN_CODE=$? if (( RETURN_CODE != EXIT_SUCCESS )); then echo "cmake failed with status $RETURN_CODE" 1>&2 @@ -191,6 +197,8 @@ Usage: ./scripts/e2e.sh [OPTIONS] Options: -h|--help Print this help text. -u|--update-lfortran Whether to update LFortran before running the end-to-end tests. + --headless Whether to run the tests in an XVFB framebuffer + (render off-screen; no visible window). EOF } diff --git a/server/src/lfortran-accessors.ts b/server/src/lfortran-accessors.ts index 17aead3..2f9ced4 100644 --- a/server/src/lfortran-accessors.ts +++ b/server/src/lfortran-accessors.ts @@ -10,6 +10,7 @@ import { Range, SymbolInformation, SymbolKind, + TextEdit, } from 'vscode-languageserver/node'; import { @@ -55,6 +56,13 @@ export interface LFortranAccessor { showErrors(uri: string, text: string, settings: ExampleSettings): Promise; + + renameSymbol(uri: string, + text: string, + line: number, + column: number, + newName: string, + settings: ExampleSettings): Promise; } /** @@ -172,8 +180,8 @@ export class LFortranCLIAccessor implements LFortranAccessor { } async showDocumentSymbols(uri: string, - text: string, - settings: ExampleSettings): Promise { + text: string, + settings: ExampleSettings): Promise { const flags = ["--show-document-symbols"]; const stdout = await this.runCompiler(settings, flags, text); let results; @@ -208,10 +216,10 @@ export class LFortranCLIAccessor implements LFortranAccessor { } async lookupName(uri: string, - text: string, - line: number, - column: number, - settings: ExampleSettings): Promise { + text: string, + line: number, + column: number, + settings: ExampleSettings): Promise { try { const flags = [ "--lookup-name", @@ -247,27 +255,92 @@ export class LFortranCLIAccessor implements LFortranAccessor { } async showErrors(uri: string, - text: string, - settings: ExampleSettings): Promise { + text: string, + settings: ExampleSettings): Promise { const diagnostics: Diagnostic[] = []; + let stdout: string | null = null; try { const flags = ["--show-errors"]; - const stdout = await this.runCompiler(settings, flags, text); - const results: ErrorDiagnostics = JSON.parse(stdout); - if (results?.diagnostics) { - const k = Math.min(results.diagnostics.length, settings.maxNumberOfProblems); - for (let i = 0; i < k; i++) { - const diagnostic: Diagnostic = results.diagnostics[i]; - diagnostic.severity = DiagnosticSeverity.Warning; - diagnostic.source = "lfortran-lsp"; - diagnostics.push(diagnostic); + stdout = await this.runCompiler(settings, flags, text); + if (stdout.length > 0) { + let results: ErrorDiagnostics; + try { + results = JSON.parse(stdout); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } catch (error: any) { + // FIXME: Remove this repair logic once the respective bug has been + // fixed (lfortran/lfortran issue #5525) + // ---------------------------------------------------------------- + console.warn("Failed to parse response, attempting to repair and re-parse it."); + const repaired: string = stdout.substring(0, 28) + "{" + stdout.substring(28); + try { + results = JSON.parse(repaired); + console.log("Repair succeeded, see: https://github.com/lfortran/lfortran/issues/5525"); + } catch { + console.error("Failed to repair response"); + throw error; + } + } + if (results?.diagnostics) { + const k = Math.min(results.diagnostics.length, settings.maxNumberOfProblems); + for (let i = 0; i < k; i++) { + const diagnostic: Diagnostic = results.diagnostics[i]; + diagnostic.severity = DiagnosticSeverity.Warning; + diagnostic.source = "lfortran-lsp"; + diagnostics.push(diagnostic); + } } } // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { console.error("Failed to show errors"); + if (stdout !== null) { + console.error("Failed to parse response: %s", stdout); + } console.error(error); } return diagnostics; } + + async renameSymbol(uri: string, + text: string, + line: number, + column: number, + newName: string, + settings: ExampleSettings): Promise { + const edits: TextEdit[] = []; + try { + const flags = [ + "--rename-symbol", + "--line=" + (line + 1), + "--column=" + (column + 1) + ]; + const stdout = await this.runCompiler(settings, flags, text); + const obj = JSON.parse(stdout); + for (let i = 0, k = obj.length; i < k; i++) { + const location = obj[i].location; + if (location) { + const range: Range = location.range; + + const start: Position = range.start; + start.character--; + + const end: Position = range.end; + end.character--; + + const edit: TextEdit = { + range: range, + newText: newName, + }; + + edits.push(edit); + } + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } catch (error: any) { + console.error("Failed to rename symbol at line=%d, column=%d", line, column); + console.error(error); + } + return edits; + } } diff --git a/server/src/lfortran-language-server.ts b/server/src/lfortran-language-server.ts index 099160d..a558ce1 100644 --- a/server/src/lfortran-language-server.ts +++ b/server/src/lfortran-language-server.ts @@ -24,11 +24,14 @@ import { Location, Position, Range, + RenameParams, SymbolInformation, TextDocumentChangeEvent, TextDocumentPositionParams, TextDocuments, TextDocumentSyncKind, + TextEdit, + WorkspaceEdit, } from 'vscode-languageserver/node'; import { TextDocument } from 'vscode-languageserver-textdocument'; @@ -101,6 +104,7 @@ export class LFortranLanguageServer { definitionProvider: true, documentSymbolProvider: true, hoverProvider: true, + renameProvider: true, textDocumentSync: TextDocumentSyncKind.Incremental, } }; @@ -304,24 +308,11 @@ export class LFortranLanguageServer { // } extractQuery(text: string, line: number, column: number): string | null { - let currLine = 0; - let currCol = 0; + let currLine: number = 0; + let currCol: number = 0; for (let i = 0, k = text.length; i < k; i++) { const c: string = text[i]; - if (c === '\n') { - currLine++; - currCol = 0; - } else if (c === '\r') { - const j = i + 1; - if ((j < k) && (text[j] === '\n')) { - i = j; - } - currLine++; - currCol = 0; - } else { - currCol++; - } if ((line === currLine) && (column === currCol)) { const re_identifiable: RegExp = RE_IDENTIFIABLE; @@ -338,6 +329,20 @@ export class LFortranLanguageServer { return query; } } + + if (c === '\n') { + currLine++; + currCol = 0; + } else if (c === '\r') { + const j = i + 1; + if ((j < k) && (text[j] === '\n')) { + i = j; + } + currLine++; + currCol = 0; + } else { + currCol++; + } } return null; @@ -350,9 +355,12 @@ export class LFortranLanguageServer { if ((document !== undefined) && (dictionary !== undefined)) { const text: string = document.getText(); const pos: Position = documentPosition.position; - const query: string | null = this.extractQuery(text, pos.line, pos.character); + const line: number = pos.line; + const column: number = pos.character - 1; + const query: string | null = this.extractQuery(text, line, column); if (query !== null) { - return Array.from(dictionary.lookup(query)) as CompletionItem[]; + const completions: CompletionItem[] = Array.from(dictionary.lookup(query)) as CompletionItem[]; + return completions; } } } @@ -368,7 +376,9 @@ export class LFortranLanguageServer { if ((document !== undefined) && (dictionary !== undefined)) { const text: string = document.getText(); const pos: Position = params.position; - const query: string | null = this.extractQuery(text, pos.line, pos.character); + const line = pos.line; + const column = pos.character; + const query: string | null = this.extractQuery(text, line, column); if (query !== null) { const completion: CompletionItem | undefined = dictionary.exactLookup(query) as CompletionItem; @@ -384,4 +394,110 @@ export class LFortranLanguageServer { } } } + + renameSymbol(text: string, symbol: string, newName: string): TextEdit[] { + // case-insensitive search + text = text.toLowerCase(); + symbol = symbol.toLowerCase(); + + const edits: TextEdit[] = []; + + let currLine: number = 0; + let currCol: number = 0; + + const k = text.length; + const l = symbol.length; + let u: string | null = null; + const w: string = symbol[0]; + const reIdentifiable = RE_IDENTIFIABLE; + for (let i = 0; i < k; i++) { + const v: string = text[i]; + + if (v === '\n') { + currLine++; + currCol = 0; + u = v; + } else if (v === '\r') { + const j = i + 1; + if ((j < k) && (text[j] === '\n')) { + i = j; + u = '\n'; + } else { + u = v; + } + currLine++; + currCol = 0; + } else if ((v === w) && ((u === null) || !reIdentifiable.test(u))) { + const startLine: number = currLine; + const startCol: number = currCol; + + let j: number = 1; + for (; (j < l) && ((i + j) < k) && (text[i + j] === symbol[j]); j++) { + // empty + } + + currCol += j; + i += (j - 1); + u = text[i]; + + if ((j === l) && (((i + 1) === k) || !reIdentifiable.test(text[i + 1]))) { + const endLine: number = currLine; + const endCol: number = currCol; + + const range: Range = { + start: { + line: startLine, + character: startCol, + }, + end: { + line: endLine, + character: endCol, + }, + }; + + const edit: TextEdit = { + range: range, + newText: newName, + }; + + edits.push(edit); + } + } else { + currCol++; + u = v; + } + } + + return edits; + } + + async onRenameRequest(params: RenameParams): Promise { + const newName: string = params.newName; + const uri: string = params.textDocument.uri; + const document = this.documents.get(uri); + if (document !== undefined) { + const text: string = document.getText(); + const pos: Position = params.position; + // ===================================================================================== + // FIXME: Once lfortran/lfortran issue #5524 is resolved, restore this call to lfortran: + // ===================================================================================== + // const settings = await this.getDocumentSettings(uri); + // const edits: TextEdit[] = + // await this.lfortran.renameSymbol(uri, text, pos.line, pos.character, newName, settings); + // return { + // changes: { + // [uri]: edits, + // }, + // }; + const query: string | null = this.extractQuery(text, pos.line, pos.character); + if (query !== null) { + const edits: TextEdit[] = this.renameSymbol(text, query, newName); + return { + changes: { + [uri]: edits, + }, + }; + } + } + } } diff --git a/server/src/main.ts b/server/src/main.ts index 74f281d..402b042 100644 --- a/server/src/main.ts +++ b/server/src/main.ts @@ -34,6 +34,7 @@ connection.onDidChangeConfiguration(server.onDidChangeConfiguration.bind(server) connection.onCompletion(server.onCompletion.bind(server)); connection.onCompletionResolve(server.onCompletionResolve.bind(server)); connection.onHover(server.onHover.bind(server)); +connection.onRenameRequest(server.onRenameRequest.bind(server)); documents.onDidClose(server.onDidClose.bind(server)); documents.onDidChangeContent(server.onDidChangeContent.bind(server)); diff --git a/server/test/spec/lfortran-accessors.spec.ts b/server/test/spec/lfortran-accessors.spec.ts index 31e0ccb..49ec425 100644 --- a/server/test/spec/lfortran-accessors.spec.ts +++ b/server/test/spec/lfortran-accessors.spec.ts @@ -5,6 +5,7 @@ import { Range, SymbolInformation, SymbolKind, + TextEdit, } from "vscode-languageserver/node"; import { LFortranCLIAccessor } from "../../src/lfortran-accessors"; @@ -251,4 +252,82 @@ describe("LFortranCLIAccessor", () => { assert.deepEqual(actual, expected); }); }); + + describe("renameSymbol", () => { + it("decrements the exclusive ending character values to make them inclusive columns", async () => { + const newName: string = "foo"; + + const stdout: string = JSON.stringify([ + { + kind: 1, + location: { + range: { + start: { + character: 5, + line: 8 + }, + end: { + character: 25, + line: 12 + } + }, + uri: "uri" + }, + name: "eval_1d" + }, + { + kind: 1, + location: { + range: { + start: { + character: 7, + line: 4 + }, + end: { + character: 28, + line: 4 + } + }, + uri: "uri" + }, + name: "eval_1d" + } + ]); + + sinon.stub(lfortran, "runCompiler").resolves(stdout); + + const expected: TextEdit[] = [ + { + range: { + start: { + line: 8, + character: 4, + }, + end: { + line: 12, + character: 24, + } + }, + newText: newName, + }, + { + range: { + start: { + line: 4, + character: 6, + }, + end: { + line: 4, + character: 27, + } + }, + newText: newName, + }, + ]; + + const actual: TextEdit[] = + await lfortran.renameSymbol(uri, "", 18, 22, newName, settings); + assert.deepEqual(actual, expected); + }); + }); }); diff --git a/server/test/spec/lfortran-language-server.spec.ts b/server/test/spec/lfortran-language-server.spec.ts index 5d0483a..eff1fa0 100644 --- a/server/test/spec/lfortran-language-server.spec.ts +++ b/server/test/spec/lfortran-language-server.spec.ts @@ -17,11 +17,14 @@ import { Position, Range, RemoteWorkspace, + RenameParams, SymbolInformation, SymbolKind, TextDocumentChangeEvent, TextDocumentPositionParams, TextDocuments, + TextEdit, + WorkspaceEdit, } from "vscode-languageserver/node"; import { TextDocument } from "vscode-languageserver-textdocument"; @@ -319,7 +322,7 @@ describe("LFortranLanguageServer", () => { await server.validateTextDocument(document); const sendDiagnostics = connection.sendDiagnostics; - assert.isTrue(sendDiagnostics.calledOnceWith({uri: uri, diagnostics })); + assert.isTrue(sendDiagnostics.calledOnceWith({ uri: uri, diagnostics })); }); }); @@ -504,10 +507,10 @@ describe("LFortranLanguageServer", () => { it("expands identifiable symbols at the given location", () => { const text: string = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."; - let query: string = server.extractQuery(text, 0, 1); + let query: string = server.extractQuery(text, 0, 0); assert.equal(query, "Lorem"); - query = server.extractQuery(text, 0, 6); // actual text, ".", is not identifiable. + query = server.extractQuery(text, 0, 26); // actual text, ",", is not identifiable. assert.isNull(query); query = server.extractQuery(text, 0, 25); @@ -582,7 +585,7 @@ describe("LFortranLanguageServer", () => { }, position: { line: 0, - character: 47, + character: 46, }, }; @@ -687,4 +690,237 @@ describe("LFortranLanguageServer", () => { assert.equal(contents.value, "def foo;"); }); }); + + describe("renameSymbol", () => { + describe("renaming a symbol over an empty string", () => { + it("should change nothing", () => { + const symbol: string = "foo"; + const newName: string = "bar"; + const text: string = ""; + const edits: TextEdit[] = server.renameSymbol(text, symbol, newName); + assert.isEmpty(edits); + }); + }); + + describe("renaming a symbol in a singleton string", () => { + it("should change the symbol as requested", () => { + const symbol: string = "lorem"; + const newName: string = "id"; + const text: string = "lorem"; + const edits: TextEdit[] = server.renameSymbol(text, symbol, newName); + assert.deepEqual(edits, [ + { + range: { + start: { + line: 0, + character: 0, + }, + end: { + line: 0, + character: 5, + }, + }, + newText: newName, + }, + ]); + }); + }); + + describe("renaming a non-existing symbol", () => { + it("should change nothing", () => { + const symbol: string = "foo"; + const newName: string = "bar"; + const text: string = "baz qux quo"; + const edits: TextEdit[] = server.renameSymbol(text, symbol, newName); + assert.isEmpty(edits); + }); + }); + + describe("renaming a symbol on a single line", () => { + it("should replace all the respective terms", () => { + const symbol: string = "foo"; + const newName: string = "abc"; + const text: string = "foo bar baz%foo foo_qux quo_foo foo"; + const edits: TextEdit[] = server.renameSymbol(text, symbol, newName); + assert.deepEqual(edits, [ + { + range: { + start: { + line: 0, + character: 0, + }, + end: { + line: 0, + character: 3, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 0, + character: 12, + }, + end: { + line: 0, + character: 15, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 0, + character: 32, + }, + end: { + line: 0, + character: 35, + }, + }, + newText: newName, + }, + ]); + }); + }); + + describe("renaming a symbol over multiple lines", () => { + it("should replace all respective symbols", () => { + const symbol: string = "foo"; + const newName: string = "bar"; + const text: string = [ + "Foo bar baz", + "qux quo", + "abc%foo", + "foo%def", + "FOO foo bar_foo", + "baz_foo quux FoO foofoo bax_foo_qux" + ].join("\n"); + const edits: TextEdit[] = server.renameSymbol(text, symbol, newName); + assert.deepEqual(edits, [ + { + range: { + start: { + line: 0, + character: 0, + }, + end: { + line: 0, + character: 3, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 2, + character: 4, + }, + end: { + line: 2, + character: 7, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 3, + character: 0, + }, + end: { + line: 3, + character: 3, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 4, + character: 0, + }, + end: { + line: 4, + character: 3, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 4, + character: 4, + }, + end: { + line: 4, + character: 7, + }, + }, + newText: newName, + }, + { + range: { + start: { + line: 5, + character: 13, + }, + end: { + line: 5, + character: 16, + }, + }, + newText: newName, + }, + ]); + }); + }); + }); + + describe("onRenameRequest", () => { + it("should wrap the edits from lfortran within a WorkspaceEdit", async () => { + const text: string = "foo bar baz qux"; + document.getText.returns(text); + + const newName: string = "quo"; + + const expected: WorkspaceEdit = { + changes: { + [uri]: [ + { + range: { + start: { + line: 0, + character: 0, + }, + end: { + line: 0, + character: 3, + }, + }, + newText: newName, + }, + ], + }, + }; + + const params: RenameParams = { + newName: newName, + textDocument: document, + position: { + line: 0, + character: 0, + }, + }; + + const actual: WorkspaceEdit = await server.onRenameRequest(params); + assert.deepEqual(actual, expected); + }); + }); });