diff --git a/packages/core/__tests__/cli/check.test.ts b/packages/core/__tests__/cli/check.test.ts index 2323f1b1f..5862db09b 100644 --- a/packages/core/__tests__/cli/check.test.ts +++ b/packages/core/__tests__/cli/check.test.ts @@ -213,6 +213,42 @@ describe('CLI: single-pass typechecking', () => { `); }); + test('reports correct diagnostics given @glint-expect-error and @glint-ignore directives', async () => { + project.write('.glintrc', 'environment: ember-loose\n'); + + let script = stripIndent` + import Component from '@glint/environment-ember-loose/ember-component'; + + export default class MyComponent extends Component {} + `; + + let template = stripIndent` + {{! @glint-ignore }} + {{@message}}, + + {{! @glint-expect-error }} + {{this.target}} + + {{! @glint-expect-error }} + Hello. + `; + + project.write('my-component.ts', script); + project.write('my-component.hbs', template); + + let checkResult = await project.check({ reject: false }); + + expect(checkResult.exitCode).toBe(1); + expect(checkResult.stdout).toEqual(''); + expect(stripAnsi(checkResult.stderr)).toMatchInlineSnapshot(` + "my-component.hbs:7:1 - error TS0: Unused '@glint-expect-error' directive. + + 7 {{! @glint-expect-error }} + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + " + `); + }); + test('honors .glintrc configuration', async () => { let code = stripIndent` import Component, { hbs } from '@glint/environment-glimmerx/component'; diff --git a/packages/core/__tests__/cli/watch.test.ts b/packages/core/__tests__/cli/watch.test.ts index b567afcee..876d9a0b0 100644 --- a/packages/core/__tests__/cli/watch.test.ts +++ b/packages/core/__tests__/cli/watch.test.ts @@ -216,4 +216,44 @@ describe('CLI: watched typechecking', () => { await watch.terminate(); }); + + test('reports correct diagnostics given @glint-expect-error and @glint-ignore directives', async () => { + project.write('.glintrc', 'environment: ember-loose\n'); + + let script = stripIndent` + import Component from '@glint/environment-ember-loose/ember-component'; + + export default class MyComponent extends Component { + // private target = 'world'; + } + `; + + let template = stripIndent` + {{! @glint-ignore }} + {{@message}}, + + {{! @glint-expect-error }} + {{this.target}} + `; + + project.write('my-component.ts', script); + project.write('my-component.hbs', template); + + let watch = project.watch(); + + let output = await watch.awaitOutput('Watching for file changes.'); + expect(output).toMatch('Found 0 errors.'); + + project.write('my-component.ts', script.replace('// ', '')); + + output = await watch.awaitOutput('Watching for file changes.'); + expect(output).toMatch('Found 1 error.'); + + project.write('my-component.ts', script); + + output = await watch.awaitOutput('Watching for file changes.'); + expect(output).toMatch('Found 0 errors.'); + + await watch.terminate(); + }); }); diff --git a/packages/core/__tests__/language-server/diagnostics.test.ts b/packages/core/__tests__/language-server/diagnostics.test.ts index 41ab0be01..6a74da924 100644 --- a/packages/core/__tests__/language-server/diagnostics.test.ts +++ b/packages/core/__tests__/language-server/diagnostics.test.ts @@ -38,22 +38,6 @@ describe('Language Server: Diagnostics', () => { expect(diagnostics).toMatchInlineSnapshot(` Array [ - Object { - "message": "Property 'startupTimee' does not exist on type 'Application'. Did you mean 'startupTime'?", - "range": Object { - "end": Object { - "character": 43, - "line": 11, - }, - "start": Object { - "character": 31, - "line": 11, - }, - }, - "severity": 1, - "source": "glint:ts(2551)", - "tags": Array [], - }, Object { "message": "'startupTime' is declared but its value is never read.", "range": Object { @@ -72,6 +56,22 @@ describe('Language Server: Diagnostics', () => { 1, ], }, + Object { + "message": "Property 'startupTimee' does not exist on type 'Application'. Did you mean 'startupTime'?", + "range": Object { + "end": Object { + "character": 43, + "line": 11, + }, + "start": Object { + "character": 31, + "line": 11, + }, + }, + "severity": 1, + "source": "glint:ts(2551)", + "tags": Array [], + }, ] `); @@ -160,4 +160,95 @@ describe('Language Server: Diagnostics', () => { expect(server.getDiagnostics(project.fileURI('index.ts'))).toEqual([]); expect(server.getDiagnostics(project.fileURI('index.hbs'))).toEqual([]); }); + + test('honors @glint-ignore and @glint-expect-error', () => { + let componentA = stripIndent` + import Component, { hbs } from '@glint/environment-glimmerx/component'; + + export default class ComponentA extends Component { + public static template = hbs\` + {{! @glint-expect-error }} + Welcome to app v{{@version}}. + \`; + } + `; + + let componentB = stripIndent` + import Component, { hbs } from '@glint/environment-glimmerx/component'; + + export default class ComponentB extends Component { + public startupTime = new Date().toISOString(); + + public static template = hbs\` + {{! @glint-ignore: this looks like a typo but for some reason it isn't }} + The current time is {{this.startupTimee}}. + \`; + } + `; + + project.write('component-a.ts', componentA); + project.write('component-b.ts', componentB); + + let server = project.startLanguageServer(); + + expect(server.getDiagnostics(project.fileURI('component-a.ts'))).toEqual([]); + expect(server.getDiagnostics(project.fileURI('component-b.ts'))).toEqual([]); + + server.openFile(project.fileURI('component-a.ts'), componentA); + server.updateFile( + project.fileURI('component-a.ts'), + componentA.replace('{{! @glint-expect-error }}', '') + ); + + expect(server.getDiagnostics(project.fileURI('component-b.ts'))).toEqual([]); + expect(server.getDiagnostics(project.fileURI('component-a.ts'))).toMatchInlineSnapshot(` + Array [ + Object { + "message": "Property 'version' does not exist on type 'EmptyObject'.", + "range": Object { + "end": Object { + "character": 36, + "line": 5, + }, + "start": Object { + "character": 29, + "line": 5, + }, + }, + "severity": 1, + "source": "glint:ts(2339)", + "tags": Array [], + }, + ] + `); + + server.updateFile(project.fileURI('component-a.ts'), componentA); + + expect(server.getDiagnostics(project.fileURI('component-a.ts'))).toEqual([]); + expect(server.getDiagnostics(project.fileURI('component-b.ts'))).toEqual([]); + + server.updateFile(project.fileURI('component-a.ts'), componentA.replace('{{@version}}', '')); + + expect(server.getDiagnostics(project.fileURI('component-b.ts'))).toEqual([]); + expect(server.getDiagnostics(project.fileURI('component-a.ts'))).toMatchInlineSnapshot(` + Array [ + Object { + "message": "Unused '@glint-expect-error' directive.", + "range": Object { + "end": Object { + "character": 30, + "line": 4, + }, + "start": Object { + "character": 4, + "line": 4, + }, + }, + "severity": 1, + "source": "glint", + "tags": Array [], + }, + ] + `); + }); }); diff --git a/packages/core/src/cli/diagnostics.ts b/packages/core/src/cli/diagnostics.ts new file mode 100644 index 000000000..effb0168d --- /dev/null +++ b/packages/core/src/cli/diagnostics.ts @@ -0,0 +1,14 @@ +import type ts from 'typescript'; + +export function buildDiagnosticFormatter( + ts: typeof import('typescript') +): (diagnostic: ts.Diagnostic) => string { + const formatDiagnosticHost: ts.FormatDiagnosticsHost = { + getCanonicalFileName: (name) => name, + getCurrentDirectory: ts.sys.getCurrentDirectory, + getNewLine: () => ts.sys.newLine, + }; + + return (diagnostic) => + ts.formatDiagnosticsWithColorAndContext([diagnostic], formatDiagnosticHost); +} diff --git a/packages/core/src/cli/perform-check.ts b/packages/core/src/cli/perform-check.ts index bda03fead..15a979b00 100644 --- a/packages/core/src/cli/perform-check.ts +++ b/packages/core/src/cli/perform-check.ts @@ -1,6 +1,7 @@ import type ts from 'typescript'; import TransformManager from '../common/transform-manager'; import { GlintConfig } from '@glint/config'; +import { buildDiagnosticFormatter } from './diagnostics'; export function performCheck( ts: typeof import('typescript'), @@ -12,6 +13,7 @@ export function performCheck( let transformManager = new TransformManager(ts, glintConfig); let parsedConfig = loadTsconfig(ts, configPath, optionsToExtend); let compilerHost = createCompilerHost(ts, parsedConfig.options, transformManager); + let formatDiagnostic = buildDiagnosticFormatter(ts); let program = ts.createProgram({ rootNames: rootNames.length ? rootNames : parsedConfig.fileNames, options: parsedConfig.options, @@ -20,12 +22,13 @@ export function performCheck( program.emit(); - let diagnostics = collectDiagnostics(program, transformManager, parsedConfig.options); - for (let diagnostic of diagnostics) { - console.error(transformManager.formatDiagnostic(diagnostic)); + let baselineDiagnostics = collectDiagnostics(program, transformManager, parsedConfig.options); + let fullDiagnostics = transformManager.rewriteDiagnostics(baselineDiagnostics); + for (let diagnostic of fullDiagnostics) { + console.error(formatDiagnostic(diagnostic)); } - process.exit(diagnostics.length ? 1 : 0); + process.exit(fullDiagnostics.length ? 1 : 0); } function collectDiagnostics( diff --git a/packages/core/src/cli/perform-watch.ts b/packages/core/src/cli/perform-watch.ts index 3f03022d1..33c68c90e 100644 --- a/packages/core/src/cli/perform-watch.ts +++ b/packages/core/src/cli/perform-watch.ts @@ -1,5 +1,6 @@ import TransformManager from '../common/transform-manager'; import { GlintConfig } from '@glint/config'; +import { buildDiagnosticFormatter } from './diagnostics'; export function performWatch( ts: typeof import('typescript'), @@ -8,12 +9,13 @@ export function performWatch( optionsToExtend: import('typescript').CompilerOptions ): void { let transformManager = new TransformManager(ts, glintConfig); + let formatDiagnostic = buildDiagnosticFormatter(ts); let host = ts.createWatchCompilerHost( tsconfigPath ?? 'tsconfig.json', optionsToExtend, sysForWatchCompilerHost(ts, transformManager), ts.createSemanticDiagnosticsBuilderProgram, - (diagnostic) => console.error(transformManager.formatDiagnostic(diagnostic)) + (diagnostic) => console.error(formatDiagnostic(diagnostic)) ); patchWatchCompilerHost(host, transformManager); @@ -44,10 +46,16 @@ function patchWatchCompilerHost(host: WatchCompilerHost, transformManager: Trans } function patchProgram(program: Program, transformManager: TransformManager): void { - let { getSyntacticDiagnostics } = program; + let { getSyntacticDiagnostics, getSemanticDiagnostics } = program; + program.getSyntacticDiagnostics = function (sourceFile, cancelationToken) { let diagnostics = getSyntacticDiagnostics.call(program, sourceFile, cancelationToken); let transformDiagnostics = transformManager.getTransformDiagnostics(sourceFile?.fileName); return [...diagnostics, ...transformDiagnostics]; }; + + program.getSemanticDiagnostics = (sourceFile, cancellationToken) => { + let diagnostics = getSemanticDiagnostics.call(program, sourceFile, cancellationToken); + return transformManager.rewriteDiagnostics(diagnostics, sourceFile?.fileName); + }; } diff --git a/packages/core/src/common/transform-manager.ts b/packages/core/src/common/transform-manager.ts index 3608fd993..ef453fb59 100644 --- a/packages/core/src/common/transform-manager.ts +++ b/packages/core/src/common/transform-manager.ts @@ -1,4 +1,11 @@ -import { TransformedModule, rewriteModule, rewriteDiagnostic } from '@glint/transform'; +import { + TransformedModule, + rewriteModule, + rewriteDiagnostic, + Directive, + SourceFile, + Range, +} from '@glint/transform'; import type ts from 'typescript'; import { GlintConfig } from '@glint/config'; import { assert } from '@glint/transform/lib/util'; @@ -22,31 +29,46 @@ export default class TransformManager { public getTransformDiagnostics(fileName?: string): Array { if (fileName) { let transformedModule = this.getTransformInfo(fileName)?.transformedModule; - return transformedModule ? this.buildDiagnostics(transformedModule) : []; + return transformedModule ? this.buildTransformDiagnostics(transformedModule) : []; } return [...this.transformCache.values()].flatMap((transformInfo) => { if (transformInfo.transformedModule) { - return this.buildDiagnostics(transformInfo.transformedModule); + return this.buildTransformDiagnostics(transformInfo.transformedModule); } return []; }); } - public rewriteDiagnostic(diagnostic: ts.Diagnostic): ts.Diagnostic { - return rewriteDiagnostic( - this.ts, - diagnostic, - (fileName) => this.getTransformInfo(fileName)?.transformedModule - ); - } + public rewriteDiagnostics( + diagnostics: ReadonlyArray, + fileName?: string + ): ReadonlyArray { + let unusedExpectErrors = new Set(this.getExpectErrorDirectives(fileName)); + let allDiagnostics = []; + for (let diagnostic of diagnostics) { + let { rewrittenDiagnostic, appliedDirective } = this.rewriteDiagnostic(diagnostic); + if (rewrittenDiagnostic) { + allDiagnostics.push(rewrittenDiagnostic); + } - public formatDiagnostic(diagnostic: ts.Diagnostic): string { - return this.ts.formatDiagnosticsWithColorAndContext( - [this.rewriteDiagnostic(diagnostic)], - this.formatDiagnosticHost - ); + if (appliedDirective?.kind === 'expect-error') { + unusedExpectErrors.delete(appliedDirective); + } + } + + for (let directive of unusedExpectErrors) { + allDiagnostics.push( + this.buildDiagnostic( + directive.source, + `Unused '@glint-expect-error' directive.`, + directive.location + ) + ); + } + + return this.ts.sortAndDeduplicateDiagnostics(allDiagnostics); } public getTransformedRange( @@ -163,6 +185,48 @@ export default class TransformManager { } }; + private getExpectErrorDirectives(filename?: string): Array { + let transformInfos = filename + ? [this.getTransformInfo(filename)] + : [...this.transformCache.values()]; + + return transformInfos.flatMap((transformInfo) => { + if (!transformInfo.transformedModule) return []; + + return transformInfo.transformedModule.directives.filter( + (directive) => directive.kind === 'expect-error' + ); + }); + } + + private rewriteDiagnostic( + diagnostic: ts.Diagnostic + ): { rewrittenDiagnostic?: ts.Diagnostic; appliedDirective?: Directive } { + if (!diagnostic.file) return {}; + + let transformInfo = this.getTransformInfo(diagnostic.file?.fileName); + let rewrittenDiagnostic = rewriteDiagnostic( + this.ts, + diagnostic, + (fileName) => this.getTransformInfo(fileName)?.transformedModule + ); + + let appliedDirective = transformInfo.transformedModule?.directives.find( + (directive) => + directive.source.filename === rewrittenDiagnostic.file?.fileName && + directive.areaOfEffect.start <= rewrittenDiagnostic.start! && + directive.areaOfEffect.end > rewrittenDiagnostic.start! + ); + + // All current directives have the effect of squashing any diagnostics they apply + // to, so if we have an applicable directive, we don't return the diagnostic. + if (appliedDirective) { + return { appliedDirective }; + } else { + return { rewrittenDiagnostic }; + } + } + private findTransformInfoForOriginalFile(originalFileName: string): TransformInfo | null { let transformedFileName = isTemplate(originalFileName) ? this.documents.getCompanionDocumentPath(originalFileName) @@ -209,24 +273,26 @@ export default class TransformManager { return cacheEntry; } - private readonly formatDiagnosticHost: ts.FormatDiagnosticsHost = { - getCanonicalFileName: (name) => name, - getCurrentDirectory: this.ts.sys.getCurrentDirectory, - getNewLine: () => this.ts.sys.newLine, - }; + private buildTransformDiagnostics( + transformedModule: TransformedModule + ): Array { + return transformedModule.errors.map((error) => + this.buildDiagnostic(error.source, error.message, error.location) + ); + } - private buildDiagnostics(transformedModule: TransformedModule): Array { - return transformedModule.errors.map((error) => ({ + private buildDiagnostic( + source: SourceFile, + message: string, + location: Range + ): ts.DiagnosticWithLocation { + return { category: this.ts.DiagnosticCategory.Error, code: 0, - file: this.ts.createSourceFile( - error.source.filename, - error.source.contents, - this.ts.ScriptTarget.Latest - ), - start: error.location.start, - length: error.location.end - error.location.start, - messageText: error.message, - })); + file: this.ts.createSourceFile(source.filename, source.contents, this.ts.ScriptTarget.Latest), + start: location.start, + length: location.end - location.start, + messageText: message, + }; } } diff --git a/packages/core/src/language-server/glint-language-server.ts b/packages/core/src/language-server/glint-language-server.ts index 94cf974ae..0ad4ab308 100644 --- a/packages/core/src/language-server/glint-language-server.ts +++ b/packages/core/src/language-server/glint-language-server.ts @@ -88,27 +88,30 @@ export default class GlintLanguageServer { let sourcePath = this.findDiagnosticsSource(filePath); if (!sourcePath) return []; - return [ + let diagnostics = [ ...this.service.getSyntacticDiagnostics(sourcePath), ...this.transformManager.getTransformDiagnostics(sourcePath), ...this.service.getSemanticDiagnostics(sourcePath), ...this.service.getSuggestionDiagnostics(sourcePath), - ].flatMap((transformedDiagnostic) => { - let diagnostic = this.transformManager.rewriteDiagnostic(transformedDiagnostic); - let { start = 0, length = 0, messageText, file } = diagnostic; - if (!file || file.fileName !== filePath) return []; - - return { - severity: severityForDiagnostic(diagnostic), - message: this.ts.flattenDiagnosticMessageText(messageText, '\n'), - source: `glint${diagnostic.code ? `:ts(${diagnostic.code})` : ''}`, - tags: tagsForDiagnostic(diagnostic), - range: { - start: offsetToPosition(file.getText(), start), - end: offsetToPosition(file.getText(), start + length), - }, - }; - }); + ]; + + return this.transformManager + .rewriteDiagnostics(diagnostics, sourcePath) + .flatMap((diagnostic) => { + let { start = 0, length = 0, messageText, file } = diagnostic; + if (!file || file.fileName !== filePath) return []; + + return { + severity: severityForDiagnostic(diagnostic), + message: this.ts.flattenDiagnosticMessageText(messageText, '\n'), + source: `glint${diagnostic.code ? `:ts(${diagnostic.code})` : ''}`, + tags: tagsForDiagnostic(diagnostic), + range: { + start: offsetToPosition(file.getText(), start), + end: offsetToPosition(file.getText(), start + length), + }, + }; + }); } public findSymbols(query: string): Array { diff --git a/packages/transform/__tests__/debug.test.ts b/packages/transform/__tests__/debug.test.ts index 68cd493a0..4fd3193ca 100644 --- a/packages/transform/__tests__/debug.test.ts +++ b/packages/transform/__tests__/debug.test.ts @@ -21,6 +21,9 @@ describe('Debug utilities', () => { static template = hbs\`

Hello, {{@foo}}! + + {{! @glint-expect-error: no @bar arg }} + {{@bar}}

\`; } @@ -79,16 +82,16 @@ describe('Debug utilities', () => { | | Mapping: Template - | hbs(0:62): hbs\`\\\\n

\\\\n Hello, {{@foo}}!\\\\n

\\\\n \` - | ts(0:378): (() => {\\\\n hbs;\\\\n let χ!: typeof import(\\"@glint/environment-glimmerx/-private/dsl\\");\\\\n return χ.template(function(𝚪: import(\\"@glint/environment-glimmerx/-private/dsl\\").ResolveContext) {\\\\n {\\\\n const 𝛄 = χ.emitElement(\\"p\\");\\\\n χ.applySplattributes(𝚪.element, 𝛄.element);\\\\n χ.emitValue(χ.resolveOrReturn(𝚪.args.foo)({}));\\\\n }\\\\n 𝚪;\\\\n });\\\\n})() + | hbs(0:124): hbs\`\\\\n

\\\\n Hello, {{@foo}}!\\\\n\\\\n {{! @glint-expect-error: no @bar arg }}\\\\n {{@bar}}\\\\n

\\\\n \` + | ts(0:433): (() => {\\\\n hbs;\\\\n let χ!: typeof import(\\"@glint/environment-glimmerx/-private/dsl\\");\\\\n return χ.template(function(𝚪: import(\\"@glint/environment-glimmerx/-private/dsl\\").ResolveContext) {\\\\n {\\\\n const 𝛄 = χ.emitElement(\\"p\\");\\\\n χ.applySplattributes(𝚪.element, 𝛄.element);\\\\n χ.emitValue(χ.resolveOrReturn(𝚪.args.foo)({}));\\\\n χ.emitValue(χ.resolveOrReturn(𝚪.args.bar)({}));\\\\n }\\\\n 𝚪;\\\\n });\\\\n})() | | | Mapping: Identifier | | hbs(0:0): | | ts(184:199): HelperComponent | | | | Mapping: ElementNode - | | hbs(9:58):

\\\\n Hello, {{@foo}}!\\\\n

- | | ts(204:360): {\\\\n const 𝛄 = χ.emitElement(\\"p\\");\\\\n χ.applySplattributes(𝚪.element, 𝛄.element);\\\\n χ.emitValue(χ.resolveOrReturn(𝚪.args.foo)({}));\\\\n } + | | hbs(9:120):

\\\\n Hello, {{@foo}}!\\\\n\\\\n {{! @glint-expect-error: no @bar arg }}\\\\n {{@bar}}\\\\n

+ | | ts(204:415): {\\\\n const 𝛄 = χ.emitElement(\\"p\\");\\\\n χ.applySplattributes(𝚪.element, 𝛄.element);\\\\n χ.emitValue(χ.resolveOrReturn(𝚪.args.foo)({}));\\\\n χ.emitValue(χ.resolveOrReturn(𝚪.args.bar)({}));\\\\n } | | | | | Mapping: AttrNode | | | hbs(12:25): ...attributes @@ -108,6 +111,24 @@ describe('Debug utilities', () => { | | | | | | | | | | | | + | | | Mapping: MustacheCommentStatement + | | | hbs(57:96): {{! @glint-expect-error: no @bar arg }} + | | | ts(354:354): + | | | + | | | Mapping: MustacheStatement + | | | hbs(103:111): {{@bar}} + | | | ts(354:407): χ.emitValue(χ.resolveOrReturn(𝚪.args.bar)({})) + | | | + | | | | Mapping: PathExpression + | | | | hbs(105:109): @bar + | | | | ts(390:401): 𝚪.args.bar + | | | | + | | | | | Mapping: Identifier + | | | | | hbs(106:109): bar + | | | | | ts(398:401): bar + | | | | | + | | | | + | | | | | |" `); diff --git a/packages/transform/__tests__/template-to-typescript.test.ts b/packages/transform/__tests__/template-to-typescript.test.ts index e46fd312a..15837b1f9 100644 --- a/packages/transform/__tests__/template-to-typescript.test.ts +++ b/packages/transform/__tests__/template-to-typescript.test.ts @@ -70,6 +70,111 @@ describe('rewriteTemplate', () => { }); }); + describe('directives', () => { + test('in a top-level mustache', () => { + let template = stripIndent` + {{! @glint-ignore: this is fine }} + + {{hello}} + + `; + + let { result, errors } = templateToTypescript(template, { typesPath: '@glint/template' }); + + expect(errors).toEqual([]); + expect(result?.directives).toEqual([ + { + kind: 'ignore', + location: { + start: template.indexOf('{{!'), + end: template.indexOf('fine }}') + 'fine }}'.length, + }, + areaOfEffect: { + start: template.indexOf('') + '|bar|>'.length + 1, + }, + }, + ]); + }); + + test('in an element bobdy', () => { + let template = stripIndent` + + {{hello}} + + `; + + let { result, errors } = templateToTypescript(template, { typesPath: '@glint/template' }); + + expect(errors).toEqual([]); + expect(result?.directives).toEqual([ + { + kind: 'ignore', + location: { + start: template.indexOf('{{!'), + end: template.indexOf('fine }}') + 'fine }}'.length, + }, + areaOfEffect: { + start: template.indexOf(' @arg='), + end: template.indexOf('"hi"') + '"hi"'.length + 1, + }, + }, + ]); + }); + + test('expect-error', () => { + let template = stripIndent` + {{! @glint-expect-error: this is fine }} + + {{hello}} + + `; + + let { result, errors } = templateToTypescript(template, { typesPath: '@glint/template' }); + + expect(errors).toEqual([]); + expect(result?.directives).toEqual([ + { + kind: 'expect-error', + location: { + start: template.indexOf('{{!'), + end: template.indexOf('fine }}') + 'fine }}'.length, + }, + areaOfEffect: { + start: template.indexOf('') + '|bar|>'.length + 1, + }, + }, + ]); + }); + + test('unknown type', () => { + let template = stripIndent` + {{! @glint-check }} + + {{hello}} + + `; + + let { result, errors } = templateToTypescript(template, { typesPath: '@glint/template' }); + + expect(result?.directives).toEqual([]); + expect(errors).toEqual([ + { + message: 'Unknown directive @glint-check', + location: { + start: template.indexOf('{{!'), + end: template.indexOf('}}') + '}}'.length, + }, + }, + ]); + }); + }); + describe('primitives', () => { describe('{{if}}', () => { test('without an alternate', () => { diff --git a/packages/transform/src/index.ts b/packages/transform/src/index.ts index 43eb11086..a3fe2fd0b 100644 --- a/packages/transform/src/index.ts +++ b/packages/transform/src/index.ts @@ -6,12 +6,14 @@ import TransformedModule, { CorrelatedSpan, TransformError, SourceFile, + Directive, + Range, } from './transformed-module'; import { CorrelatedSpansResult, PartialCorrelatedSpan } from './inlining'; import { calculateTaggedTemplateSpans } from './inlining/tagged-strings'; import { calculateCompanionTemplateSpans } from './inlining/companion-file'; -export { TransformedModule }; +export { TransformedModule, Directive, Range, SourceFile }; /** * Given a TypeScript diagnostic object from a module that was rewritten @@ -108,7 +110,7 @@ export function rewriteModule( return null; } - let { errors, partialSpans } = calculateCorrelatedSpans( + let { errors, directives, partialSpans } = calculateCorrelatedSpans( input.script, input.template, scriptAST, @@ -122,7 +124,7 @@ export function rewriteModule( let sparseSpans = completeCorrelatedSpans(partialSpans); let { contents, correlatedSpans } = calculateTransformedSource(input.script, sparseSpans); - return new TransformedModule(contents, errors, correlatedSpans); + return new TransformedModule(contents, errors, directives, correlatedSpans); } /** @@ -138,6 +140,7 @@ function calculateCorrelatedSpans( ast: t.File | t.Program, environment: GlintEnvironment ): CorrelatedSpansResult { + let directives: Array = []; let errors: Array = []; let partialSpans: Array = []; let defaultExportSeen = false; @@ -146,6 +149,7 @@ function calculateCorrelatedSpans( TaggedTemplateExpression(path) { let result = calculateTaggedTemplateSpans(path, script, environment); + directives.push(...result.directives); errors.push(...result.errors); partialSpans.push(...result.partialSpans); }, @@ -161,6 +165,7 @@ function calculateCorrelatedSpans( let result = calculateCompanionTemplateSpans(declaration, script, template, environment); defaultExportSeen = true; + directives.push(...result.directives); errors.push(...result.errors); partialSpans.push(...result.partialSpans); } @@ -175,7 +180,7 @@ function calculateCorrelatedSpans( }); } - return { errors, partialSpans }; + return { errors, directives, partialSpans }; } /** diff --git a/packages/transform/src/inlining/companion-file.ts b/packages/transform/src/inlining/companion-file.ts index eef1bbc30..c80e6b4e7 100644 --- a/packages/transform/src/inlining/companion-file.ts +++ b/packages/transform/src/inlining/companion-file.ts @@ -3,7 +3,7 @@ import { GlintEnvironment } from '@glint/config'; import { CorrelatedSpansResult, getContainingTypeInfo, PartialCorrelatedSpan } from '.'; import MappingTree, { ParseError } from '../mapping-tree'; import { templateToTypescript } from '../template-to-typescript'; -import { SourceFile, TransformError } from '../transformed-module'; +import { Directive, SourceFile, TransformError } from '../transformed-module'; import { assert } from '../util'; const STANDALONE_TEMPLATE_FIELD = `'~template'`; @@ -15,6 +15,7 @@ export function calculateCompanionTemplateSpans( environment: GlintEnvironment ): CorrelatedSpansResult { let errors: Array = []; + let directives: Array = []; let partialSpans: Array = []; let typesPath = environment.getTypesForStandaloneTemplate(); if (!typesPath) { @@ -24,7 +25,7 @@ export function calculateCompanionTemplateSpans( message: `Glint environment ${environment.name} does not support standalone template files`, }); - return { errors, partialSpans }; + return { errors, directives, partialSpans }; } let targetPath = findCompanionTemplateTarget(path); @@ -38,7 +39,7 @@ export function calculateCompanionTemplateSpans( }, }); - return { errors, partialSpans }; + return { errors, directives, partialSpans }; } let target = targetPath.node; @@ -70,6 +71,15 @@ export function calculateCompanionTemplateSpans( ); if (transformedTemplate.result) { + directives.push( + ...transformedTemplate.result.directives.map(({ kind, location, areaOfEffect }) => ({ + kind, + location, + areaOfEffect, + source: template, + })) + ); + partialSpans.push( { originalFile: template, @@ -115,7 +125,7 @@ export function calculateCompanionTemplateSpans( // TODO: handle opaque expression like an imported identifier or `templateOnlyComponent()` } - return { errors, partialSpans }; + return { errors, directives, partialSpans }; } type CompanionTemplateTarget = NodePath | NodePath | null; diff --git a/packages/transform/src/inlining/index.ts b/packages/transform/src/inlining/index.ts index c1ec763f5..0671e4087 100644 --- a/packages/transform/src/inlining/index.ts +++ b/packages/transform/src/inlining/index.ts @@ -1,11 +1,12 @@ import { NodePath, types as t } from '@babel/core'; import generate from '@babel/generator'; -import { CorrelatedSpan, TransformError } from '../transformed-module'; +import { CorrelatedSpan, Directive, TransformError } from '../transformed-module'; export type PartialCorrelatedSpan = Omit; export type CorrelatedSpansResult = { errors: Array; + directives: Array; partialSpans: Array; }; diff --git a/packages/transform/src/inlining/tagged-strings.ts b/packages/transform/src/inlining/tagged-strings.ts index f73a8b078..c4c80675f 100644 --- a/packages/transform/src/inlining/tagged-strings.ts +++ b/packages/transform/src/inlining/tagged-strings.ts @@ -2,7 +2,7 @@ import { NodePath, types as t } from '@babel/core'; import { GlintEnvironment } from '@glint/config'; import { CorrelatedSpansResult, getContainingTypeInfo, PartialCorrelatedSpan } from '.'; import { templateToTypescript } from '../template-to-typescript'; -import { SourceFile, TransformError } from '../transformed-module'; +import { Directive, SourceFile, TransformError, Range } from '../transformed-module'; import { assert } from '../util'; export function calculateTaggedTemplateSpans( @@ -10,12 +10,13 @@ export function calculateTaggedTemplateSpans( script: SourceFile, environment: GlintEnvironment ): CorrelatedSpansResult { + let directives: Array = []; let errors: Array = []; let partialSpans: Array = []; let tag = path.get('tag'); if (!tag.isIdentifier()) { - return { errors, partialSpans }; + return { errors, directives, partialSpans }; } let typesPath = determineTypesPathForTag(tag, environment); @@ -59,10 +60,7 @@ export function calculateTaggedTemplateSpans( errors.push({ source: script, message, - location: { - start: path.node.start + location.start, - end: path.node.start + location.end, - }, + location: addOffset(location, path.node.start), }); } else { assert(path.node.tag.start, 'Missing location info'); @@ -80,20 +78,34 @@ export function calculateTaggedTemplateSpans( } if (transformedTemplate.result) { - let { code, mapping } = transformedTemplate.result; + for (let { kind, location, areaOfEffect } of transformedTemplate.result.directives) { + directives.push({ + kind: kind, + source: script, + location: addOffset(location, path.node.start), + areaOfEffect: addOffset(areaOfEffect, path.node.start), + }); + } partialSpans.push({ originalFile: script, originalStart: path.node.start, originalLength: path.node.end - path.node.start, insertionPoint: path.node.start, - transformedSource: code, - mapping: mapping, + transformedSource: transformedTemplate.result.code, + mapping: transformedTemplate.result.mapping, }); } } - return { errors, partialSpans }; + return { errors, directives, partialSpans }; +} + +function addOffset(location: Range, offset: number): Range { + return { + start: location.start + offset, + end: location.end + offset, + }; } function determineTypesPathForTag( diff --git a/packages/transform/src/map-template-contents.ts b/packages/transform/src/map-template-contents.ts index c621d1c31..ddfb41357 100644 --- a/packages/transform/src/map-template-contents.ts +++ b/packages/transform/src/map-template-contents.ts @@ -1,6 +1,6 @@ import { AST, preprocess } from '@glimmer/syntax'; import MappingTree from './mapping-tree'; -import { Range } from './transformed-module'; +import { Directive, DirectiveKind, Range } from './transformed-module'; import { assert } from './util'; /** @@ -24,6 +24,25 @@ export type Mapper = { */ rangeForNode: (node: AST.Node) => Range; + /** + * Given a 0-based line number, returns the corresponding start and + * end offsets for that line. + */ + rangeForLine: (line: number) => Range; + + record: { + /** + * Captures the existence of a directive specified by the given source + * node and affecting the given range of text. + */ + directive: (type: DirectiveKind, location: Range, areaOfEffect: Range) => void; + + /** + * Records an error at the given location. + */ + error: (message: string, location: Range) => void; + }; + emit: { /** Emit a newline in the transformed source */ newline(): void; @@ -43,6 +62,13 @@ export type Mapper = { */ synthetic(value: string): void; + /** + * Essentially the inverse of `emit.synthetic`, this notes the + * presence of a template AST node at a given location while not + * emitting anything in the resulting TS translation. + */ + nothing(node: AST.Node): void; + /** * Append the given value to the transformed source, mapping * that span back to the given offset in the original source. @@ -57,6 +83,8 @@ export type Mapper = { }; }; +type LocalDirective = Omit; + /** The result of rewriting a template */ export type RewriteResult = { /** @@ -72,6 +100,7 @@ export type RewriteResult = { */ result?: { code: string; + directives: Array; mapping: MappingTree; }; }; @@ -111,6 +140,10 @@ export function mapTemplateContents( } let rangeForNode = buildRangeForNode(lineOffsets); + let rangeForLine = (line: number): Range => ({ + start: lineOffsets[line], + end: lineOffsets[line + 1] ?? template.length, + }); let segmentsStack: string[][] = [[]]; let mappingsStack: MappingTree[][] = [[]]; @@ -118,6 +151,7 @@ export function mapTemplateContents( let offset = 0; let needsIndent = false; let errors: Array<{ message: string; location: Range }> = []; + let directives: Array = []; // Associates all content emitted during the given callback with the // given range in the template source and corresponding AST node. @@ -157,6 +191,15 @@ export function mapTemplateContents( } }; + let record = { + error(message: string, location: Range) { + errors.push({ message, location }); + }, + directive(kind: DirectiveKind, location: Range, areaOfEffect: Range) { + directives.push({ kind, location, areaOfEffect }); + }, + }; + let emit = { indent() { indent += ' '; @@ -184,6 +227,9 @@ export function mapTemplateContents( emit.identifier(value, 0, 0); } }, + nothing(node: AST.Node) { + captureMapping(rangeForNode(node), node, true, () => {}); + }, identifier(value: string, hbsOffset: number, hbsLength = value.length) { // If there's a pending indent, flush that so it's not included in // the range mapping for the identifier we're about to emit @@ -200,7 +246,7 @@ export function mapTemplateContents( }, }; - callback(ast, { emit, rangeForNode }); + callback(ast, { emit, record, rangeForLine, rangeForNode }); assert(segmentsStack.length === 1); @@ -212,7 +258,7 @@ export function mapTemplateContents( ast ); - return { errors, result: { code, mapping } }; + return { errors, result: { code, directives, mapping } }; } const LEADING_WHITESPACE = /^\s+/; diff --git a/packages/transform/src/template-to-typescript.ts b/packages/transform/src/template-to-typescript.ts index e7d41922d..2caa0ce72 100644 --- a/packages/transform/src/template-to-typescript.ts +++ b/packages/transform/src/template-to-typescript.ts @@ -33,7 +33,7 @@ export function templateToTypescript( preamble = [], }: TemplateToTypescriptOptions ): RewriteResult { - return mapTemplateContents(template, (ast, { emit, rangeForNode }) => { + return mapTemplateContents(template, (ast, { emit, record, rangeForLine, rangeForNode }) => { let scope = new ScopeStack(identifiersInScope); emit.text('(() => {'); @@ -81,11 +81,13 @@ export function templateToTypescript( throw new Error(`Internal error: unexpected top-level ${node.type}`); case 'TextNode': - case 'CommentStatement': - case 'MustacheCommentStatement': // Nothing to be done return; + case 'CommentStatement': + case 'MustacheCommentStatement': + return emitComment(node); + case 'MustacheStatement': return emitTopLevelMustacheStatement(node); @@ -100,6 +102,24 @@ export function templateToTypescript( } } + function emitComment(node: AST.MustacheCommentStatement | AST.CommentStatement): void { + emit.nothing(node); + + let text = node.value.trim(); + let match = /^@glint-([a-z-]+)/i.exec(text); + if (!match) return; + + let kind = match[1]; + let location = rangeForNode(node); + if (kind === 'ignore') { + record.directive(kind, location, rangeForLine(node.loc.end.line + 1)); + } else if (kind === 'expect-error') { + record.directive(kind, location, rangeForLine(node.loc.end.line + 1)); + } else { + record.error(`Unknown directive @glint-${kind}`, location); + } + } + // Captures the context in which a given invocation (i.e. a mustache or // sexpr) is being performed. Certain keywords like `yield` are only // valid in certain positions, and whether a param-less mustache implicitly @@ -332,6 +352,10 @@ export function templateToTypescript( emit.forNode(node, () => { let { start, path, kind } = tagNameToPathContents(node); + for (let comment of node.comments) { + emitComment(comment); + } + emit.text('{'); emit.newline(); emit.indent(); @@ -382,6 +406,11 @@ export function templateToTypescript( let blocks = determineBlockChildren(node); if (blocks.type === 'named') { for (let child of blocks.children) { + if (child.type === 'CommentStatement' || child.type === 'MustacheCommentStatement') { + emitComment(child); + continue; + } + let childStart = rangeForNode(child).start; let nameStart = template.indexOf(child.tag, childStart) + ':'.length; let blockParamsStart = template.indexOf('|', childStart); @@ -430,8 +459,9 @@ export function templateToTypescript( return node.type === 'ElementNode' && node.tag.startsWith(':'); } + type NamedBlockChild = AST.ElementNode | AST.CommentStatement | AST.MustacheCommentStatement; type BlockChildren = - | { type: 'named'; children: AST.ElementNode[] } + | { type: 'named'; children: NamedBlockChild[] } | { type: 'default'; children: AST.TopLevelStatement[] }; function determineBlockChildren(node: AST.ElementNode): BlockChildren { @@ -456,8 +486,11 @@ export function templateToTypescript( return { type: 'named', children: node.children.filter( - // Filter out blank `TextNode`s, comments, etc - (child): child is AST.ElementNode => child.type === 'ElementNode' + // Filter out ignorable content between named blocks + (child): child is NamedBlockChild => + child.type === 'ElementNode' || + child.type === 'CommentStatement' || + child.type === 'MustacheCommentStatement' ), }; } else { @@ -481,6 +514,10 @@ export function templateToTypescript( function emitPlainElement(node: AST.ElementNode): void { emit.forNode(node, () => { + for (let comment of node.comments) { + emitComment(comment); + } + emit.text('{'); emit.newline(); emit.indent(); diff --git a/packages/transform/src/transformed-module.ts b/packages/transform/src/transformed-module.ts index 13aac0116..a42ac6830 100644 --- a/packages/transform/src/transformed-module.ts +++ b/packages/transform/src/transformed-module.ts @@ -23,6 +23,14 @@ export type CorrelatedSpan = { mapping?: MappingTree; }; +export type DirectiveKind = 'ignore' | 'expect-error'; +export type Directive = { + kind: DirectiveKind; + source: SourceFile; + location: Range; + areaOfEffect: Range; +}; + export type TransformError = { message: string; location: Range; @@ -48,6 +56,7 @@ export default class TransformedModule { public constructor( public readonly transformedContents: string, public readonly errors: ReadonlyArray, + public readonly directives: ReadonlyArray, private readonly correlatedSpans: Array ) {} diff --git a/test-packages/ts-ember-app/app/components/bar/index.hbs b/test-packages/ts-ember-app/app/components/bar/index.hbs index 4be59c84f..8947cbaab 100644 --- a/test-packages/ts-ember-app/app/components/bar/index.hbs +++ b/test-packages/ts-ember-app/app/components/bar/index.hbs @@ -1,3 +1,4 @@ {{this.name}}-{{@grault}} -{{@waldo}} \ No newline at end of file +{{! @glint-expect-error: invalid arg }} +{{@waldo}} diff --git a/test-packages/ts-ember-app/app/components/foo.hbs b/test-packages/ts-ember-app/app/components/foo.hbs index 7946b50fe..8974386b2 100644 --- a/test-packages/ts-ember-app/app/components/foo.hbs +++ b/test-packages/ts-ember-app/app/components/foo.hbs @@ -1,32 +1,59 @@ {{this.name}} +{{! @glint-expect-error: invalid property }} {{this.corge}} - + - + +{{! @glint-expect-error: missing required arg }} +{{! @glint-expect-error: missing required arg }} - + - + - + + + + {{! @glint-expect-error: bad named block }} + <:foo>hello + @@ -34,12 +61,14 @@ {{! should work, even though MaybeComponent might not be present }} - {{! should fail, since a string is not invokable }} + {{! @glint-expect-error: strings aren't invokable }} {{repeat "foo" 3}} +{{! @glint-expect-error: missing second arg }} {{repeat "foo"}} +{{! @glint-expect-error: incorrect type for second arg }} {{repeat "foo" "bar"}} diff --git a/test-packages/ts-ember-app/app/pods/components/baz/template.hbs b/test-packages/ts-ember-app/app/pods/components/baz/template.hbs index fc2163d36..f44fe6aa9 100644 --- a/test-packages/ts-ember-app/app/pods/components/baz/template.hbs +++ b/test-packages/ts-ember-app/app/pods/components/baz/template.hbs @@ -1,5 +1,7 @@ {{this.name}} +{{! @glint-expect-error: invalid property }} {{this.plugh}} +{{! @glint-expect-error: invalid arg }} {{@thud}} diff --git a/test-packages/ts-ember-app/package.json b/test-packages/ts-ember-app/package.json index c96556e88..87e6ded4d 100644 --- a/test-packages/ts-ember-app/package.json +++ b/test-packages/ts-ember-app/package.json @@ -16,7 +16,9 @@ "lint:hbs": "ember-template-lint .", "lint:js": "eslint .", "start": "ember serve", - "test": "ember test" + "test": "npm-run-all typecheck test:browsers", + "typecheck": "glint", + "test:browsers": "ember test" }, "devDependencies": { "@ember/optional-features": "^2.0.0", @@ -41,6 +43,7 @@ "ember-fetch": "^8.0.2", "ember-load-initializers": "^2.1.1", "ember-modifier": "^2.1.1", + "ember-named-blocks-polyfill": "^0.2.4", "ember-qunit": "^4.6.0", "ember-resolver": "^8.0.2", "ember-source": "~3.22.0", diff --git a/test-packages/ts-ember-app/tests/integration/components/foo-test.ts b/test-packages/ts-ember-app/tests/integration/components/foo-test.ts index 649b2e579..584043ba4 100644 --- a/test-packages/ts-ember-app/tests/integration/components/foo-test.ts +++ b/test-packages/ts-ember-app/tests/integration/components/foo-test.ts @@ -46,6 +46,9 @@ module('Integration | Component | foo', function (hooks) { 'false', 'req', 'false', + 'hi', + 'defaultValue', + 'hi', 'req', '1', 'defaultValue', diff --git a/yarn.lock b/yarn.lock index a49250934..ba5b3a617 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6719,6 +6719,14 @@ ember-modifier@^2.1.1: ember-destroyable-polyfill "^2.0.2" ember-modifier-manager-polyfill "^1.2.0" +ember-named-blocks-polyfill@^0.2.4: + version "0.2.4" + resolved "https://registry.yarnpkg.com/ember-named-blocks-polyfill/-/ember-named-blocks-polyfill-0.2.4.tgz#f5f30711ee89244927b55aae7fa9630edaadc974" + integrity sha512-PsohC7ejjS7V++6i/JSy0pl1hXLV3IS3Qs+O7SrjIPYcg1UEmUwqgPiDmXqNgy0p2dc5TK5bIJTtX8wofCI63Q== + dependencies: + ember-cli-babel "^7.19.0" + ember-cli-version-checker "^5.1.1" + ember-qunit@^4.6.0: version "4.6.0" resolved "https://registry.yarnpkg.com/ember-qunit/-/ember-qunit-4.6.0.tgz#ad79fd3ff00073a8779400cc5a4b44829517590f"