Skip to content

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Jul 4, 2024

Fixes a couple of issues when we check if a @let declaration is used before it is defined:

fix(compiler-cli): avoid duplicate diagnostics for let declarations read before definition

Fixes that in some cases @let declarations that are read before they're defined were producing multiple diagnostics.

fix(compiler-cli): used before declared diagnostic not firing for control flow blocks

When we process @if and @for blocks, we create a scope around their expressions in order to encapsulate the aliases to them. The problem is that this doesn't represent the actual structure since the expression is part of the outer scope. This surfaces by not raising the "used before declared" diagnostic for @let declarations.

These changes resolve the issue by processing the expression as a part of the parent scope.

Fixes #56842.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler target: rc This PR is targeted for the next release-candidate labels Jul 4, 2024
@ngbot ngbot bot modified the milestone: Backlog Jul 4, 2024
…ead before definition

Fixes that in some cases `@let` declarations that are read before they're defined were producing multiple diagnostics.
@crisbeto crisbeto force-pushed the let-duplicate-diag branch from fe0a8b6 to ed10722 Compare July 4, 2024 08:19
…trol flow blocks

When we process `@if` and `@for` blocks, we create a scope around their expressions in order to encapsulate the aliases to them. The problem is that this doesn't represent the actual structure since the expression is part of the outer scope. This surfaces by not raising the "used before declared" diagnostic for `@let` declarations.

These changes resolve the issue by processing the expression as a part of the parent scope.

Fixes angular#56842.
@crisbeto crisbeto changed the title fix(compiler-cli): avoid duplicate diagnostics for let declarations read before definition @let validation fixes Jul 4, 2024
this.expressionScopes.set(branch, expressionScope);
const outerScope = Scope.forNodes(this.tcb, this.scope, branch, [], null);
outerScope.render().forEach((stmt) => this.scope.addStatement(stmt));
this.expressionScopes.set(branch, outerScope);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only line that actually changed. The rest of the diff here is renaming expressionScope to outerScope to be a bit more accurate.

@crisbeto crisbeto marked this pull request as ready for review July 4, 2024 13:20
@crisbeto
Copy link
Member Author

crisbeto commented Jul 4, 2024

Since these changes also affect @if and @for, I've run a TGP. It's passing, aside from a few unrelated broken targets.

@crisbeto crisbeto requested a review from JoostK July 4, 2024 14:21
@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 4, 2024
@pkozlowski-opensource
Copy link
Member

This PR was merged into the repository by commit d7ab5c3.

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

pkozlowski-opensource pushed a commit that referenced this pull request Jul 5, 2024
…trol flow blocks (#56843)

When we process `@if` and `@for` blocks, we create a scope around their expressions in order to encapsulate the aliases to them. The problem is that this doesn't represent the actual structure since the expression is part of the outer scope. This surfaces by not raising the "used before declared" diagnostic for `@let` declarations.

These changes resolve the issue by processing the expression as a part of the parent scope.

Fixes #56842.

PR Close #56843
pkozlowski-opensource pushed a commit that referenced this pull request Jul 5, 2024
…ead before definition (#56843)

Fixes that in some cases `@let` declarations that are read before they're defined were producing multiple diagnostics.

PR Close #56843
pkozlowski-opensource pushed a commit that referenced this pull request Jul 5, 2024
…trol flow blocks (#56843)

When we process `@if` and `@for` blocks, we create a scope around their expressions in order to encapsulate the aliases to them. The problem is that this doesn't represent the actual structure since the expression is part of the outer scope. This surfaces by not raising the "used before declared" diagnostic for `@let` declarations.

These changes resolve the issue by processing the expression as a part of the parent scope.

Fixes #56842.

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

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@let variable used in @if condition but declared after the block doesn't throw NG8016

3 participants