Skip to content

Commit e7ee53c

Browse files
ivanwonderalxhub
ivanwonder
authored and
alxhub
committed
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
1 parent 120555a commit e7ee53c

File tree

6 files changed

+221
-11
lines changed

6 files changed

+221
-11
lines changed

‎packages/language-service/src/codefixes/all_codefixes_metas.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {fixInvalidBananaInBoxMeta} from './fix_invalid_banana_in_box';
910
import {missingMemberMeta} from './fix_missing_member';
1011
import {CodeActionMeta} from './utils';
1112

12-
export const ALL_CODE_FIXES_METAS: CodeActionMeta[] = [missingMemberMeta];
13+
export const ALL_CODE_FIXES_METAS: CodeActionMeta[] = [
14+
missingMemberMeta,
15+
fixInvalidBananaInBoxMeta,
16+
];

‎packages/language-service/src/codefixes/code_fixes.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ export class CodeFixes {
4343
* and collect all the responses from the `codeActionMetas` which could handle the `errorCodes`.
4444
*/
4545
getCodeFixesAtPosition(
46-
templateInfo: TemplateInfo, compiler: NgCompiler, start: number, end: number,
47-
errorCodes: readonly number[], diagnostics: tss.Diagnostic[],
46+
fileName: string, templateInfo: TemplateInfo, compiler: NgCompiler, start: number,
47+
end: number, errorCodes: readonly number[], diagnostics: tss.Diagnostic[],
4848
formatOptions: tss.FormatCodeSettings,
4949
preferences: tss.UserPreferences): readonly tss.CodeFixAction[] {
5050
const codeActions: tss.CodeFixAction[] = [];
@@ -55,6 +55,7 @@ export class CodeFixes {
5555
}
5656
for (const meta of metas) {
5757
const codeActionsForMeta = meta.getCodeActions({
58+
fileName,
5859
templateInfo,
5960
compiler,
6061
start,
@@ -98,7 +99,8 @@ export class CodeFixes {
9899
preferences,
99100
tsLs: this.tsLS,
100101
scope,
101-
diagnostics,
102+
// only pass the diagnostics the `meta` cares about.
103+
diagnostics: diagnostics.filter(diag => meta.errorCodes.includes(diag.code)),
102104
});
103105
}
104106
}

‎packages/language-service/src/codefixes/fix_invalid_banana_in_box.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics';
10+
import {BoundEvent} from '@angular/compiler/src/render3/r3_ast';
11+
import * as tss from 'typescript/lib/tsserverlibrary';
12+
13+
import {getTargetAtPosition, TargetNodeKind} from '../template_target';
14+
import {getTemplateInfoAtPosition, TemplateInfo} from '../utils';
15+
16+
import {CodeActionMeta, FixIdForCodeFixesAll} from './utils';
17+
18+
/**
19+
* fix [invalid banana-in-box](https://angular.io/extended-diagnostics/NG8101)
20+
*/
21+
export const fixInvalidBananaInBoxMeta: CodeActionMeta = {
22+
errorCodes: [ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX)],
23+
getCodeActions({start, fileName, templateInfo}) {
24+
const boundEvent = getTheBoundEventAtPosition(templateInfo, start);
25+
if (boundEvent === null) {
26+
return [];
27+
}
28+
const textChanges = convertBoundEventToTsTextChange(boundEvent);
29+
return [{
30+
fixName: FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX,
31+
fixId: FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX,
32+
fixAllDescription: 'fix all invalid banana-in-box',
33+
description: `fix invalid banana-in-box for '${boundEvent.sourceSpan.toString()}'`,
34+
changes: [{
35+
fileName,
36+
textChanges,
37+
}],
38+
}];
39+
},
40+
fixIds: [FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX],
41+
getAllCodeActions({diagnostics, compiler}) {
42+
const fileNameToTextChangesMap = new Map<string, tss.TextChange[]>();
43+
for (const diag of diagnostics) {
44+
const fileName = diag.file?.fileName;
45+
if (fileName === undefined) {
46+
continue;
47+
}
48+
const start = diag.start;
49+
if (start === undefined) {
50+
continue;
51+
}
52+
const templateInfo = getTemplateInfoAtPosition(fileName, start, compiler);
53+
if (templateInfo === undefined) {
54+
continue;
55+
}
56+
57+
/**
58+
* This diagnostic has detected a likely mistake that puts the square brackets inside the
59+
* parens (the BoundEvent `([thing])`) when it should be the other way around `[(thing)]` so
60+
* this function is trying to find the bound event in order to flip the syntax.
61+
*/
62+
const boundEvent = getTheBoundEventAtPosition(templateInfo, start);
63+
if (boundEvent === null) {
64+
continue;
65+
}
66+
67+
if (!fileNameToTextChangesMap.has(fileName)) {
68+
fileNameToTextChangesMap.set(fileName, []);
69+
}
70+
const fileTextChanges = fileNameToTextChangesMap.get(fileName)!;
71+
const textChanges = convertBoundEventToTsTextChange(boundEvent);
72+
fileTextChanges.push(...textChanges);
73+
}
74+
75+
const fileTextChanges: tss.FileTextChanges[] = [];
76+
for (const [fileName, textChanges] of fileNameToTextChangesMap) {
77+
fileTextChanges.push({
78+
fileName,
79+
textChanges,
80+
});
81+
}
82+
return {
83+
changes: fileTextChanges,
84+
};
85+
},
86+
};
87+
88+
function getTheBoundEventAtPosition(templateInfo: TemplateInfo, start: number): BoundEvent|null {
89+
// It's safe to get the bound event at the position `start + 1` because the `start` is at the
90+
// start of the diagnostic, and the node outside the attribute key and value spans are skipped by
91+
// the function `getTargetAtPosition`.
92+
// https://github.com/angular/vscode-ng-language-service/blob/8553115972ca40a55602747667c3d11d6f47a6f8/server/src/session.ts#L220
93+
// https://github.com/angular/angular/blob/4e10a7494130b9bb4772ee8f76b66675867b2145/packages/language-service/src/template_target.ts#L347-L356
94+
const positionDetail = getTargetAtPosition(templateInfo.template, start + 1);
95+
if (positionDetail === null) {
96+
return null;
97+
}
98+
99+
if (positionDetail.context.kind !== TargetNodeKind.AttributeInKeyContext ||
100+
!(positionDetail.context.node instanceof BoundEvent)) {
101+
return null;
102+
}
103+
104+
return positionDetail.context.node;
105+
}
106+
107+
/**
108+
* Flip the invalid "box in a banana" `([thing])` to the correct "banana in a box" `[(thing)]`.
109+
*/
110+
function convertBoundEventToTsTextChange(node: BoundEvent): readonly tss.TextChange[] {
111+
const name = node.name;
112+
const boundSyntax = node.sourceSpan.toString();
113+
const expectedBoundSyntax = boundSyntax.replace(`(${name})`, `[(${name.slice(1, -1)})]`);
114+
115+
return [
116+
{
117+
span: {
118+
start: node.sourceSpan.start.offset,
119+
length: boundSyntax.length,
120+
},
121+
newText: expectedBoundSyntax,
122+
},
123+
];
124+
}

‎packages/language-service/src/codefixes/utils.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {TemplateInfo} from '../utils';
2121
*/
2222
export interface CodeActionContext {
2323
templateInfo: TemplateInfo;
24+
fileName: string;
2425
compiler: NgCompiler;
2526
start: number;
2627
end: number;
@@ -132,4 +133,5 @@ export function isFixAllAvailable(meta: CodeActionMeta, diagnostics: tss.Diagnos
132133
export enum FixIdForCodeFixesAll {
133134
FIX_SPELLING = 'fixSpelling',
134135
FIX_MISSING_MEMBER = 'fixMissingMember',
136+
FIX_INVALID_BANANA_IN_BOX = 'fixInvalidBananaInBox',
135137
}

‎packages/language-service/src/language_service.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,8 @@ export class LanguageService {
307307
return [];
308308
}
309309
return this.codeFixes.getCodeFixesAtPosition(
310-
templateInfo, compiler, start, end, errorCodes, diags, formatOptions, preferences);
310+
fileName, templateInfo, compiler, start, end, errorCodes, diags, formatOptions,
311+
preferences);
311312
});
312313
}
313314

