From b934e1971f933efd878002d4af83ff9dd16ec558 Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Sun, 17 Dec 2023 21:56:21 +0000 Subject: [PATCH] fix: file end of line character changes Maintain the current predominant end of line character for files when making edits. This changes editing to use the standard TextEdit insert and replace creation methods to reduce complexity and overhead. Fix callback handling in tests, adding missing await and reject handling. Use option chaining and early return in getUpdatedSource to improve readability. Remove use of deprecated isNullOrUndefined. Fixes #176 --- .gitattributes | 3 + CHANGELOG.md | 2 + client/src/test/examples/tiny-cr.groovy | 1 + client/src/test/examples/tiny-crlf.groovy | 56 ++++++++ .../examples/{tiny.groovy => tiny-lf.groovy} | 0 client/src/test/suite/extension.test.ts | 120 +++++++++++++--- server/src/DocumentsManager.ts | 13 +- server/src/clientUtils.ts | 134 ++++++++++-------- server/src/codeActions.ts | 35 +++-- server/src/linter.ts | 14 +- 10 files changed, 273 insertions(+), 105 deletions(-) create mode 100644 .gitattributes create mode 100644 client/src/test/examples/tiny-cr.groovy create mode 100644 client/src/test/examples/tiny-crlf.groovy rename client/src/test/examples/{tiny.groovy => tiny-lf.groovy} (100%) diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..bc7fe7f --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +client/src/test/examples/tiny-lf.groovy text eol=lf +client/src/test/examples/tiny-cr.groovy text eol=cr +client/src/test/examples/tiny-crlf.groovy text eol=crlf diff --git a/CHANGELOG.md b/CHANGELOG.md index 8705c7c..d716896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased] +- fix: file end of line character changes ([#176](https://github.com/nvuillam/vscode-groovy-lint/issues/176)) + ## [3.1.0] 2023-12-17 - Added new setting `showProblemsView` that controls if Problems View should open after initial lint pass diff --git a/client/src/test/examples/tiny-cr.groovy b/client/src/test/examples/tiny-cr.groovy new file mode 100644 index 0000000..2e0f6ca --- /dev/null +++ b/client/src/test/examples/tiny-cr.groovy @@ -0,0 +1 @@ +import groovy.io.FileType import groovy.json.* import groovy.time.TimeCategory import static groovyx.gpars.GParsPool.withPool def script = new GroovyScriptEngine( "." ).with{ loadScriptByName( 'Utils.groovy' ) ; } this.metaClass.mixin script def returnCode = 0 Exception eThrow = null ; try { initialize(args) ; } catch (Exception e){ eThrow = e ; returnCode = 1 } if (eThrow == null){ return 0 ; } else { throw eThrow ; return 1 ; } ////////////////////////////////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////// SCRIPT ///////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////////////////////////////// def initialize(args3) { // def executor = new TestExecutor(args3) return executor } class TestExecutor { public TestExecutor( args2) { this.testExternalGlobalProps() } public testExternalGlobalProps() { Utils.printlnLog( '########## testExternalGlobalProps') def globalKeyName = new Random().with { (1..9).collect { (('a'..'z')).join()[ nextInt((('a'..'z')).join().length())]}.join()} Utils.printlnLog( "Generated random key: ${globalKeyName}") Utils.setExternalValue(globalKeyName , 'lelama' , 'nul') def storedValue = Utils.getExternalValue(globalKeyName , 'lelama') assert storedValue == 'nul' , 'Error in global prop key storage/ retrieval (1)' Utils.setExternalValue(globalKeyName , 'lelama2' , 'nul2') def storedValue2 = Utils.getExternalValue(globalKeyName , 'lelama2') assert storedValue2 == 'nul2' , 'Error in global prop key storage/ retrieval (2)' def storedValueBack = Utils.getExternalValue(globalKeyName , 'lelama') assert storedValueBack == 'nul' , 'Error in global prop key storage/ retrieval (3)' Utils.printlnLog( Utils.getExternalValue(globalKeyName)) } } \ No newline at end of file diff --git a/client/src/test/examples/tiny-crlf.groovy b/client/src/test/examples/tiny-crlf.groovy new file mode 100644 index 0000000..a25979e --- /dev/null +++ b/client/src/test/examples/tiny-crlf.groovy @@ -0,0 +1,56 @@ +import groovy.io.FileType +import groovy.json.* +import groovy.time.TimeCategory +import static groovyx.gpars.GParsPool.withPool + + def script = new GroovyScriptEngine( "." ).with{ + loadScriptByName( 'Utils.groovy' ) ; + } +this.metaClass.mixin script + +def returnCode = 0 + Exception eThrow = null ; + try { + initialize(args) ; +} catch (Exception e){ + eThrow = e ; + returnCode = 1 + } + if (eThrow == null){ + return 0 ; +} +else { + throw eThrow ; + return 1 ; +} + +////////////////////////////////////////////////////////////////////////////////////////////////////////// +///////////////////////////////////////////////// SCRIPT ///////////////////////////////////////////////// +////////////////////////////////////////////////////////////////////////////////////////////////////////// +def initialize(args3) { // + def executor = new TestExecutor(args3) + return executor +} + +class TestExecutor { + + public TestExecutor( args2) { + this.testExternalGlobalProps() + } + + public testExternalGlobalProps() { + Utils.printlnLog( '########## testExternalGlobalProps') + def globalKeyName = new Random().with { (1..9).collect { (('a'..'z')).join()[ nextInt((('a'..'z')).join().length())]}.join()} + Utils.printlnLog( "Generated random key: ${globalKeyName}") + Utils.setExternalValue(globalKeyName , 'lelama' , 'nul') + def storedValue = Utils.getExternalValue(globalKeyName , 'lelama') + assert storedValue == 'nul' , 'Error in global prop key storage/ retrieval (1)' + Utils.setExternalValue(globalKeyName , 'lelama2' , 'nul2') + def storedValue2 = Utils.getExternalValue(globalKeyName , 'lelama2') + assert storedValue2 == 'nul2' , 'Error in global prop key storage/ retrieval (2)' + def storedValueBack = Utils.getExternalValue(globalKeyName , 'lelama') + assert storedValueBack == 'nul' , 'Error in global prop key storage/ retrieval (3)' + Utils.printlnLog( Utils.getExternalValue(globalKeyName)) + } + +} diff --git a/client/src/test/examples/tiny.groovy b/client/src/test/examples/tiny-lf.groovy similarity index 100% rename from client/src/test/examples/tiny.groovy rename to client/src/test/examples/tiny-lf.groovy diff --git a/client/src/test/suite/extension.test.ts b/client/src/test/suite/extension.test.ts index e550b38..8168a03 100644 --- a/client/src/test/suite/extension.test.ts +++ b/client/src/test/suite/extension.test.ts @@ -21,7 +21,7 @@ const testsFolder = '../../../src/test'; const examples = 'examples'; const testConfig = '.groovylintrc.json'; const extensionId = 'NicolasVuillamy.vscode-groovy-lint'; -const tinyGroovy = 'tiny.groovy'; +const tinyGroovy = 'tiny-lf.groovy'; const validGroovy = 'valid.groovy'; const numberOfGroovyLintCommands = 9; @@ -29,6 +29,9 @@ const numberOfGroovyLintCommands = 9; // allow for server startup timeout. let additionalTimeout = defaultTimeout; +// RegExp to determine predominate End Of Line (EOL) sequence. +const eolRegExp: RegExp = new RegExp(/(\r\n|\n|\r)/g); + /** * testDocumentDetails represents the expected results for a testDocument. */ @@ -54,6 +57,8 @@ const documentDetails = new Map(); [ new testDocumentDetails(validGroovy, 0, 0, false), new testDocumentDetails(tinyGroovy, 50, 19), + new testDocumentDetails('tiny-cr.groovy', 50, 19), + new testDocumentDetails('tiny-crlf.groovy', 50, 19), new testDocumentDetails('big.groovy', 4114, 789, true, 10 * second), new testDocumentDetails('Jenkinsfile', 380, 151, true, 10 * second), new testDocumentDetails('parseError.groovy', 2, 1, false), @@ -428,15 +433,19 @@ class testDocuments extends Map { this.forEach(doc => doc.reject()); }, timeout); - // Close stale editors. - await executeCommand('workbench.action.closeAllEditors'); + try { + // Close stale editors. + await executeCommand('workbench.action.closeAllEditors'); - doc = await doc.setupTest(); - if (action) { - action(doc, this); - } + doc = await doc.setupTest(); + if (action) { + await action(doc, this); + } - resolve(); + resolve(); + } catch (err) { + reject(err); + } }).finally(async () => { clearTimeout(timer); await this.cleanup(); @@ -505,6 +514,7 @@ suite('VsCode GroovyLint Test Suite', async function() { test(`Format "${name}"`, async function() { await testSingle(this, name, async function(doc: testDocument): Promise { const textBefore = doc.text; + const origEOL: string = textEOL(textBefore); const textEdits = await executeCommand('vscode.executeFormatDocumentProvider', doc.uri, {}); if (!doc.formats) { // Not expected for format. @@ -513,9 +523,12 @@ suite('VsCode GroovyLint Test Suite', async function() { } await doc.applyEdits(textEdits); - const textAfter = doc.text; + const textAfter = doc.text; assert(textBefore !== textAfter, 'Text not updated format'); + + const newEOL: string = textEOL(textAfter); + assert(origEOL === newEOL, `end of line sequence changed from ${origEOL} to ${newEOL}`); }); }); }); @@ -527,6 +540,7 @@ suite('VsCode GroovyLint Test Suite', async function() { const docDetails = documentDetails.get(name); await testSingle(this, name, async function(doc: testDocument): Promise { const textBefore = doc.text; + const origEOL: string = textEOL(textBefore); const promiseNames: string[] = []; if (doc.lint !== doc.lintFix) { promiseNames.push(testDocument.diagnosticPromise, testDocument.documentEditPromise); @@ -544,36 +558,57 @@ suite('VsCode GroovyLint Test Suite', async function() { assert(textBefore === textAfter, 'Unexpected text update'); } assert(diagsLen === doc.lintFix, `Found ${diagsLen} diagnostics expected ${doc.lintFix}`); + + const newEOL: string = textEOL(textAfter); + assert(origEOL === newEOL, `end of line sequence changed from ${origEOL} to ${newEOL}`); }, docDetails.timeout * 3); }); }); // Disable rules for a line. - test('Disable next line', async function() { - await testSingle(this, tinyGroovy, async function(doc: testDocument): Promise { - const lineNb = 5; - await doc.disableRule('groovyLint.disableRule', 'SpaceBeforeOpeningBrace', lineNb); - await doc.disableRule('groovyLint.disableRule', 'UnnecessaryGString', lineNb + 1); // Disable added a line. + documentDetails.forEach(async (_, name) => { + if (!name.startsWith('tiny')) { + return; + } - const disableLine = doc.doc.lineAt(lineNb).text; + test.only(`Disable "${name}" next line`, async function() { + await testSingle(this, name, async function(doc: testDocument): Promise { + const origEOL: string = textEOL(doc.text); + const lineNb = 5; + // Add a disable line. + await doc.disableRule('groovyLint.disableRule', 'SpaceBeforeOpeningBrace', lineNb); - assert(disableLine.includes(`/* groovylint-disable-next-line SpaceBeforeOpeningBrace, UnnecessaryGString */`), - `groovylint-disable-next-line not added correctly found: ${disableLine}` - ); + // Amend the disable line. + await doc.disableRule('groovyLint.disableRule', 'UnnecessaryGString', lineNb + 1); + + const disableLine = doc.doc.lineAt(lineNb).text; + assert(disableLine.includes(`/* groovylint-disable-next-line SpaceBeforeOpeningBrace, UnnecessaryGString */`), + `groovylint-disable-next-line not added correctly found: ${disableLine}` + ); + + const newEOL: string = textEOL(doc.text); + assert(origEOL === newEOL, `end of line sequence changed from ${origEOL} to ${newEOL}`); + }); }); }); // Disable rules for entire file. test('Disable rules in all file', async function() { await testSingle(this, tinyGroovy, async function(doc: testDocument): Promise { + const origEOL: string = textEOL(doc.text); + // Add a disable line. await doc.disableRule('groovyLint.disableRuleInFile', 'CompileStatic'); + + // Amend the disable line. await doc.disableRule('groovyLint.disableRuleInFile', 'DuplicateStringLiteral'); const disableLine = doc.doc.lineAt(0).text; - assert(disableLine.includes('/* groovylint-disable CompileStatic, DuplicateStringLiteral */'), `groovylint-disable not added correctly found: ${disableLine}` ); + + const newEOL: string = textEOL(doc.text); + assert(origEOL === newEOL, `end of line sequence changed from ${origEOL} to ${newEOL}`); }); }); @@ -581,6 +616,7 @@ suite('VsCode GroovyLint Test Suite', async function() { test('Quick fix error in tiny document', async function() { await testSingle(this, tinyGroovy, async function(doc: testDocument): Promise { const textBefore = doc.text; + const origEOL: string = textEOL(textBefore); const diagnostic = doc.diags.find(diag => (diag.code as string).startsWith('UnnecessarySemicolon')); // Request code actions. await doc.execute([], 'vscode.executeCodeActionProvider', doc.uri, diagnostic.range); @@ -590,6 +626,9 @@ suite('VsCode GroovyLint Test Suite', async function() { const textAfter = doc.text; assert(textBefore !== textAfter, 'Text not updated'); + + const newEOL: string = textEOL(textAfter); + assert(origEOL === newEOL, `end of line sequence changed from ${origEOL} to ${newEOL}`); }); }); @@ -597,6 +636,7 @@ suite('VsCode GroovyLint Test Suite', async function() { test('Quick fix error in entire file in tiny document', async function() { await testSingle(this, tinyGroovy, async function(doc: testDocument): Promise { const textBefore = doc.text; + const origEOL: string = textEOL(textBefore); const diagnostic = doc.diags.find(diag => (diag.code as string).startsWith('UnnecessarySemicolon')); // Request code actions. @@ -604,9 +644,12 @@ suite('VsCode GroovyLint Test Suite', async function() { // Apply Quick Fix. await doc.execute([testDocument.diagnosticPromise, testDocument.documentEditPromise], 'groovyLint.quickFixFile', doc.uri.toString(), diagnostic); - const textAfter = doc.text; + const textAfter = doc.text; assert(textBefore !== textAfter, 'Text not updated'); + + const newEOL: string = textEOL(textAfter); + assert(origEOL === newEOL, `end of line sequence changed from ${origEOL} to ${newEOL}`); }); }); @@ -657,3 +700,40 @@ async function executeCommand(command: string, ...args: any[]): Promise { debug(`Execute command ${command} with args ${JSON.stringify(args)}`); return vscode.commands.executeCommand(command, ...args); } + +/** + * Returns the name of the predominate End Of Line (EOL) of the given text. + * + * @param text the text to determine the EOL from. + * @returns the name of the predominate EOL sequence of text. + */ +function textEOL(text: string): string { + const eols: string[] = text.match(eolRegExp) || []; + let dos: number = 0; + let unix: number = 0; + let mac: number = 0; + eols.forEach(eol => { + switch (eol) { + case '\r\n': + dos++; + break; + case '\n': + unix++; + break; + case '\r': + mac++; + break; + } + }); + + let eol = 'CRLF'; + if (unix > dos) { + eol = 'LF'; + } + + if (mac > unix) { + eol = 'CR'; + } + + return eol; +} diff --git a/server/src/DocumentsManager.ts b/server/src/DocumentsManager.ts index 3a3f994..4f8a203 100644 --- a/server/src/DocumentsManager.ts +++ b/server/src/DocumentsManager.ts @@ -2,9 +2,8 @@ import { TextDocuments, Diagnostic, WorkspaceFolder } from 'vscode-languageserve import { TextDocument, DocumentUri, TextEdit } from 'vscode-languageserver-textdocument'; import { executeLinter } from './linter'; import { applyQuickFixes, applyQuickFixesInFile, disableErrorWithComment, disableErrorForProject } from './codeActions'; -import { isTest, showRuleDocumentation } from './clientUtils'; +import { isTest, showRuleDocumentation, eolAndLines } from './clientUtils'; import { URI } from 'vscode-uri'; -import { EOL } from 'os'; import { resolve } from 'path'; import { StatusNotification, VsCodeGroovyLintSettings } from './types'; import { lintFolder } from './folder'; @@ -377,16 +376,6 @@ export class DocumentsManager { return this.documents.get(textDocument.uri) || textDocument; // Or expression, in case the textDocument is not opened yet } - // Split source string into array of lines - getTextDocumentLines(textDocument: TextDocument): string[] { - // TODO(steve): Use TextDocument.eol instead of EOL so we - // maintain the original line endings. - let normalizedString = textDocument.getText() + ""; - normalizedString = normalizedString.replace(/\r/g, ""); - normalizedString = normalizedString.replace(/\n/g, EOL); - return normalizedString.split(EOL); - } - // Update diagnostics on client and store them in docsDiagnostics field async updateDiagnostics(docUri: string, diagnostics: Diagnostic[]): Promise { debug(`Update diagnostics for ${docUri}: ${diagnostics.length} diagnostics sent`); diff --git a/server/src/clientUtils.ts b/server/src/clientUtils.ts index 36a6d45..bc5b993 100644 --- a/server/src/clientUtils.ts +++ b/server/src/clientUtils.ts @@ -1,16 +1,28 @@ -import { TextDocument, TextEdit } from 'vscode-languageserver-textdocument'; -import { TextDocumentEdit, WorkspaceEdit, ShowMessageRequestParams, MessageType, ShowMessageParams, ShowMessageRequest } from 'vscode-languageserver'; +import { TextDocument } from 'vscode-languageserver-textdocument'; +import { + TextDocumentEdit, + WorkspaceEdit, + ShowMessageRequestParams, + MessageType, + ShowMessageRequest, + TextEdit, + Range, +} from 'vscode-languageserver'; import { DocumentsManager } from './DocumentsManager'; import { OpenNotification } from './types'; const debug = require("debug")("vscode-groovy-lint"); -import { EOL } from 'os'; + +// RegExp to find and capture End Of Line (EOL) sequences. +export const eolCaptureRegExp: RegExp = new RegExp(/(\r\n|\n|\r)/); + +// RegExp to replace End Of Line (EOL) sequences. +export const eolReplaceRegExp: RegExp = new RegExp(/\r\n|\n|\r/g); const defaultDocUrl = "https://codenarc.github.io/CodeNarc/codenarc-rule-index.html"; // Apply updated source into the client TextDocument -export async function applyTextDocumentEditOnWorkspace(docManager: DocumentsManager, textDocument: TextDocument, updatedSource: string, where: any = {}) { - textDocument = docManager.getUpToDateTextDocument(textDocument); - const textDocEdit: TextDocumentEdit = createTextDocumentEdit(docManager, textDocument, updatedSource, where); +export async function applyTextDocumentEditOnWorkspace(docManager: DocumentsManager, textDocument: TextDocument, textEdit: TextEdit) { + const textDocEdit: TextDocumentEdit = TextDocumentEdit.create({ uri: textDocument.uri, version: textDocument.version }, [textEdit]); const applyWorkspaceEdits: WorkspaceEdit = { documentChanges: [textDocEdit] }; @@ -18,67 +30,75 @@ export async function applyTextDocumentEditOnWorkspace(docManager: DocumentsMana debug(`Updated ${textDocument.uri} using WorkspaceEdit (${JSON.stringify(applyEditResult)})`); } -// Create a TextDocumentEdit that will be applied on client workspace -export function createTextDocumentEdit(docManager: DocumentsManager, textDocument: TextDocument, updatedSource: string, where: any = {}): TextDocumentEdit { - const textEdit: TextEdit = createTextEdit(docManager, textDocument, updatedSource, where); - const textDocEdit: TextDocumentEdit = TextDocumentEdit.create({ uri: textDocument.uri, version: textDocument.version }, [textEdit]); - return textDocEdit; -} +/** + * Create text edit to replace the whole file maintaining line endings. + * + * @param originalText the original text. + * @param newText the new text. + * @returns a TextEdit which replaces currentText with newText. + */ +export function createTextEditReplaceAll(originalText: string, newText: string): TextEdit { + const [eol, lines]: [string, string[]] = eolAndLines(originalText); -// Create text edit for the whole file from updated source -export function createTextEdit(docManager: DocumentsManager, textDocument: TextDocument, updatedSource: string, where: any = {}): TextEdit { - const allLines = docManager.getTextDocumentLines(textDocument); - // If range is not sent, replace all file lines - let textEdit: TextEdit; - // Insert at position - if (where.insertLinePos || where.insertLinePos === 0) { - allLines.splice(where.insertLinePos, 0, updatedSource); - textEdit = { - range: { - start: { line: 0, character: 0 }, - end: { line: allLines.length - 1, character: allLines[allLines.length - 1].length } - }, - newText: allLines.join(EOL) - }; - } - // Replace line at position - else if (where.replaceLinePos || where.replaceLinePos === 0) { - textEdit = { - range: { - start: { line: where.replaceLinePos, character: 0 }, - end: { line: where.replaceLinePos, character: allLines[where.replaceLinePos].length } - }, - newText: updatedSource - }; + // Pop is faster than indexed access and also avoids having to check the index going negative. + const lastLine: string = lines.pop() || ""; + let lastLineCharIdx: number = lastLine.length; + if (lastLineCharIdx > 0) { + lastLineCharIdx--; } - // Replace all source - else if (!where?.range) { - textEdit = { - range: { - start: { line: 0, character: 0 }, - end: { line: allLines.length - 1, character: allLines[allLines.length - 1].length } - }, - newText: updatedSource - }; + + const range: Range = Range.create(0, 0, lines.length, lastLineCharIdx); + return TextEdit.replace(range, newText.replace(eolReplaceRegExp, eol)); +} + +/** + * Returns the predominant end of line sequence and lines of a string. + * + * @param text the string to process. + * @returns the predominant end of line sequence and the lines. + */ +export function eolAndLines(text: string): [string, string[]] { + const parts: string[] = text.split(eolCaptureRegExp); + let dos: number = 0; + let unix: number = 0; + let mac: number = 0; + const lines: string[] = []; + parts.forEach(val => { + switch (val) { + case '\r\n': + dos++; + break; + case '\n': + unix++; + break; + case '\r': + mac++; + break; + default: + lines.push(val); + break; + } + }); + + let eol = '\r\n'; + if (unix > dos) { + eol = '\n'; } - // Provided range - else { - textEdit = { - range: where.range, - newText: updatedSource - }; + + if (mac > unix) { + eol = '\r'; } - return textEdit; + + return [eol, lines]; } // Return updated source export function getUpdatedSource(docLinter: any, prevSource: string) { - if (docLinter && docLinter.lintResult && docLinter.lintResult.files && docLinter.lintResult.files[0]) { + if (docLinter?.lintResult?.files && docLinter.lintResult.files[0]) { return docLinter.lintResult.files[0].updatedSource; } - else { - return prevSource; - } + + return prevSource; } // Shows the documentation of a rule diff --git a/server/src/codeActions.ts b/server/src/codeActions.ts index 9f3d07b..64fbd7d 100644 --- a/server/src/codeActions.ts +++ b/server/src/codeActions.ts @@ -6,14 +6,22 @@ import { CodeActionKind, Diagnostic, MessageType, - ShowMessageRequestParams + ShowMessageRequestParams, + Position, + Range, + TextEdit, } from 'vscode-languageserver'; import { URI } from 'vscode-uri'; -import { isNullOrUndefined } from "util"; import * as fse from "fs-extra"; import { DocumentsManager } from './DocumentsManager'; -import { applyTextDocumentEditOnWorkspace, getUpdatedSource, notifyFixFailures } from './clientUtils'; +import { + applyTextDocumentEditOnWorkspace, + getUpdatedSource, + notifyFixFailures, + createTextEditReplaceAll, + eolAndLines, +} from './clientUtils'; import { parseLinterResults } from './linterParser'; import { StatusNotification, OpenNotification } from './types'; import { @@ -30,7 +38,7 @@ const debug = require("debug")("vscode-groovy-lint"); const trace = require("debug")("vscode-groovy-lint-trace"); /** - * Provide quick-fixes for a piece of code * + * Provide quick-fixes for a piece of code * @export * @param {TextDocument} textDocument * @param {CodeActionParams} codeActionParams @@ -39,7 +47,7 @@ const trace = require("debug")("vscode-groovy-lint-trace"); export function provideQuickFixCodeActions(textDocument: TextDocument, codeActionParams: CodeActionParams, docQuickFixes: any): CodeAction[] { const diagnostics = codeActionParams.context.diagnostics; const quickFixCodeActions: CodeAction[] = []; - if (isNullOrUndefined(diagnostics) || diagnostics.length === 0) { + if (diagnostics === undefined || diagnostics === null || diagnostics.length === 0) { return quickFixCodeActions; } @@ -233,7 +241,10 @@ export async function applyQuickFixes(diagnostics: Diagnostic[], textDocumentUri if (docLinter.status !== 0) { debug(`Error while fixing ${textDocument.uri} status: ${docLinter.status} error: ${docLinter.error ? docLinter.error.msg : 'unknown'}}`); } else if (docLinter.lintResult.summary.totalFixedNumber > 0) { - await applyTextDocumentEditOnWorkspace(docManager, textDocument, getUpdatedSource(docLinter, textDocument.getText())); + const source: string = textDocument.getText(); + const updatedSource: string = getUpdatedSource(docLinter, source); + const textEdit: TextEdit = createTextEditReplaceAll(source, updatedSource); + await applyTextDocumentEditOnWorkspace(docManager, textDocument, textEdit); setTimeout(() => { // Wait 500ms so we are more sure that the textDocument is already updated const newDoc = docManager.getUpToDateTextDocument(textDocument); docManager.validateTextDocument(newDoc, { force: true }); @@ -272,7 +283,7 @@ export async function disableErrorWithComment(diagnostic: Diagnostic, textDocume } const textDocument: TextDocument = docManager.getDocumentFromUri(textDocumentUri); - const allLines = docManager.getTextDocumentLines(textDocument); + const [eol, allLines]: [string, string[]] = eolAndLines(textDocument.getText()); // Get line to check or create let linePos: number = 0; let disableKey: string = ''; @@ -299,12 +310,16 @@ export async function disableErrorWithComment(diagnostic: Diagnostic, textDocume commentRules.push(errorCode); commentRules.sort(); const disableLine = indent + `/* ${disableKey} ${[...new Set(commentRules)].join(", ")} */`; - await applyTextDocumentEditOnWorkspace(docManager, textDocument, disableLine, { replaceLinePos: prevLinePos }); + const range: Range = Range.create(prevLinePos, 0, prevLinePos, prevLine.length); + const textEdit: TextEdit = TextEdit.replace(range, disableLine); + await applyTextDocumentEditOnWorkspace(docManager, textDocument, textEdit); } else { // Add new /* groovylint-disable */ or /* groovylint-disable-next-line */ - const disableLine = indent + `/* ${disableKey} ${errorCode} */`; - await applyTextDocumentEditOnWorkspace(docManager, textDocument, disableLine, { insertLinePos: linePos }); + const disableLine = indent + `/* ${disableKey} ${errorCode} */${eol}`; + const position: Position = Position.create(linePos, 0); + const textEdit: TextEdit = TextEdit.insert(position, disableLine); + await applyTextDocumentEditOnWorkspace(docManager, textDocument, textEdit); } // The content of textDocument.getText() will be out of date, so request diff --git a/server/src/linter.ts b/server/src/linter.ts index 701cf9f..fe8eb55 100644 --- a/server/src/linter.ts +++ b/server/src/linter.ts @@ -4,7 +4,7 @@ import { URI } from 'vscode-uri'; import * as path from 'path'; import { DocumentsManager } from './DocumentsManager'; -import { applyTextDocumentEditOnWorkspace, getUpdatedSource, createTextEdit, notifyFixFailures } from './clientUtils'; +import { applyTextDocumentEditOnWorkspace, getUpdatedSource, createTextEditReplaceAll, notifyFixFailures } from './clientUtils'; import { parseLinterResults } from './linterParser'; import { StatusNotification, OpenNotification } from './types'; import { ShowMessageRequestParams, MessageType, ShowMessageRequest } from 'vscode-languageserver'; @@ -311,11 +311,11 @@ export async function executeLinter(textDocument: TextDocument, docManager: Docu // Send updated sources to client if format mode else if (format === true && linter.status === 0 && linter.lintResult.summary.totalFixedNumber > 0) { const updatedSource = getUpdatedSource(linter, source); + const textEdit: TextEdit = createTextEditReplaceAll(source, updatedSource); if (opts.applyNow) { - await applyTextDocumentEditOnWorkspace(docManager, textDocument, updatedSource); + await applyTextDocumentEditOnWorkspace(docManager, textDocument, textEdit); } else { - const textEdit = createTextEdit(docManager, textDocument, updatedSource); textEdits.push(textEdit); } // Display fix failures if existing @@ -324,7 +324,8 @@ export async function executeLinter(textDocument: TextDocument, docManager: Docu // Send updated sources to client if fix mode else if (fix === true && linter.status === 0 && linter.lintResult.summary.totalFixedNumber > 0) { const updatedSource = getUpdatedSource(linter, source); - await applyTextDocumentEditOnWorkspace(docManager, textDocument, updatedSource); + const textEdit: TextEdit = createTextEditReplaceAll(source, updatedSource); + await applyTextDocumentEditOnWorkspace(docManager, textDocument, textEdit); // Display fix failures if existing await notifyFixFailures(fixFailures, docManager); } @@ -421,8 +422,9 @@ async function manageFixSourceBeforeCallingLinter(source: string, textDocument: const replaceChars = " ".repeat(indentLength); const newSources = source.replace(/\t/g, replaceChars); if (newSources !== source) { - await applyTextDocumentEditOnWorkspace(docManager, textDocument, newSources); - debug(`Replaces tabs by spaces in ${textDocument.uri}`); + const textEdit: TextEdit = createTextEditReplaceAll(source, newSources); + await applyTextDocumentEditOnWorkspace(docManager, textDocument, textEdit); + debug(`Replaced tabs with spaces in ${textDocument.uri}`); return 'updated'; } }