Some private identifier errors are duplicated and uninformative

From privateNameInInExpression.errors.txt:

            const b = #fiel in v; // Bad - typo in privateID
                      ~~~~~
!!! error TS2304: Cannot find name '#fiel'.
                      ~~~~~
!!! error TS2339: Property '#fiel' does not exist on type 'any'.
            for (#field in v) { /**/ } // Bad - 'in' not allowed
                 ~~~~~~
!!! error TS1451: 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
                 ~~~~~~
!!! error TS2406: The left-hand side of a 'for...in' statement must be a variable or a property access.

Expected: One error on each line of code: "Cannot find name '#fiel'" and "The left-hand side of a 'for...in' statement must be a variable or property access."
Actual: Two errors on each line.

The cause is that checkGrammarPrivateIdentifierExpression does not check whether other errors might apply. From my most recent read of it, I think only the first error is actually valuable, and that the "Cannot find name '#fiel'" error can be issued elsewhere. It's not really a grammatical error in any case -- it's likely just issued there because the ES spec refers to these errors as syntactic.

Nathan Shively-Sanders (@sandersn)

Hi Nathan Shively-Sanders (@sandersn), can I try to solve this one?

Go ahead; let me know if you need any help.

Sorry, it took me a while to understand the setup and flow of the project.

From the issue, I assume you want to remove the "Cannot find name '#fiel'" when there is:
"The left-hand side of a 'for...in' statement must be a variable or property access." or
"Private identifiers are not allowed outside class bodies.".

From what I understand, the checkGrammarPrivateIdentifierExpression has 3 checks matching the 3 errors above, and it is only called when the expression syntax is private. So I cannot remove any of those if check because it affects other cases, but rather find a way to show only one of them in the case of multiple privateName errors?

// Code reference

function checkGrammarPrivateIdentifierExpression(privId: PrivateIdentifier): boolean {
    if (!getContainingClass(privId)) {
        return grammarErrorOnNode(privId, Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies);
    }
    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);
    }
    if (!getSymbolForPrivateIdentifierExpression(privId)) {
        return grammarErrorOnNode(privId, Diagnostics.Cannot_find_name_0, idText(privId));
    }
    return false;
}

Having thought about it a little more, I think I'd like to see

"Property '#fiel' does not exist on type 'any'.

for the first error and

The left-hand side of a 'for...in' statement must be a variable or a property access.

for the second. For the first error, the check for "cannot find name {0}" in checkGrammarPrivateIdentifierExpression needs to also check that privId's parent isn't an in expression. In the same way, the check for "Private identifiers are only allowed in class bodies" should also check that privId's parent isn't a for...in statement. That should avoid the duplicated errors.

Thanks, it's pretty clear now. I'll try to send a PR tomorrow.

Fix baselines from bad merge

Fix baselines from bad merge (#47177)

Fix baselines from bad merge (microsoft#47177)

fix(58754): fix missing check of private field in for-in expression, …

Navigation Menu

Repository navigation

Some private identifier errors are duplicated and uninformative #46824

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Some private identifier errors are duplicated and uninformative #46824

Description

Activity

Andrewnt219 commented on Dec 6, 2021

sandersn commented on Dec 6, 2021

Andrewnt219 commented on Dec 9, 2021

sandersn commented on Dec 10, 2021

Andrewnt219 commented on Dec 11, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions