From dba4aa3567f305bc739de419ae36c6034da5cb08 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 17:09:10 +0200 Subject: [PATCH] chore: align warning and error objects, add frame property (#12326) This aligns warning and error objects to contain the same properties and have their toString methods return the same shape. It's implemented by warnings becoming class objects, too, and sharing the same base class with errors. It also adds back the `frame` property that got lost in the Svelte 4->5 transition. The only difference to Svelte 4 now is a slightly adjusted toString property (which is consistent between warnings and errors now) and a `position` property that contains a tuple of start/end offsets instead of a `pos` property only containing the start offset closes #12151 --- .changeset/small-owls-remain.md | 5 + .../templates/compile-errors.js | 45 +------- .../templates/compile-warnings.js | 30 +++-- packages/svelte/src/compiler/errors.js | 40 +------ packages/svelte/src/compiler/state.js | 11 +- packages/svelte/src/compiler/types/index.d.ts | 14 +-- .../src/compiler/utils/compile_diagnostic.js | 105 ++++++++++++++++++ packages/svelte/src/compiler/warnings.js | 31 +++--- packages/svelte/tests/css/test.ts | 2 + packages/svelte/types/index.d.ts | 50 ++++----- 10 files changed, 191 insertions(+), 142 deletions(-) create mode 100644 .changeset/small-owls-remain.md create mode 100644 packages/svelte/src/compiler/utils/compile_diagnostic.js diff --git a/.changeset/small-owls-remain.md b/.changeset/small-owls-remain.md new file mode 100644 index 0000000000..a63ff64935 --- /dev/null +++ b/.changeset/small-owls-remain.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: align warning and error objects, add frame property diff --git a/packages/svelte/scripts/process-messages/templates/compile-errors.js b/packages/svelte/scripts/process-messages/templates/compile-errors.js index e129eef9f6..349e0b38ef 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-errors.js +++ b/packages/svelte/scripts/process-messages/templates/compile-errors.js @@ -1,54 +1,17 @@ -/** @import { Location } from 'locate-character' */ -import * as state from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -export class InternalCompileError extends Error { +export class InternalCompileError extends CompileDiagnostic { name = 'CompileError'; - filename = state.filename; - - /** @type {[number, number] | undefined} */ - position = undefined; - - /** @type {Location | undefined} */ - start = undefined; - - /** @type {Location | undefined} */ - end = undefined; - /** - * * @param {string} code * @param {string} message * @param {[number, number] | undefined} position */ constructor(code, message, position) { - super(message); - - this.code = code; - this.position = position; - - if (position) { - this.start = state.locator(position[0]); - this.end = state.locator(position[1]); - } - } - - toString() { - let out = `${this.name}: ${this.message}`; - - out += `\n(${this.code})`; - - if (this.filename) { - out += `\n${this.filename}`; - - if (this.start) { - out += `${this.start.line}:${this.start.column}`; - } - } - - return out; + super(code, message, position); } } @@ -65,7 +28,7 @@ function e(node, code, message) { throw new InternalCompileError( code, message, - start !== undefined && end !== undefined ? [start, end] : undefined + start !== undefined ? [start, end ?? start] : undefined ); } diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index d6a0cd6a0b..c8bd7993c0 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -1,7 +1,21 @@ -import { filename, locator, warnings, ignore_stack, ignore_map } from './state.js'; +import { warnings, ignore_stack, ignore_map } from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ +export class InternalCompileWarning extends CompileDiagnostic { + name = 'CompileWarning'; + + /** + * @param {string} code + * @param {string} message + * @param {[number, number] | undefined} position + */ + constructor(code, message, position) { + super(code, message, position); + } +} + /** * @param {null | NodeLike} node * @param {string} code @@ -14,13 +28,13 @@ function w(node, code, message) { } if (stack && stack.at(-1)?.has(code)) return; - warnings.push({ - code, - message, - filename, - start: node?.start !== undefined ? locator(node.start) : undefined, - end: node?.end !== undefined ? locator(node.end) : undefined - }); + warnings.push( + new InternalCompileWarning( + code, + message, + node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined + ) + ); } export const codes = CODES; diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index c26586fb1c..7e0b5dd995 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -1,50 +1,18 @@ /* This file is generated by scripts/process-messages/index.js. Do not edit! */ -/** @import { Location } from 'locate-character' */ -import * as state from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -export class InternalCompileError extends Error { +export class InternalCompileError extends CompileDiagnostic { name = 'CompileError'; - filename = state.filename; - /** @type {[number, number] | undefined} */ - position = undefined; - /** @type {Location | undefined} */ - start = undefined; - /** @type {Location | undefined} */ - end = undefined; /** - * * @param {string} code * @param {string} message * @param {[number, number] | undefined} position */ constructor(code, message, position) { - super(message); - this.code = code; - this.position = position; - - if (position) { - this.start = state.locator(position[0]); - this.end = state.locator(position[1]); - } - } - - toString() { - let out = `${this.name}: ${this.message}`; - - out += `\n(${this.code})`; - - if (this.filename) { - out += `\n${this.filename}`; - - if (this.start) { - out += `${this.start.line}:${this.start.column}`; - } - } - - return out; + super(code, message, position); } } @@ -58,7 +26,7 @@ function e(node, code, message) { const start = typeof node === 'number' ? node : node?.start; const end = typeof node === 'number' ? node : node?.end; - throw new InternalCompileError(code, message, start !== undefined && end !== undefined ? [start, end] : undefined); + throw new InternalCompileError(code, message, start !== undefined ? [start, end ?? start] : undefined); } /** diff --git a/packages/svelte/src/compiler/state.js b/packages/svelte/src/compiler/state.js index 1e094d95fd..64b9fe591c 100644 --- a/packages/svelte/src/compiler/state.js +++ b/packages/svelte/src/compiler/state.js @@ -14,6 +14,12 @@ export let warnings = []; */ export let filename; +/** + * The original source code + * @type {string} + */ +export let source; + export let locator = getLocator('', { offsetLine: 1 }); /** @@ -43,10 +49,11 @@ export function pop_ignore() { } /** - * @param {string} source + * @param {string} _source * @param {{ filename?: string, rootDir?: string }} options */ -export function reset(source, options) { +export function reset(_source, options) { + source = _source; const root_dir = options.rootDir?.replace(/\\/g, '/'); filename = options.filename?.replace(/\\/g, '/'); diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 129b5da175..9574491756 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -6,13 +6,12 @@ import type { Identifier, ImportDeclaration } from 'estree'; -import type { Location } from 'locate-character'; import type { SourceMap } from 'magic-string'; import type { Context } from 'zimmerframe'; import type { Scope } from '../phases/scope.js'; import type { Css } from './css.js'; import type { EachBlock, Namespace, SvelteNode, SvelteOptions } from './template.js'; -import type { InternalCompileError } from '../errors.js'; +import type { ICompileDiagnostic } from '../utils/compile_diagnostic.js'; /** The return value of `compile` from `svelte/compiler` */ export interface CompileResult { @@ -51,16 +50,9 @@ export interface CompileResult { ast: any; } -export interface Warning { - start?: Location; - end?: Location; - // TODO there was pos: number in Svelte 4 - do we want to add it back? - code: string; - message: string; - filename?: string; -} +export interface Warning extends ICompileDiagnostic {} -export interface CompileError extends InternalCompileError {} +export interface CompileError extends ICompileDiagnostic {} export type CssHashGetter = (args: { name: string; diff --git a/packages/svelte/src/compiler/utils/compile_diagnostic.js b/packages/svelte/src/compiler/utils/compile_diagnostic.js new file mode 100644 index 0000000000..fa36416a06 --- /dev/null +++ b/packages/svelte/src/compiler/utils/compile_diagnostic.js @@ -0,0 +1,105 @@ +/** @import { Location } from 'locate-character' */ +import * as state from '../state.js'; + +const regex_tabs = /^\t+/; + +/** + * @param {string} str + */ +function tabs_to_spaces(str) { + return str.replace(regex_tabs, (match) => match.split('\t').join(' ')); +} + +/** + * @param {string} source + * @param {number} line + * @param {number} column + */ +function get_code_frame(source, line, column) { + const lines = source.split('\n'); + const frame_start = Math.max(0, line - 2); + const frame_end = Math.min(line + 3, lines.length); + const digits = String(frame_end + 1).length; + return lines + .slice(frame_start, frame_end) + .map((str, i) => { + const is_error_line = frame_start + i === line; + const line_num = String(i + frame_start + 1).padStart(digits, ' '); + if (is_error_line) { + const indicator = + ' '.repeat(digits + 2 + tabs_to_spaces(str.slice(0, column)).length) + '^'; + return `${line_num}: ${tabs_to_spaces(str)}\n${indicator}`; + } + return `${line_num}: ${tabs_to_spaces(str)}`; + }) + .join('\n'); +} + +/** + * @typedef {{ + * code: string; + * message: string; + * filename?: string; + * start?: Location; + * end?: Location; + * position?: [number, number]; + * frame?: string; + * }} ICompileDiagnostic */ + +/** @implements {ICompileDiagnostic} */ +export class CompileDiagnostic extends Error { + name = 'CompileDiagnostic'; + + /** + * @param {string} code + * @param {string} message + * @param {[number, number] | undefined} position + */ + constructor(code, message, position) { + super(message); + this.code = code; + + if (state.filename) { + this.filename = state.filename; + } + + if (position) { + this.position = position; + this.start = state.locator(position[0]); + this.end = state.locator(position[1]); + if (this.start && this.end) { + this.frame = get_code_frame(state.source, this.start.line - 1, this.end.column); + } + } + } + + toString() { + let out = `${this.code}: ${this.message}`; + + if (this.filename) { + out += `\n${this.filename}`; + + if (this.start) { + out += `:${this.start.line}:${this.start.column}`; + } + } + + if (this.frame) { + out += `\n${this.frame}`; + } + + return out; + } + + toJSON() { + return { + code: this.code, + message: this.message, + filename: this.filename, + start: this.start, + end: this.end, + position: this.position, + frame: this.frame + }; + } +} diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 7f40eec02b..2518c6c38a 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -1,14 +1,22 @@ /* This file is generated by scripts/process-messages/index.js. Do not edit! */ -import { - filename, - locator, - warnings, - ignore_stack, - ignore_map -} from './state.js'; +import { warnings, ignore_stack, ignore_map } from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ +export class InternalCompileWarning extends CompileDiagnostic { + name = 'CompileWarning'; + + /** + * @param {string} code + * @param {string} message + * @param {[number, number] | undefined} position + */ + constructor(code, message, position) { + super(code, message, position); + } +} + /** * @param {null | NodeLike} node * @param {string} code @@ -22,14 +30,7 @@ function w(node, code, message) { } if (stack && stack.at(-1)?.has(code)) return; - - warnings.push({ - code, - message, - filename, - start: node?.start !== undefined ? locator(node.start) : undefined, - end: node?.end !== undefined ? locator(node.end) : undefined - }); + warnings.push(new InternalCompileWarning(code, message, node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined)); } export const codes = [ diff --git a/packages/svelte/tests/css/test.ts b/packages/svelte/tests/css/test.ts index d17f7e9c6b..b4b25fc479 100644 --- a/packages/svelte/tests/css/test.ts +++ b/packages/svelte/tests/css/test.ts @@ -10,6 +10,8 @@ import type { CompileOptions, Warning } from '#compiler'; function normalize_warning(warning: Warning) { delete warning.filename; + delete warning.position; + delete warning.frame; return warning; } diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 305e1240db..775fd7394b 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -562,9 +562,9 @@ declare module 'svelte/animate' { declare module 'svelte/compiler' { import type { AssignmentExpression, ClassDeclaration, Expression, FunctionDeclaration, Identifier, ImportDeclaration, ArrayExpression, MemberExpression, ObjectExpression, Pattern, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, FunctionExpression, Node, Program, ChainExpression, SimpleCallExpression } from 'estree'; - import type { Location } from 'locate-character'; import type { SourceMap } from 'magic-string'; import type { Context } from 'zimmerframe'; + import type { Location } from 'locate-character'; /** * `compile` converts your `.svelte` source code into a JavaScript module that exports a component * @@ -714,16 +714,9 @@ declare module 'svelte/compiler' { ast: any; } - export interface Warning { - start?: Location; - end?: Location; - // TODO there was pos: number in Svelte 4 - do we want to add it back? - code: string; - message: string; - filename?: string; - } + export interface Warning extends ICompileDiagnostic {} - export interface CompileError extends InternalCompileError {} + export interface CompileError extends ICompileDiagnostic {} type CssHashGetter = (args: { name: string; @@ -1882,18 +1875,15 @@ declare module 'svelte/compiler' { content: Program; attributes: Attribute[]; } - class InternalCompileError extends Error { - - constructor(code: string, message: string, position: [number, number] | undefined); - filename: string | undefined; - - position: [number, number] | undefined; - - start: Location | undefined; - - end: Location | undefined; + type ICompileDiagnostic = { code: string; - } + message: string; + filename?: string; + start?: Location; + end?: Location; + position?: [number, number]; + frame?: string; + }; export {}; } @@ -2539,14 +2529,7 @@ declare module 'svelte/types/compiler/interfaces' { export type CompileOptions = CompileOptions_1; /** @deprecated import this from 'svelte' instead */ export type Warning = Warning_1; - interface Warning_1 { - start?: Location; - end?: Location; - // TODO there was pos: number in Svelte 4 - do we want to add it back? - code: string; - message: string; - filename?: string; - } + interface Warning_1 extends ICompileDiagnostic {} type CssHashGetter = (args: { name: string; @@ -2709,6 +2692,15 @@ declare module 'svelte/types/compiler/interfaces' { * (also see https://github.com/sveltejs/svelte/pull/5652) */ type Namespace = 'html' | 'svg' | 'mathml' | 'foreign'; + type ICompileDiagnostic = { + code: string; + message: string; + filename?: string; + start?: Location; + end?: Location; + position?: [number, number]; + frame?: string; + }; export {}; }declare module '*.svelte' {