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

feat(compiler): warn when style suffixes are used with attribute bindings (#46651)

Browse source 
This commit adds an extended diagnostic which warns when style suffixes such as '.px'
are used with attribute bindings (attr.width.px).

Fixes #36256

PR Close #46651
This commit is contained in:
Andrew Scott 2022-06-30 13:10:34 -07:00 committed by Jessica Janiuk
parent ea0f07a858
commit 9e836c232f
10 changed files with 194 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -67,6 +67,7 @@ export enum ErrorCode {
SCHEMA_INVALID_ATTRIBUTE = 8002,
SCHEMA_INVALID_ELEMENT = 8001,
SPLIT_TWO_WAY_BINDING = 8007,
SUFFIX_NOT_SUPPORTED = 8106,
SUGGEST_STRICT_TEMPLATES = 10001,
SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002,
// (undocumented)

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

@ -15,6 +15,8 @@ export enum ExtendedTemplateDiagnosticName {
// (undocumented)
NULLISH_COALESCING_NOT_NULLABLE = "nullishCoalescingNotNullable",
// (undocumented)
SUFFIX_NOT_SUPPORTED = "suffixNotSupported",
// (undocumented)
TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding"
}

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

@ -266,6 +266,17 @@ export enum ErrorCode {
* ```
*/
MISSING_NGFOROF_LET = 8105,
/**
* Indicates that the binding suffix is not supported
*
* Style bindings support suffixes like `stlye.width.px`, `.em`, and `.%`.
* These suffixes are _not_ supported for attribute bindings.
*
* For example `[attr.width.px]="5"` becomes `width.px="5"` when bound.
* This is almost certainly unintentional and this error is meant to
* surface this mistake to the developer.
*/
SUFFIX_NOT_SUPPORTED = 8106,
/**
* The template type-checking engine would need to generate an inline type check block for a

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

@ -20,5 +20,6 @@ export enum ExtendedTemplateDiagnosticName {
NULLISH_COALESCING_NOT_NULLABLE = 'nullishCoalescingNotNullable',
MISSING_CONTROL_FLOW_DIRECTIVE = 'missingControlFlowDirective',
TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding',
MISSING_NGFOROF_LET = 'missingNgForOfLet'
MISSING_NGFOROF_LET = 'missingNgForOfLet',
SUFFIX_NOT_SUPPORTED = 'suffixNotSupported',
}

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

@ -16,6 +16,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
"@npm//typescript",
],

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

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

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

@ -0,0 +1,49 @@
/**
* @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, TmplAstBoundAttribute, TmplAstNode} from '@angular/compiler';
import ts from 'typescript';
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
import {NgTemplateDiagnostic} from '../../../api';
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
const STYLE_SUFFIXES = ['px', '%', 'em'];
/**
* A check which detects when the `.px`, `.%`, and `.em` suffixes are used with an attribute
* binding. These suffixes are only available for style bindings.
*/
class SuffixNotSupportedCheck extends TemplateCheckWithVisitor<ErrorCode.SUFFIX_NOT_SUPPORTED> {
override code = ErrorCode.SUFFIX_NOT_SUPPORTED as const;
override visitNode(
ctx: TemplateContext<ErrorCode.SUFFIX_NOT_SUPPORTED>, component: ts.ClassDeclaration,
node: TmplAstNode|AST): NgTemplateDiagnostic<ErrorCode.SUFFIX_NOT_SUPPORTED>[] {
if (!(node instanceof TmplAstBoundAttribute)) return [];
if (!node.keySpan.toString().startsWith('attr.') ||
!STYLE_SUFFIXES.some(suffix => node.name.endsWith(`.${suffix}`))) {
return [];
}
const diagnostic = ctx.makeTemplateDiagnostic(
node.keySpan,
`The ${
STYLE_SUFFIXES.map(suffix => `'.${suffix}'`)
.join(', ')} suffixes are only supported on style bindings.`);
return [diagnostic];
}
}
export const factory: TemplateCheckFactory<
ErrorCode.SUFFIX_NOT_SUPPORTED, ExtendedTemplateDiagnosticName.SUFFIX_NOT_SUPPORTED> = {
code: ErrorCode.SUFFIX_NOT_SUPPORTED,
name: ExtendedTemplateDiagnosticName.SUFFIX_NOT_SUPPORTED,
create: () => new SuffixNotSupportedCheck(),
};

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

@ -13,6 +13,7 @@ import {factory as invalidBananaInBoxFactory} from './checks/invalid_banana_in_b
import {factory as missingControlFlowDirectiveFactory} from './checks/missing_control_flow_directive';
import {factory as missingNgForOfLetFactory} from './checks/missing_ngforof_let';
import {factory as nullishCoalescingNotNullableFactory} from './checks/nullish_coalescing_not_nullable';
import {factory as suffixNotSupportedFactory} from './checks/suffix_not_supported';
import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding';
export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker';
@ -24,4 +25,5 @@ export const ALL_DIAGNOSTIC_FACTORIES:
missingControlFlowDirectiveFactory,
textAttributeNotBindingFactory,
missingNgForOfLetFactory,
suffixNotSupportedFactory,
];

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

@ -0,0 +1,27 @@
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
ts_library(
name = "test_lib",
testonly = True,
srcs = ["suffix_not_supported_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/suffix_not_supported",
"//packages/compiler-cli/src/ngtsc/typecheck/testing",
"@npm//typescript",
],
)
jasmine_node_test(
name = "test",
bootstrap = ["//tools/testing:node_no_angular_es2015"],
deps = [
":test_lib",
],
)

84
packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/suffix_not_supported/suffix_not_supported_spec.ts Normal file
View file

@ -0,0 +1,84 @@
/**
* @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 {DiagnosticCategoryLabel} from '@angular/compiler-cli/src/ngtsc/core/api';
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 suffixNotSupportedFactory} from '../../../checks/suffix_not_supported';
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
runInEachFileSystem(() => {
describe('SuffixNotSupportedCheck', () => {
it('binds the error code to its extended template diagnostic name', () => {
expect(suffixNotSupportedFactory.code).toBe(ErrorCode.SUFFIX_NOT_SUPPORTED);
expect(suffixNotSupportedFactory.name)
.toBe(ExtendedTemplateDiagnosticName.SUFFIX_NOT_SUPPORTED);
});
it('should produce suffix not supported warning', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'TestCmp': `<div [attr.width.px]="1"></div>`,
},
source: 'export class TestCmp {}'
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [suffixNotSupportedFactory], {});
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SUFFIX_NOT_SUPPORTED));
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`attr.width.px`);
});
it('should not produce suffix not supported warning on a style binding', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'TestCmp': `<div [style.width.px]="1"></div>`,
},
source: 'export class TestCmp {}'
}]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [suffixNotSupportedFactory], {});
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
it('should not produce suffix not supported warning on an input', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': `<div [myInput.px]="1"></div>`,
},
source: 'export class TestCmp {}',
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker, program.getTypeChecker(), [suffixNotSupportedFactory], {});
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
expect(diags.length).toBe(0);
});
});
});