Skip to content
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.

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

for (const x in #b) is allowed #58754

Open
Josh-Cena opened this issue Jun 3, 2024 · 1 comment Β· May be fixed by #58798
Open

for (const x in #b) is allowed #58754

Josh-Cena opened this issue Jun 3, 2024 · 1 comment Β· May be fixed by #58798
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@Josh-Cena
Copy link
Contributor

πŸ”Ž Search Terms

Private identifier, for...in

πŸ•— Version & Regression Information

  • This changed between versions 4.5.5 and 4.6.4

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.5.0-dev.20240603#code/MYGwhgzhAECC0G8BQ1oGIBGAKAlIlq0AZgPYBO0WwJAdhAC7RjQCWN6Geyh0Avgf35A

πŸ’» Code

class A {
  #b() {
    for (const a in #b) {
    }
  }
}

πŸ™ Actual behavior

No errors

πŸ™‚ Expected behavior

Error: Private identifiers are only allowed in class bodies and may only be used as part of a class member declaration, property access, or on the left-hand-side of an 'in' expression

Additional information about the issue

Noticed this while working on typescript-eslint/typescript-eslint#9232. There's a very suspicious condition in code: https://github.com/microsoft/TypeScript/blob/5a4134470128a062a8297f404dfb3321f8f55798/src/compiler/checker.ts#L33699

    function checkGrammarPrivateIdentifierExpression(privId: PrivateIdentifier): boolean {
        if (!getContainingClass(privId)) {
            return grammarErrorOnNode(privId, Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies);
        }

        if (!isForInStatement(privId.parent)) {
            if (!isExpressionNode(privId)) {
                return grammarErrorOnNode(privId, Diagnostics.Private_identifiers_are_only_allowed_in_class_bodies_and_may_only_be_used_as_part_of_a_class_member_declaration_property_access_or_on_the_left_hand_side_of_an_in_expression);
            }

            const isInOperation = isBinaryExpression(privId.parent) && privId.parent.operatorToken.kind === SyntaxKind.InKeyword;
            if (!getSymbolForPrivateIdentifierExpression(privId) && !isInOperation) {
                return grammarErrorOnNode(privId, Diagnostics.Cannot_find_name_0, idText(privId));
            }
        }

        return false;
    }

I don't know what isForInStatement is supposed to do there. I was unable to find the commit that introduced thisβ€”git blame took me to #44648 but that didn't have this condition.

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Jun 3, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jun 3, 2024
@bernard-wang
Copy link

Launched bug fix #58798 it seems that some of the logics were introduced in the bug fix #46824 .

Some of the thoughts here: first into logic getContainingClass(), after that, check whether it's in a for-in block and privId is the initializer part of the for-in, if so, this would be caught by the checkForInStatement().
Then into the isExpressionNode() logic, which would only allow privId be the left side of the In-expression, which the problem of right sight of for-in block would solved with a Error Diagnosis.
After these logic, we can make sure only privIds in In-expression come out and then we check its declaration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
3 participants