From 50524a0a603decdf6720dc1c3a1aec7347cb2e04 Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Fri, 10 Jun 2022 10:28:43 +0900 Subject: [PATCH 1/3] Add new diagnostic: deprecated-color-setting-shorthand --- package-lock.json | 14 ++ package.json | 2 + src/__mocks__/vscode.ts | 6 + ...deprecated-color-setting-shorthand.test.ts | 173 ++++++++++++++++++ .../deprecated-color-setting-shorthand.ts | 102 +++++++++++ src/diagnostics/deprecated-dollar-prefix.ts | 2 + src/diagnostics/index.ts | 3 + src/directives/parser.ts | 43 +++-- 8 files changed, 332 insertions(+), 13 deletions(-) create mode 100644 src/diagnostics/deprecated-color-setting-shorthand.test.ts create mode 100644 src/diagnostics/deprecated-color-setting-shorthand.ts diff --git a/package-lock.json b/package-lock.json index a8ab07f7..680498f6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "devDependencies": { "@babel/preset-env": "^7.18.2", "@marp-team/marp-core": "^3.2.1", + "@types/color-string": "^1.5.2", "@types/express": "^4.17.13", "@types/jest": "^27.5.2", "@types/lodash.debounce": "^4.0.7", @@ -23,6 +24,7 @@ "@typescript-eslint/parser": "^5.27.0", "@vscode/test-web": "^0.0.24", "abort-controller": "^3.0.0", + "color-string": "^1.9.1", "dedent": "^0.7.0", "esbuild": "^0.14.42", "esbuild-loader": "^2.19.0", @@ -3004,6 +3006,12 @@ "@types/node": "*" } }, + "node_modules/@types/color-string": { + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/@types/color-string/-/color-string-1.5.2.tgz", + "integrity": "sha512-hAhTmfFYVdzgsKwpC9Flc6h9Do64PhKoNxy3YxE0ze+0LIh3a7TrDQAxiujmANQbDRDgGduEz+9sMS+Zd+J7hA==", + "dev": true + }, "node_modules/@types/connect": { "version": "3.4.35", "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.35.tgz", @@ -20636,6 +20644,12 @@ "@types/node": "*" } }, + "@types/color-string": { + "version": "1.5.2", + "resolved": "https://registry.npmjs.org/@types/color-string/-/color-string-1.5.2.tgz", + "integrity": "sha512-hAhTmfFYVdzgsKwpC9Flc6h9Do64PhKoNxy3YxE0ze+0LIh3a7TrDQAxiujmANQbDRDgGduEz+9sMS+Zd+J7hA==", + "dev": true + }, "@types/connect": { "version": "3.4.35", "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.35.tgz", diff --git a/package.json b/package.json index e7f7c12a..78297798 100644 --- a/package.json +++ b/package.json @@ -260,6 +260,7 @@ "devDependencies": { "@babel/preset-env": "^7.18.2", "@marp-team/marp-core": "^3.2.1", + "@types/color-string": "^1.5.2", "@types/express": "^4.17.13", "@types/jest": "^27.5.2", "@types/lodash.debounce": "^4.0.7", @@ -269,6 +270,7 @@ "@typescript-eslint/parser": "^5.27.0", "@vscode/test-web": "^0.0.24", "abort-controller": "^3.0.0", + "color-string": "^1.9.1", "dedent": "^0.7.0", "esbuild": "^0.14.42", "esbuild-loader": "^2.19.0", diff --git a/src/__mocks__/vscode.ts b/src/__mocks__/vscode.ts index d9b49c95..2176f687 100644 --- a/src/__mocks__/vscode.ts +++ b/src/__mocks__/vscode.ts @@ -55,6 +55,11 @@ export class Diagnostic { ) {} } +export enum DiagnosticTag { + Unnecessary = 1, + Deprecated = 2, +} + export enum DiagnosticSeverity { Error, Warning, @@ -244,6 +249,7 @@ export const workspace = { export class WorkspaceEdit { readonly delete = jest.fn() readonly insert = jest.fn() + readonly replace = jest.fn() } beforeEach(() => { diff --git a/src/diagnostics/deprecated-color-setting-shorthand.test.ts b/src/diagnostics/deprecated-color-setting-shorthand.test.ts new file mode 100644 index 00000000..18255a99 --- /dev/null +++ b/src/diagnostics/deprecated-color-setting-shorthand.test.ts @@ -0,0 +1,173 @@ +import { + CancellationToken, + CodeAction, + CodeActionTriggerKind, + CodeActionKind, + Diagnostic, + DiagnosticSeverity, + languages, + Position, + Range, + TextDocument, + WorkspaceEdit, +} from 'vscode' +import { DirectiveParser } from '../directives/parser' +import * as rule from './deprecated-color-setting-shorthand' + +jest.mock('vscode') + +const doc = (text: string): TextDocument => + ({ + getText: () => text, + positionAt: (offset: number) => { + const lines = text.slice(0, offset).split('\n') + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return new Position(lines.length - 1, lines.pop()!.length) + }, + uri: '/test/document', + } as any) + +describe('[Diagnostics rule] Deprecated color setting shorthand', () => { + const register = (doc: TextDocument): Diagnostic[] => { + const parser = new DirectiveParser() + const diagnostics: Diagnostic[] = [] + + rule.register(parser, diagnostics) + + parser.parse(doc) + return diagnostics + } + + describe('#register', () => { + it('does not add diagnostics, to the image syntax not for setting colors', () => { + expect(register(doc('![](/image.jpg)'))).toHaveLength(0) + expect(register(doc('![unknown](blue)'))).toHaveLength(0) + }) + + it('adds diagnostics to warn deprecated directives when used shorthand syntax for setting color', () => { + // Text + const [$text] = register(doc('![](red)')) + + expect($text).toBeInstanceOf(Diagnostic) + expect($text.code).toBe(rule.code) + expect($text.source).toBe('marp-vscode') + expect($text.severity).toBe(DiagnosticSeverity.Warning) + expect($text.range).toStrictEqual( + new Range(new Position(0, 0), new Position(0, 8)) + ) + + // Background + const [$bg] = register(doc('![bg](currentColor)')) + + expect($bg).toBeInstanceOf(Diagnostic) + expect($bg.code).toBe(rule.code) + expect($bg.source).toBe('marp-vscode') + expect($bg.severity).toBe(DiagnosticSeverity.Warning) + expect($bg.range).toStrictEqual( + new Range(new Position(0, 0), new Position(0, 19)) + ) + + // Multiple syntaxes in inline + const [$inlineText, $inlineBg] = register( + doc('![](#abc)![bg](rgb(0,0,0))') + ) + + expect($inlineText).toBeInstanceOf(Diagnostic) + expect($inlineText.code).toBe(rule.code) + expect($inlineText.range).toStrictEqual( + new Range(new Position(0, 0), new Position(0, 9)) + ) + expect($inlineBg).toBeInstanceOf(Diagnostic) + expect($inlineBg.code).toBe(rule.code) + expect($inlineBg.range).toStrictEqual( + new Range(new Position(0, 9), new Position(0, 26)) + ) + }) + }) + + describe('#subscribe', () => { + it('subscribes registered DeprecatedColorSettingShorthand code action provider', () => { + const subscriptions: any[] = [] + rule.subscribe(subscriptions) + + expect(languages.registerCodeActionsProvider).toHaveBeenCalledWith( + 'markdown', + expect.any(rule.DeprecatedColorSettingShorthand), + { + providedCodeActionKinds: [CodeActionKind.QuickFix], + } + ) + }) + }) + + describe('DeprecatedColorSettingShorthand code action', () => { + describe('#provideCodeActions', () => { + const dummyRange = new Range(new Position(0, 0), new Position(0, 0)) + const dummyToken = {} as CancellationToken + + it('returns created code actions for corresponding diagnostics', () => { + const document = doc('![](#012abc)\n![bg](rgba(1,2,3,0.5))') + const diagnostics = register(document) + const codeActions = + new rule.DeprecatedColorSettingShorthand().provideCodeActions( + document, + dummyRange, + { + diagnostics, + triggerKind: CodeActionTriggerKind.Invoke, + only: undefined, + }, + dummyToken + ) + + expect(codeActions).toHaveLength(2) + expect(codeActions?.[0]).toBeInstanceOf(CodeAction) + expect(codeActions?.[1]).toBeInstanceOf(CodeAction) + + // Quick fix action + const textAction: CodeAction = codeActions?.[0] + expect(textAction.kind).toBe(CodeActionKind.QuickFix) + expect(textAction.diagnostics).toStrictEqual([diagnostics[0]]) + expect(textAction.edit).toBeInstanceOf(WorkspaceEdit) + expect(textAction.isPreferred).toBe(true) + expect(textAction.edit?.replace).toHaveBeenCalledTimes(1) + expect(textAction.edit?.replace).toHaveBeenCalledWith( + document.uri, + new Range(new Position(0, 0), new Position(0, 12)), + '' + ) + + const bgAction: CodeAction = codeActions?.[1] + expect(bgAction.kind).toBe(CodeActionKind.QuickFix) + expect(bgAction.diagnostics).toStrictEqual([diagnostics[1]]) + expect(bgAction.edit).toBeInstanceOf(WorkspaceEdit) + expect(bgAction.isPreferred).toBe(true) + expect(bgAction.edit?.replace).toHaveBeenCalledTimes(1) + expect(bgAction.edit?.replace).toHaveBeenCalledWith( + document.uri, + new Range(new Position(1, 0), new Position(1, 22)), + '' + ) + }) + + it('does not create code actions when corresponding diagnostics have not passed', () => { + const document = doc('![unknown](red)') + const diagnostics = register(document) + const codeActions = + new rule.DeprecatedColorSettingShorthand().provideCodeActions( + document, + dummyRange, + { + diagnostics, + triggerKind: CodeActionTriggerKind.Invoke, + only: undefined, + }, + dummyToken + ) + + expect(codeActions).toHaveLength(0) + }) + }) + }) +}) diff --git a/src/diagnostics/deprecated-color-setting-shorthand.ts b/src/diagnostics/deprecated-color-setting-shorthand.ts new file mode 100644 index 00000000..ae48a73f --- /dev/null +++ b/src/diagnostics/deprecated-color-setting-shorthand.ts @@ -0,0 +1,102 @@ +import colorString from 'color-string' +import { + CodeAction, + CodeActionKind, + CodeActionProvider, + Diagnostic, + DiagnosticSeverity, + DiagnosticTag, + Disposable, + TextDocument, + WorkspaceEdit, + languages, +} from 'vscode' +import { DirectiveParser } from '../directives/parser' + +const diagnosticMeta: unique symbol = Symbol() + +interface DeprecatedColorSettingShorthandDiagnostic extends Diagnostic { + [diagnosticMeta]: { replacement: string } +} + +export const code = 'deprecated-color-setting-shorthand' + +export function register( + directiveParser: DirectiveParser, + diagnostics: Diagnostic[] +) { + directiveParser.on('image', ({ alt, range, url }) => { + const directive = (() => { + if (alt === '') return 'color' as const + if (alt === 'bg') return 'backgroundColor' as const + return undefined + })() + if (directive === undefined) return + + if (colorString.get(url) || url.toLowerCase() === 'currentcolor') { + const diagnostic = Object.assign( + new Diagnostic( + range, + `Shorthand for setting colors via Markdown image syntax is deprecated, and will be removed in future. Please replace to the scoped local direcitve , or consider to use the scoped style.`, + DiagnosticSeverity.Warning + ), + { + source: 'marp-vscode', + code, + tags: [DiagnosticTag.Deprecated], + [diagnosticMeta]: { + replacement: ``, + }, + } + ) + + diagnostics.push(diagnostic) + } + }) +} + +export class DeprecatedColorSettingShorthand implements CodeActionProvider { + static readonly providedCodeActionKinds = [CodeActionKind.QuickFix] + + readonly provideCodeActions: CodeActionProvider['provideCodeActions'] = ( + doc, + _, + context + ) => + context.diagnostics + .filter( + (d): d is DeprecatedColorSettingShorthandDiagnostic => + d.source === 'marp-vscode' && d.code === code && d[diagnosticMeta] + ) + .map((d) => this.createCodeAction(d, doc)) + + private createCodeAction( + diag: DeprecatedColorSettingShorthandDiagnostic, + doc: TextDocument + ): CodeAction { + const act = new CodeAction( + `Replace to the scoped local direcitve: ${diag[diagnosticMeta].replacement}`, + CodeActionKind.QuickFix + ) + + act.diagnostics = [diag] + act.edit = new WorkspaceEdit() + act.edit.replace(doc.uri, diag.range, diag[diagnosticMeta].replacement) + act.isPreferred = true + + return act + } +} + +export function subscribe(subscriptions: Disposable[]) { + subscriptions.push( + languages.registerCodeActionsProvider( + 'markdown', + new DeprecatedColorSettingShorthand(), + { + providedCodeActionKinds: + DeprecatedColorSettingShorthand.providedCodeActionKinds, + } + ) + ) +} diff --git a/src/diagnostics/deprecated-dollar-prefix.ts b/src/diagnostics/deprecated-dollar-prefix.ts index aaae1ac1..e3d23208 100644 --- a/src/diagnostics/deprecated-dollar-prefix.ts +++ b/src/diagnostics/deprecated-dollar-prefix.ts @@ -4,6 +4,7 @@ import { CodeActionProvider, Diagnostic, DiagnosticSeverity, + DiagnosticTag, Disposable, Range, TextDocument, @@ -42,6 +43,7 @@ export function register( diagnostic.source = 'marp-vscode' diagnostic.code = code + diagnostic.tags = [DiagnosticTag.Deprecated] diagnostics.push(diagnostic) } diff --git a/src/diagnostics/index.ts b/src/diagnostics/index.ts index f5f7f1ea..0af07bba 100644 --- a/src/diagnostics/index.ts +++ b/src/diagnostics/index.ts @@ -10,6 +10,7 @@ import { import { DirectiveParser } from '../directives/parser' import { detectMarpDocument } from '../utils' import * as defineMathGlobalDirective from './define-math-global-directive' +import * as deprecatedColorSettingShorthand from './deprecated-color-setting-shorthand' import * as deprecatedDollarPrefix from './deprecated-dollar-prefix' import * as ignoredMathGlobalDirective from './ignored-math-global-directive' import * as overloadingGlobalDirective from './overloading-global-directive' @@ -23,6 +24,7 @@ const setDiagnostics = lodashDebounce((doc: TextDocument) => { const diagnostics: Diagnostic[] = [] defineMathGlobalDirective.register(directiveParser, diagnostics) + deprecatedColorSettingShorthand.register(directiveParser, diagnostics) deprecatedDollarPrefix.register(doc, directiveParser, diagnostics) ignoredMathGlobalDirective.register(doc, directiveParser, diagnostics) overloadingGlobalDirective.register(doc, directiveParser, diagnostics) @@ -53,6 +55,7 @@ export function subscribe(subscriptions: Disposable[]) { // Actions defineMathGlobalDirective.subscribe(subscriptions, debouncedRefresh) + deprecatedColorSettingShorthand.subscribe(subscriptions) deprecatedDollarPrefix.subscribe(subscriptions) ignoredMathGlobalDirective.subscribe(subscriptions, debouncedRefresh) diff --git a/src/directives/parser.ts b/src/directives/parser.ts index 39744d43..4acd8b02 100644 --- a/src/directives/parser.ts +++ b/src/directives/parser.ts @@ -38,6 +38,14 @@ export interface DirectiveSectionEventHandler { range: Range } +export interface ImageEventHandler { + alt: string + body: string + range: Range + title: string | null + url: string +} + export interface MathEventHandler { body: string range: Range @@ -48,6 +56,7 @@ type DirectiveParserEvents = { directive: (event: DirectiveEventHandler) => void endParse: (event: { document: TextDocument }) => void frontMatter: (event: DirectiveSectionEventHandler) => void + image: (event: ImageEventHandler) => void maybeMath: (event: MathEventHandler) => void startParse: (event: { document: TextDocument }) => void } @@ -119,10 +128,15 @@ export class DirectiveParser extends (EventEmitter as new () => TypedEmitter { + visit(parsed, ['html', 'image', 'math', 'inlineMath'], (n: any) => { + const range = new Range( + doc.positionAt(index + n.position.start.offset), + doc.positionAt(index + n.position.end.offset) + ) + switch (n.type) { case 'html': visit(parseHtml(n.value), 'comment', (c: any) => { @@ -131,13 +145,7 @@ export class DirectiveParser extends (EventEmitter as new () => TypedEmitter TypedEmitter TypedEmitter Date: Fri, 10 Jun 2022 11:11:11 +0900 Subject: [PATCH 2/3] Update README.md --- README.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ad8d39d5..96a82407 100644 --- a/README.md +++ b/README.md @@ -90,17 +90,19 @@ Marp for VS Code can detect some basic problems in Marp directives. Diagnostics Diagnostics

-| Name | Description | [Quick Fix] | -| :------------------------------ | :------------------------------------------------------------------------------------------------- | :---------: | -| `define-math-global-directive` | Recommend to declare math typesetting library via [`math` global directive][math global directive] | ✅ | -| `deprecated-dollar-prefix` | Check [obsoleted directives prefixed by `$`][dollar-prefix] | ✅ | -| `ignored-math-global-directive` | Report ignored `math` global directive if disabled math by the extension setting | | -| `overloading-global-directive` | Find out overloaded global directives | | -| `unknown-size` | Notify if the specified [size preset] was not defined in a theme | | -| `unknown-theme` | Notify a not recognized theme name | | +| Name | Description | [Quick Fix] | +| :----------------------------------- | :------------------------------------------------------------------------------------------------- | :---------: | +| `define-math-global-directive` | Recommend to declare math typesetting library via [`math` global directive][math global directive] | ✅ | +| `deprecated-color-setting-shorthand` | Check [deprecated shorthands for setting slide colors][color setting shorthand] | ✅ | +| `deprecated-dollar-prefix` | Check [obsoleted directives prefixed by `$`][dollar-prefix] | ✅ | +| `ignored-math-global-directive` | Report ignored `math` global directive if disabled math by the extension setting | | +| `overloading-global-directive` | Find out overloaded global directives | | +| `unknown-size` | Notify if the specified [size preset] was not defined in a theme | | +| `unknown-theme` | Notify a not recognized theme name | | [quick fix]: https://code.visualstudio.com/docs/editor/refactoring#_code-actions-quick-fixes-and-refactorings [dollar-prefix]: https://github.com/marp-team/marpit/issues/182 +[color setting shorthand]: https://github.com/marp-team/marpit/issues/331 [math global directive]: https://github.com/marp-team/marp-core#math-global-directive [size preset]: https://github.com/marp-team/marp-core/tree/main/themes#size-name-width-height From 24c918eeba28a4e418e5e76803093a59a9aa605e Mon Sep 17 00:00:00 2001 From: Yuki Hattori Date: Fri, 10 Jun 2022 11:13:14 +0900 Subject: [PATCH 3/3] [ci skip] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1656c9..758f1083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ## [Unreleased] +### Added + +- `deprecated-color-setting-shorthand` auto-fixable diagnostic for replacing [deprecated shorthands for setting colors](https://github.com/marp-team/marpit/issues/331) ([#358](https://github.com/marp-team/marp-vscode/issues/358), [#361](https://github.com/marp-team/marp-vscode/pull/366)) + ## v2.0.1 - 2022-06-06 ### Changed