Skip to content

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

Closed
crisbeto wants to merge 2 commits intoangular:mainfrom
crisbeto:unused-let
Closed

fix(compiler-cli): add warning for unused let declarations#57033
crisbeto wants to merge 2 commits intoangular:mainfrom
crisbeto:unused-let

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Jul 18, 2024

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.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler labels Jul 18, 2024
@ngbot ngbot Bot modified the milestone: Backlog Jul 18, 2024
@crisbeto crisbeto marked this pull request as ready for review July 18, 2024 07:57
@pullapprove pullapprove Bot requested a review from atscott July 18, 2024 07:57
thePunderWoman
thePunderWoman approved these changes Jul 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove pullapprove Bot requested a review from alxhub July 19, 2024 14:43
alxhub
alxhub approved these changes Jul 19, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 19, 2024
@crisbeto crisbeto removed the request for review from dylhunn July 19, 2024 15:47
atscott
atscott approved these changes Jul 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Jul 19, 2024

This PR was merged into the repository by commit c76b440.

The changes were merged into the following branches: main, 18.1.x

atscott pushed a commit that referenced this pull request Jul 19, 2024
@crisbeto @atscott
fix(compiler-cli): add warning for unused let declarations (#57033)
e904f34 
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
@atscott atscott closed this in c76b440 Jul 19, 2024
atscott added a commit to atscott/angular that referenced this pull request Jul 22, 2024
@atscott
Revert "fix(compiler-cli): add warning for unused let declarations (a…
718b9d5 
…ngular#57033)"

This reverts commit c76b440.
@atscott atscott reopened this Jul 22, 2024
@atscott
Copy link
Copy Markdown
Contributor

atscott commented Jul 22, 2024 •
edited
Loading

Reopening - this broke at least 1 place in g3 that looks to be an incorrect identification of an unused let variable (it is used)
https://critique.corp.google.com/cl/654849455/analysis

Edit: Also, should this really target patch? I think we usually consider diagnostics features.

atscott added a commit that referenced this pull request Jul 22, 2024
@atscott
Revert "fix(compiler-cli): add warning for unused let declarations (#…
915b0e0 
…57033)" (#57088)

This reverts commit c76b440.

PR Close #57088
atscott added a commit that referenced this pull request Jul 22, 2024
@atscott
Revert "fix(compiler-cli): add warning for unused let declarations (#…
4ac39ae 
…57033)" (#57088)

This reverts commit c76b440.

PR Close #57088
@crisbeto crisbeto removed the action: merge The PR is ready for merge by the caretaker label Jul 23, 2024
@crisbeto crisbeto added action: global presubmit The PR is in need of a google3 global presubmit target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jul 23, 2024
crisbeto added 2 commits July 23, 2024 06:56
@crisbeto
fix(compiler-cli): extended diagnostic visitor not visiting template …
9af619f 
…attributes

Fixes that the visitor which is used to implement template diagnostics isn't visiting the template attributes of structural directives.
@crisbeto
fix(compiler-cli): add warning for unused let declarations
02ac364 
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.
@crisbeto crisbeto mentioned this pull request Jul 23, 2024
Closed
@crisbeto
Copy link
Copy Markdown
Member Author

crisbeto commented Jul 23, 2024 •
edited
Loading

Also, should this really target patch? I think we usually consider diagnostics features.

My thinking was that since @let is still in developer preview, it would be fine to land this in a patch. I don't feel strongly about it so I'll switch it to minor.

Also the issue that caused the target to break has been fixed and I've run a TGP just in case (note the one unrelated failing target).

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Jul 23, 2024
@atscott
Copy link
Copy Markdown
Contributor

atscott commented Jul 23, 2024

This PR was merged into the repository by commit d4ff6bc.

The changes were merged into the following branches: main

@atscott atscott closed this in 6c2fbda Jul 23, 2024
atscott pushed a commit that referenced this pull request Jul 23, 2024
@crisbeto @atscott
fix(compiler-cli): add warning for unused let declarations (#57033)
d4ff6bc 
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
vladboisa pushed a commit to vladboisa/angular that referenced this pull request Jul 29, 2024
@crisbeto @vladboisa
fix(compiler-cli): add warning for unused let declarations (angular#5…
3ce0094 
…7033)

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 angular#57033
vladboisa pushed a commit to vladboisa/angular that referenced this pull request Jul 29, 2024
@atscott @vladboisa
Revert "fix(compiler-cli): add warning for unused let declarations (a…
059afb5 
…ngular#57033)" (angular#57088)

This reverts commit c76b440.

PR Close angular#57088
vladboisa pushed a commit to vladboisa/angular that referenced this pull request Jul 29, 2024
@crisbeto @vladboisa
fix(compiler-cli): extended diagnostic visitor not visiting template …
f0612e9 
…attributes (angular#57033)

Fixes that the visitor which is used to implement template diagnostics isn't visiting the template attributes of structural directives.

PR Close angular#57033
vladboisa pushed a commit to vladboisa/angular that referenced this pull request Jul 29, 2024
@crisbeto @vladboisa
fix(compiler-cli): add warning for unused let declarations (angular#5…
570182a 
…7033)

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 angular#57033
@josuedeleonaguilar josuedeleonaguilar mentioned this pull request Aug 2, 2024
Open
@mayankv87 mayankv87 mentioned this pull request Aug 4, 2024
Open
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot Bot commented Aug 23, 2024

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 Aug 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@atscott atscott atscott approved these changes

@thePunderWoman thePunderWoman thePunderWoman approved these changes

@alxhub alxhub alxhub 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 target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

4 participants

@crisbeto @atscott @thePunderWoman @alxhub