From 2a0af747866778e4b63af0b25ab08aa557b7144d Mon Sep 17 00:00:00 2001 From: Armano Date: Sat, 6 Jul 2024 16:25:10 +0200 Subject: [PATCH] refactor: reuse codeFrame helper in logger and deduplicate code (#350) * refactor: reuse codeFrame helper in logger and deduplicate code * chore: revert unnecessary change * chore: correct linting * chore: add missing changeset * test: update snapshots * fix: reuse already computed pos for ts location start * fix: apply changes from code review * fix: rename locationToBabelLocation to lineColLocToBabelLoc --- .changeset/violet-moons-shake.md | 5 + .../__tests__/codeFrame.spec.ts | 22 ++++- packages/vite-plugin-checker/src/codeFrame.ts | 37 ++++--- packages/vite-plugin-checker/src/logger.ts | 96 +++---------------- .../__tests__/__snapshots__/test.spec.ts.snap | 32 +++---- 5 files changed, 76 insertions(+), 116 deletions(-) create mode 100644 .changeset/violet-moons-shake.md diff --git a/.changeset/violet-moons-shake.md b/.changeset/violet-moons-shake.md new file mode 100644 index 00000000..4fdf9185 --- /dev/null +++ b/.changeset/violet-moons-shake.md @@ -0,0 +1,5 @@ +--- +'vite-plugin-checker': patch +--- + +refactor: reuse codeFrame helper in logger and deduplicate code diff --git a/packages/vite-plugin-checker/__tests__/codeFrame.spec.ts b/packages/vite-plugin-checker/__tests__/codeFrame.spec.ts index 4c220609..fcd66381 100644 --- a/packages/vite-plugin-checker/__tests__/codeFrame.spec.ts +++ b/packages/vite-plugin-checker/__tests__/codeFrame.spec.ts @@ -1,12 +1,10 @@ import { describe, expect, it } from 'vitest' -import type { SourceLocation } from '@babel/code-frame' - -import { tsLocationToBabelLocation } from '../src/codeFrame' +import { lineColLocToBabelLoc, tsLikeLocToBabelLoc } from '../src/codeFrame' describe('code frame', () => { it('should add 1 offset to TS location', () => { - const babelLoc = tsLocationToBabelLocation({ + const babelLoc = tsLikeLocToBabelLoc({ start: { line: 1, character: 2 }, end: { line: 3, character: 4 }, }) @@ -14,6 +12,20 @@ describe('code frame', () => { expect(babelLoc).toEqual({ start: { line: 2, column: 3 }, end: { line: 4, column: 5 }, - } as SourceLocation) + }) + }) + + it('transform location without offset', () => { + const babelLoc = lineColLocToBabelLoc({ + line: 1, + column: 2, + endLine: 3, + endColumn: 4, + }) + + expect(babelLoc).toEqual({ + start: { line: 1, column: 2 }, + end: { line: 3, column: 4 }, + }) }) }) diff --git a/packages/vite-plugin-checker/src/codeFrame.ts b/packages/vite-plugin-checker/src/codeFrame.ts index e3c7e451..91d896dc 100644 --- a/packages/vite-plugin-checker/src/codeFrame.ts +++ b/packages/vite-plugin-checker/src/codeFrame.ts @@ -1,30 +1,39 @@ import os from 'node:os' -import type ts from 'typescript' import { type SourceLocation, codeFrameColumns } from '@babel/code-frame' -export function createFrame({ - source, - location, -}: { - source: string // file source code - location: SourceLocation -}) { - const frame = codeFrameColumns(source, location, { - highlightCode: true, +/** + * Create a code frame from source code and location + * @param source source code + * @param location babel compatible location to highlight + */ +export function createFrame(source: string, location: SourceLocation): string { + return codeFrameColumns(source, location, { + // worker tty did not fork parent process stdout, let's make a workaround + forceColor: true, }) .split('\n') .map((line) => ` ${line}`) .join(os.EOL) - - return frame } -export function tsLocationToBabelLocation( - tsLoc: Record<'start' | 'end', ts.LineAndCharacter /** 0-based */> +export function tsLikeLocToBabelLoc( + tsLoc: Record<'start' | 'end', { line: number; character: number } /** 0-based */> ): SourceLocation { return { start: { line: tsLoc.start.line + 1, column: tsLoc.start.character + 1 }, end: { line: tsLoc.end.line + 1, column: tsLoc.end.character + 1 }, } } + +export function lineColLocToBabelLoc(d: { + line: number + column: number + endLine?: number + endColumn?: number +}): SourceLocation { + return { + start: { line: d.line, column: d.column }, + end: { line: d.endLine || 0, column: d.endColumn }, + } +} diff --git a/packages/vite-plugin-checker/src/logger.ts b/packages/vite-plugin-checker/src/logger.ts index 67e35deb..f2f7419d 100644 --- a/packages/vite-plugin-checker/src/logger.ts +++ b/packages/vite-plugin-checker/src/logger.ts @@ -11,9 +11,10 @@ import * as _vscodeUri from 'vscode-uri' const URI = _vscodeUri?.default?.URI ?? _vscodeUri.URI import { parentPort } from 'node:worker_threads' -import { type SourceLocation, codeFrameColumns } from '@babel/code-frame' +import type { SourceLocation } from '@babel/code-frame' import { WS_CHECKER_ERROR_EVENT } from './client/index.js' +import { createFrame, lineColLocToBabelLoc, tsLikeLocToBabelLoc } from './codeFrame.js' import { ACTION_TYPES, type ClientDiagnosticPayload, @@ -25,14 +26,12 @@ import { isMainThread } from './utils.js' const _require = createRequire(import.meta.url) import type { ESLint } from 'eslint' import type Stylelint from 'stylelint' -import type { Range } from 'vscode-languageclient' import type { Diagnostic as LspDiagnostic, PublishDiagnosticsParams, } from 'vscode-languageclient/node' import type { - LineAndCharacter, Diagnostic as TsDiagnostic, flattenDiagnosticMessageText as flattenDiagnosticMessageTextType, } from 'typescript' @@ -162,34 +161,6 @@ export function toClientPayload( } } -export function createFrame({ - source, - location, -}: { - /** file source code */ - source: string - location: SourceLocation -}) { - const frame = codeFrameColumns(source, location, { - // worker tty did not fork parent process stdout, let's make a workaround - forceColor: true, - }) - .split('\n') - .map((line) => ` ${line}`) - .join(os.EOL) - - return frame -} - -export function tsLocationToBabelLocation( - tsLoc: Record<'start' | 'end', LineAndCharacter /** 0-based */> -): SourceLocation { - return { - start: { line: tsLoc.start.line + 1, column: tsLoc.start.character + 1 }, - end: { line: tsLoc.end.line + 1, column: tsLoc.end.character + 1 }, - } -} - export function wrapCheckerSummary(checkerName: string, rawSummary: string): string { return `[${checkerName}] ${rawSummary}` } @@ -224,18 +195,15 @@ export function normalizeTsDiagnostic(d: TsDiagnostic): NormalizedDiagnostic { let loc: SourceLocation | undefined const pos = d.start === undefined ? null : d.file?.getLineAndCharacterOfPosition?.(d.start) if (pos && d.file && typeof d.start === 'number' && typeof d.length === 'number') { - loc = tsLocationToBabelLocation({ - start: d.file?.getLineAndCharacterOfPosition(d.start), - end: d.file?.getLineAndCharacterOfPosition(d.start + d.length), + loc = tsLikeLocToBabelLoc({ + start: pos, + end: d.file.getLineAndCharacterOfPosition(d.start + d.length), }) } let codeFrame: string | undefined if (loc) { - codeFrame = createFrame({ - source: d.file!.text, - location: loc, - }) + codeFrame = createFrame(d.file!.text, loc) } return { @@ -262,8 +230,8 @@ export function normalizeLspDiagnostic({ fileText: string }): NormalizedDiagnostic { let level = DiagnosticLevel.Error - const loc = lspRange2Location(diagnostic.range) - const codeFrame = codeFrameColumns(fileText, loc) + const loc = tsLikeLocToBabelLoc(diagnostic.range) + const codeFrame = createFrame(fileText, loc) switch (diagnostic.severity) { case 1: // Error @@ -315,19 +283,6 @@ export function uriToAbsPath(documentUri: string): string { return URI.parse(documentUri).fsPath } -export function lspRange2Location(range: Range): SourceLocation { - return { - start: { - line: range.start.line + 1, - column: range.start.character + 1, - }, - end: { - line: range.end.line + 1, - column: range.end.character + 1, - }, - } -} - /* --------------------------------- vue-tsc -------------------------------- */ export function normalizeVueTscDiagnostic(d: TsDiagnostic): NormalizedDiagnostic { @@ -360,21 +315,9 @@ export function normalizeEslintDiagnostic(diagnostic: ESLint.LintResult): Normal break } - const loc: SourceLocation = { - start: { - line: d.line, - column: d.column, - }, - end: { - line: d.endLine || 0, - column: d.endColumn, - }, - } + const loc = lineColLocToBabelLoc(d) - const codeFrame = createFrame({ - source: diagnostic.source ?? '', - location: loc, - }) + const codeFrame = createFrame(diagnostic.source ?? '', loc) return { message: `${d.message} (${d.ruleId})`, @@ -410,22 +353,13 @@ export function normalizeStylelintDiagnostic( return null } - const loc: SourceLocation = { - start: { - line: d.line, - column: d.column, - }, - end: { - line: d.endLine || 0, - column: d.endColumn, - }, - } + const loc = lineColLocToBabelLoc(d) - const codeFrame = createFrame({ + const codeFrame = createFrame( // @ts-ignore - source: diagnostic._postcssResult.css ?? '', - location: loc, - }) + diagnostic._postcssResult.css ?? '', + loc + ) return { message: `${d.text} (${d.rule})`, diff --git a/playground/vls-vue2/__tests__/__snapshots__/test.spec.ts.snap b/playground/vls-vue2/__tests__/__snapshots__/test.spec.ts.snap index 57c34620..751d2a44 100644 --- a/playground/vls-vue2/__tests__/__snapshots__/test.spec.ts.snap +++ b/playground/vls-vue2/__tests__/__snapshots__/test.spec.ts.snap @@ -1,38 +1,38 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`vue2-vls > serve > get initial error and subsequent error 1`] = `"[{"checkerId":"VLS","frame":" 1 |