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

fix(semantic): remove inherting ScopeFlags::Modifiers from parent scope #7932

Conversation

Dunqing
Copy link
Member

@Dunqing Dunqing commented Dec 16, 2024

close: #7900

After #4283 changed, we don't need to inherit ScopeFlags from the constructor, set, get anymore, I think this is a logic of forgetting to remove

@Dunqing Dunqing requested a review from Boshen as a code owner December 16, 2024 06:59
Copy link
Member Author

Dunqing commented Dec 16, 2024


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

@github-actions github-actions bot added A-semantic Area - Semantic C-bug Category - Bug labels Dec 16, 2024
@Dunqing Dunqing force-pushed the 12-16-fix_semantic_remove_inherting_scopeflags_modifier_from_parent_scope branch from e7a4bfb to f5da19a Compare December 16, 2024 06:59
@github-actions github-actions bot added the A-transformer Area - Transformer / Transpiler label Dec 16, 2024
Copy link

codspeed-hq bot commented Dec 16, 2024

CodSpeed Performance Report

Merging #7932 will not alter performance

Comparing 12-16-fix_semantic_remove_inherting_scopeflags_modifier_from_parent_scope (14c51ff) with main (850dd43)

Summary

✅ 29 untouched benchmarks

@Dunqing
Copy link
Member Author

Dunqing commented Dec 16, 2024

https://github.com/oxc-project/oxc/actions/runs/12347529516/job/34454785628?pr=7932
I just realized why we still need to inherit when I saw the failed linter test, But I think we should fix this test in that linter rule rather than by inheriting ScopeFlags.

EDIT:
Fixed in 7e8ba1a

@github-actions github-actions bot added the A-linter Area - Linter label Dec 16, 2024
crates/oxc_semantic/examples/semantic.rs Outdated Show resolved Hide resolved
crates/oxc_semantic/examples/semantic.rs Outdated Show resolved Hide resolved
@Dunqing Dunqing force-pushed the 12-16-fix_semantic_remove_inherting_scopeflags_modifier_from_parent_scope branch from 7e8ba1a to 220f2cb Compare December 16, 2024 08:39
@Dunqing Dunqing requested a review from Boshen December 16, 2024 08:46
@Boshen Boshen removed their request for review December 16, 2024 09:11
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Dec 16, 2024
Copy link
Contributor

overlookmotel commented Dec 16, 2024

Merge activity

  • Dec 16, 6:25 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 16, 6:25 AM EST: A user added this pull request to the Graphite merge queue.
  • Dec 16, 6:30 AM EST: A user merged this pull request with the Graphite merge queue.

