Skip to content

Commit 8f0a811

Browse files
committed
feat(lsp): drop untagged hint diagnostics, style unused/deprecated code
Hint-severity (4) LSP diagnostics without tags are refactoring suggestions (e.g. tsserver's "File is a CommonJS module; it may be converted to an ES module") - drop them in the publishDiagnostics path so they never reach the problems panel or the quickfix layer. Tagged hints stay: unused symbols (tag Unnecessary) and deprecated usages (tag Deprecated) keep their panel row and squiggle, and now additionally render faded / struck-through in the editor via new editor-text-fragment-unnecessary/-deprecated mark classes applied from the diagnostic tags. Specs: tagged hints survive with panel rows + text marks; the untagged CommonJS suggestion never comes through (publish-level capture).
1 parent 6169ce2 commit 8f0a811

5 files changed

Lines changed: 122 additions & 5 deletions

File tree

src/extensions/default/TypeScriptSupport/unittests.js

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*
1919
*/
2020

21-
/*global describe, it, expect, beforeAll, afterAll, afterEach, awaitsFor, awaitsForDone, path, jsPromise */
21+
/*global describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, awaitsFor, awaitsForDone, path, jsPromise */
2222

2323
define(function (require, exports, module) {
2424

@@ -864,5 +864,87 @@ define(function (require, exports, module) {
864864
}, "parameter hint popup to show again", 30000);
865865
}, 90000);
866866
});
867+
868+
// ----- hint-severity diagnostics: untagged suggestions dropped, tagged ones styled -----
869+
describe("hint diagnostics and tags", function () {
870+
let publishedByFile;
871+
let origSetInspectionResults;
872+
873+
beforeAll(function () {
874+
// Capture what actually reaches the linting layer (post the LSPClient hint filter)
875+
// so the specs can await/assert on publishes deterministically instead of relying
876+
// only on problems-panel text.
877+
const DefaultProviders = testWindow.require("languageTools/DefaultProviders");
878+
const proto = DefaultProviders.LintingProvider.prototype;
879+
publishedByFile = {};
880+
origSetInspectionResults = proto.setInspectionResults;
881+
proto.setInspectionResults = function (msgObj) {
882+
const name = msgObj.uri.substring(msgObj.uri.lastIndexOf("/") + 1);
883+
publishedByFile[name] = publishedByFile[name] || [];
884+
publishedByFile[name].push(msgObj.diagnostics || []);
885+
return origSetInspectionResults.call(this, msgObj);
886+
};
887+
});
888+
889+
afterAll(function () {
890+
const DefaultProviders = testWindow.require("languageTools/DefaultProviders");
891+
DefaultProviders.LintingProvider.prototype.setInspectionResults = origSetInspectionResults;
892+
});
893+
894+
beforeEach(async function () {
895+
// Isolate from whatever earlier specs left behind (dirty documents, an open
896+
// parameter-hint popup, stale panel state).
897+
await awaitsForDone(CommandManager.execute(Commands.FILE_CLOSE_ALL, { _forceClose: true }),
898+
"close all files");
899+
});
900+
901+
it("should keep tagged hints (unused/deprecated) in the panel and style the text", async function () {
902+
await _openInProject("ts/", "type-error.ts");
903+
const editor = EditorManager.getActiveEditor();
904+
editor.document.setText(
905+
"const unusedVar: number = 1;\n" +
906+
"/** @deprecated use newFn */\n" +
907+
"function oldFn(): number { return 1; }\n" +
908+
"oldFn();\n" +
909+
"export {};\n"
910+
);
911+
// tagged hints (unused tag 1, deprecated tag 2) must survive the hint filter
912+
await awaitsFor(function () {
913+
const all = (publishedByFile["type-error.ts"] || []).flat();
914+
return all.some(function (d) { return d.tags && d.tags.indexOf(1) !== -1; }) &&
915+
all.some(function (d) { return d.tags && d.tags.indexOf(2) !== -1; });
916+
}, "tagged unused + deprecated diagnostics to be published", 30000);
917+
await awaitsFor(function () {
918+
return panelText().includes("never read") && panelText().includes("deprecated");
919+
}, "unused + deprecated hint rows in the problems panel", 30000);
920+
// the marked text carries the tag styles on top of the info squiggle
921+
await awaitsFor(function () {
922+
return $(".editor-text-fragment-unnecessary").length > 0 &&
923+
$(".editor-text-fragment-deprecated").length > 0;
924+
}, "faded + strikethrough text marks in the editor", 30000);
925+
}, 90000);
926+
927+
it("should drop untagged hint suggestions like the CommonJS to ESM nag", async function () {
928+
await _openInProject("js-plain/", "implicit.js");
929+
const editor = EditorManager.getActiveEditor();
930+
const publishesBeforeEdit = (publishedByFile["implicit.js"] || []).length;
931+
// "require(" is concatenated so RequireJS's static dependency scan of this module's
932+
// source doesn't read the string literal as an AMD dependency named "http" - that
933+
// fails the whole file's load and silently unregisters every suite in it.
934+
const requireCall = "require";
935+
editor.document.setText("const http = " + requireCall + "('http');\nconsole.log(http);\n");
936+
// the server re-publishes (possibly empty) diagnostics after the edit - wait for
937+
// one, then assert the untagged 80001 CommonJS suggestion never came through
938+
await awaitsFor(function () {
939+
return (publishedByFile["implicit.js"] || []).length > publishesBeforeEdit;
940+
}, "a diagnostics publish for implicit.js after the edit", 30000);
941+
const all = (publishedByFile["implicit.js"] || []).flat();
942+
expect(all.some(function (d) { return d.message.indexOf("CommonJS") !== -1; })).toBe(false);
943+
expect(all.some(function (d) {
944+
return d.severity === 4 && !(d.tags && d.tags.length);
945+
})).toBe(false);
946+
expect(panelText().includes("CommonJS")).toBe(false);
947+
}, 90000);
948+
});
867949
});
868950
});

