feat(language-service): support to fix invalid banana in box

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,
+];

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)),
     });
   }
 }

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<string, tss.TextChange[]>();
+    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,
+    },
+  ];
+}

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',
 }

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);
         });
   }
 

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': `<input ([ngModel])="title">`,
+    };
+
+    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: '<input ([ngModel])="title"><input ([value])="title">',
+      })
+      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;
     }