Skip to content

Commit

Permalink
feat(language-service): Add optional quick fix data to template diagn…
Browse files Browse the repository at this point in the history
…ostics

language service implementation for angular#44941. A follow-up PR will be
needed in the language server code to support this information.
  • Loading branch information
atscott committed Apr 14, 2022
1 parent 000363e commit 5e29ed1
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 29 deletions.
12 changes: 11 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/api/api.ts
Expand Up @@ -8,8 +8,8 @@

import {AbsoluteSourceSpan, BoundTarget, DirectiveMeta, ParseSourceSpan, SchemaMetadata} from '@angular/compiler';
import ts from 'typescript';
import {ErrorCode} from '../../diagnostics';

import {ErrorCode} from '../../diagnostics';
import {AbsoluteFsPath} from '../../file_system';
import {Reference} from '../../imports';
import {ClassPropertyMapping, DirectiveTypeCheckMeta} from '../../metadata';
Expand Down Expand Up @@ -43,6 +43,16 @@ export interface TemplateDiagnostic extends ts.Diagnostic {
* The template id of the component that resulted in this diagnostic.
*/
templateId: TemplateId;

quickFix?: TemplateQuickFixData;
}

export interface TemplateQuickFixData {
title: string;

replacementSpan: ts.TextSpan;

replacementText: string;
}

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts
Expand Up @@ -12,7 +12,7 @@ import ts from 'typescript';
import {AbsoluteFsPath} from '../../../../src/ngtsc/file_system';
import {ErrorCode} from '../../diagnostics';

import {FullTemplateMapping, NgTemplateDiagnostic, TypeCheckableDirectiveMeta} from './api';
import {FullTemplateMapping, NgTemplateDiagnostic, TemplateQuickFixData, TypeCheckableDirectiveMeta} from './api';
import {GlobalCompletion} from './completion';
import {DirectiveInScope, PipeInScope} from './scope';
import {ElementSymbol, Symbol, TcbLocation, TemplateSymbol} from './symbols';
Expand Down Expand Up @@ -180,7 +180,8 @@ export interface TemplateTypeChecker {
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): NgTemplateDiagnostic<T>;
}[],
quickFix?: TemplateQuickFixData): NgTemplateDiagnostic<T>;
}

/**
Expand Down
Expand Up @@ -9,7 +9,7 @@
import {ParseSourceSpan} from '@angular/compiler';
import ts from 'typescript';

import {ExternalTemplateSourceMapping, TemplateDiagnostic, TemplateId, TemplateSourceMapping} from '../../api';
import {ExternalTemplateSourceMapping, TemplateDiagnostic, TemplateId, TemplateQuickFixData, TemplateSourceMapping} from '../../api';

/**
* Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
Expand All @@ -22,7 +22,8 @@ export function makeTemplateDiagnostic(
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): TemplateDiagnostic {
}[],
quickFix?: TemplateQuickFixData): TemplateDiagnostic {
if (mapping.type === 'direct') {
let relatedInformation: ts.DiagnosticRelatedInformation[]|undefined = undefined;
if (relatedMessages !== undefined) {
Expand Down Expand Up @@ -52,6 +53,7 @@ export function makeTemplateDiagnostic(
start: span.start.offset,
length: span.end.offset - span.start.offset,
relatedInformation,
quickFix,
};
} else if (mapping.type === 'indirect' || mapping.type === 'external') {
// For indirect mappings (template was declared inline, but ngtsc couldn't map it directly
Expand Down Expand Up @@ -107,6 +109,7 @@ export function makeTemplateDiagnostic(
length: span.end.offset - span.start.offset,
// Show a secondary message indicating the component whose template contains the error.
relatedInformation,
quickFix,
};
} else {
throw new Error(`Unexpected source mapping type: ${(mapping as {type: string}).type}`);
Expand Down
19 changes: 12 additions & 7 deletions packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts
Expand Up @@ -11,7 +11,7 @@ import ts from 'typescript';

import {NgCompilerOptions} from '../../../core/api';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../diagnostics';
import {NgTemplateDiagnostic, TemplateTypeChecker} from '../../api';
import {NgTemplateDiagnostic, TemplateQuickFixData, TemplateTypeChecker} from '../../api';

/**
* A Template Check receives information about the template it's checking and returns
Expand Down Expand Up @@ -43,12 +43,17 @@ export interface TemplateContext<Code extends ErrorCode> {
* Creates a template diagnostic with the given information for the template being processed and
* using the diagnostic category configured for the extended template diagnostic.
*/
makeTemplateDiagnostic(sourceSpan: ParseSourceSpan, message: string, relatedInformation?: {
text: string,
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): NgTemplateDiagnostic<Code>;
makeTemplateDiagnostic(
sourceSpan: ParseSourceSpan,
message: string,
relatedInformation?: {
text: string,
start: number,
end: number,
sourceFile: ts.SourceFile,
}[],
quickFix?: TemplateQuickFixData,
): NgTemplateDiagnostic<Code>;
}