‎packages/language-service/test/code_fixes_spec.ts‎

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
1010

11+
import {FixIdForCodeFixesAll} from '../src/codefixes/utils';
1112
import {createModuleAndProjectWithDeclarations, LanguageServiceTestEnv} from '../testing';
1213

1314
describe('code fixes', () => {
@@ -157,6 +158,74 @@ describe('code fixes', () => {
157158
fileName: 'app.ts'
158159
});
159160
});
161+
162+
it('should fix invalid banana-in-box error', () => {
163+
const files = {
164+
'app.ts': `
165+
import {Component, NgModule} from '@angular/core';
166+
167+
@Component({
168+
templateUrl: './app.html'
169+
})
170+
export class AppComponent {
171+
title = '';
172+
}
173+
`,
174+
'app.html': `<input ([ngModel])="title">`,
175+
};
176+
177+
const project = createModuleAndProjectWithDeclarations(env, 'test', files);
178+
const diags = project.getDiagnosticsForFile('app.html');
179+
const appFile = project.openFile('app.html');
180+
appFile.moveCursorToText('¦([ngModel');
181+
182+
const codeActions =
183+
project.getCodeFixesAtPosition('app.html', appFile.cursor, appFile.cursor, [diags[0].code]);
184+
expectIncludeReplacementText({
185+
codeActions,
186+
content: appFile.contents,
187+
text: `([ngModel])="title"`,
188+
newText: `[(ngModel)]="title"`,
189+
fileName: 'app.html',
190+
description: `fix invalid banana-in-box for '([ngModel])="title"'`
191+
});
192+
});
193+
194+
it('should fix all invalid banana-in-box errors', () => {
195+
const files = {
196+
'app.ts': `
197+
import {Component, NgModule} from '@angular/core';
198+
199+
@Component({
200+
template: '<input ([ngModel])="title"><input ([value])="title">',
201+
})
202+
export class AppComponent {
203+
title = '';
204+
banner = '';
205+
}
206+
`,
207+
};
208+
209+
const project = createModuleAndProjectWithDeclarations(env, 'test', files);
210+
const appFile = project.openFile('app.ts');
211+
212+
const fixesAllActions =
213+
project.getCombinedCodeFix('app.ts', FixIdForCodeFixesAll.FIX_INVALID_BANANA_IN_BOX);
214+
expectIncludeReplacementTextForFileTextChange({
215+
fileTextChanges: fixesAllActions.changes,
216+
content: appFile.contents,
217+
text: `([ngModel])="title"`,
218+
newText: `[(ngModel)]="title"`,
219+
fileName: 'app.ts'
220+
});
221+
expectIncludeReplacementTextForFileTextChange({
222+
fileTextChanges: fixesAllActions.changes,
223+
content: appFile.contents,
224+
text: `([value])="title"`,
225+
newText: `[(value)]="title"`,
226+
fileName: 'app.ts'
227+
});
228+
});
160229
});
161230

