Skip to content
Permalink
Browse files
feat(language-service): support to fix invalid banana in box (#47393)
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

PR Close #47393
  • Loading branch information
ivanwonder authored and alxhub committed Sep 27, 2022
1 parent 120555a commit e7ee53c541da0a1f85c217354ec9901010ae0de9
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 11 deletions.
@@ -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,
];
@@ -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)),
});
}
}
@@ -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,
},
];
}
@@ -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',
}
@@ -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);
});
}

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

0 comments on commit e7ee53c

Please sign in to comment.