Skip to content

Commit

Permalink
fix: file end of line character changes
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@stevenh
stevenh committed Dec 17, 2023
1 parent 4aac003 commit b934e19
Show file tree
Hide file tree
Showing 10 changed files with 273 additions and 105 deletions.
3 changes: 3 additions & 0 deletions .gitattributes
View file
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions client/src/test/examples/tiny-cr.groovy
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import groovy.io.FileTypeimport groovy.json.*import groovy.time.TimeCategoryimport static groovyx.gpars.GParsPool.withPool def script = new GroovyScriptEngine( "." ).with{ loadScriptByName( 'Utils.groovy' ) ; }this.metaClass.mixin scriptdef 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)) }}
Expand Down
56 changes: 56 additions & 0 deletions client/src/test/examples/tiny-crlf.groovy
View file
Original file line number Diff line number Diff line change
@@ -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))
}

}
0 client/src/test/examples/tiny.groovy → client/src/test/examples/tiny-lf.groovy
View file
File renamed without changes.
120 changes: 100 additions & 20 deletions client/src/test/suite/extension.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ 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;

// Additional timeout for the first test to
// 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.
*/
Expand All @@ -54,6 +57,8 @@ const documentDetails = new Map<string, testDocumentDetails>();
[
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),
Expand Down Expand Up @@ -428,15 +433,19 @@ class testDocuments extends Map<string, testDocument> {
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();
Expand Down Expand Up @@ -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<void> {
const textBefore = doc.text;
const origEOL: string = textEOL(textBefore);
const textEdits = await executeCommand('vscode.executeFormatDocumentProvider', doc.uri, {});
if (!doc.formats) {
// Not expected for format.
Expand All @@ -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}`);
});
});
});
Expand All @@ -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<void> {
const textBefore = doc.text;
const origEOL: string = textEOL(textBefore);
const promiseNames: string[] = [];
if (doc.lint !== doc.lintFix) {
promiseNames.push(testDocument.diagnosticPromise, testDocument.documentEditPromise);
Expand All @@ -544,43 +558,65 @@ 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<void> {
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<void> {
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<void> {
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}`);
});
});

// Quick fix error.
test('Quick fix error in tiny document', async function() {
await testSingle(this, tinyGroovy, async function(doc: testDocument): Promise<void> {
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);
Expand All @@ -590,23 +626,30 @@ 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}`);
});
});

// Quick fix all error types.
test('Quick fix error in entire file in tiny document', async function() {
await testSingle(this, tinyGroovy, async function(doc: testDocument): Promise<void> {
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);

// 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}`);
});
});

Expand Down Expand Up @@ -657,3 +700,40 @@ async function executeCommand(command: string, ...args: any[]): Promise<any> {
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;
}
13 changes: 1 addition & 12 deletions server/src/DocumentsManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<void> {
debug(`Update diagnostics for ${docUri}: ${diagnostics.length} diagnostics sent`);
Expand Down
Loading

0 comments on commit b934e19

Please sign in to comment.