src/language/CodeInspection.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,11 +423,24 @@ define(function (require, exports, module) {
423423
}
424424

425425
function _getMarkOptions(error){
426+
let options;
426427
switch (error.type) {
427-
case Type.ERROR: return Editor.getMarkOptionUnderlineError();
428-
case Type.WARNING: return Editor.getMarkOptionUnderlineWarn();
429-
case Type.META: return Editor.getMarkOptionUnderlineInfo();
428+
case Type.ERROR: options = Editor.getMarkOptionUnderlineError(); break;
429+
case Type.WARNING: options = Editor.getMarkOptionUnderlineWarn(); break;
430+
case Type.META: options = Editor.getMarkOptionUnderlineInfo(); break;
430431
}
432+
// LSP DiagnosticTag values on the error (1 Unnecessary - unused symbol, 2 Deprecated)
433+
// add a text style on top of the squiggle: faded for unused, strikethrough for
434+
// deprecated. Panel row and squiggle treatment stay unchanged.
435+
if (options && Array.isArray(error.tags)) {
436+
if (error.tags.indexOf(1) !== -1) {
437+
options.className += " editor-text-fragment-unnecessary";
438+
}
439+
if (error.tags.indexOf(2) !== -1) {
440+
options.className += " editor-text-fragment-deprecated";
441+
}
442+
}
443+
return options;
431444
}
432445

433446
function _getMarkTypePriority(type){

src/languageTools/DefaultProviders.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -943,7 +943,10 @@ define(function (require, exports, module) {
943943
ch: obj.range.end.character
944944
},
945945
message: obj.message,
946-
type: (obj.severity === 1 ? CodeInspection.Type.ERROR : (obj.severity === 2 ? CodeInspection.Type.WARNING : CodeInspection.Type.META))
946+
type: (obj.severity === 1 ? CodeInspection.Type.ERROR : (obj.severity === 2 ? CodeInspection.Type.WARNING : CodeInspection.Type.META)),
947+
// LSP DiagnosticTag values (1 Unnecessary, 2 Deprecated) - CodeInspection styles
948+
// the marked text off these (faded / strikethrough) on top of the squiggle.
949+
tags: obj.tags
947950
};
948951
});
949952

src/languageTools/LSPClient.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,14 @@ define(function (require, exports, module) {
212212
if (filterFn && diagnostics.length) {
213213
diagnostics = filterFn(diagnostics, { languageId: langId, filePath: vfsPath });
214214
}
215+
// Severity 4 (Hint) without tags is a refactoring suggestion (e.g. tsserver's "File is
216+
// a CommonJS module; it may be converted to an ES module") - noise in a problems panel,
217+
// so drop it (this also drops its quickfix). Tagged hints stay: tag 1 Unnecessary
218+
// (unused symbol) and tag 2 Deprecated are real signal - they keep their panel row and
219+
// squiggle, and additionally drive the faded/strikethrough text styling.
220+
diagnostics = diagnostics.filter(function (d) {
221+
return d.severity !== 4 || (d.tags && d.tags.length);
222+
});
215223
client.lintingProvider.setInspectionResults({
216224
uri: vfsUri,
217225
diagnostics: diagnostics

src/styles/brackets.less

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,17 @@ html, body {
217217
background: url('images/wavy-info.svg') repeat-x bottom left;
218218
}
219219

220+
// LSP DiagnosticTag text styles, added on top of the squiggle classes above:
221+
// unused symbols (tag Unnecessary) render faded, deprecated usages (tag Deprecated)
222+
// render struck through.
223+
.editor-text-fragment-unnecessary {
224+
opacity: 0.6;
225+
}
226+
227+
.editor-text-fragment-deprecated {
228+
text-decoration: line-through;
229+
}
230+
220231
.hidden-element{
221232
display: none !important;
222233
}

0 commit comments

Comments
 (0)