Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use content-tag instead of ember-template-imports #655

Merged
merged 4 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"name": "Debug Extension (Glint + TS)",
"type": "extensionHost",
"request": "launch",
"preLaunchTask": "npm: build",
// this errors. Run `yarn build` before launching this task
// "preLaunchTask": "npm: build",
"autoAttachChildProcesses": true,
"runtimeExecutable": "${execPath}",
"outFiles": [
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
],
"scripts": {
"lint": "yarn lint:scripts && yarn lint:formatting",
"lint:fix": "yarn lint:scripts --fix && yarn prettier --write .",
"lint:scripts": "yarn eslint --max-warnings 0 --cache .",
"lint:formatting": "yarn prettier --check .",
"test": "yarn workspaces run test",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,103 @@ describe('Language Server: Diagnostic Augmentation', () => {
await project.destroy();
});

test('There is a content-tag parse error (for a template-only component)', async () => {
project.setGlintConfig({ environment: ['ember-loose', 'ember-template-imports'] });
project.write({
'index.gts': stripIndent`
function expectsTwoArgs(a: string, b: number) {
console.log(a, b);
}

<template>
{{expectsTwoArgs "one"}}
`,
});

let server = project.startLanguageServer();
let diagnostics = server.getDiagnostics(project.fileURI('index.gts'));

expect(diagnostics).toMatchInlineSnapshot(`
[
{
"code": 0,
"message": "Unexpected eof

5 │ <template>
6 │ {{expectsTwoArgs \\"one\\"}}
╰────",
"range": {
"end": {
"character": 7,
"line": 4,
},
"start": {
"character": 6,
"line": 4,
},
},
"severity": 1,
"source": "glint",
"tags": [],
},
]
`);
});

test('There is a content-tag parse error (for a class component)', async () => {
project.setGlintConfig({ environment: ['ember-loose', 'ember-template-imports'] });
project.write({
'index.gts': stripIndent`
import Component from '@glimmer/component';

export interface AppSignature {
Blocks: {
expectsTwoParams: [a: string, b: number];
expectsAtLeastOneParam: [a: string, ...rest: Array<string>];
}
}

function expectsTwoArgs(a: string, b: number) {
console.log(a, b);
}

export default class App extends Component<AppSignature> {
<template>
{{expectsTwoArgs "one"}}
}
`,
});

let server = project.startLanguageServer();
let diagnostics = server.getDiagnostics(project.fileURI('index.gts'));

expect(diagnostics).toMatchInlineSnapshot(`
[
{
"code": 0,
"message": "Unexpected token \`<lexing error: Error { error: (Span { lo: BytePos(382), hi: BytePos(382), ctxt: #0 }, Eof) }>\`. Expected content tag

16 │ {{expectsTwoArgs \\"one\\"}}
17 │ }
╰────",
"range": {
"end": {
"character": 14,
"line": 15,
},
"start": {
"character": 13,
"line": 15,
},
},
"severity": 1,
"source": "glint",
"tags": [],
},
]
`);
});

test('expected argument count', async () => {
project.setGlintConfig({ environment: ['ember-loose', 'ember-template-imports'] });
project.write({
Expand Down
18 changes: 17 additions & 1 deletion packages/core/src/common/transform-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ export default class TransformManager {
);
}

// When we have syntax errors we get _too many errors_
// if we have an issue with <template> tranformation, we should
// make the user fix their syntax before revealing all the other errors.
let contentTagErrors = allDiagnostics.filter(
(diagnostic) => (diagnostic as Diagnostic).isContentTagError
);
if (contentTagErrors.length) {
return this.ts.sortAndDeduplicateDiagnostics(contentTagErrors);
}

return this.ts.sortAndDeduplicateDiagnostics(allDiagnostics);
}

Expand Down Expand Up @@ -450,7 +460,13 @@ export default class TransformManager {

private buildTransformDiagnostics(transformedModule: TransformedModule): Array<Diagnostic> {
return transformedModule.errors.map((error) =>
createTransformDiagnostic(this.ts, error.source, error.message, error.location)
createTransformDiagnostic(
this.ts,
error.source,
error.message,
error.location,
error.isContentTagError
)
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ export function createTransformDiagnostic(
ts: TSLib,
source: SourceFile,
message: string,
location: Range
location: Range,
isContentTagError = false
): Diagnostic {
return {
isContentTagError,
isGlintTransformDiagnostic: true,
category: ts.DiagnosticCategory.Error,
code: 0,
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/transform/diagnostics/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type * as ts from 'typescript';

export type Diagnostic = ts.Diagnostic & { isGlintTransformDiagnostic?: boolean };
export type Diagnostic = ts.Diagnostic & {
isGlintTransformDiagnostic?: boolean;
isContentTagError?: boolean;
};

export { rewriteDiagnostic } from './rewrite-diagnostic.js';
export { createTransformDiagnostic } from './create-transform-diagnostic.js';
119 changes: 115 additions & 4 deletions packages/core/src/transform/template/rewrite-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,39 @@ function calculateCorrelatedSpans(
let errors: Array<TransformError> = [];
let partialSpans: Array<PartialCorrelatedSpan> = [];

let { ast, emitMetadata } = parseScript(ts, script, environment);
let { ast, emitMetadata, error } = parseScript(ts, script, environment);

if (error) {
if (typeof error === 'string') {
errors.push({
message: error,
location: { start: 0, end: script.contents.length - 1 },
source: script,
});
} else if ('isContentTagError' in error && error.isContentTagError) {
// these lines exclude the line with the error, because
// adding the column offset will get us on to the line with the error
let lines = script.contents.split('\n').slice(0, error.line);
let start = lines.reduce((sum, line) => sum + line.length, 0) + error.column - 1;
let end = start + 1;

errors.push({
isContentTagError: true,
// we have to show the "help" because content-tag has different line numbers
// than we are able to discern ourselves.
message: error.message + '\n\n' + error.help,
source: script,
location: {
start,
end,
},
});
}

// We've hit a parsing error, so we need to immediately return as the parsed
// document must be correct before we can continue.
return { errors, directives, partialSpans };
}

ts.transform(ast, [
(context) =>
Expand Down Expand Up @@ -103,9 +135,22 @@ function calculateCorrelatedSpans(
return { errors, directives, partialSpans };
}

type ParseError =
| string
| {
isContentTagError: true;
message: string;
line: number;
column: number;
file: string;
help: string;
raw: string;
};

type ParseResult = {
ast: ts.SourceFile;
emitMetadata: WeakMap<ts.Node, GlintEmitMetadata>;
error?: ParseError;
};

function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironment): ParseResult {
Expand All @@ -116,15 +161,37 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
void emitMetadata.set(node, Object.assign(emitMetadata.get(node) ?? {}, data));

let { preprocess, transform } = environment.getConfigForExtension(extension) ?? {};
let preprocessed = preprocess?.(contents, filename) ?? { contents };

let original: {
contents: string;
data?: {
// SAFETY: type exists elsewhere (the environments)
templateLocations: any[];
};
} = { contents, data: { templateLocations: [] } };

let preprocessed = original;
let error: ParseError | undefined;

try {
preprocessed = preprocess?.(contents, filename) ?? original;
} catch (e) {
error = parseError(e, filename);
}

let ast = ts.createSourceFile(
filename,
// contents will be transformed and placeholder'd content
// or, in the event of a parse error, the original content
// in which case, TS will report a ton of errors about some goofy syntax.
// We'll want to ignore all of that and only display our parsing error from content-tag.
preprocessed.contents,
ts.ScriptTarget.Latest,
true // setParentNodes
);

if (transform) {
// Only transform if we don't have a parse error
if (!error && transform) {
let { transformed } = ts.transform(ast, [
(context) => transform!(preprocessed.data, { ts, context, setEmitMetadata }),
]);
Expand All @@ -133,7 +200,51 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
ast = transformed[0];
}

return { ast, emitMetadata };
return { ast, emitMetadata, error };
}

function parseError(e: unknown, filename: string): ParseError {
if (typeof e === 'object' && e !== null) {
// Parse Errors from the rust parser
if ('source_code' in e) {
// We remove the blank links in the error because swc
// pads errors with a leading and trailing blank line.
// the error is typically meant for the terminal, so making it
// stand out a bit more is a good, but that's more a presentation
// concern than just pure error information (which is what we need).
// @ts-expect-error object / property narrowing isn't available until TS 5.1
let lines = e.source_code.split('\n').filter(Boolean);
// Example:
// ' × Unexpected eof'
// ' ╭─[/home/nullvoxpopuli/Development/OpenSource/glint/test-packages/ts-template-imports-app/src/index.gts:6:1]'
// ' 6 │ '
// ' 7 │ export const X = <tem'
// ' ╰────'
let raw = lines.join('\n');
let message = lines[0].replace('×', '').trim();
let info = lines[1];
// a filename may have numbers in it, so we want to remove the filename
// before regex searching for numbers at the end of this line
let strippedInfo = info.replace(filename, '');
let matches = [...strippedInfo.matchAll(/\d+/g)];
let line = parseInt(matches[0][0], 10);
let column = parseInt(matches[1][0], 10);
// The help omits the original file name, because TS will provide that.
let help = lines.slice(2).join('\n');

return {
isContentTagError: true,
raw,
message,
line,
column,
file: filename,
help,
};
}
}

return `${e}`;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/transform/template/transformed-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type Directive = {
};

export type TransformError = {
isContentTagError?: boolean;
message: string;
location: Range;
source: SourceFile;
Expand Down
Loading
Loading