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

Check reserved identifiers in function params #129

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

littledivy
Copy link
Contributor

Adds TS1359 validation for function parameter identifiers.

error[TS1359]: ExpectedIdentifierReservedKeyword {
    span: Span {
        lo: BytePos(
            21,
        ),
        hi: BytePos(
            26,
        ),
        ctxt: #0,
    },
}
 --> a.ts:1:20
  |
1 | async function foo(await): Promise<void> {
  |                    ^^^^^

New passing conforming tests:

async/es2017/functionDeclarations/asyncFunctionDeclaration5_es2017.ts

Comment on lines +485 to +486
// TODO(@littledivy): Generalize this to work regardless of 'function' context.
fn validate_parameter_name(ctx: &mut Analyzer, p: &RParam, is_async: bool, is_generator: bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be made context-aware so it can be used with in top level and not just function params. Maybe in the parser? I saw @kdy1 that you have a similar TODO a few lines above.

Similar to https://github.com/microsoft/TypeScript/blob/3d2b4017eb6b9a2b94bc673291e56ae95e8beddd/src/compiler/binder.ts#L2178

Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to make a method of Analyzer for consistency.
I may refactor it to split the analyzer, but let's use a method at now

@kdy1
Copy link
Member

kdy1 commented Oct 28, 2022

Also, I think this should be handled by the parser.

TS1xxx errors are for parser/lexer

@kdy1 kdy1 self-assigned this Nov 1, 2022
@kdy1 kdy1 removed their assignment Nov 13, 2022
@kdy1 kdy1 added this to the 1. No false positive milestone Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants