From e89c6096fa0321ec11db71ff4b13c7e6aad8572b Mon Sep 17 00:00:00 2001 From: ivanwonder Date: Sat, 20 Aug 2022 20:11:46 +0800 Subject: [PATCH] feat(language-service): support to fix invalid banana in box The Angular compiler will report the invalid banana in box, this code fixes will try to fix the error and all the same errors in the selected file. Fixes #44941 --- .../src/codefixes/all_codefixes_metas.ts | 6 +- .../src/codefixes/code_fixes.ts | 8 +- .../codefixes/fix_invalid_banana_in_box.ts | 124 ++++++++++++++++++ .../language-service/src/codefixes/utils.ts | 2 + .../language-service/src/language_service.ts | 3 +- .../language-service/test/code_fixes_spec.ts | 89 ++++++++++++- 6 files changed, 221 insertions(+), 11 deletions(-) create mode 100644 packages/language-service/src/codefixes/fix_invalid_banana_in_box.ts 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; }