diff --git a/packages/language-service/src/codefixes/all_codefixes_metas.ts b/packages/language-service/src/codefixes/all_codefixes_metas.ts index b2a5bb0d80ec6..6894a897a374d 100644 --- a/packages/language-service/src/codefixes/all_codefixes_metas.ts +++ b/packages/language-service/src/codefixes/all_codefixes_metas.ts @@ -6,7 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ +import {fixInvalidBananaInBoxMeta} from './fix_invalid_banana_in_box'; import {missingMemberMeta} from './fix_missing_member'; import {CodeActionMeta} from './utils'; -export const ALL_CODE_FIXES_METAS: CodeActionMeta[] = [missingMemberMeta]; +export const ALL_CODE_FIXES_METAS: CodeActionMeta[] = [ + missingMemberMeta, + fixInvalidBananaInBoxMeta, +]; diff --git a/packages/language-service/src/codefixes/code_fixes.ts b/packages/language-service/src/codefixes/code_fixes.ts index e03a40fc1f2fb..08516c49b480c 100644 --- a/packages/language-service/src/codefixes/code_fixes.ts +++ b/packages/language-service/src/codefixes/code_fixes.ts @@ -43,8 +43,8 @@ export class CodeFixes { * and collect all the responses from the `codeActionMetas` which could handle the `errorCodes`. */ getCodeFixesAtPosition( - templateInfo: TemplateInfo, compiler: NgCompiler, start: number, end: number, - errorCodes: readonly number[], diagnostics: tss.Diagnostic[], + fileName: string, templateInfo: TemplateInfo, compiler: NgCompiler, start: number, + end: number, errorCodes: readonly number[], diagnostics: tss.Diagnostic[], formatOptions: tss.FormatCodeSettings, preferences: tss.UserPreferences): readonly tss.CodeFixAction[] { const codeActions: tss.CodeFixAction[] = []; @@ -55,6 +55,7 @@ export class CodeFixes { } for (const meta of metas) { const codeActionsForMeta = meta.getCodeActions({ + fileName, templateInfo, compiler, start, @@ -98,7 +99,8 @@ export class CodeFixes { preferences, tsLs: this.tsLS, scope, - diagnostics, + // only pass the diagnostics the `meta` cares about. + diagnostics: diagnostics.filter(diag => meta.errorCodes.includes(diag.code)), }); } } diff --git a/packages/language-service/src/codefixes/fix_invalid_banana_in_box.ts b/packages/language-service/src/codefixes/fix_invalid_banana_in_box.ts new file mode 100644 index 0000000000000..53495aafa294d --- /dev/null +++ b/packages/language-service/src/codefixes/fix_invalid_banana_in_box.ts @@ -0,0 +1,124 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics'; +import {BoundEvent} from '@angular/compiler/src/render3/r3_ast'; +import * as tss from 'typescript/lib/tsserverlibrary'; + +import {getTargetAtPosition, TargetNodeKind} from '../template_target'; +import {getTemplateInfoAtPosition, TemplateInfo} from '../utils'; + +import {CodeActionMeta, FixIdForCodeFixesAll} from './utils'; + +/** + * fix [invalid banana-in-box](https://angular.io/extended-diagnostics/NG8101) + */ +export const fixInvalidBananaInBoxMeta: CodeActionMeta = { + errorCodes: [ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX)], + getCodeActions({start, fileName, templateInfo}) { + const boundEvent = getTheBoundEventAtPosition(templateInfo, start); + if (boundEvent === null) { + return []; + } + const textChanges = convertBoundEventToTsTextChange(boundEvent); + return [{ + fixName: FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX, + fixId: FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX, + fixAllDescription: 'fix all invalid banana-in-box', + description: `fix invalid banana-in-box for '${boundEvent.sourceSpan.toString()}'`, + changes: [{ + fileName, + textChanges, + }], + }]; + }, + fixIds: [FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX], + getAllCodeActions({diagnostics, compiler}) { + const fileNameToTextChangesMap = new Map(); + for (const diag of diagnostics) { + const fileName = diag.file?.fileName; + if (fileName === undefined) { + continue; + } + const start = diag.start; + if (start === undefined) { + continue; + } + const templateInfo = getTemplateInfoAtPosition(fileName, start, compiler); + if (templateInfo === undefined) { + continue; + } + + /** + * This diagnostic has detected a likely mistake that puts the square brackets inside the + * parens (the BoundEvent `([thing])`) when it should be the other way around `[(thing)]` so + * this function is trying to find the bound event in order to flip the syntax. + */ + const boundEvent = getTheBoundEventAtPosition(templateInfo, start); + if (boundEvent === null) { + continue; + } + + if (!fileNameToTextChangesMap.has(fileName)) { + fileNameToTextChangesMap.set(fileName, []); + } + const fileTextChanges = fileNameToTextChangesMap.get(fileName)!; + const textChanges = convertBoundEventToTsTextChange(boundEvent); + fileTextChanges.push(...textChanges); + } + + const fileTextChanges: tss.FileTextChanges[] = []; + for (const [fileName, textChanges] of fileNameToTextChangesMap) { + fileTextChanges.push({ + fileName, + textChanges, + }); + } + return { + changes: fileTextChanges, + }; + }, +}; + +function getTheBoundEventAtPosition(templateInfo: TemplateInfo, start: number): BoundEvent|null { + // It's safe to get the bound event at the position `start + 1` because the `start` is at the + // start of the diagnostic, and the node outside the attribute key and value spans are skipped by + // the function `getTargetAtPosition`. + // https://github.com/angular/vscode-ng-language-service/blob/8553115972ca40a55602747667c3d11d6f47a6f8/server/src/session.ts#L220 + // https://github.com/angular/angular/blob/4e10a7494130b9bb4772ee8f76b66675867b2145/packages/language-service/src/template_target.ts#L347-L356 + const positionDetail = getTargetAtPosition(templateInfo.template, start + 1); + if (positionDetail === null) { + return null; + } + + if (positionDetail.context.kind !== TargetNodeKind.AttributeInKeyContext || + !(positionDetail.context.node instanceof BoundEvent)) { + return null; + } + + return positionDetail.context.node; +} + +/** + * Flip the invalid "box in a banana" `([thing])` to the correct "banana in a box" `[(thing)]`. + */ +function convertBoundEventToTsTextChange(node: BoundEvent): readonly tss.TextChange[] { + const name = node.name; + const boundSyntax = node.sourceSpan.toString(); + const expectedBoundSyntax = boundSyntax.replace(`(${name})`, `[(${name.slice(1, -1)})]`); + + return [ + { + span: { + start: node.sourceSpan.start.offset, + length: boundSyntax.length, + }, + newText: expectedBoundSyntax, + }, + ]; +} diff --git a/packages/language-service/src/codefixes/utils.ts b/packages/language-service/src/codefixes/utils.ts index 587e69c6fda20..4d6f18331ebbc 100644 --- a/packages/language-service/src/codefixes/utils.ts +++ b/packages/language-service/src/codefixes/utils.ts @@ -21,6 +21,7 @@ import {TemplateInfo} from '../utils'; */ export interface CodeActionContext { templateInfo: TemplateInfo; + fileName: string; compiler: NgCompiler; start: number; end: number; @@ -132,4 +133,5 @@ export function isFixAllAvailable(meta: CodeActionMeta, diagnostics: tss.Diagnos export enum FixIdForCodeFixesAll { FIX_SPELLING = 'fixSpelling', FIX_MISSING_MEMBER = 'fixMissingMember', + FIX_INVALID_BANANA_IN_BOX = 'fixInvalidBananaInBox', } diff --git a/packages/language-service/src/language_service.ts b/packages/language-service/src/language_service.ts index 2bae90435b55d..ae455f10655fa 100644 --- a/packages/language-service/src/language_service.ts +++ b/packages/language-service/src/language_service.ts @@ -307,7 +307,8 @@ export class LanguageService { return []; } return this.codeFixes.getCodeFixesAtPosition( - templateInfo, compiler, start, end, errorCodes, diags, formatOptions, preferences); + fileName, templateInfo, compiler, start, end, errorCodes, diags, formatOptions, + preferences); }); } diff --git a/packages/language-service/test/code_fixes_spec.ts b/packages/language-service/test/code_fixes_spec.ts index 9beb937ef238a..1091948f1933b 100644 --- a/packages/language-service/test/code_fixes_spec.ts +++ b/packages/language-service/test/code_fixes_spec.ts @@ -8,6 +8,7 @@ import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; +import {FixIdForCodeFixesAll} from '../src/codefixes/utils'; import {createModuleAndProjectWithDeclarations, LanguageServiceTestEnv} from '../testing'; describe('code fixes', () => { @@ -157,6 +158,74 @@ describe('code fixes', () => { fileName: 'app.ts' }); }); + + it('should fix invalid banana-in-box error', () => { + const files = { + 'app.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + templateUrl: './app.html' + }) + export class AppComponent { + title = ''; + } + `, + 'app.html': ``, + }; + + const project = createModuleAndProjectWithDeclarations(env, 'test', files); + const diags = project.getDiagnosticsForFile('app.html'); + const appFile = project.openFile('app.html'); + appFile.moveCursorToText('¦([ngModel'); + + const codeActions = + project.getCodeFixesAtPosition('app.html', appFile.cursor, appFile.cursor, [diags[0].code]); + expectIncludeReplacementText({ + codeActions, + content: appFile.contents, + text: `([ngModel])="title"`, + newText: `[(ngModel)]="title"`, + fileName: 'app.html', + description: `fix invalid banana-in-box for '([ngModel])="title"'` + }); + }); + + it('should fix all invalid banana-in-box errors', () => { + const files = { + 'app.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: '', + }) + export class AppComponent { + title = ''; + banner = ''; + } + `, + }; + + const project = createModuleAndProjectWithDeclarations(env, 'test', files); + const appFile = project.openFile('app.ts'); + + const fixesAllActions = + project.getCombinedCodeFix('app.ts', FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX); + expectIncludeReplacementTextForFileTextChange({ + fileTextChanges: fixesAllActions.changes, + content: appFile.contents, + text: `([ngModel])="title"`, + newText: `[(ngModel)]="title"`, + fileName: 'app.ts' + }); + expectIncludeReplacementTextForFileTextChange({ + fileTextChanges: fixesAllActions.changes, + content: appFile.contents, + text: `([value])="title"`, + newText: `[(value)]="title"`, + fileName: 'app.ts' + }); + }); }); function expectNotIncludeFixAllInfo(codeActions: readonly ts.CodeFixAction[]) { @@ -166,14 +235,22 @@ function expectNotIncludeFixAllInfo(codeActions: readonly ts.CodeFixAction[]) { } } -function expectIncludeReplacementText({codeActions, content, text, newText, fileName}: { - codeActions: readonly ts.CodeAction[]; content: string; text: string; newText: string; - fileName: string; -}) { +/** + * The `description` is optional because if the description comes from the ts server, no need to + * check it. + */ +function expectIncludeReplacementText( + {codeActions, content, text, newText, fileName, description}: { + codeActions: readonly ts.CodeAction[]; content: string; text: string; newText: string; + fileName: string; + description?: string; + }) { let includeReplacementText = false; for (const codeAction of codeActions) { - includeReplacementText = includeReplacementTextInChanges( - {fileTextChanges: codeAction.changes, content, text, newText, fileName}); + includeReplacementText = + includeReplacementTextInChanges( + {fileTextChanges: codeAction.changes, content, text, newText, fileName}) && + (description === undefined ? true : (description === codeAction.description)); if (includeReplacementText) { return; }