Skip to content

Commit

Permalink
Add unused diagnostic subtype (#49646)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mjbvz committed May 17, 2018
1 parent 0f7ac44 commit 8bb27cd
Show file tree
Hide file tree
Showing 17 changed files with 92 additions and 90 deletions.
4 changes: 4 additions & 0 deletions build/monaco/monaco.d.ts.recipe
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ declare namespace monaco {
Error = 3,
}

export enum MarkerTag {
Unnecessary = 1,
}

export enum MarkerSeverity {
Hint = 1,
Info = 2,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 })[] = [];
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 };
}
Expand Down
7 changes: 6 additions & 1 deletion src/vs/editor/browser/widget/codeEditorWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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}; }`);
}
});
3 changes: 2 additions & 1 deletion src/vs/editor/common/model/intervalTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
};

/**
Expand Down
16 changes: 13 additions & 3 deletions src/vs/editor/common/services/modelServiceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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:
Expand All @@ -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;

Expand Down Expand Up @@ -193,7 +202,8 @@ class ModelMarkerHandler {
darkColor,
position: OverviewRulerLane.Right
},
zIndex
zIndex,
inlineClassName,
};
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/vs/editor/common/standalone/standaloneBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ export enum Severity {
Error = 3,
}

export enum MarkerTag {
Unnecessary = 1,
}

export enum MarkerSeverity {
Hint = 1,
Info = 2,
Expand Down Expand Up @@ -246,6 +250,7 @@ export function createMonacoBaseAPI(): typeof monaco {
SelectionDirection: SelectionDirection,
Severity: Severity,
MarkerSeverity: MarkerSeverity,
MarkerTag: MarkerTag,
Promise: TPromise,
Uri: <any>URI,
Token: Token
Expand Down
2 changes: 2 additions & 0 deletions src/vs/editor/common/view/editorColorRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.'));
Expand Down
1 change: 1 addition & 0 deletions src/vs/editor/editor.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/vs/monaco.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ declare namespace monaco {
Error = 3,
}

export enum MarkerTag {
Unnecessary = 1,
}

export enum MarkerSeverity {
Hint = 1,
Info = 2,
Expand Down Expand Up @@ -1089,6 +1093,7 @@ declare namespace monaco.editor {
endLineNumber: number;
endColumn: number;
relatedInformation?: IRelatedInformation[];
customTags?: MarkerTag[];
}

/**
Expand All @@ -1104,6 +1109,7 @@ declare namespace monaco.editor {
endLineNumber: number;
endColumn: number;
relatedInformation?: IRelatedInformation[];
customTags?: MarkerTag[];
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/vs/platform/markers/common/markerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ export class MarkerService implements IMarkerService {
code, severity,
message, source,
startLineNumber, startColumn, endLineNumber, endColumn,
relatedInformation
relatedInformation,
customTags,
} = data;

if (!message) {
Expand All @@ -208,7 +209,8 @@ export class MarkerService implements IMarkerService {
startColumn,
endLineNumber,
endColumn,
relatedInformation
relatedInformation,
customTags,
};
}

Expand Down
6 changes: 6 additions & 0 deletions src/vs/platform/markers/common/markers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export interface IRelatedInformation {
endColumn: number;
}

export enum MarkerTag {
Unnecessary = 1,
}

export enum MarkerSeverity {
Hint = 1,
Info = 2,
Expand Down Expand Up @@ -83,6 +87,7 @@ export interface IMarkerData {
endLineNumber: number;
endColumn: number;
relatedInformation?: IRelatedInformation[];
customTags?: MarkerTag[];
}

export interface IResourceMarker {
Expand All @@ -102,6 +107,7 @@ export interface IMarker {
endLineNumber: number;
endColumn: number;
relatedInformation?: IRelatedInformation[];
customTags?: MarkerTag[];
}

export interface MarkerStatistics {
Expand Down
20 changes: 20 additions & 0 deletions src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
1 change: 1 addition & 0 deletions src/vs/workbench/api/node/extHost.api.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 8bb27cd

Please sign in to comment.