/**
Expand Down
Expand Up @@ -10,7 +10,7 @@ import {AST, TmplAstBoundEvent, TmplAstNode} from '@angular/compiler';
import ts from 'typescript';

import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
import {NgTemplateDiagnostic} from '../../../api';
import {NgTemplateDiagnostic, TemplateQuickFixData} from '../../../api';
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';

/**
Expand All @@ -33,11 +33,20 @@ class InvalidBananaInBoxCheck extends TemplateCheckWithVisitor<ErrorCode.INVALID

const boundSyntax = node.sourceSpan.toString();
const expectedBoundSyntax = boundSyntax.replace(`(${name})`, `[(${name.slice(1, -1)})]`);
const quickFixData: TemplateQuickFixData = {
title: 'Place parentheses inside square brackets',
replacementSpan: ts.createTextSpan(
node.sourceSpan.start.offset, node.sourceSpan.end.offset - node.sourceSpan.start.offset),
replacementText: expectedBoundSyntax,
};
const diagnostic = ctx.makeTemplateDiagnostic(
node.sourceSpan,
`In the two-way binding syntax the parentheses should be inside the brackets, ex. '${
expectedBoundSyntax}'.
Find more at https://angular.io/guide/two-way-binding`);
Find more at https://angular.io/guide/two-way-binding`,
undefined,
quickFixData,
);
return [diagnostic];
}
}
Expand Down
Expand Up @@ -11,7 +11,7 @@ import ts from 'typescript';

import {DiagnosticCategoryLabel, NgCompilerOptions} from '../../../core/api';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../diagnostics';
import {NgTemplateDiagnostic, TemplateDiagnostic, TemplateTypeChecker} from '../../api';
import {NgTemplateDiagnostic, TemplateDiagnostic, TemplateQuickFixData, TemplateTypeChecker} from '../../api';
import {ExtendedTemplateChecker, TemplateCheck, TemplateCheckFactory, TemplateContext} from '../api';

export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
Expand Down Expand Up @@ -67,14 +67,19 @@ export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
...this.partialCtx,
// Wrap `templateTypeChecker.makeTemplateDiagnostic()` to implicitly provide all the known
// options.
makeTemplateDiagnostic: (span: ParseSourceSpan, message: string, relatedInformation?: {
text: string,
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): NgTemplateDiagnostic<ErrorCode> => {
makeTemplateDiagnostic: (
span: ParseSourceSpan,
message: string,
relatedInformation?: {
text: string,
start: number,
end: number,
sourceFile: ts.SourceFile,
}[],
quickFix?: TemplateQuickFixData,
): NgTemplateDiagnostic<ErrorCode> => {
return this.partialCtx.templateTypeChecker.makeTemplateDiagnostic(
component, span, category, check.code, message, relatedInformation);
component, span, category, check.code, message, relatedInformation, quickFix);
},
};

Expand Down
18 changes: 12 additions & 6 deletions packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
Expand Up @@ -19,7 +19,7 @@ import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../r
import {ComponentScopeReader, TypeCheckScopeRegistry} from '../../scope';
import {isShim} from '../../shims';
import {getSourceFileOrNull, isSymbolWithValueDeclaration} from '../../util/src/typescript';
import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PipeInScope, ProgramTypeCheckAdapter, Symbol, TcbLocation, TemplateDiagnostic, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PipeInScope, ProgramTypeCheckAdapter, Symbol, TcbLocation, TemplateDiagnostic, TemplateId, TemplateQuickFixData, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';

import {CompletionEngine} from './completion';
Expand Down Expand Up @@ -335,13 +335,19 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
}

makeTemplateDiagnostic<T extends ErrorCode>(
clazz: ts.ClassDeclaration, sourceSpan: ParseSourceSpan, category: ts.DiagnosticCategory,
errorCode: T, message: string, relatedInformation?: {
clazz: ts.ClassDeclaration,
sourceSpan: ParseSourceSpan,
category: ts.DiagnosticCategory,
errorCode: T,
message: string,
relatedInformation?: {
text: string,
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): NgTemplateDiagnostic<T> {
}[],
quickFix?: TemplateQuickFixData,
): NgTemplateDiagnostic<T> {
const sfPath = absoluteFromSourceFile(clazz.getSourceFile());
const fileRecord = this.state.get(sfPath)!;
const templateId = fileRecord.sourceManager.getTemplateId(clazz);
Expand All @@ -350,8 +356,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
return {
...makeTemplateDiagnostic(
templateId, mapping, sourceSpan, category, ngErrorCode(errorCode), message,
relatedInformation),
__ngCode: errorCode
relatedInformation, quickFix),
__ngCode: errorCode,
};
}

Expand Down
1 change: 1 addition & 0 deletions packages/language-service/BUILD.bazel
Expand Up @@ -11,6 +11,7 @@ ts_library(
],
prodmode_module = "commonjs",
deps = [
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"@npm//@types/node",
"@npm//typescript",
],
Expand Down
8 changes: 8 additions & 0 deletions packages/language-service/api.ts
Expand Up @@ -46,8 +46,15 @@ export type GetTcbResponse = {
selections: ts.TextSpan[],
};

export interface TemplateQuickFixData {
title: string;
replacementSpan: ts.TextSpan;
replacementText: string;
}

export type GetComponentLocationsForTemplateResponse = ts.DocumentSpan[];
export type GetTemplateLocationForComponentResponse = ts.DocumentSpan|undefined;
export type GetQuickFixDataForDiagnosticResponse = TemplateQuickFixData|undefined

/**
* `NgLanguageService` describes an instance of an Angular language service,
Expand All @@ -58,6 +65,7 @@ export interface NgLanguageService extends ts.LanguageService {
getComponentLocationsForTemplate(fileName: string): GetComponentLocationsForTemplateResponse;
getTemplateLocationForComponent(fileName: string, position: number):
GetTemplateLocationForComponentResponse;
getQuickFixDataForDiagnostic(diagnostic: ts.Diagnostic): GetQuickFixDataForDiagnosticResponse;
}

export function isNgLanguageService(ls: ts.LanguageService|
Expand Down
1 change: 1 addition & 0 deletions packages/language-service/src/BUILD.bazel
Expand Up @@ -21,6 +21,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/shims",
"//packages/compiler-cli/src/ngtsc/typecheck",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/diagnostics",
"//packages/compiler-cli/src/ngtsc/util",
"//packages/language-service:api",
"@npm//@types/node",
Expand Down
11 changes: 10 additions & 1 deletion packages/language-service/src/ts_plugin.ts
Expand Up @@ -6,9 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import {isTemplateDiagnostic} from '@angular/compiler-cli/src/ngtsc/typecheck/diagnostics';
import * as ts from 'typescript/lib/tsserverlibrary';

import {GetComponentLocationsForTemplateResponse, GetTcbResponse, GetTemplateLocationForComponentResponse, NgLanguageService} from '../api';
import {GetComponentLocationsForTemplateResponse, GetTcbResponse, GetTemplateLocationForComponentResponse, NgLanguageService, TemplateQuickFixData} from '../api';

import {LanguageService} from './language_service';

Expand Down Expand Up @@ -164,6 +165,13 @@ export function create(info: ts.server.PluginCreateInfo): NgLanguageService {
return ngLS.getTemplateLocationForComponent(fileName, position);
}

function getQuickFixDataForDiagnostic(diagnostic: ts.Diagnostic): TemplateQuickFixData|undefined {
if (!isTemplateDiagnostic(diagnostic)) {
return undefined;
}
return diagnostic.quickFix;
}

return {
...tsLS,
getSemanticDiagnostics,
Expand All @@ -181,6 +189,7 @@ export function create(info: ts.server.PluginCreateInfo): NgLanguageService {
getComponentLocationsForTemplate,
getSignatureHelpItems,
getTemplateLocationForComponent,
getQuickFixDataForDiagnostic,
};
}

Expand Down

0 comments on commit 5e29ed1

Please sign in to comment.