Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(compiler-cli): add warning for unused let declarations (#57033)

Browse source 
Adds a new extended diagnostic that will flag `@let` declarations that aren't used within the template. The diagnostic can be turned off through the `extendedDiagnostics` compiler option.

PR Close #57033
This commit is contained in:
Kristiyan Kostadinov 2024-07-23 06:19:37 +02:00 committed by Andrew Scott
parent 6c2fbda694
commit d4ff6bc0b2
12 changed files with 280 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
goldens/public-api/compiler-cli/error_code.api.md
View file

@ -105,6 +105,7 @@ export enum ErrorCode {
UNDECORATED_PROVIDER = 2005,
UNINVOKED_FUNCTION_IN_EVENT_BINDING = 8111,
UNSUPPORTED_INITIALIZER_API_USAGE = 8110,
UNUSED_LET_DECLARATION = 8112,
// (undocumented)
VALUE_HAS_WRONG_TYPE = 1010,
// (undocumented)

4
goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md
View file

@ -27,7 +27,9 @@ export enum ExtendedTemplateDiagnosticName {
// (undocumented)
TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding",
// (undocumented)
UNINVOKED_FUNCTION_IN_EVENT_BINDING = "uninvokedFunctionInEventBinding"
UNINVOKED_FUNCTION_IN_EVENT_BINDING = "uninvokedFunctionInEventBinding",
// (undocumented)
UNUSED_LET_DECLARATION = "unusedLetDeclaration"
}
// (No @packageDocumentation comment for this package)

13
packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
View file

@ -495,6 +495,19 @@ export enum ErrorCode {
*/
UNINVOKED_FUNCTION_IN_EVENT_BINDING = 8111,
/**
* A `@let` declaration in a template isn't used.
*
* For example:
* ```
* @let used = 1; <!-- Not an error -->
* @let notUsed = 2; <!-- Error -->
*
* {{used}}
* ```
*/
UNUSED_LET_DECLARATION = 8112,
/**
* The template type-checking engine would need to generate an inline type check block for a
* component, but the current type-checking environment doesn't support it.

1
packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts
View file

@ -27,4 +27,5 @@ export enum ExtendedTemplateDiagnosticName {
SKIP_HYDRATION_NOT_STATIC = 'skipHydrationNotStatic',
INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked',
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 'controlFlowPreventingContentProjection',
UNUSED_LET_DECLARATION = 'unusedLetDeclaration',
}

1
packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel
View file

@ -22,6 +22,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_function_in_event_binding",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/unused_let_declaration",
"@npm//typescript",
],
)

17
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/unused_let_declaration/BUILD.bazel Normal file
View file

@ -0,0 +1,17 @@
load("//tools:defaults.bzl", "ts_library")
ts_library(
name = "unused_let_declaration",
srcs = ["index.ts"],
visibility = [
"//packages/compiler-cli/src/ngtsc:__subpackages__",
"//packages/compiler-cli/test/ngtsc:__pkg__",
],
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
"@npm//typescript",
],
)

87
packages/compiler-cli/src/ngtsc/typecheck/extended/checks/unused_let_declaration/index.ts Normal file
View file

@ -0,0 +1,87 @@
/**
* @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 {AST, ASTWithSource, TmplAstLetDeclaration, TmplAstNode} from '@angular/compiler';
import ts from 'typescript';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
import {NgTemplateDiagnostic} from '../../../api';
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
interface ClassAnalysis {
allLetDeclarations: Set<TmplAstLetDeclaration>;
usedLetDeclarations: Set<TmplAstLetDeclaration>;
}
/**
* Ensures that all `@let` declarations in a template are used.
*/
class UnusedLetDeclarationCheck extends TemplateCheckWithVisitor<ErrorCode.UNUSED_LET_DECLARATION> {
override code = ErrorCode.UNUSED_LET_DECLARATION as const;
private analysis = new Map<ts.ClassDeclaration, ClassAnalysis>();
override run(
ctx: TemplateContext<ErrorCode.UNUSED_LET_DECLARATION>,
component: ts.ClassDeclaration,
template: TmplAstNode[],
): NgTemplateDiagnostic<ErrorCode.UNUSED_LET_DECLARATION>[] {
super.run(ctx, component, template);
const diagnostics: NgTemplateDiagnostic<ErrorCode.UNUSED_LET_DECLARATION>[] = [];
const {allLetDeclarations, usedLetDeclarations} = this.getAnalysis(component);
for (const decl of allLetDeclarations) {
if (!usedLetDeclarations.has(decl)) {
diagnostics.push(
ctx.makeTemplateDiagnostic(
decl.sourceSpan,
`@let ${decl.name} is declared but its value is never read.`,
),
);
}
}
this.analysis.clear();
return diagnostics;
}
override visitNode(
ctx: TemplateContext<ErrorCode.UNUSED_LET_DECLARATION>,
component: ts.ClassDeclaration,
node: TmplAstNode | AST,
): NgTemplateDiagnostic<ErrorCode.UNUSED_LET_DECLARATION>[] {
if (node instanceof TmplAstLetDeclaration) {
this.getAnalysis(component).allLetDeclarations.add(node);
} else if (node instanceof AST) {
const unwrappedNode = node instanceof ASTWithSource ? node.ast : node;
const target = ctx.templateTypeChecker.getExpressionTarget(unwrappedNode, component);
if (target !== null && target instanceof TmplAstLetDeclaration) {
this.getAnalysis(component).usedLetDeclarations.add(target);
}
}
return [];
}
private getAnalysis(node: ts.ClassDeclaration): ClassAnalysis {
if (!this.analysis.has(node)) {
this.analysis.set(node, {allLetDeclarations: new Set(), usedLetDeclarations: new Set()});
}
return this.analysis.get(node)!;
}
}
export const factory: TemplateCheckFactory<
ErrorCode.UNUSED_LET_DECLARATION,
ExtendedTemplateDiagnosticName.UNUSED_LET_DECLARATION
> = {
code: ErrorCode.UNUSED_LET_DECLARATION,
name: ExtendedTemplateDiagnosticName.UNUSED_LET_DECLARATION,
create: () => new UnusedLetDeclarationCheck(),
};

2
packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts
View file

@ -18,6 +18,7 @@ import {factory as optionalChainNotNullableFactory} from './checks/optional_chai
import {factory as suffixNotSupportedFactory} from './checks/suffix_not_supported';
import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding';
import {factory as uninvokedFunctionInEventBindingFactory} from './checks/uninvoked_function_in_event_binding';
import {factory as unusedLetDeclarationFactory} from './checks/unused_let_declaration';
export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker';
@ -34,6 +35,7 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory<
suffixNotSupportedFactory,
interpolatedSignalNotInvoked,
uninvokedFunctionInEventBindingFactory,
unusedLetDeclarationFactory,
];
export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([

30
packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/unused_let_declaration/BUILD.bazel Normal file
View file

@ -0,0 +1,30 @@
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
ts_library(
name = "test_lib",
testonly = True,
srcs = ["unused_let_declaration_spec.ts"],
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/core:api",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/compiler-cli/src/ngtsc/testing",
"//packages/compiler-cli/src/ngtsc/typecheck/extended",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/unused_let_declaration",
"//packages/compiler-cli/src/ngtsc/typecheck/testing",
"@npm//typescript",
],
)
jasmine_node_test(
name = "test",
bootstrap = ["//tools/testing:node_no_angular"],
data = [
"//packages/core:npm_package",
],
deps = [
":test_lib",
],
)

112
packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/unused_let_declaration/unused_let_declaration_spec.ts Normal file
View file

@ -0,0 +1,112 @@
/**
* @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 ts from 'typescript';
import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics';
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
import {runInEachFileSystem} from '../../../../../file_system/testing';
import {getSourceCodeForDiagnostic} from '../../../../../testing';
import {getClass, setup} from '../../../../testing';
import {factory as unusedLetDeclarationFactory} from '../../../checks/unused_let_declaration/index';
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
runInEachFileSystem(() => {
describe('UnusedLetDeclarationCheck', () => {
function diagnose(template: string) {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': template,
},
source: `
export class TestCmp {
eventCallback(value: any) {}
}
`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker,
program.getTypeChecker(),
[unusedLetDeclarationFactory],
{},
);
return extendedTemplateChecker.getDiagnosticsForComponent(component);
}
it('binds the error code to its extended template diagnostic name', () => {
expect(unusedLetDeclarationFactory.code).toBe(ErrorCode.UNUSED_LET_DECLARATION);
expect(unusedLetDeclarationFactory.name).toBe(
ExtendedTemplateDiagnosticName.UNUSED_LET_DECLARATION,
);
});
it('should report a @let declaration that is not used', () => {
const diags = diagnose(`
@let used = 1;
@let unused = 2;
{{used}}
`);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNUSED_LET_DECLARATION));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('@let unused = 2');
});
it('should report a @let declaration that is not used', () => {
const diags = diagnose(`
@let foo = 1;
@if (true) {
@let foo = 2;
{{foo}}
}
`);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNUSED_LET_DECLARATION));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('@let foo = 1');
});
it('should not report a @let declaration that is only used in other @let declarations', () => {
const diags = diagnose(`
@let one = 1;
@let two = 2;
@let three = one + two;
{{three}}
`);
expect(diags.length).toBe(0);
});
it('should not report a @let declaration that is only used in an event listener', () => {
const diags = diagnose(`
@let foo = 1;
<button (click)="eventCallback(foo + 1)">Click me</button>
`);
expect(diags.length).toBe(0);
});
it('should not report a @let declaration that is only used in a structural directive', () => {
const diags = diagnose(`
@let foo = null;
<div *ngIf="foo"></div>
`);
expect(diags.length).toBe(0);
});
});
});

13
packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
View file

@ -6611,7 +6611,18 @@ suppress
});
describe('@let declarations', () => {
beforeEach(() => env.tsconfig({strictTemplates: true}));
beforeEach(() =>
env.tsconfig({
strictTemplates: true,
extendedDiagnostics: {
checks: {
// Suppress the diagnostic for unused @let since some of the error cases
// we're checking for here also qualify as being unused which adds noise.
unusedLetDeclaration: 'suppress',
},
},
}),
);
it('should infer the type of a let declaration', () => {
env.write(

2
packages/language-service/test/quick_info_spec.ts
View file

@ -708,7 +708,7 @@ describe('quick info', () => {
describe('let declarations', () => {
it('should get quick info for a let declaration', () => {
expectQuickInfo({
templateOverride: `@let na¦me = 'Frodo';`,
templateOverride: `@let na¦me = 'Frodo'; {{name}}`,
expectedSpanText: `@let name = 'Frodo'`,
expectedDisplayString: `(let) name: "Frodo"`,
});