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.
microsoft · May 17, 2018 · 9ed74df · 9ed74df
1 parent ab69262
commit 9ed74df
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 89 deletions.
diff --git a/extensions/typescript-language-features/src/features/unusedHighlighter.ts b/extensions/typescript-language-features/src/features/unusedHighlighter.ts
diff --git 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<boolean>('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
@@ -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
@@ -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
@@ -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
@@ -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, WellKnownMarkerTags } 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(WellKnownMarkerTags.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(WellKnownMarkerTags.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/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/monaco.d.ts b/src/vs/monaco.d.ts
@@ -1089,6 +1089,7 @@ declare namespace monaco.editor {
 		endLineNumber: number;
 		endColumn: number;
 		relatedInformation?: IRelatedInformation[];
+		customTags?: string[];
 	}
 
 	/**
@@ -1104,6 +1105,7 @@ declare namespace monaco.editor {
 		endLineNumber: number;
 		endColumn: number;
 		relatedInformation?: IRelatedInformation[];
+		customTags?: string[];
 	}
 
 	/**

diff --git 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
@@ -38,6 +38,10 @@ export interface IRelatedInformation {
 	endColumn: number;
 }
 
+export enum WellKnownMarkerTags {
+	Unnecessary = 'unnecessary'
+}
+
 export enum MarkerSeverity {
 	Hint = 1,
 	Info = 2,
@@ -83,6 +87,7 @@ export interface IMarkerData {
 	endLineNumber: number;
 	endColumn: number;
 	relatedInformation?: IRelatedInformation[];
+	customTags?: string[];
 }
 
 export interface IResourceMarker {
@@ -102,6 +107,7 @@ export interface IMarker {
 	endLineNumber: number;
 	endColumn: number;
 	relatedInformation?: IRelatedInformation[];
+	customTags?: string[];
 }
 
 export interface MarkerStatistics {

diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts
@@ -3845,6 +3845,16 @@ declare module 'vscode' {
 		Hint = 3
 	}
 
+	/**
+	 * Additional metadata about the type of diagnostic.
+	 */
+	export enum DiagnosticTag {
+		/**
+		 * Unused or unnecessary code.
+		 */
+		Unnecessary = 'unnecessary'
+	}
+
 	/**
 	 * Represents a related message and source code location for a diagnostic. This should be
 	 * used to point to code locations that cause or related to a diagnostics, e.g when duplicating
@@ -3892,6 +3902,11 @@ declare module 'vscode' {
 		 */
 		severity: DiagnosticSeverity;
 
+		/**
+		 * Additional metadata about the type of the diagnostic.
+		 */
+		customTags?: DiagnosticTag[];
+
 		/**
 		 * A human-readable string describing the source of this
 		 * diagnostic, e.g. 'typescript' or 'super lint'.

diff --git 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
@@ -96,7 +96,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: value.customTags,
 		};
 	}
 }

diff --git 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 = 'unnecessary',
+}
+
 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;