Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Extended template checks #42984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Extended template checks #42984

wants to merge 8 commits into from

Conversation

danieltre23
Copy link
Contributor

@danieltre23 danieltre23 commented Jul 28, 2021 •
edited
Loading

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

The compiler currently doesn't really generates warning.

Issue Number: #42966

What is the new behavior?

This PR introduces the extended package which should be able to generate diagnostics after type checking (integration with the compiler should come in a future PR). The API provides an interface for TemplateChecks to be authored, the package also has a function to generate the extended template diagnostics. The banana in a box check, that validates the two-way binding syntax is implemented with it's unit test.

The extended package is private, the only modification to the public API is the new BANANA_IN_BOX ErrorCode.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@danieltre23 danieltre23 added the target: patch This PR is targeted for the next patch release label Jul 28, 2021
@google-cla google-cla bot added the cla: yes label Jul 28, 2021
@danieltre23 danieltre23 force-pushed the template-checks branch 2 times, most recently from 1db63b9 to 915515b Compare July 28, 2021 19:38
atscott
atscott requested changes Jul 28, 2021
View reviewed changes
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/api/api.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/api/api.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/test/template_checks_spec.ts Outdated Show resolved Hide resolved
@dylhunn dylhunn added the area: compiler Issues related to `ngc`, Angular's template compiler label Jul 28, 2021
@ngbot ngbot bot added this to the Backlog milestone Jul 28, 2021
dgp1130
dgp1130 requested changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on an initial proposal here, looking really good!

Mostly just a bunch of nitpicks, a few stylistic comments, and a couple questions about edge cases. Nothing too concerning with this PR, hope we can land it soon!

packages/compiler-cli/src/ngtsc/extended_template_diagnostics/BUILD.bazel Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
...piler-cli/src/ngtsc/extended_template_diagnostics/src/template_checks/banana_in_box_check.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/test/template_checks_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/test/template_checks_spec.ts Outdated Show resolved Hide resolved
@danieltre23 danieltre23 force-pushed the template-checks branch 2 times, most recently from 737eb07 to 9da58bc Compare August 2, 2021 18:46
alxhub
alxhub previously requested changes Aug 3, 2021
View reviewed changes
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/BUILD.bazel Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/api/api.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/extended_template_diagnostics/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts Outdated Show resolved Hide resolved
@danieltre23 danieltre23 force-pushed the template-checks branch 2 times, most recently from df24977 to e12f69e Compare August 4, 2021 17:05
dgp1130
dgp1130 reviewed Aug 4, 2021
View reviewed changes
packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/extended/src/template_checker.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts Outdated Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/testing/src/utils.ts Outdated Show resolved Hide resolved
packages/compiler/src/render3/r3_ast.ts
@@ -203,11 +203,15 @@ export class NullVisitor implements Visitor<void> {
export class RecursiveVisitor implements Visitor<void> {
visitElement(element: Element): void {
visitAll(this, element.attributes);
visitAll(this, element.inputs);
visitAll(this, element.outputs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this fix and for keeping it in a separate commit, much easier to understand the context this way!

AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): add test for BananaInBoxCheck (#42984)
e25d35f 
Create the test package for extended_template_diagnostics and add a unit
test for the `BananaInBoxCheck`.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): introduce extended package to typecheck with …
21e2b93 
…it's API (#42984)

This commit introduces //packages/compiler-cli/src/ngtsc/typecheck/extended
as a container for a new phase of diagnostics generation. The API provides an
interface for new template checks to implement and generate template diagnostics.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): make makeTemplateDiagnostic wrapper error (#…
a53d268 
…42984)

specific

This commit makes the wrapper function `makeTemplateDiagnostic` take an
ErrorCode as a type for the `ts.Diagnostic`s to be generated.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): introduce `getExtendedTemplateDiagnosticsForC…
029060f 
…omponent` function (#42984)

Add function to get extended template diagnostics generated by all the template checks.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): move typecheck test utils into its own package (
387dcc6 
#42984)

This commit moves the test utils used in the typechecking tests into its
own package. This makes them available to be used in the tests of a
different package.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): move getSourceCodeForDiagnostic to utils (#…
a3bc29b 
…42984)

Export `getSourceCodeForDiagnostic` from `ngtsc/testing` to make it
available for other packages. This will help confirm that the source
code is correct in other tests.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): visit inputs and outputs for elements and tem…
c42b50c 
…plates (#42984)

This commit adds to the exisiting `RecursiveVisitor` and visits inputs
and outputs for elements and templates.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): add BananaInBoxCheck to the template checks (…
0f8ae8b 
…#42984)

Add the implementation of a Template Check that ensures the correct
use of two-way binding syntax. Generates a warning when
'([foo])="bar"' is found instead of '[(foo)]="bar"'.

Refs #42966

PR Close #42984
AndrewKushnir pushed a commit that referenced this pull request Aug 10, 2021
@danieltre23 @AndrewKushnir
refactor(compiler-cli): add test for BananaInBoxCheck (#42984)
4a1d9a7 
Create the test package for extended_template_diagnostics and add a unit
test for the `BananaInBoxCheck`.

Refs #42966

PR Close #42984
alxhub
alxhub reviewed Aug 18, 2021
View reviewed changes
packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts Show resolved Hide resolved
...compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/index.ts Show resolved Hide resolved
...compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/index.ts Show resolved Hide resolved
...compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/index.ts Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts Show resolved Hide resolved
packages/compiler-cli/src/ngtsc/typecheck/extended/api/api.ts Show resolved Hide resolved
...compiler-cli/src/ngtsc/typecheck/extended/src/template_checks/invalid_banana_in_box/index.ts Show resolved Hide resolved
@danieltre23 danieltre23 mentioned this pull request Aug 19, 2021
Closed
14 tasks
@rafaelss95 rafaelss95 mentioned this pull request Sep 2, 2021
Closed
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoostK JoostK JoostK left review comments

@alxhub alxhub alxhub left review comments

@dgp1130 dgp1130 dgp1130 approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir left review comments

@IgorMinar IgorMinar IgorMinar approved these changes

@atscott atscott atscott approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.
9 participants
@danieltre23 @JoostK @IgorMinar @atscott @alxhub @dgp1130 @AndrewKushnir @jessicajaniuk @dylhunn
Morty Proxy This is a proxified and sanitized view of the page, visit original site.