Skip to content

Commit

Permalink
refactor: reuse codeFrame helper in logger and deduplicate code (#350)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
armano2 authored Jul 6, 2024
1 parent b4da388 commit 2a0af74
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 116 deletions.
5 changes: 5 additions & 0 deletions .changeset/violet-moons-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'vite-plugin-checker': patch
---

refactor: reuse codeFrame helper in logger and deduplicate code
22 changes: 17 additions & 5 deletions packages/vite-plugin-checker/__tests__/codeFrame.spec.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
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 },
})

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 },
})
})
})
37 changes: 23 additions & 14 deletions packages/vite-plugin-checker/src/codeFrame.ts
Original file line number Diff line number Diff line change
@@ -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 },
}
}
96 changes: 15 additions & 81 deletions packages/vite-plugin-checker/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'
Expand Down Expand Up @@ -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}`
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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})`,
Expand Down Expand Up @@ -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})`,
Expand Down
32 changes: 16 additions & 16 deletions playground/vls-vue2/__tests__/__snapshots__/test.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -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 | <template>/n 2 | <div class=/"hello/">/n> 3 | <h1>{{ msg1 }}</h1>/n | ^^^^/n 4 | <p>/n 5 | For a guide and recipes on how to configure / customize this project,<br />/n 6 | check out the","id":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","level":1,"loc":{"column":12,"file":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","line":3},"message":"Property 'msg1' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?","stack":""}]"`;
exports[`vue2-vls > serve > get initial error and subsequent error 1`] = `"[{"checkerId":"VLS","frame":" 1 | <template>/n 2 | <div class=/"hello/">/n > 3 | <h1>{{ msg1 }}</h1>/n | ^^^^/n 4 | <p>/n 5 | For a guide and recipes on how to configure / customize this project,<br />/n 6 | check out the","id":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","level":1,"loc":{"column":12,"file":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","line":3},"message":"Property 'msg1' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?","stack":""}]"`;

exports[`vue2-vls > serve > get initial error and subsequent error 2`] = `
[
" ERROR(VLS) Property 'msg1' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?
FILE <PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue:3:12
1 | <template>
2 | <div class="hello">
> 3 | <h1>{{ msg1 }}</h1>
| ^^^^
4 | <p>
5 | For a guide and recipes on how to configure / customize this project,<br />
6 | check out the
1 | <template>
2 | <div class="hello">
> 3 | <h1>{{ msg1 }}</h1>
| ^^^^
4 | <p>
5 | For a guide and recipes on how to configure / customize this project,<br />
6 | check out the
",
"[VLS] Found 1 error and 0 warning",
]
`;
exports[`vue2-vls > serve > get initial error and subsequent error 3`] = `"[{"checkerId":"VLS","frame":" 1 | <template>/n 2 | <div class=/"hello/">/n> 3 | <h1>{{ msg1 }}</h1>/n | ^^^^/n 4 | <p>/n 5 | For a guide and recipes on how to configure / customize this project,<br />/n 6 | check out the","id":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","level":1,"loc":{"column":12,"file":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","line":3},"message":"Property 'msg1' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?","stack":""},{"checkerId":"VLS","frame":" 1 | <template>/n 2 | <div class=/"hello/">/n> 3 | <h1>{{ msg2 }}</h1>/n | ^^^^/n 4 | <p>/n 5 | For a guide and recipes on how to configure / customize this project,<br />/n 6 | check out the","id":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","level":1,"loc":{"column":12,"file":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","line":3},"message":"Property 'msg2' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?","stack":""}]"`;
exports[`vue2-vls > serve > get initial error and subsequent error 3`] = `"[{"checkerId":"VLS","frame":" 1 | <template>/n 2 | <div class=/"hello/">/n > 3 | <h1>{{ msg1 }}</h1>/n | ^^^^/n 4 | <p>/n 5 | For a guide and recipes on how to configure / customize this project,<br />/n 6 | check out the","id":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","level":1,"loc":{"column":12,"file":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","line":3},"message":"Property 'msg1' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?","stack":""},{"checkerId":"VLS","frame":" 1 | <template>/n 2 | <div class=/"hello/">/n > 3 | <h1>{{ msg2 }}</h1>/n | ^^^^/n 4 | <p>/n 5 | For a guide and recipes on how to configure / customize this project,<br />/n 6 | check out the","id":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","level":1,"loc":{"column":12,"file":"<PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue","line":3},"message":"Property 'msg2' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?","stack":""}]"`;
exports[`vue2-vls > serve > get initial error and subsequent error 4`] = `
[
" ERROR(VLS) Property 'msg2' does not exist on type 'CombinedVueInstance<ExtractComputedReturns<{}> & { msg: string; } & Vue<Record<string, any>, Record<string, any>, never, never, (event: string, ...args: any[]) => Vue<...>> & ShallowUnwrapRef<...> & Vue<...>, ... 7 more ..., OptionTypesType<...>>'. Did you mean 'msg'?
FILE <PROJECT_ROOT>/playground-temp/vls-vue2/src/components/HelloWorld.vue:3:12
1 | <template>
2 | <div class="hello">
> 3 | <h1>{{ msg2 }}</h1>
| ^^^^
4 | <p>
5 | For a guide and recipes on how to configure / customize this project,<br />
6 | check out the
1 | <template>
2 | <div class="hello">
> 3 | <h1>{{ msg2 }}</h1>
| ^^^^
4 | <p>
5 | For a guide and recipes on how to configure / customize this project,<br />
6 | check out the
",
"[VLS] Found 1 error and 0 warning",
]
Expand Down

0 comments on commit 2a0af74

Please sign in to comment.