• test(semantic): add a test for UpdateExpression #7495
• test(semantic): re-organize snapshot-based tests #7494
• fix(semantic): correctly resolve binding for return type of functions #6388 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

CodSpeed Performance Report

Merging #6388 will create unknown performance changes

_{Comparing 10-09-fix_semantic_correctly_resolve_binding_for_return_type_of_functions (f3850eb) with main (b0e1c03)}

Summary

🆕 30 new benchmarks

Benchmarks breakdown

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

...or at least I think it's a fix. @Dunqing could you please help me with this one?

Questions:

1. There are various TS types which contain FormalParameters:

• TSCallSignatureDeclaration
• TSMethodSignature
• TSConstructSignatureDeclaration
• TSFunctionType
• TSConstructorType

None of these function types have a body, so I assume we're OK to skip the extra call to resolve_references_for_current_scope for them, right?

2. Is the func.params.kind != FormalParameterKind::Signature check required for Function and ArrowFunctionExpression? Or can their params never have kind: FormalParameterKind::Signature anyway?

3. How/where would I add tests for this?

Questions:

1. There are various TS types which contain FormalParameters:

• TSCallSignatureDeclaration
• TSMethodSignature
• TSConstructSignatureDeclaration
• TSFunctionType
• TSConstructorType

None of these function types have a body, so I assume we're OK to skip the extra call to resolve_references_for_current_scope for them, right?

Right

2. Is the func.params.kind != FormalParameterKind::Signature check required for Function and ArrowFunctionExpression? Or can their params never have kind: FormalParameterKind::Signature anyway?

Skipping func.params.kind != FormalParameterKind::Signature is ok

3. How/where would I add tests for this?

I think as long as there are no test failures, it’s fine, so we don’t need to add tests for it.

2. Is the func.params.kind != FormalParameterKind::Signature check required for Function and ArrowFunctionExpression? Or can their params never have kind: FormalParameterKind::Signature anyway?

Skipping func.params.kind != FormalParameterKind::Signature is ok

Great. I've removed that.

3. How/where would I add tests for this?

I think as long as there are no test failures, it’s fine, so we don’t need to add tests for it.

But #6387 was a bug, and this PR fixes it. So there should have been a test failing before, but there wasn't. So I think we should add one. I just don't know where to put it. Can you point me in right direction please?

But #6387 was a bug, and this PR fixes it. So there should have been a test failing before, but there wasn't. So I think we should add one. I just don't know where to put it. Can you point me in right direction please?

Oh, sorry, I didn't notice that. We have two ways to write semantic tests:

1. The snapshot-based tests, you can add a test here, and then run https://github.com/oxc-project/oxc/blob/c822b48d4f7eedf4db803d158cfe7bcde24c25ea/crates/oxc_semantic/tests/main.rs#L158-L172

2. The integration test, you can add here

Either way is ok. I prefer the first way because I can see what scopes/symbols have in the test code

Let me add tests to it

Merge activity

• Nov 26, 10:41 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Nov 26, 10:47 AM EST: A user added this pull request to the Graphite merge queue.
• Nov 26, 10:53 AM EST: A user merged this pull request with the Graphite merge queue.

Thank you very much for cleaning up the mess I left!