feat: fix problems infrastructure and eslint fix error impl

abose · abose · commit dfa49d0fc952 · 2024-06-10T21:47:11.000+05:30
diff --git a/src/editor/Editor.js b/src/editor/Editor.js
@@ -1168,6 +1168,7 @@ define(function (require, exports, module) {
      * @param {Object} [options] - When given, it should be  one of the predefined `Editor.MARK_OPTION_UNDERLINE*` or
      * it should be an object that may contain the following configuration options:
      *
+     * @param {string} [options.metadata] - If you want to store any metadata object with the mark, use this.
      * @param {string} [options.className] -Assigns a CSS class to the marked stretch of text.
      * @param {string} [options.css] -A string of CSS to be applied to the covered text. For example "color: #fe3".
      * @param {string} [options.startStyle] -Can be used to specify an extra CSS class to be applied to the leftmost
@@ -1226,6 +1227,7 @@ define(function (require, exports, module) {
     Editor.prototype.markText = function (markType, cursorFrom, cursorTo, options) {
         let newMark = this._codeMirror.markText(cursorFrom, cursorTo, options);
         newMark.markType = markType;
+        newMark.metadata = options && options.metadata;
         return newMark;
     };
 
diff --git a/src/extensions/default/JSLint/ESLint.js b/src/extensions/default/JSLint/ESLint.js
@@ -102,14 +102,25 @@ define(function (require, exports, module) {
 
     function _getErrors(resultArray) {
         return resultArray.map(function (lintError) {
+            let fix = null;
+            if(lintError.fix && lintError.fix.range && typeof lintError.fix.text === "string") {
+                fix = {
+                    replaceText: lintError.fix.text,
+                    rangeOffset: {
+                        start: lintError.fix.range[0],
+                        end: lintError.fix.range[1]
+                    }
+                };
+            }
             return {
                 pos: { line: _0Based(lintError.line), ch: _0Based(lintError.column)},
                 endPos: {
                     line: _0Based(lintError.endLine, lintError.line),
                     ch: _0Based(lintError.endColumn, lintError.column)
                 },
                 message: `${lintError.message} ESLint (${lintError.ruleId})`,
-                type: _getErrorClass(lintError.severity)
+                type: _getErrorClass(lintError.severity),
+                fix: fix
             };
         });
     }
diff --git a/src/htmlContent/problems-panel-table.html b/src/htmlContent/problems-panel-table.html
@@ -7,6 +7,10 @@
                 <input type="hidden" value="{{providerName}}" />
                 {{providerName}} ({{results.length}})
             </td>
+            <td></td>
+            <td></td>
+            <td></td>
+            <td></td>
         </tr>
         {{#results}}
         <tr class="{{display}} inspector-line">
@@ -16,11 +20,19 @@
             </td>
             <td class="line-text">{{message}}{{{htmlMessage}}}</td>
             <td>
-                <button class="btn btn-mini table-copy-err-button" title="{{Strings.COPY_ERROR}}"
+                <button class="btn btn-mini table-copy-err-button ph-copy-problem" title="{{Strings.COPY_ERROR}}"
                         data-message="{{message}}">
-                    <i class="fa-solid fa-copy"></i>
+                    <i class="fa-solid fa-copy ph-copy-problem"></i>
                 </button>
             </td>
+            <td>
+                {{#fix.id}}
+                <button class="btn btn-mini table-fix-err-button ph-fix-problem" title="{{Strings.FIX_ERROR}}"
+                        data-fixid="{{fix.id}}">
+                    {{Strings.FIX}}
+                </button>
+                {{/fix.id}}
+            </td>
             <td class="line-snippet">{{codeSnippet}}</td>
         </tr>
         {{/results}}
diff --git a/src/language/CodeInspection.js b/src/language/CodeInspection.js
@@ -43,6 +43,7 @@ define(function (require, exports, module) {
         CommandManager          = require("command/CommandManager"),
         DocumentManager         = require("document/DocumentManager"),
         EditorManager           = require("editor/EditorManager"),
+        Dialogs                 = require("widgets/Dialogs"),
         Editor                  = require("editor/Editor").Editor,
         MainViewManager         = require("view/MainViewManager"),
         LanguageManager         = require("language/LanguageManager"),
@@ -61,6 +62,8 @@ define(function (require, exports, module) {
     const CODE_INSPECTION_GUTTER_PRIORITY      = 500,
         CODE_INSPECTION_GUTTER = "code-inspection-gutter";
 
+    const EDIT_ORIGIN_LINT_FIX = "lint_fix";
+
     const INDICATOR_ID = "status-inspection";
 
     /** Values for problem's 'type' property */
@@ -504,20 +507,35 @@ define(function (require, exports, module) {
         });
     }
 
+    let fixIDCounter = 1;
+    let documentFixes = new Map(), lastDocumentScanTimeStamp;
+    function _registerNewFix(editor, fix) {
+        if(!editor || !fix || !fix.rangeOffset) {
+            return null;
+        }
+        if(editor.document.lastChangeTimestamp !== lastDocumentScanTimeStamp){
+            // the document changed from the last time the fixes where registered, we have to
+            // invalidate all existing fixes in that case.
+            lastDocumentScanTimeStamp = editor.document.lastChangeTimestamp;
+            documentFixes.clear();
+        }
+        fixIDCounter++;
+        documentFixes.set(`${fixIDCounter}`, fix);
+        return fixIDCounter;
+    }
 
     /**
      * Adds gutter icons and squiggly lines under err/warn/info to editor after lint.
+     * also updates  the passed in resultProviderEntries with fixes that can be applied.
      * @param resultProviderEntries
      * @private
      */
-    function _updateEditorMarks(resultProviderEntries) {
+    function _updateEditorMarksAndFixResults(resultProviderEntries) {
         let editor = EditorManager.getCurrentFullEditor();
         if(!(editor && resultProviderEntries && resultProviderEntries.length)) {
             return;
         }
         editor.operation(function () {
-            editor.clearAllMarks(CODE_MARK_TYPE_INSPECTOR);
-            editor.clearGutter(CODE_INSPECTION_GUTTER);
             editor.off("viewportChange.codeInspection");
             editor.on("viewportChange.codeInspection", _editorVieportChangeHandler);
             let gutterErrorMessages = {};
@@ -532,11 +550,16 @@ define(function (require, exports, module) {
                     // add squiggly lines
                     if (_shouldMarkTokenAtPosition(editor, error)) {
                         let mark;
+                        const markOptions = _getMarkOptions(error);
+                        const fixID = _registerNewFix(editor, error.fix);
+                        if(fixID) {
+                            markOptions.metadata = fixID;
+                            error.fix.id = fixID;
+                        }
                         if(error.endPos){
-                            mark = editor.markText(CODE_MARK_TYPE_INSPECTOR, error.pos, error.endPos,
-                                _getMarkOptions(error));
+                            mark = editor.markText(CODE_MARK_TYPE_INSPECTOR, error.pos, error.endPos, markOptions);
                         } else {
-                            mark = editor.markToken(CODE_MARK_TYPE_INSPECTOR, error.pos, _getMarkOptions(error));
+                            mark = editor.markToken(CODE_MARK_TYPE_INSPECTOR, error.pos, markOptions);
                         }
                         mark.type = error.type;
                         mark.message = error.message;
@@ -547,6 +570,7 @@ define(function (require, exports, module) {
         });
     }
 
+    let linterHadRun = false;
     /**
      * Run inspector applicable to current document. Updates status bar indicator and refreshes error list in
      * bottom panel. Does not run if inspection is disabled or if a providerName is given and does not
@@ -574,6 +598,14 @@ define(function (require, exports, module) {
             return !provider.canInspect || provider.canInspect(currentDoc.file.fullPath);
         });
 
+        let editor = EditorManager.getCurrentFullEditor();
+        if(editor){
+            lastDocumentScanTimeStamp = editor.document.lastChangeTimestamp;
+            documentFixes.clear();
+            editor.clearAllMarks(CODE_MARK_TYPE_INSPECTOR);
+            editor.clearGutter(CODE_INSPECTION_GUTTER);
+        }
+
         if (providerList && providerList.length) {
             var numProblems = 0;
             var aborted = false;
@@ -584,7 +616,7 @@ define(function (require, exports, module) {
 
             // run all the providers registered for this file type
             (_currentPromise = inspectFile(currentDoc.file, providerList)).then(function (results) {
-                _updateEditorMarks(results);
+                _updateEditorMarksAndFixResults(results);
                 // check if promise has not changed while inspectFile was running
                 if (this !== _currentPromise) {
                     return;
@@ -675,6 +707,7 @@ define(function (require, exports, module) {
             // No provider for current file
             _hasErrors = false;
             _currentPromise = null;
+            updatePanelTitleAndStatusBar(0, [], false);
             if(problemsPanel){
                 problemsPanel.hide();
             }
@@ -708,9 +741,18 @@ define(function (require, exports, module) {
      * @param {{name:string, scanFileAsync:?function(string, string):!{$.Promise},
      *         scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}} provider
      *
-     * Each error is: { pos:{line,ch}, endPos:?{line,ch}, message:string, type:?Type }
+     * Each error is: { pos:{line,ch}, endPos:?{line,ch}, message:string, htmlMessage:string, type:?Type ,
+     *                     fix: { // an optional fix, if present will show the fix button
+     *                     replace: "text to replace the offset given below",
+     *                     rangeOffset: {
+     *                         start: number,
+     *                         end: number
+     *                     }}}
      * If type is unspecified, Type.WARNING is assumed.
      * If no errors found, return either null or an object with a zero-length `errors` array.
+     * `message` will be printed as text as is. This is needed when the error text contains HTML that may be
+     * mis interpreted as html to display. If you want to display html, pass in `htmlMessage`. Both can be used
+     * at the same time, in which case both will be displayed.
      */
     function register(languageId, provider) {
         if (!_providers[languageId]) {
@@ -898,11 +940,35 @@ define(function (require, exports, module) {
         var $selectedRow;
         $problemsPanelTable = $problemsPanel.find(".table-container")
             .on("click", "tr", function (e) {
-                if ($(e.target).hasClass('table-copy-err-button')) {
+                if ($(e.target).hasClass('ph-copy-problem')) {
                     // Retrieve the message from the data attribute of the clicked element
                     const message = $(e.target).parent().parent().find(".line-text").text();
                     message && Phoenix.app.copyToClipboard(message);
+                    e.preventDefault();
+                    e.stopPropagation();
+                    MainViewManager.focusActivePane();
+                    return;
+                }
+                if ($(e.target).hasClass('ph-fix-problem')) {
+                    // Retrieve the message from the data attribute of the clicked element
+                    const fixid = "" + $(e.target).data("fixid");
+                    const fixDetails = documentFixes.get(fixid);
+                    const editor = EditorManager.getCurrentFullEditor();
+                    if(!editor || !fixDetails || editor.document.lastChangeTimestamp !== lastDocumentScanTimeStamp) {
+                        Dialogs.showErrorDialog(Strings.CANNOT_FIX_TITLE, Strings.CANNOT_FIX_MESSAGE);
+                    } else {
+                        const from = editor.posFromIndex(fixDetails.rangeOffset.start),
+                            to =  editor.posFromIndex(fixDetails.rangeOffset.end);
+                        editor.setSelection(from, to, true, Editor.BOUNDARY_BULLSEYE, EDIT_ORIGIN_LINT_FIX);
+                        editor.replaceSelection(fixDetails.replaceText, "around");
+                    }
+                    e.preventDefault();
+                    e.stopPropagation();
+                    MainViewManager.focusActivePane();
+                    run();
+                    return;
                 }
+
                 if ($selectedRow) {
                     $selectedRow.removeClass("selected");
                 }
diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js
@@ -418,6 +418,10 @@ define({
     "NO_LINT_AVAILABLE": "No linter available for {0}",
     "NOTHING_TO_LINT": "Nothing to lint",
     "COPY_ERROR": "Copy Error Message",
+    "FIX_ERROR": "Fix Problem",
+    "FIX": "Fix",
+    "CANNOT_FIX_TITLE": "Failed to Apply Fix",
+    "CANNOT_FIX_MESSAGE": "The document has been modified since the fix was prepared. Please try again.",
     "LINTER_TIMED_OUT": "{0} has timed out after waiting for {1} ms",
     "LINTER_FAILED": "{0} terminated with error: {1}",
 
diff --git a/src/styles/brackets.less b/src/styles/brackets.less
@@ -160,73 +160,42 @@ html, body {
 // selectors like .warning or .info. Even if error, warning, and info are used on the same div in any order,
 // the style for .error.error will take precedence due to its higher specificity.
 .editor-text-fragment-error.editor-text-fragment-error.editor-text-fragment-error.editor-text-fragment-error{
-    border-bottom:2px dotted @error-underline-text;
-    display: inline-block;
-    position: relative;
-}
-
-.editor-text-fragment-error.editor-text-fragment-error.editor-text-fragment-error.editor-text-fragment-error:after{
-    content: '';
-    width: 100%;
-    border-bottom:2px dotted @error-underline-text;
-    position: absolute;
-    display:block;
-    bottom: -3px;
-    left: 2px;
-}
-
-.editor-text-fragment-warn.editor-text-fragment-warn.editor-text-fragment-warn{
-    border-bottom:2px dotted @warn-underline-text;
-    .dark &{
-        border-bottom:2px dotted @dark-warn-underline-text;
-    }
-    display: inline-block;
-    position: relative;
+    text-decoration-style: wavy;
+    text-decoration-line: underline;
+    text-decoration-skip-ink: none;
+    text-decoration-thickness: 1px;
+    text-underline-offset: 2px;
+    text-decoration-color: @error-underline-text;
 }
 
-.editor-text-fragment-warn.editor-text-fragment-warn.editor-text-fragment-warn:after{
-    content: '';
-    width: 100%;
-    border-bottom:2px dotted @warn-underline-text;
+.editor-text-fragment-warn.editor-text-fragment-warn.editor-text-fragment-warn {
+    text-decoration-style: wavy;
+    text-decoration-line: underline;
+    text-decoration-skip-ink: none;
+    text-decoration-thickness: 1px;
+    text-underline-offset: 2px;
+    text-decoration-color: @warn-underline-text;
     .dark &{
-        border-bottom:2px dotted @dark-warn-underline-text;
+        text-decoration-color: @dark-warn-underline-text;
     }
-    position: absolute;
-    display:block;
-    bottom: -3px;
-    left: 2px;
 }
 
-.editor-text-fragment-info{
-    border-bottom:2px dotted @info-underline-text;
-    display: inline-block;
-    position: relative;
-}
-
-.editor-text-fragment-info:after{
-    content: '';
-    width: 100%;
-    border-bottom:2px dotted @info-underline-text;
-    position: absolute;
-    display:block;
-    bottom: -3px;
-    left: 2px;
-}
-
-.editor-text-fragment-spell-error.editor-text-fragment-spell-error{
-    border-bottom:2px dotted @spell-underline-text;
-    display: inline-block;
-    position: relative;
+.editor-text-fragment-spell-error.editor-text-fragment-spell-error {
+    text-decoration-style: wavy;
+    text-decoration-line: underline;
+    text-decoration-skip-ink: none;
+    text-decoration-thickness: 1px;
+    text-underline-offset: 2px;
+    text-decoration-color: @spell-underline-text;
 }
 
-.editor-text-fragment-spell-error.editor-text-fragment-spell-error:after{
-    content: '';
-    width: 100%;
-    border-bottom:2px dotted @spell-underline-text;
-    position: absolute;
-    display:block;
-    bottom: -3px;
-    left: 2px;
+.editor-text-fragment-info {
+    text-decoration-style: wavy;
+    text-decoration-line: underline;
+    text-decoration-skip-ink: none;
+    text-decoration-thickness: 1px;
+    text-underline-offset: 2px;
+    text-decoration-color: @info-underline-text;
 }
 
 .hidden-element{
@@ -3005,7 +2974,6 @@ textarea.exclusions-editor {
 
     .table-copy-err-button {
         visibility: hidden;
-        cursor: pointer;
         margin: 0 0 0 0;
     }
 
diff --git a/src/styles/brackets_patterns_override.less b/src/styles/brackets_patterns_override.less
@@ -1940,8 +1940,8 @@ input[type="color"],
         }
     }
 
-    // Update Button Type
-    &.update {
+    // Safe(Green) or Update Button Type
+    &.safe, &.update {
         background-color: @bc-secondary-btn-bg;
         border-color: @bc-secondary-btn-border;
         box-shadow: inset 0 1px 0 @bc-highlight;

Original file line number	Diff line number	Diff line change
`@@ -1940,8 +1940,8 @@ input[type="color"],`
`1940`	`1940`	`}`
`1941`	`1941`	`}`
`1942`	`1942`
`1943`		`- // Update Button Type`
`1944`		`- &.update {`
	`1943`	`+ // Safe(Green) or Update Button Type`
	`1944`	`+ &.safe, &.update {`
`1945`	`1945`	`background-color: @bc-secondary-btn-bg;`
`1946`	`1946`	`border-color: @bc-secondary-btn-border;`
`1947`	`1947`	`box-shadow: inset 0 1px 0 @bc-highlight;`