Skip to content

Conversation

jakebailey
Copy link
Member

Fixes #1870

@Copilot Copilot AI review requested due to automatic review settings October 10, 2025 20:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR re-ports the isUsedInFunctionOrInstanceProperty function to fix issue #1870. The refactoring replaces a traditional for-loop traversal with the ast.FindAncestorOrQuit utility function for cleaner code structure and improved maintainability.

Key changes:

  • Converted loop-based ancestor traversal to use ast.FindAncestorOrQuit pattern
  • Updated return statements to use appropriate ast.FindAncestorResult values
  • Added a test case to verify blocked scope variable behavior

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
internal/checker/checker.go Refactored isUsedInFunctionOrInstanceProperty to use ast.FindAncestorOrQuit instead of manual loop traversal
testdata/tests/cases/compiler/blockedScopeVariableNotUnused1.ts Added test case for blocked scope variable usage validation
testdata/baselines/reference/compiler/blockedScopeVariableNotUnused1.* Generated baseline files for the new test case

Comment on lines +1921 to +1922
initializerOfProperty := propertyDeclaration.Initializer() == current
if initializerOfProperty {
Copy link
Member

Choose a reason for hiding this comment

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

Nit on naming and comparison order to make it more readable

Suggested change
initializerOfProperty := propertyDeclaration.Initializer() == current
if initializerOfProperty {
currentIsInitializer := current == propertyDeclaration.Initializer()
if currentIsInitializer {

or if you flip the order, I'd argue you don't need a variable anyway

Suggested change
initializerOfProperty := propertyDeclaration.Initializer() == current
if initializerOfProperty {
if current == propertyDeclaration.Initializer() {

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 1:1 ported so I don't really want to futz with it.

@jakebailey jakebailey added this pull request to the merge queue Oct 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 11, 2025
@jakebailey jakebailey added this pull request to the merge queue Oct 11, 2025
Merged via the queue into main with commit a05e479 Oct 11, 2025
22 checks passed
@jakebailey jakebailey deleted the jabaile/fix-1870 branch October 11, 2025 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TS2448 Block-scoped variable used before its declaration

2 participants