Skip to content

Commit 5b3c92b

Browse files
aboseabose
committed
test: code inspection integ tests invalid fixes supplied
1 parent e990b1b commit 5b3c92b

3 files changed

Lines changed: 76 additions & 15 deletions

File tree

‎src/language/CodeInspection.js‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ define(function (require, exports, module) {
575575

576576
let fixIDCounter = 1;
577577
let documentFixes = new Map(), lastDocumentScanTimeStamp;
578-
function _registerNewFix(editor, fix, providerName) {
578+
function _registerNewFix(editor, fix, providerName, maxOffset) {
579579
if(!editor || !fix || !fix.rangeOffset) {
580580
return null;
581581
}
@@ -585,6 +585,9 @@ define(function (require, exports, module) {
585585
lastDocumentScanTimeStamp = editor.document.lastChangeTimestamp;
586586
documentFixes.clear();
587587
}
588+
if(_isInvalidFix(fix, maxOffset)){
589+
return null;
590+
}
588591
fixIDCounter++;
589592
fix.providerName = providerName;
590593
documentFixes.set(`${fixIDCounter}`, fix);
@@ -602,6 +605,7 @@ define(function (require, exports, module) {
602605
if(!(editor && resultProviderEntries && resultProviderEntries.length)) {
603606
return;
604607
}
608+
const maxOffset = editor.document.getText().length;
605609
editor.operation(function () {
606610
editor.off("viewportChange.codeInspection");
607611
editor.on("viewportChange.codeInspection", _editorVieportChangeHandler);
@@ -618,7 +622,7 @@ define(function (require, exports, module) {
618622
if (_shouldMarkTokenAtPosition(editor, error)) {
619623
let mark;
620624
const markOptions = _getMarkOptions(error);
621-
const fixID = _registerNewFix(editor, error.fix, resultProvider.provider.name);
625+
const fixID = _registerNewFix(editor, error.fix, resultProvider.provider.name, maxOffset);
622626
if(fixID) {
623627
markOptions.metadata = fixID;
624628
error.fix.id = fixID;
@@ -1009,16 +1013,21 @@ define(function (require, exports, module) {
10091013
description: Strings.DESCRIPTION_USE_PREFERED_ONLY
10101014
});
10111015

1016+
function _isInvalidFix(fixDetails, maxOffset) {
1017+
return (!_.isNumber(fixDetails.rangeOffset.start) || !_.isNumber(fixDetails.rangeOffset.end) ||
1018+
fixDetails.rangeOffset.start < 0 || fixDetails.rangeOffset.end < 0 ||
1019+
fixDetails.rangeOffset.start > maxOffset || fixDetails.rangeOffset.end > maxOffset ||
1020+
typeof fixDetails.replaceText !== "string");
1021+
}
1022+
10121023
function _fixProblem(fixID) {
10131024
const fixDetails = documentFixes.get(fixID);
10141025
const editor = EditorManager.getCurrentFullEditor();
10151026
const maxOffset = editor.document.getText().length;
1016-
if(fixDetails.rangeOffset.start < 0 || fixDetails.rangeOffset.end < 0 ||
1017-
fixDetails.rangeOffset.start > maxOffset || fixDetails.rangeOffset.end > maxOffset ){
1018-
Dialogs.showErrorDialog(Strings.CANNOT_FIX_TITLE,
1019-
StringUtils.format(Strings.CANNOT_FIX_INVALID_MESSAGE, fixDetails.providerName));
1020-
} else if(!editor || !fixDetails || editor.document.lastChangeTimestamp !== lastDocumentScanTimeStamp) {
1027+
if(!editor || !fixDetails || editor.document.lastChangeTimestamp !== lastDocumentScanTimeStamp) {
10211028
Dialogs.showErrorDialog(Strings.CANNOT_FIX_TITLE, Strings.CANNOT_FIX_MESSAGE);
1029+
} else if(_isInvalidFix(fixDetails, maxOffset)){
1030+
console.error("Invalid fix:", fixDetails); // this should never happen as we filter the fix while inserting
10221031
} else {
10231032
const from = editor.posFromIndex(fixDetails.rangeOffset.start),
10241033
to = editor.posFromIndex(fixDetails.rangeOffset.end);
@@ -1039,7 +1048,11 @@ define(function (require, exports, module) {
10391048
return;
10401049
}
10411050
const replacements = [];
1051+
const maxOffset = editor.document.getText().length;
10421052
for(let fixDetails of documentFixes.values()){
1053+
if(_isInvalidFix(fixDetails, maxOffset)){
1054+
console.error("Invalid fix:", fixDetails); // this should never happen
1055+
}
10431056
replacements.push({
10441057
from: editor.posFromIndex(fixDetails.rangeOffset.start),
10451058
to: editor.posFromIndex(fixDetails.rangeOffset.end),

‎src/nls/root/strings.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,8 +425,8 @@ define({
425425
"FIX_ERROR": "Fix Problem",
426426
"FIX": "Fix",
427427
"CANNOT_FIX_TITLE": "Failed to Apply Fix",
428+
"CANNOT_FIX_SOME_TITLE": "Failed to Apply Some Fixes",
428429
"CANNOT_FIX_MESSAGE": "The document has been modified since the fix was prepared. Please try again.",
429-
"CANNOT_FIX_INVALID_MESSAGE": "Invalid Fix supplied by {0}",
430430
"LINTER_TIMED_OUT": "{0} has timed out after waiting for {1} ms",
431431
"LINTER_FAILED": "{0} terminated with error: {1}",
432432

‎test/spec/CodeInspection-fix-integ-test.js‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ define(function (require, exports, module) {
5050
};
5151
}
5252

53+
let invalidFix;
54+
5355
function scanFile(text, fullPath) {
5456
const currentEditor = EditorManager.getActiveEditor();
5557
if(currentEditor.document.file.fullPath !== fullPath) {
@@ -72,7 +74,7 @@ define(function (require, exports, module) {
7274

7375
if (line.includes('this line a fixable error')) {
7476
const wordPosition = findWordPosition(line, 'fixable');
75-
error.fix = {
77+
error.fix = invalidFix ? invalidFix : {
7678
replaceText: "no",
7779
rangeOffset: {
7880
start: currentEditor.indexFromPos({line: lineNumber, ch: wordPosition.start}),
@@ -88,11 +90,10 @@ define(function (require, exports, module) {
8890
return {errors};
8991
}
9092

91-
let linterCount = 0;
93+
let linterName = "vbscript mock linter";
9294
function createVBScriptInspector() {
93-
linterCount++;
9495
const provider = {
95-
name: "vbscript mock linter" + linterCount,
96+
name: linterName,
9697
scanFile
9798
};
9899

@@ -120,6 +121,7 @@ define(function (require, exports, module) {
120121

121122
afterEach(async function () {
122123
await testWindow.closeAllFiles();
124+
invalidFix = null;
123125
});
124126

125127
afterAll(async function () {
@@ -287,7 +289,7 @@ define(function (require, exports, module) {
287289
expect($("#problems-panel .ph-fix-problem").length).toBe(5); // 5 fix buttons should be there
288290
});
289291

290-
async function _validateCannotFix(fixAll) {
292+
async function _validateCannotFix(fixAll, message=Strings.CANNOT_FIX_MESSAGE) {
291293
await _openProjectFile("testFix.vbs");
292294
const editor = EditorManager.getActiveEditor();
293295
editor.setCursorPos(1, 1);
@@ -309,7 +311,7 @@ define(function (require, exports, module) {
309311
await SpecRunnerUtils.waitForModalDialog();
310312
const dialogText = $(".error-dialog").text();
311313
expect(dialogText.includes(Strings.CANNOT_FIX_TITLE)).toBeTrue();
312-
expect(dialogText.includes(Strings.CANNOT_FIX_MESSAGE)).toBeTrue();
314+
expect(dialogText.includes(message)).toBeTrue();
313315

314316
await SpecRunnerUtils.clickDialogButton();
315317
}
@@ -322,7 +324,53 @@ define(function (require, exports, module) {
322324
await _validateCannotFix(true);
323325
});
324326

325-
// todo invalid fixes test, doc changed after fix dialog
327+
async function _validateNoFixableErrors() {
328+
await _openProjectFile("testFix.vbs");
329+
const editor = EditorManager.getActiveEditor();
330+
editor.setCursorPos(1, 1);
331+
332+
expect($("#problems-panel").is(":visible")).toBeTrue();
333+
expect($("#problems-panel .ph-fix-problem").length).toBe(0); // 5 fix buttons should be there
334+
}
335+
336+
it("should not be able to fix if invalid fix supplied", async function () {
337+
invalidFix = {
338+
replaceText: "no",
339+
rangeOffset: {
340+
start: -1,
341+
end: -1
342+
}
343+
};
344+
await _validateNoFixableErrors();
345+
});
326346

347+
it("should not be able to fix if invalid fix supplied 2", async function () {
348+
invalidFix = {
349+
replaceText: "no",
350+
rangeOffset: {
351+
end: 999999
352+
}
353+
};
354+
await _validateNoFixableErrors();
355+
});
356+
357+
it("should not be able to fix if invalid fix supplied 3", async function () {
358+
invalidFix = {
359+
replaceText: "no",
360+
rangeOffset: {}
361+
};
362+
await _validateNoFixableErrors();
363+
});
364+
365+
it("should not be able to fix if invalid fix supplied 4", async function () {
366+
invalidFix = {
367+
replaceText: {}, // only text can be provided here
368+
rangeOffset: { // range valid but not text
369+
start: 2,
370+
end: 5
371+
}
372+
};
373+
await _validateNoFixableErrors();
374+
});
327375
});
328376
});

0 commit comments

Comments
 (0)