Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(compiler): Add extended diagnostic to warn when text attributes …
…are intended to be bindings https://angular.io/guide/attribute-binding#attribute-class-and-style-bindings Angular supports `attr.`, `style.`, and `class.` binding prefixes to bind attributes, styles, and classes. If the key does not have the binding syntax `[]` or the value does not have an interpolation `{{}}`, the attribute will not be interpreted as a binding. This diagnostic warns the user when the attributes listed above will not be interpreted as bindings. resolves #46137
- Loading branch information
Showing
8 changed files
with
225 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
...s/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding/BUILD.bazel
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
load("//tools:defaults.bzl", "ts_library") | ||
|
||
ts_library( | ||
name = "text_attribute_not_binding", | ||
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", | ||
], | ||
) |
69 changes: 69 additions & 0 deletions
69
...ages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding/index.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
/** | ||
* @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, TmplAstBoundEvent, TmplAstNode} from '@angular/compiler'; | ||
import {TmplAstTextAttribute} from '@angular/compiler/public_api'; | ||
import ts from 'typescript'; | ||
|
||
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics'; | ||
import {NgTemplateDiagnostic} from '../../../api'; | ||
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api'; | ||
|
||
/** | ||
* Ensures that attributes that have the "special" angular binding prefix (attr., style., and | ||
* class.) are interpreted as bindings. For example, `<div attr.id="my-id"></div>` will not | ||
* interperet this as an `AttributeBinding` to `id` but rather just a `TmplAstTextAttribute`. This | ||
* is likely not the intent of the developer. Instead, the intent is likely to have the `id` be set | ||
* to 'my-id'. | ||
*/ | ||
class TextAttributeNotBindingSpec extends | ||
TemplateCheckWithVisitor<ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING> { | ||
override code = ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING as const; | ||
|
||
override visitNode( | ||
ctx: TemplateContext<ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING>, | ||
component: ts.ClassDeclaration, | ||
node: TmplAstNode|AST, | ||
): NgTemplateDiagnostic<ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING>[] { | ||
if (!(node instanceof TmplAstTextAttribute)) return []; | ||
|
||
const name = node.name; | ||
if ((!name.startsWith('attr.') && !name.startsWith('style.') && !name.startsWith('class.')) || | ||
!node.value) { | ||
return []; | ||
} | ||
|
||
const boundSyntax = node.sourceSpan.toString(); | ||
let errorString: string; | ||
if (name.startsWith('attr.')) { | ||
const staticAttr = name.replace('attr.', ''); | ||
errorString = `Static attributes should be written without the 'attr.' prefix: ${ | ||
staticAttr}="${node.value}".`; | ||
|
||
} else { | ||
const expectedKey = `[${name}]`; | ||
const expectedValue = | ||
node.value === 'true' || node.value === 'false' ? node.value : `'${node.value}'`; | ||
errorString = boundSyntax.replace( | ||
name, | ||
`Attribute, style, and class bindings should be enclosed with square braces: '${ | ||
expectedKey}="${expectedValue}"'.`); | ||
} | ||
const diagnostic = ctx.makeTemplateDiagnostic( | ||
node.sourceSpan, `${errorString} Find more at https://angular.io/guide/TODO`); | ||
return [diagnostic]; | ||
} | ||
} | ||
|
||
export const factory: TemplateCheckFactory< | ||
ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING, | ||
ExtendedTemplateDiagnosticName.TEXT_ATTRIBUTE_NOT_BINDING> = { | ||
code: ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING, | ||
name: ExtendedTemplateDiagnosticName.TEXT_ATTRIBUTE_NOT_BINDING, | ||
create: () => new TextAttributeNotBindingSpec(), | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
...piler-cli/src/ngtsc/typecheck/extended/test/checks/text_attribute_not_binding/BUILD.bazel
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") | ||
|
||
ts_library( | ||
name = "test_lib", | ||
testonly = True, | ||
srcs = ["text_attribute_not_binding_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/text_attribute_not_binding", | ||
"//packages/compiler-cli/src/ngtsc/typecheck/testing", | ||
"@npm//typescript", | ||
], | ||
) | ||
|
||
jasmine_node_test( | ||
name = "test", | ||
bootstrap = ["//tools/testing:node_no_angular_es2015"], | ||
deps = [ | ||
":test_lib", | ||
], | ||
) |
91 changes: 91 additions & 0 deletions
91
...echeck/extended/test/checks/text_attribute_not_binding/text_attribute_not_binding_spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/** | ||
* @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 textAttributeNotBindingFactory} from '../../../checks/text_attribute_not_binding'; | ||
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker'; | ||
|
||
runInEachFileSystem(() => { | ||
describe('TextAttributeNotBindingCheck', () => { | ||
it('binds the error code to its extended template diagnostic name', () => { | ||
expect(textAttributeNotBindingFactory.code).toBe(ErrorCode.TEXT_ATTRIBUTE_NOT_BINDING); | ||
expect(textAttributeNotBindingFactory.name) | ||
.toBe(ExtendedTemplateDiagnosticName.TEXT_ATTRIBUTE_NOT_BINDING); | ||
}); | ||
|
||
it('should produce class binding warning', () => { | ||
const fileName = absoluteFrom('/main.ts'); | ||
const {program, templateTypeChecker} = setup([{ | ||
fileName, | ||
templates: { | ||
'TestCmp': `<div class.blue="true"></div>`, | ||
}, | ||
source: 'export class TestCmp { }' | ||
}]); | ||
const sf = getSourceFileOrError(program, fileName); | ||
const component = getClass(sf, 'TestCmp'); | ||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( | ||
templateTypeChecker, program.getTypeChecker(), [textAttributeNotBindingFactory], | ||
{} /* options */); | ||
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.TEXT_ATTRIBUTE_NOT_BINDING)); | ||
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`class.blue="true"`); | ||
}); | ||
|
||
it('should produce an attribute binding warning', () => { | ||
const fileName = absoluteFrom('/main.ts'); | ||
const {program, templateTypeChecker} = setup([{ | ||
fileName, | ||
templates: { | ||
'TestCmp': `<div attr.id="bar"></div>`, | ||
}, | ||
source: 'export class TestCmp { }' | ||
}]); | ||
const sf = getSourceFileOrError(program, fileName); | ||
const component = getClass(sf, 'TestCmp'); | ||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( | ||
templateTypeChecker, program.getTypeChecker(), [textAttributeNotBindingFactory], | ||
{} /* options */); | ||
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.TEXT_ATTRIBUTE_NOT_BINDING)); | ||
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`attr.id="bar"`); | ||
}); | ||
|
||
it('should produce a style binding warning', () => { | ||
const fileName = absoluteFrom('/main.ts'); | ||
const {program, templateTypeChecker} = setup([{ | ||
fileName, | ||
templates: { | ||
'TestCmp': `<div style.margin-right.px="5"></div>`, | ||
}, | ||
source: 'export class TestCmp { }' | ||
}]); | ||
const sf = getSourceFileOrError(program, fileName); | ||
const component = getClass(sf, 'TestCmp'); | ||
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( | ||
templateTypeChecker, program.getTypeChecker(), [textAttributeNotBindingFactory], | ||
{} /* options */); | ||
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.TEXT_ATTRIBUTE_NOT_BINDING)); | ||
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`style.margin-right.px="5"`); | ||
}); | ||
}); | ||
}); |