…ope (#7932)

close: #7900

After #4283 changed, we don't need to inherit `ScopeFlags` from the `constructor`, `set`, `get` anymore, I think this is a logic of forgetting to remove
@overlookmotel overlookmotel force-pushed the 12-16-fix_semantic_remove_inherting_scopeflags_modifier_from_parent_scope branch from 220f2cb to 14c51ff Compare December 16, 2024 11:26
@overlookmotel overlookmotel changed the title fix(semantic): remove inherting ScopeFlags::Modifier from parent scope fix(semantic): remove inherting ScopeFlags::Modifiers from parent scope Dec 16, 2024
@graphite-app graphite-app bot merged commit 14c51ff into main Dec 16, 2024
27 checks passed
@graphite-app graphite-app bot deleted the 12-16-fix_semantic_remove_inherting_scopeflags_modifier_from_parent_scope branch December 16, 2024 11:30
Dunqing pushed a commit that referenced this pull request Dec 16, 2024
#7932 removed the only usage of `ScopeFlags::Modifiers`. So we don't need it any more. Remove it.
Boshen added a commit that referenced this pull request Dec 17, 2024
## [0.15.3] - 2024-12-17

### Features

- 25ddb35 linter: Add the import/no_named_default rule (#7902)
(Guillaume Piedigrossi)
- ee26b44 linter: Enhance `get_element_type` to resolve more element
types (#7885) (dalaoshu)

### Bug Fixes

- 6f41d92 linter: False positive in `unicorn/no-useless-spread` (#7940)
(dalaoshu)
- 0867b40 linter: Fix configuration parser for `no-restricted-imports`
(#7921) (Alexander S.)
- 9c9b73d linter: Fix incorrect fixer for `prefer-regexp-test` (#7898)
(Cameron)
- 32935e6 linter: False positive in
`jsx-a11y/label-has-associated-control` (#7881) (dalaoshu)
- 14c51ff semantic: Remove inherting `ScopeFlags::Modifier` from parent
scope (#7932) (Dunqing)

### Refactor

- 3858221 global: Sort imports (#7883) (overlookmotel)
- b99ee37 linter: Move rule "no-restricted-imports" to nursery (#7897)
(Alexander S.)
- ff2a68f linter/yoda: Simplify code (#7941) (overlookmotel)

### Styling

- 7fb9d47 rust: `cargo +nightly fmt` (#7877) (Boshen)

Co-authored-by: Boshen <[email protected]>
Dunqing added a commit that referenced this pull request Dec 18, 2024
## [0.42.0] - 2024-12-18

- 84b75a0 semantic: [**BREAKING**] Remove `ScopeFlags::Modifiers`
(#7935) (overlookmotel)

- c071494 semantic: [**BREAKING**] Remove `SymbolTable::rename` method
(#7868) (overlookmotel)

### Features

- 8b7c5ae ast: Add `AstBuilder::atom_from_cow` (#7974) (overlookmotel)
- 46e2e27 data_structures: Implement `Default` for `NonEmptyStack`
(#7946) (overlookmotel)
- db9e93b mangler: Mangle top level variables (#7907) (翠 / green)
- 075bd16 minifier: Fold bitwise operation (#7908) (翠 / green)
- c16a851 napi/transform: Add `jsx: 'preserve'` option (#7965) (Boshen)
- 81eedb1 parser: 'readonly' type modifier is only permitted on array
and tuple literal types. (#7880) (Boshen)
- b9322c6 semantic: Re-export all flags and ID types (#7886)
(overlookmotel)
- c30a982 span: Add `impl From<ArenaString> for Atom` (#7973)
(overlookmotel)
- 02b653c transformer/class-properties: Do not create temp var for
template literal computed key (#7919) (overlookmotel)
- feac02e transformer/class-properties: Only rename symbols if necessary
(#7896) (overlookmotel)
- 6bc530d transformer/class-properties: Transform super call expression
that is inside static prop initializer (#7831) (Dunqing)
- 53e2bc0 traverse: Add `TraverseScoping::rename_symbol` method (#7871)
(overlookmotel)

### Bug Fixes

- 3659e6d cfg: Include export default code in CFG instructions (#7862)
(Jan Olaf Martin)
- 850dd43 codegen: Missing `,` when generating type parameters with jsx
(#7929) (Dunqing)
- 4799471 minfier: Bigint bitwise operation only works with bigint
(#7937) (Boshen)
- de8a86e minifier: Incorrect minification in `try_fold_left_child_op`
(#7949) (翠 / green)
- 9a30910 oxc_transformer: Inject_global_variables should considering
string imported name (#7768) (IWANABETHATGUY)
- 111dc52 parser: Include export token in spans of
TSNamespaceExportDeclaration (#7963) (branchseer)
- 14c51ff semantic: Remove inherting `ScopeFlags::Modifier` from parent
scope (#7932) (Dunqing)
- 596aead semantic: Reset references flags when resolved (#7923)
(Dunqing)
- 4924073 semantic: `ScopeTree::rename_binding` preserve order of
bindings (#7870) (overlookmotel)
- bb38065 transformer/class-properties: Do not transform `super.prop` in
nested method within static prop initializer (#7978) (overlookmotel)
- e76fbb0 transformer/class-properties: Fix symbol clashes in instance
prop initializers (#7872) (overlookmotel)
- c0576fa transformer/class-properties: Use UID for `args` in created
class constructor (#7866) (overlookmotel)
- d660d8d transformer/optional-chaining: Do not create unused reference
when `noDocumentAll` assumption (#7847) (overlookmotel)
- 4920c6a transformer/optional-chaining: Avoid creating a useless
reference when `noDocumentAll` is true (#7832) (Dunqing)

### Performance

- a5f04a7 ast: Faster `Comment::is_jsdoc` (#7905) (overlookmotel)
- 4b24335 codegen: Improve printing of statement comments (#7857)
(Boshen)
- 71a40a2 codegen: Guard comment printing comments when there are no
comments (#7856) (Boshen)
- b31f123 transformer/class-properties: Do not re-generate same method
key (#7915) (overlookmotel)
- 8ca8fce transformer/class-properties: Reduce work updating scopes when
transforming static prop initializers (#7904) (overlookmotel)
- 80d0b3e transformer/class-properties: Fast path for instance prop
initializer scope re-parenting (#7901) (overlookmotel)
- 38aafa2 transformer/class-properties: Reduce size of
`transform_call_expression_for_super_member_expr` (#7859)
(overlookmotel)

### Documentation

- e49de81 ast: Document `Expression::is_*` methods (#7853)
(overlookmotel)
- 10a86b9 transformer: Fix comments (#7925) (overlookmotel)
- f4cb5d3 transformer: Clarify comment (#7918) (overlookmotel)
- 41a1456 transformer/class-properties: Correct doc comments (#7966)
(overlookmotel)
- 18441af transformer/class-properties: Remove oudated todo for
assignment expression (#7955) (Dunqing)
- 1317c00 transformer/class-properties: Clarify doc comments (#7914)
(overlookmotel)
- 9989b58 transformer/class-properties: Re-order file list in doc
comment (#7911) (overlookmotel)
- 7390048 transformer/class-properties: Reformat doc comment (#7909)
(overlookmotel)

### Refactor

- beb982a ast: Use exhaustive match for `Argument` to
`ArrayExpressionElement` conversion (#7848) (overlookmotel)
- 3858221 global: Sort imports (#7883) (overlookmotel)
- 1314c97 minifier: Expose dce as an API instead of an option (#7957)
(Boshen)
- 6551dfe semantic: Pass `&str` instead of `Cow` (#7972) (overlookmotel)
- b8d2bd2 semantic: Move determining references flags for export
specifier to `visit_export_named_declaration` (#7924) (Dunqing)
- 98d7946 semantic: Import flags and ID types from `oxc_syntax` (#7887)
(overlookmotel)
- 1cf8f8f semantic: `SymbolTable::set_name` return old name (#7869)
(overlookmotel)
- 5d42df8 semantic: Use `Expression::is_super` (#7851) (overlookmotel)
- 8cf9766 semantic, syntax, wasm: Remove `#![allow(non_snake_case)]`
(#7863) (overlookmotel)
- d59bbae transformer: Remove unneeded lint `#[allow]` (#7971)
(overlookmotel)
- 2c94236 transformer: Improve encapsulation of transforms (#7888)
(overlookmotel)
- 34091b2 transformer: Use `Expression::is_super` (#7852)
(overlookmotel)
- d4d7bc0 transformer/async-to-generator: Avoid allocating unnecessary
`Atom`s (#7975) (overlookmotel)
- 2e5ffd3 transformer/class-properties: Store `temp_var_is_created` on
`ClassBindings` (#7981) (overlookmotel)
- 27cc6da transformer/class-properties: Store `is_declaration` only on
`ClassDetails` (#7980) (overlookmotel)
- ee282f8 transformer/class-properties: Remove `move_expression`s
(#7979) (overlookmotel)
- 94b376a transformer/class-properties: Simplify logic for when to
create temp binding (#7977) (overlookmotel)
- ff9d1b3 transformer/class-properties: Comments about shorter output
(#7976) (overlookmotel)
- 6fc40f0 transformer/class-properties: Pass `BoundIdentifier`s by
reference (#7968) (overlookmotel)
- 69eeeea transformer/class-properties: Methods take `&self` where
possible (#7967) (overlookmotel)
- 98340bb transformer/class-properties: Use stack of `ClassDetails`
(#7947) (overlookmotel)
- 088dd48 transformer/class-properties: Shorten code (#7913)
(overlookmotel)
- 544ffbf transformer/class-properties: Split up code into multiple
files (#7912) (overlookmotel)
- dcaf674 transformer/class-properties: Rename file (#7910)
(overlookmotel)
- 6243980 transformer/class-properties: Instance prop inits visitor use
`Visit` (#7867) (overlookmotel)
- eb47d43 transformer/class-properties: Re-use existing `Vec` (#7854)
(overlookmotel)
- 1380b7b transformer/class-properties: Reduce visibility of method
(#7858) (overlookmotel)
- 0f5e078 transformer/class-properties: Rename `*_owner` to `owned_*`
(#7855) (Dunqing)
- 4ea90d4 transformer/react-refresh: Calculate signature key once
(#7970) (Dunqing)
- 15b9bff transformer/typescript: Reuse `Atom` (#7969) (overlookmotel)

### Styling

- fb897f6 data_structures: Add line break (#7882) (overlookmotel)
- 7fb9d47 rust: `cargo +nightly fmt` (#7877) (Boshen)

### Testing

- 523d48c transformer: Move named test to exports folder (#7922)
(Dunqing)
- e766051 transformer: Skip test which uses filesystem under miri
(#7874) (overlookmotel)
- f39e65e transformer: Prevent lint error when running miri (#7873)
(overlookmotel)

Co-authored-by: Dunqing <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-merge Merge with Graphite Merge Queue A-linter Area - Linter A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler C-bug Category - Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Semantic: ScopeFlags for a class in a class constructor has ScopeFlags::Constructor
3 participants