Skip to content

Commit

Permalink
fix(compiler-cli): Show template syntax errors in local compilation m…
Browse files Browse the repository at this point in the history
…odified

Currently the template syntax errors are extracted in the template type check phase. But in local compilation mode we skip the type check phase. As a result template syntax errors are not displayed. With this change we show the template syntax diagnostics in local mode.
  • Loading branch information
pmvald committed May 26, 2024
1 parent 1360110 commit 382d8d9
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/scope",
"//packages/compiler-cli/src/ngtsc/transform",
"//packages/compiler-cli/src/ngtsc/typecheck",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
"//packages/compiler-cli/src/ngtsc/typecheck/template_semantics/api",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ import {
HandlerPrecedence,
ResolveResult,
} from '../../../transform';
import {TypeCheckableDirectiveMeta, TypeCheckContext} from '../../../typecheck/api';
import {TemplateId, TypeCheckableDirectiveMeta, TypeCheckContext} from '../../../typecheck/api';
import {ExtendedTemplateChecker} from '../../../typecheck/extended/api';
import {TemplateSemanticsChecker} from '../../../typecheck/template_semantics/api/api';
import {getSourceFile} from '../../../util/src/typescript';
Expand Down Expand Up @@ -176,6 +176,7 @@ import {
collectAnimationNames,
validateAndFlattenComponentImports,
} from './util';
import {getTemplateDiagnostics} from '../../../typecheck';

const EMPTY_ARRAY: any[] = [];

Expand Down Expand Up @@ -624,6 +625,23 @@ export class ComponentDecoratorHandler
},
this.compilationMode,
);
if (
this.compilationMode === CompilationMode.LOCAL &&
template.errors &&
template.errors.length > 0
) {
if (diagnostics === undefined) {
diagnostics = [];
}

diagnostics.push(
...getTemplateDiagnostics(
template.errors,
'' as TemplateId, // Template ID is not required for a single diagnostic.
template.sourceMapping,
),
);
}
}
const templateResource = template.declaration.isInline
? {path: null, expression: component.get('template')!}
Expand Down Expand Up @@ -1578,10 +1596,6 @@ export class ComponentDecoratorHandler
resolution: Readonly<Partial<ComponentResolutionData>>,
pool: ConstantPool,
): CompileResult[] {
if (analysis.template.errors !== null && analysis.template.errors.length > 0) {
return [];
}

// In the local compilation mode we can only rely on the information available
// within the `@Component.deferredImports` array, because in this mode compiler
// doesn't have information on which dependencies belong to which defer blocks.
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
*/

export {FileTypeCheckingData, TemplateTypeCheckerImpl} from './src/checker';
export {TypeCheckContextImpl} from './src/context';
export {TypeCheckContextImpl, getTemplateDiagnostics} from './src/context';
export {TypeCheckShimGenerator} from './src/shim';
export {typeCheckFilePath} from './src/type_check_file';
54 changes: 26 additions & 28 deletions packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
const templateDiagnostics: TemplateDiagnostic[] = [];

if (parseErrors !== null) {
templateDiagnostics.push(
...this.getTemplateDiagnostics(parseErrors, templateId, sourceMapping),
);
templateDiagnostics.push(...getTemplateDiagnostics(parseErrors, templateId, sourceMapping));
}

const boundTarget = binder.bind({template});
Expand Down Expand Up @@ -559,33 +557,33 @@ export class TypeCheckContextImpl implements TypeCheckContext {

return this.fileMap.get(sfPath)!;
}
}

private getTemplateDiagnostics(
parseErrors: ParseError[],
templateId: TemplateId,
sourceMapping: TemplateSourceMapping,
): TemplateDiagnostic[] {
return parseErrors.map((error) => {
const span = error.span;

if (span.start.offset === span.end.offset) {
// Template errors can contain zero-length spans, if the error occurs at a single point.
// However, TypeScript does not handle displaying a zero-length diagnostic very well, so
// increase the ending offset by 1 for such errors, to ensure the position is shown in the
// diagnostic.
span.end.offset++;
}
export function getTemplateDiagnostics(
parseErrors: ParseError[],
templateId: TemplateId,
sourceMapping: TemplateSourceMapping,
): TemplateDiagnostic[] {
return parseErrors.map((error) => {
const span = error.span;

if (span.start.offset === span.end.offset) {
// Template errors can contain zero-length spans, if the error occurs at a single point.
// However, TypeScript does not handle displaying a zero-length diagnostic very well, so
// increase the ending offset by 1 for such errors, to ensure the position is shown in the
// diagnostic.
span.end.offset++;
}

return makeTemplateDiagnostic(
templateId,
sourceMapping,
span,
ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.TEMPLATE_PARSE_ERROR),
error.msg,
);
});
}
return makeTemplateDiagnostic(
templateId,
sourceMapping,
span,
ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.TEMPLATE_PARSE_ERROR),
error.msg,
);
});
}

/**
Expand Down
62 changes: 62 additions & 0 deletions packages/compiler-cli/test/ngtsc/local_compilation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2623,5 +2623,67 @@ runInEachFileSystem(() => {
);
});
});

describe('template diagnostics', () => {
it('should show correct error message for syntatic template errors - case of inline template', () => {
env.write(
'test.ts',
`
import {Component} from '@angular/core';
@Component({
template: '<span Hello! </span>',
})
export class Main {
}
`,
);

const errors = env.driveDiagnostics();

expect(errors.length).toBeGreaterThanOrEqual(1);

const {code, messageText} = errors[0];

expect(code).toBe(ngErrorCode(ErrorCode.TEMPLATE_PARSE_ERROR));

const text = ts.flattenDiagnosticMessageText(messageText, '\n');

expect(text).toContain('Opening tag "span" not terminated');
});

it('should show correct error message for syntatic template errors - case of external template', () => {
env.write(
'test.ts',
`
import {Component} from '@angular/core';
@Component({
templateUrl: 'test.ng.html',
})
export class Main {
}
`,
);
env.write(
'test.ng.html',
`
<span Hello! </span>
`,
);

const errors = env.driveDiagnostics();

expect(errors.length).toBeGreaterThanOrEqual(1);

const {code, messageText} = errors[0];

expect(code).toBe(ngErrorCode(ErrorCode.TEMPLATE_PARSE_ERROR));

const text = ts.flattenDiagnosticMessageText(messageText, '\n');

expect(text).toContain('Opening tag "span" not terminated');
});
});
});
});

0 comments on commit 382d8d9

Please sign in to comment.