162231
function expectNotIncludeFixAllInfo(codeActions: readonly ts.CodeFixAction[]) {
@@ -166,14 +235,22 @@ function expectNotIncludeFixAllInfo(codeActions: readonly ts.CodeFixAction[]) {
166235
}
167236
}
168237

169-
function expectIncludeReplacementText({codeActions, content, text, newText, fileName}: {
170-
codeActions: readonly ts.CodeAction[]; content: string; text: string; newText: string;
171-
fileName: string;
172-
}) {
238+
/**
239+
* The `description` is optional because if the description comes from the ts server, no need to
240+
* check it.
241+
*/
242+
function expectIncludeReplacementText(
243+
{codeActions, content, text, newText, fileName, description}: {
244+
codeActions: readonly ts.CodeAction[]; content: string; text: string; newText: string;
245+
fileName: string;
246+
description?: string;
247+
}) {
173248
let includeReplacementText = false;
174249
for (const codeAction of codeActions) {
175-
includeReplacementText = includeReplacementTextInChanges(
176-
{fileTextChanges: codeAction.changes, content, text, newText, fileName});
250+
includeReplacementText =
251+
includeReplacementTextInChanges(
252+
{fileTextChanges: codeAction.changes, content, text, newText, fileName}) &&
253+
(description === undefined ? true : (description === codeAction.description));
177254
if (includeReplacementText) {
178255
return;
179256
}

0 commit comments

Comments
 (0)