From 8bb27cd255508611021969f79061d1b5b6a6283f Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 17 May 2018 15:43:59 -0700 Subject: [PATCH] Add unused diagnostic subtype (#49646) * Add unused diagnostic subtype Fixes #15710 Adds a new `DiagnosticTag` class that provide additional information about a diagnostic. Introduce `DiagnosticTag.Unnecessary` to mark when a diagnostic is for unused / unnecessary code The design comes from Rosyln's diagnostic object and allows us to modify how a diagnostic is rendered without changing its serverity. Hooks up JS and TS to use this new tag. This is controlled by the `javascript.showUnused.enabled` setting which is enabled by default - Introduce a new diagnostic severity for unused. However, using this approach, if a user sets `noUnusedLocals` in their `tsconfig.json`, the resulting diagnostic could only show the squiggly OR be grayed out. Using `customTags` allows us to support both graying out and showing the squiggly - Custom JS/TS implementation using decorators Not themable. We want a standard experience across languages. * - Move to proposed - Use numeric enum --- build/monaco/monaco.d.ts.recipe | 4 + .../src/features/unusedHighlighter.ts | 73 ------------------- .../src/languageProvider.ts | 7 -- .../src/typeScriptServiceClientHost.ts | 5 +- .../editor/browser/widget/codeEditorWidget.ts | 7 +- src/vs/editor/common/model/intervalTree.ts | 3 +- .../common/services/modelServiceImpl.ts | 16 +++- .../common/standalone/standaloneBase.ts | 5 ++ .../editor/common/view/editorColorRegistry.ts | 2 + src/vs/editor/editor.api.ts | 1 + src/vs/monaco.d.ts | 6 ++ .../platform/markers/common/markerService.ts | 6 +- src/vs/platform/markers/common/markers.ts | 6 ++ src/vs/vscode.proposed.d.ts | 20 +++++ src/vs/workbench/api/node/extHost.api.impl.ts | 1 + .../api/node/extHostTypeConverters.ts | 15 +++- src/vs/workbench/api/node/extHostTypes.ts | 5 ++ 17 files changed, 92 insertions(+), 90 deletions(-) delete mode 100644 extensions/typescript-language-features/src/features/unusedHighlighter.ts diff --git a/build/monaco/monaco.d.ts.recipe b/build/monaco/monaco.d.ts.recipe index 652c154d3d5..218d4cdf8f5 100644 --- a/build/monaco/monaco.d.ts.recipe +++ b/build/monaco/monaco.d.ts.recipe @@ -32,6 +32,10 @@ declare namespace monaco { Error = 3, } + export enum MarkerTag { + Unnecessary = 1, + } + export enum MarkerSeverity { Hint = 1, Info = 2, diff --git a/extensions/typescript-language-features/src/features/unusedHighlighter.ts b/extensions/typescript-language-features/src/features/unusedHighlighter.ts deleted file mode 100644 index f5c34a91ec8..00000000000 --- a/extensions/typescript-language-features/src/features/unusedHighlighter.ts +++ /dev/null @@ -1,73 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as vscode from 'vscode'; -import { DiagnosticSet } from './diagnostics'; - - -export class UnusedHighlighter { - - private readonly _decorationType: vscode.TextEditorDecorationType; - - private readonly _diagnostics = new DiagnosticSet(); - private _validate: boolean = true; - - constructor( - ) { - this._decorationType = vscode.window.createTextEditorDecorationType({ - opacity: '0.7' - }); - } - - public dispose() { - this._decorationType.dispose(); - } - - public reInitialize(): void { - this._diagnostics.clear(); - - for (const editor of vscode.window.visibleTextEditors) { - editor.setDecorations(this._decorationType, []); - } - } - - public set validate(value: boolean) { - if (this._validate === value) { - return; - } - - this._validate = value; - if (!value) { - for (const editor of vscode.window.visibleTextEditors) { - editor.setDecorations(this._decorationType, []); - } - } - } - - public diagnosticsReceived( - file: vscode.Uri, - diagnostics: vscode.Diagnostic[] - ): void { - // Undocumented flag to enable - if (!vscode.workspace.getConfiguration('typescript').get('showUnused.experimentalFade')) { - return; - } - this._diagnostics.set(file, diagnostics); - this._updateCurrentHighlights(file); - } - - private _updateCurrentHighlights(file: vscode.Uri) { - for (const editor of vscode.window.visibleTextEditors) { - if (editor.document.uri.fsPath !== file.fsPath) { - continue; - } - - const diagnostics = this._diagnostics.get(editor.document.uri); - if (diagnostics) { - editor.setDecorations(this._decorationType, diagnostics.map(x => x.range)); - } - } - } -} \ No newline at end of file diff --git a/extensions/typescript-language-features/src/languageProvider.ts b/extensions/typescript-language-features/src/languageProvider.ts index bb4b14033c2..312921c5671 100644 --- a/extensions/typescript-language-features/src/languageProvider.ts +++ b/extensions/typescript-language-features/src/languageProvider.ts @@ -21,7 +21,6 @@ import { CachedNavTreeResponse } from './features/baseCodeLensProvider'; import { memoize } from './utils/memoize'; import { disposeAll } from './utils/dispose'; import TelemetryReporter from './utils/telemetry'; -import { UnusedHighlighter } from './features/unusedHighlighter'; const validateSetting = 'validate.enable'; const suggestionSetting = 'suggestionActions.enabled'; @@ -30,7 +29,6 @@ const foldingSetting = 'typescript.experimental.syntaxFolding'; export default class LanguageProvider { private readonly diagnosticsManager: DiagnosticsManager; private readonly bufferSyncSupport: BufferSyncSupport; - private readonly ununsedHighlighter: UnusedHighlighter; private readonly fileConfigurationManager: FileConfigurationManager; private readonly toUpdateOnConfigurationChanged: ({ updateConfiguration: () => void })[] = []; @@ -58,7 +56,6 @@ export default class LanguageProvider { }, this._validate); this.diagnosticsManager = new DiagnosticsManager(description.diagnosticOwner); - this.ununsedHighlighter = new UnusedHighlighter(); workspace.onDidChangeConfiguration(this.configurationChanged, this, this.disposables); this.configurationChanged(); @@ -229,7 +226,6 @@ export default class LanguageProvider { public reInitialize(): void { this.diagnosticsManager.reInitialize(); - this.ununsedHighlighter.reInitialize(); this.bufferSyncSupport.reOpenDocuments(); this.bufferSyncSupport.requestAllDiagnostics(); this.fileConfigurationManager.reset(); @@ -265,9 +261,6 @@ export default class LanguageProvider { public diagnosticsReceived(diagnosticsKind: DiagnosticKind, file: Uri, diagnostics: (Diagnostic & { reportUnnecessary: any })[]): void { const config = workspace.getConfiguration(this.id, file); const reportUnnecessary = config.get('showUnused.enabled', true); - if (diagnosticsKind === DiagnosticKind.Suggestion) { - this.ununsedHighlighter.diagnosticsReceived(file, diagnostics.filter(diag => diag.reportUnnecessary)); - } this.diagnosticsManager.diagnosticsReceived(diagnosticsKind, file, diagnostics.filter(diag => diag.reportUnnecessary ? reportUnnecessary : true)); } diff --git a/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts b/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts index 9106c52bdc9..9de6075d98d 100644 --- a/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts +++ b/extensions/typescript-language-features/src/typeScriptServiceClientHost.ts @@ -8,7 +8,7 @@ * https://github.com/Microsoft/TypeScript-Sublime-Plugin/blob/master/TypeScript%20Indent.tmPreferences * ------------------------------------------------------------------------------------------ */ -import { workspace, Memento, Diagnostic, Range, Disposable, Uri, DiagnosticSeverity } from 'vscode'; +import { workspace, Memento, Diagnostic, Range, Disposable, Uri, DiagnosticSeverity, DiagnosticTag } from 'vscode'; import * as Proto from './protocol'; import * as PConst from './protocol.const'; @@ -256,6 +256,9 @@ export default class TypeScriptServiceClientHost { if (diagnostic.code) { converted.code = diagnostic.code; } + if (diagnostic.reportsUnnecessary) { + converted.customTags = [DiagnosticTag.Unnecessary]; + } (converted as Diagnostic & { reportUnnecessary: any }).reportUnnecessary = diagnostic.reportsUnnecessary; return converted as Diagnostic & { reportUnnecessary: any }; } diff --git a/src/vs/editor/browser/widget/codeEditorWidget.ts b/src/vs/editor/browser/widget/codeEditorWidget.ts index 03840a1669f..e35b26ddd3a 100644 --- a/src/vs/editor/browser/widget/codeEditorWidget.ts +++ b/src/vs/editor/browser/widget/codeEditorWidget.ts @@ -46,7 +46,7 @@ import { IMouseEvent } from 'vs/base/browser/mouseEvent'; import { InternalEditorAction } from 'vs/editor/common/editorAction'; import { ICommandDelegate } from 'vs/editor/browser/view/viewController'; import { CoreEditorCommand } from 'vs/editor/browser/controller/coreCommands'; -import { editorErrorForeground, editorErrorBorder, editorWarningForeground, editorWarningBorder, editorInfoBorder, editorInfoForeground, editorHintForeground, editorHintBorder } from 'vs/editor/common/view/editorColorRegistry'; +import { editorErrorForeground, editorErrorBorder, editorWarningForeground, editorWarningBorder, editorInfoBorder, editorInfoForeground, editorHintForeground, editorHintBorder, editorUnnecessaryForeground } from 'vs/editor/common/view/editorColorRegistry'; import { Color } from 'vs/base/common/color'; import { ClassName } from 'vs/editor/common/model/intervalTree'; @@ -1795,4 +1795,9 @@ registerThemingParticipant((theme, collector) => { if (hintForeground) { collector.addRule(`.monaco-editor .${ClassName.EditorHintDecoration} { background: url("data:image/svg+xml,${getDotDotDotSVGData(hintForeground)}") no-repeat bottom left; }`); } + + const unnecessaryForeground = theme.getColor(editorUnnecessaryForeground); + if (unnecessaryForeground) { + collector.addRule(`.monaco-editor .${ClassName.EditorUnnecessaryDecoration} { color: ${unnecessaryForeground}; }`); + } }); diff --git a/src/vs/editor/common/model/intervalTree.ts b/src/vs/editor/common/model/intervalTree.ts index cad96482e42..ca11c6f9cb4 100644 --- a/src/vs/editor/common/model/intervalTree.ts +++ b/src/vs/editor/common/model/intervalTree.ts @@ -16,7 +16,8 @@ export const ClassName = { EditorHintDecoration: 'squiggly-hint', EditorInfoDecoration: 'squiggly-info', EditorWarningDecoration: 'squiggly-warning', - EditorErrorDecoration: 'squiggly-error' + EditorErrorDecoration: 'squiggly-error', + EditorUnnecessaryDecoration: 'squiggly-overlay-unnecessary' }; /** diff --git a/src/vs/editor/common/services/modelServiceImpl.ts b/src/vs/editor/common/services/modelServiceImpl.ts index 3a3da61c476..69619294284 100644 --- a/src/vs/editor/common/services/modelServiceImpl.ts +++ b/src/vs/editor/common/services/modelServiceImpl.ts @@ -11,7 +11,7 @@ import { MarkdownString } from 'vs/base/common/htmlContent'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; import URI from 'vs/base/common/uri'; import { TPromise } from 'vs/base/common/winjs.base'; -import { IMarker, IMarkerService, MarkerSeverity } from 'vs/platform/markers/common/markers'; +import { IMarker, IMarkerService, MarkerSeverity, MarkerTag } from 'vs/platform/markers/common/markers'; import { Range } from 'vs/editor/common/core/range'; import { TextModel, createTextBuffer } from 'vs/editor/common/model/textModel'; import { IMode, LanguageIdentifier } from 'vs/editor/common/modes'; @@ -127,10 +127,13 @@ class ModelMarkerHandler { let color: ThemeColor; let darkColor: ThemeColor; let zIndex: number; + let inlineClassName: string; switch (marker.severity) { case MarkerSeverity.Hint: - className = ClassName.EditorHintDecoration; + if (!marker.customTags || marker.customTags.indexOf(MarkerTag.Unnecessary) === -1) { + className = ClassName.EditorHintDecoration; + } zIndex = 0; break; case MarkerSeverity.Warning: @@ -154,6 +157,12 @@ class ModelMarkerHandler { break; } + if (marker.customTags) { + if (marker.customTags.indexOf(MarkerTag.Unnecessary) !== -1) { + inlineClassName = ClassName.EditorUnnecessaryDecoration; + } + } + let hoverMessage: MarkdownString = null; let { message, source, relatedInformation } = marker; @@ -193,7 +202,8 @@ class ModelMarkerHandler { darkColor, position: OverviewRulerLane.Right }, - zIndex + zIndex, + inlineClassName, }; } } diff --git a/src/vs/editor/common/standalone/standaloneBase.ts b/src/vs/editor/common/standalone/standaloneBase.ts index 6d75f1091ee..c4683e6d6ed 100644 --- a/src/vs/editor/common/standalone/standaloneBase.ts +++ b/src/vs/editor/common/standalone/standaloneBase.ts @@ -25,6 +25,10 @@ export enum Severity { Error = 3, } +export enum MarkerTag { + Unnecessary = 1, +} + export enum MarkerSeverity { Hint = 1, Info = 2, @@ -246,6 +250,7 @@ export function createMonacoBaseAPI(): typeof monaco { SelectionDirection: SelectionDirection, Severity: Severity, MarkerSeverity: MarkerSeverity, + MarkerTag: MarkerTag, Promise: TPromise, Uri: URI, Token: Token diff --git a/src/vs/editor/common/view/editorColorRegistry.ts b/src/vs/editor/common/view/editorColorRegistry.ts index d875cb44bdc..c8b310fb7de 100644 --- a/src/vs/editor/common/view/editorColorRegistry.ts +++ b/src/vs/editor/common/view/editorColorRegistry.ts @@ -49,6 +49,8 @@ export const editorInfoBorder = registerColor('editorInfo.border', { dark: null, export const editorHintForeground = registerColor('editorHint.foreground', { dark: Color.fromHex('#eeeeee').transparent(0.7), light: '#6c6c6c', hc: null }, nls.localize('hintForeground', 'Foreground color of hint squigglies in the editor.')); export const editorHintBorder = registerColor('editorHint.border', { dark: null, light: null, hc: Color.fromHex('#eeeeee').transparent(0.8) }, nls.localize('hintBorder', 'Border color of hint squigglies in the editor.')); +export const editorUnnecessaryForeground = registerColor('editorUnnecessary.foreground', { dark: Color.fromHex('#eeeeee').transparent(0.7), light: '#6c6c6c', hc: null }, nls.localize('unnecessaryForeground', 'Foreground color of unnecessary code in the editor.')); + const rulerRangeDefault = new Color(new RGBA(0, 122, 204, 0.6)); export const overviewRulerRangeHighlight = registerColor('editorOverviewRuler.rangeHighlightForeground', { dark: rulerRangeDefault, light: rulerRangeDefault, hc: rulerRangeDefault }, nls.localize('overviewRulerRangeHighlight', 'Overview ruler marker color for range highlights. The color must not be opaque to not hide underlying decorations.'), true); export const overviewRulerError = registerColor('editorOverviewRuler.errorForeground', { dark: new Color(new RGBA(255, 18, 18, 0.7)), light: new Color(new RGBA(255, 18, 18, 0.7)), hc: new Color(new RGBA(255, 50, 50, 1)) }, nls.localize('overviewRuleError', 'Overview ruler marker color for errors.')); diff --git a/src/vs/editor/editor.api.ts b/src/vs/editor/editor.api.ts index 96ba99b2d8b..5594a22627c 100644 --- a/src/vs/editor/editor.api.ts +++ b/src/vs/editor/editor.api.ts @@ -37,6 +37,7 @@ export const Selection = api.Selection; export const SelectionDirection = api.SelectionDirection; export const Severity = api.Severity; export const MarkerSeverity = api.MarkerSeverity; +export const MarkerTag = api.MarkerTag; export const Promise = api.Promise; export const Uri = api.Uri; export const Token = api.Token; diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 9aeabdb729a..71a6444b1a4 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -32,6 +32,10 @@ declare namespace monaco { Error = 3, } + export enum MarkerTag { + Unnecessary = 1, + } + export enum MarkerSeverity { Hint = 1, Info = 2, @@ -1089,6 +1093,7 @@ declare namespace monaco.editor { endLineNumber: number; endColumn: number; relatedInformation?: IRelatedInformation[]; + customTags?: MarkerTag[]; } /** @@ -1104,6 +1109,7 @@ declare namespace monaco.editor { endLineNumber: number; endColumn: number; relatedInformation?: IRelatedInformation[]; + customTags?: MarkerTag[]; } /** diff --git a/src/vs/platform/markers/common/markerService.ts b/src/vs/platform/markers/common/markerService.ts index df0b4024418..8329796d9b6 100644 --- a/src/vs/platform/markers/common/markerService.ts +++ b/src/vs/platform/markers/common/markerService.ts @@ -183,7 +183,8 @@ export class MarkerService implements IMarkerService { code, severity, message, source, startLineNumber, startColumn, endLineNumber, endColumn, - relatedInformation + relatedInformation, + customTags, } = data; if (!message) { @@ -208,7 +209,8 @@ export class MarkerService implements IMarkerService { startColumn, endLineNumber, endColumn, - relatedInformation + relatedInformation, + customTags, }; } diff --git a/src/vs/platform/markers/common/markers.ts b/src/vs/platform/markers/common/markers.ts index ba99cf9285c..69f34f9400f 100644 --- a/src/vs/platform/markers/common/markers.ts +++ b/src/vs/platform/markers/common/markers.ts @@ -38,6 +38,10 @@ export interface IRelatedInformation { endColumn: number; } +export enum MarkerTag { + Unnecessary = 1, +} + export enum MarkerSeverity { Hint = 1, Info = 2, @@ -83,6 +87,7 @@ export interface IMarkerData { endLineNumber: number; endColumn: number; relatedInformation?: IRelatedInformation[]; + customTags?: MarkerTag[]; } export interface IResourceMarker { @@ -102,6 +107,7 @@ export interface IMarker { endLineNumber: number; endColumn: number; relatedInformation?: IRelatedInformation[]; + customTags?: MarkerTag[]; } export interface MarkerStatistics { diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 71389801e42..cc1adc633bb 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -620,4 +620,24 @@ declare module 'vscode' { } //#endregion + + //#region mjbvz: Unused diagnostics + /** + * Additional metadata about the type of diagnostic. + */ + export enum DiagnosticTag { + /** + * Unused or unnecessary code. + */ + Unnecessary = 1, + } + + export interface Diagnostic { + /** + * Additional metadata about the type of the diagnostic. + */ + customTags?: DiagnosticTag[]; + } + + //#endregion } diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index 79da560b9d3..c4152220f51 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -677,6 +677,7 @@ export function createApiFactory( DebugAdapterExecutable: extHostTypes.DebugAdapterExecutable, Diagnostic: extHostTypes.Diagnostic, DiagnosticRelatedInformation: extHostTypes.DiagnosticRelatedInformation, + DiagnosticTag: extHostTypes.DiagnosticTag, DiagnosticSeverity: extHostTypes.DiagnosticSeverity, Disposable: extHostTypes.Disposable, DocumentHighlight: extHostTypes.DocumentHighlight, diff --git a/src/vs/workbench/api/node/extHostTypeConverters.ts b/src/vs/workbench/api/node/extHostTypeConverters.ts index 1112265e305..e0183392127 100644 --- a/src/vs/workbench/api/node/extHostTypeConverters.ts +++ b/src/vs/workbench/api/node/extHostTypeConverters.ts @@ -20,7 +20,7 @@ import * as htmlContent from 'vs/base/common/htmlContent'; import { IRelativePattern } from 'vs/base/common/glob'; import * as languageSelector from 'vs/editor/common/modes/languageSelector'; import { WorkspaceEditDto, ResourceTextEditDto } from 'vs/workbench/api/node/extHost.protocol'; -import { MarkerSeverity, IRelatedInformation, IMarkerData } from 'vs/platform/markers/common/markers'; +import { MarkerSeverity, IRelatedInformation, IMarkerData, MarkerTag } from 'vs/platform/markers/common/markers'; export interface PositionLike { line: number; @@ -88,6 +88,16 @@ export namespace Position { } } +export namespace DiagnosticTag { + export function from(value: vscode.DiagnosticTag): MarkerTag { + switch (value) { + case types.DiagnosticTag.Unnecessary: + return MarkerTag.Unnecessary; + } + return undefined; + } +} + export namespace Diagnostic { export function from(value: vscode.Diagnostic): IMarkerData { return { @@ -96,7 +106,8 @@ export namespace Diagnostic { source: value.source, code: String(value.code), severity: DiagnosticSeverity.from(value.severity), - relatedInformation: value.relatedInformation && value.relatedInformation.map(DiagnosticRelatedInformation.from) + relatedInformation: value.relatedInformation && value.relatedInformation.map(DiagnosticRelatedInformation.from), + customTags: Array.isArray(value.customTags) ? value.customTags.map(DiagnosticTag.from) : undefined, }; } } diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 32b718698e9..2416526dcab 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -673,6 +673,10 @@ export class SnippetString { } } +export enum DiagnosticTag { + Unnecessary = 1, +} + export enum DiagnosticSeverity { Hint = 3, Information = 2, @@ -747,6 +751,7 @@ export class Diagnostic { code: string | number; severity: DiagnosticSeverity; relatedInformation: DiagnosticRelatedInformation[]; + customTags?: DiagnosticTag[]; constructor(range: Range, message: string, severity: DiagnosticSeverity = DiagnosticSeverity.Error) { this.range = range;