refactor(ast)!: Change order of fields in CallExpression #4859

cblh · 2024-08-13T01:02:39Z

graphite-app · 2024-08-13T01:02:44Z

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

Add the label “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-hq · 2024-08-13T01:11:19Z

CodSpeed Performance Report

Merging #4859 will not alter performance

_{Comparing cblh:fix/4821 (8eaae38) with main (38d4434)}

Summary

✅ 29 untouched benchmarks

cblh · 2024-08-13T01:53:00Z

This test case failed.

oxc/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs

Lines 384 to 387 in 7a75e0f

    
           // disallows `this`/`super` in arguments of `super()`. 
        
           ("class A extends B { constructor() { super(this.c); } }", None), 
        
           ("class A extends B { constructor() { super(this.c()); } }", None), 
        
           ("class A extends B { constructor() { super(super.c()); } }", None),

Dunqing · 2024-08-13T01:59:52Z

This test case failed.

oxc/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs

Lines 384 to 387 in 7a75e0f

// disallows `this`/`super` in arguments of `super()`.

("class A extends B { constructor() { super(this.c); } }", None),

("class A extends B { constructor() { super(this.c()); } }", None),

("class A extends B { constructor() { super(super.c()); } }", None),

Seems as expected, try to fix it

rzvxa · 2024-08-13T10:59:05Z

This PR should be merged with #4851 to minimize our minor version bumps. Ideally, we'd get #4188 in this version too(at least the initial implementation).

Probably requires some minor refactoring in the rolldown codebase(if they are using builder calls that are changed).

crates/oxc_linter/src/rules/eslint/no_this_before_super.rs

overlookmotel · 2024-08-14T08:32:05Z

Sorry I closed and re-opened. I made a mistake!

overlookmotel · 2024-08-15T15:52:04Z

#4851 has already been merged, so I don't think we need to wait to merge this.

@cblh Could you please rebase on main to remove merge conflicts? (don't worry about fixing merge conflicts in generated/derive_clone_in.rs - just rebase then run just ast again to regenerate the file)

@rzvxa This currently shows as "change requested". If your requested changes have been made, please can you mark that as resolved?

rzvxa · 2024-08-15T23:45:24Z

#4851 has already been merged, so I don't think we need to wait to merge this.

@cblh Could you please rebase on main to remove merge conflicts? (don't worry about fixing merge conflicts in generated/derive_clone_in.rs - just rebase then run just ast again to regenerate the file)

@rzvxa This currently shows as "change requested". If your requested changes have been made, please can you mark that as resolved?

That one proved not to contain any breaking changes for our downstream, But I guess we can do the same thing with this PR as well.

I'll test it against rolldown and let you know. If it doesn't break anything there we may drop the bang in the title. I'll take care of merging it when CI is green once more.

@cblh We had some naming refactors recently, You should be able to make the CI green by running just ast and committing the changes.

cblh · 2024-08-16T00:18:23Z

#4851 has already been merged, so I don't think we need to wait to merge this.
@cblh Could you please rebase on main to remove merge conflicts? (don't worry about fixing merge conflicts in generated/derive_clone_in.rs - just rebase then run just ast again to regenerate the file)
@rzvxa This currently shows as "change requested". If your requested changes have been made, please can you mark that as resolved?

That one proved not to contain any breaking changes for our downstream, But I guess we can do the same thing with this PR as well.

I'll test it against rolldown and let you know. If it doesn't break anything there we may drop the bang in the title. I'll take care of merging it when CI is green once more.

@cblh We had some naming refactors recently, You should be able to make the CI green by running just ast and committing the changes.

done

rzvxa · 2024-08-16T00:34:45Z

Thank you❤️, I'll take care of the rest.

rzvxa · 2024-08-16T01:12:34Z

@Boshen This PR contains a few breaking changes that need to be addressed in the rolldown/ast_snippet.rs.

I'll let you decide on merging it now or waiting to add it with other breaking changes to avoid bumping the minor version multiple times.

It contains a few linter updates so merging would be more desirable to avoid conflicts, However, most of the diff is from the generated code and snapshots. Resolving conflicts here shouldn't be that hard.

graphite-app · 2024-08-17T14:27:11Z

Merge activity

Aug 17, 10:27 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Aug 17, 10:27 AM EDT: Boshen added this pull request to the Graphite merge queue.
Aug 17, 10:29 AM EDT: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (Stack merges are not currently supported for forked repositories. Please create a branch in the target repository in order to merge).

…e-super` rule - Added `contains_this_or_super_in_args` method to check for `this` or `super` usage in function call arguments. - Modified logic in `NoThisBeforeSuper` rule to ensure that `super()` is correctly identified as being called before any usage of `this`. - Introduced `contains_this_or_super` helper method for deeper inspection of arguments within expressions. - Updated `Semantic` handling to support the new checks.

…ore-super` rule - Added a new test case in the `no-this-before-super` rule to ensure it correctly disallows the usage of `this` or `super` in nested arguments like `super(a(b(this.c)))`. - Updated the snapshot to reflect the new test case.

…_before_super` rule - Removed the `Rc<Semantic>` reference from the `contains_this_or_super` and `contains_this_or_super_in_args` methods, simplifying their signatures. - Removed the unused `Rc` import from `std::rc`. - Updated method calls to reflect the removal of the `Semantic` argument. These changes streamline the code by eliminating redundant dependencies, improving maintainability.

…nt positioning - Updated `derive_clone_in.rs` to use the correct `allocator` parameter for consistency. - Fixed argument positioning in function calls across `exponentiation_operator.rs` and `react/refresh.rs` to ensure proper parameter passing. - These changes resolve potential runtime errors due to incorrect argument placement.

Dunqing · 2024-08-20T01:43:16Z

This change may affect the development of the transformer so I would like to merge this ASAP

## [0.8.0] - 2024-08-23 - 5f4c9ab semantic: [**BREAKING**] Rename `SymbolTable::get_flag` to `get_flags` (#5030) (overlookmotel) - ce4d469 codegen: [**BREAKING**] Remove const generic `MINIFY` (#5001) (Boshen) - b2ff2df parser: [**BREAKING**] Remove builder pattern from `Parser` struct (#5000) (Boshen) - f88970b ast: [**BREAKING**] Change order of fields in CallExpression (#4859) (Burlin) ### Features - 2292606 linter: Typescript-eslint/no-wrapper-object-types (#5022) (camc314) - a0effab linter: Support more flexible config.globals values (#4990) (Don Isaac) - cdbfcfb linter: Start import fixer for eslint/no-unused-vars (#4849) (DonIsaac) - 915cb4d linter: Add dangerous fixer for oxc only used in recursion (#4805) (camc314) - 3f28c77 linter/eslint: Improve no-dupe-keys (#4943) (DonIsaac) - e1582a5 linter/eslint: Improve no-duplicate-case rule (#4942) (DonIsaac) - f1e4611 linter/eslint-plugin-vitest: Implement no-conditional-in-test (#4971) (dalaoshu) - 14bf5d5 linter/eslint-plugin-vitest: Implement no-restricted-vi-methods (#4956) (dalaoshu) - ed9a1c4 linter/eslint-plugin-vitest: Implement require-local-test-context-for-concurrent-snapshots (#4951) (dalaoshu) - 7859f58 linter/eslint-plugin-vitest: Implement no-conditional-tests (#4955) (dalaoshu) - 841174f linter/no-unused-vars: Delete non-root arrows, skip `await` (#5083) (Don Isaac) ### Bug Fixes - 86d0c0c linter: Change consistent-function-scoping to suspicious (#5010) (DonIsaac) - 7b99386 linter: Missing closing ticks in some example blocks (#4994) (DonIsaac) - 9c64b12 linter: Improve no-zero-fractions rule for member expressions and scientific notation (#4793) (Burlin) - c43945c linter/consistent-function-scoping: Allow functions passed as arguments (#5011) (Don Isaac) - 9354779 linter/no-unused-vars: Give `argsIgnorePattern` the same default behavior as `varsIgnorePattern` (#5018) (DonIsaac) - 5a55dcf linter/no-unused-vars: `type` specifier not deleted for type imports (#5029) (DonIsaac) - 4081293 linter/no-unused-vars: Panic in fixer when removing destructures (#4923) (Don Isaac) - ddf83ff linter/react: Fixed false positive with missing key inside React.Children.toArray() (#4945) (Earl Chase) - 508644a linter/tree-shaking: Correct the calculation of `>>`, `<<` and `>>>` (#4932) (mysteryven) - e99836d linter/unicorn: Allow set spreading in no-useless-spread (#4944) (Don Isaac) - 5f8a7c2 oxlint: Rules in the configuration file are not being correctly … (#4949) (dalaoshu) ### Documentation - e331ca0 linter: Improve documentation for several rules (#4997) (DonIsaac) - cd9f1cd linter/consistent-function-scoping: Improve rule documentation (#5015) (DonIsaac) ### Refactor - eca6fdb linter: Move plugin options into separate struct (#5100) (DonIsaac) - 06f2d81 linter: Avoid unnecessary temp `Vec`s (#4963) (overlookmotel) - 4cb8c37 linter: Move default_true to utils (#4947) (Don Isaac) - ca70cc7 linter, mangler, parser, semantic, transformer, traverse, wasm: Rename various `flag` vars to `flags` (#5028) (overlookmotel) - 59d15c7 semantic: `root_unresolved_references` contain only `ReferenceId` (#4959) (overlookmotel) ### Testing - c21d735 linter/no-unused-vars: Add ignored destructuring test cases (#4922) (Don Isaac) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>

## [0.25.0] - 2024-08-23 - 78f135d ast: [**BREAKING**] Remove `ReferenceFlag` from `IdentifierReference` (#5077) (Boshen) - f2b8d82 semantic: [**BREAKING**] `ScopeTree::get_child_ids` + `get_child_ids_mut` return value not `Option` (#5058) (overlookmotel) - 5f4c9ab semantic: [**BREAKING**] Rename `SymbolTable::get_flag` to `get_flags` (#5030) (overlookmotel) - 58bf215 semantic: [**BREAKING**] Rename `Reference::flag` and `flag_mut` methods to plural (#5025) (overlookmotel) - c4c08a7 ast: [**BREAKING**] Rename `IdentifierReference::reference_flags` field (#5024) (overlookmotel) - d262a58 syntax: [**BREAKING**] Rename `ReferenceFlag` to `ReferenceFlags` (#5023) (overlookmotel) - c30e2e9 semantic: [**BREAKING**] `Reference::flag` method return `ReferenceFlag` (#5019) (overlookmotel) - ce4d469 codegen: [**BREAKING**] Remove const generic `MINIFY` (#5001) (Boshen) - b2ff2df parser: [**BREAKING**] Remove builder pattern from `Parser` struct (#5000) (Boshen) - f88970b ast: [**BREAKING**] Change order of fields in CallExpression (#4859) (Burlin) ### Features - 714373d ast: `inherit_variants!` macro add `into_*` methods (#5005) (overlookmotel) - 6800e69 oxc: Add `Compiler` and `CompilerInterface` (#4954) (Boshen) - 2b21be3 oxc_minifier: Define plugin with postfix wildcard (#4979) (IWANABETHATGUY) - afe728a parser: Parse regular expression with regex parser (#4998) (Boshen) - 4b49cf8 transformer: Always pass in symbols and scopes (#5087) (Boshen) - f51d3f9 transformer/nullish-coalescing-operator: Handles nullish coalescing expression in the FormalParamter (#4975) (Dunqing) - f794870 transformer/nullish-coalescing-operator: Generate the correct binding name (#4974) (Dunqing) - 72ff2c6 transformer/nullish-coalescing-operator: Add comments in top of file (#4972) (Dunqing) - 6b885fe traverse: Expose `generate_uid_based_on_node` and `generate_uid_in_current_scope_based_on_node` from `TraverseCtx` (#4965) (Dunqing) ### Bug Fixes - 7f3129e ast: Correct code comment (#5004) (overlookmotel) - 1bd9365 coverage: Correctly check semantic data after transform (#5035) (Boshen) - 185eb20 isolated_declarations: Namespaces that are default exported should be considered for expando functions (#4935) (michaelm) - 2a5e15d npm: `libc` field should not be `null` (Boshen) - efbdced parser: Only show flow error if it's a flow file (#5069) (Boshen) - ad2be97 semantic: Incorrect semantic check for label has same name (#5041) (heygsc) - d5de97d semantic: Transform checker check reference flags (#5092) (overlookmotel) - 90c74ee semantic: Transform checker check reference symbol IDs (#5090) (overlookmotel) - a8005b9 semantic: Transform checker check symbol redeclarations (#5089) (overlookmotel) - 205bff7 semantic: Transform checker check symbol references (#5088) (overlookmotel) - 4a57086 semantic: Transform checker check symbol IDs (#5078) (overlookmotel) - ea7d216 semantic: Transform checker check symbol spans (#5076) (overlookmotel) - 1b6b27a semantic: Transform checker check symbol flags (#5074) (overlookmotel) - 6d87b0f semantic: Fix error message for duplicated label (#5071) (Boshen) - 05fff16 semantic: Transform checker compare binding symbol IDs (#5057) (overlookmotel) - f187b71 semantic: Transform checker compare scope children (#5056) (overlookmotel) - b52c6a4 semantic: Transform checker compare scope parents (#5055) (overlookmotel) - da64014 semantic: Transform checker catch more scope flags mismatches (#5054) (overlookmotel) - 67d1a96 semantic: Transform checker compare scope flags (#5052) (overlookmotel) - 863b9cb semantic: Transform checker handle conditional scopes (#5040) (overlookmotel) - 47029c4 semantic: Transform checker output symbol names in errors (#5038) (overlookmotel) - 6ffbd78 transformer: Remove an `AstBuilder::copy` call from TS namespace transform (#4987) (overlookmotel) - a8dfdda transformer: Remove an `AstBuilder::copy` call from TS module transform (#4986) (overlookmotel) - 1467eb3 transformer: Remove an `AstBuilder::copy` call from TS enum transform (#4985) (overlookmotel) - 1365feb transformer: Remove an `AstBuilder::copy` call for TS `AssignmentTarget` transform (#4984) (overlookmotel) - edacf93 transformer: Remove an `AstBuilder::copy` call (#4983) (overlookmotel) - 3b35332 transformer/logical-assignment-operators: Fix semantic errors (#5047) (Dunqing)- b7db235 Comments gen regression (#5003) (IWANABETHATGUY) ### Documentation - 178d1bd transformer: Add documentation for exponentiation-operator plugin (#5084) (Dunqing) - d50eb72 transformer: Add documentation for `optional-catch-binding` plugin (#5064) (Dunqing) - 4425b17 transformer: Add documentation for `logical-assignment-operators` plugin (#5012) (Dunqing) - 1bd5853 transformer: Updated README re: order of methods (#4993) (overlookmotel) ### Refactor - a4247e9 allocator: Move `Box` and `Vec` into separate files (#5034) (overlookmotel) - cca7440 ast: Replace `AstBuilder::move_statement_vec` with `move_vec` (#4988) (overlookmotel) - 4012260 ast: `AstBuilder::move_identifier_reference` do not allocate empty string (#4977) (overlookmotel) - 96422b6 ast: Make AstBuilder non-exhaustive (#4925) (DonIsaac) - ca70cc7 linter, mangler, parser, semantic, transformer, traverse, wasm: Rename various `flag` vars to `flags` (#5028) (overlookmotel) - 0f64d10 minifier: Remove duplicated helper `move_out_expression` (#5007) (IWANABETHATGUY) - cd9cf5e oxc: Remove `remove_whitespace` (Boshen) - b4407c4 oxc,mangler: `oxc` crate add mangler; mangler use options API (Boshen) - 9da6a21 semantic: Rename transform checker output for reference symbol mismatches (#5091) (overlookmotel) - fb46eaf semantic: Add remap functions to transform checker (#5082) (overlookmotel) - a00bf18 semantic: Add `IdMapping` to transform checker (#5079) (overlookmotel) - b14a302 semantic: Transform checker: change symbol name mismatch error (#5075) (overlookmotel) - b8c6ce5 semantic: Rename vars in transform checker (#5072) (overlookmotel) - 7156fd2 semantic: Transform checker `Pair` structure (#5053) (overlookmotel) - 0ba6f50 semantic: Simplify raising errors in transform checker (#5051) (overlookmotel) - ee7ac8b semantic: Store all data in `PostTransformChecker` in transform checker (#5050) (overlookmotel) - 4e1f4ab semantic: Add `SemanticIds` to transformer checker (#5048) (overlookmotel) - c1da574 semantic: Add comments to transformer checker (#5045) (overlookmotel) - 8cded08 semantic: Rename error labels in transformer checker snapshots (#5044) (overlookmotel) - 602244f semantic: Rename vars in transformer checker (#5043) (overlookmotel) - ae94b9a semantic: Remove unused function params in transformer checker (#5042) (overlookmotel) - 586e15c semantic: Reformat transform checker errors (#5039) (overlookmotel) - d69e34e semantic: Fix indentation (#5037) (overlookmotel) - 4336a32 semantic: Rename fields in snapshots from `flag` to `flags` (#5032) (overlookmotel) - 83dfb14 semantic: Rename vars from `flag` to `flags` (#5031) (overlookmotel) - 3b7de18 semantic: Rename `SemanticBuilder::current_reference_flags` field (#5027) (overlookmotel) - 0bacdd8 semantic: Rename `Reference::flag` field to `flags` (#5026) (overlookmotel) - 896b92f semantic: Correct typo in doc comment (#5009) (overlookmotel) - d677b8e semantic: Do not reserve space in `resolved_references` (#4962) (overlookmotel) - a7ef30d semantic: `UnresolvedReferencesStack` contain only `ReferenceId` (#4960) (overlookmotel) - 59d15c7 semantic: `root_unresolved_references` contain only `ReferenceId` (#4959) (overlookmotel) - 7706523 span: Clarify `Atom` conversion methods lifetimes (#4978) (overlookmotel) - 4fdf26d transform_conformance: Add driver (#4969) (Boshen) - 8d15e65 transformer: Use `into_member_expression` (#5006) (overlookmotel) - 4796ece transformer: TS annotations transform use `move_expression` (#4982) (overlookmotel) - a9fcf29 transformer/es2016: Move all entry points to implementation of Traverse trait (#5085) (Dunqing) - deda6ac transformer/es2019: Move all entry points to implementation of Traverse trait (#5065) (Dunqing) - 9df2f80 transformer/es2020: Move all entry points to implementation of Traverse trait (#4973) (Dunqing) - 3f9433c transformer/es2021: Move all entry points to implementation of Traverse trait (#5013) (Dunqing) - c60a50d transformer/exponentiation-operator: Use built-in `ctx.clone_identifier_reference` (#5086) (Dunqing) - bcc8da9 transformer/logical-assignment-operator: Use `ctx.clone_identifier_reference` (#5014) (Dunqing) - 38d4434 transformer/nullish-coalescing-operator: Move internal methods to bottom of the file (#4996) (Dunqing) ### Testing - 0df1a94 semantic: Add more symbol and reference checks to `PostTransformChecker` (Boshen) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>

github-actions bot added A-linter Area - Linter A-parser Area - Parser A-ast Area - AST A-transformer Area - Transformer / Transpiler labels Aug 13, 2024

github-actions bot added the A-semantic Area - Semantic label Aug 13, 2024

cblh force-pushed the fix/4821 branch 2 times, most recently from 9cfc2c0 to fedc220 Compare August 13, 2024 10:47

rzvxa changed the title ~~chore(ast): Change order of fields in CallExpression~~ refactor(ast)!: Change order of fields in CallExpression Aug 13, 2024

rzvxa suggested changes Aug 13, 2024

View reviewed changes

crates/oxc_linter/src/rules/eslint/no_this_before_super.rs Outdated Show resolved Hide resolved

cblh requested a review from rzvxa August 13, 2024 11:44

overlookmotel closed this Aug 14, 2024

overlookmotel reopened this Aug 14, 2024

cblh force-pushed the fix/4821 branch from 2f2d691 to a70e158 Compare August 15, 2024 23:33

rzvxa approved these changes Aug 16, 2024

View reviewed changes

Boshen added the 0-merge Merge with Graphite Merge Queue label Aug 17, 2024

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Aug 17, 2024

cblh added 2 commits August 18, 2024 14:18

chore(ast): Change order of fields in CallExpression

6ebe9ff

chore(ast): Change order of fields in CallExpression

228f673

cblh added 4 commits August 18, 2024 14:18

Boshen force-pushed the fix/4821 branch from c94b41d to f7ce469 Compare August 18, 2024 06:18

Dunqing added 3 commits August 20, 2024 09:17

Merge remote-tracking branch 'origin/main' into fix/4821

5c61079

Fix

c14e30c

update semantic snap

8eaae38

Dunqing approved these changes Aug 20, 2024

View reviewed changes

Dunqing merged commit f88970b into oxc-project:main Aug 20, 2024
28 checks passed

oxc-bot mentioned this pull request Aug 23, 2024

Release oxlint v0.8.0 #5104

Merged

oxc-bot mentioned this pull request Aug 23, 2024

Release crates v0.25.0 #5107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ast)!: Change order of fields in CallExpression #4859

refactor(ast)!: Change order of fields in CallExpression #4859

cblh commented Aug 13, 2024

graphite-app bot commented Aug 13, 2024

codspeed-hq bot commented Aug 13, 2024 •

edited

Loading

cblh commented Aug 13, 2024

Dunqing commented Aug 13, 2024

rzvxa commented Aug 13, 2024 •

edited

Loading

overlookmotel commented Aug 14, 2024

overlookmotel commented Aug 15, 2024

rzvxa commented Aug 15, 2024

cblh commented Aug 16, 2024

rzvxa commented Aug 16, 2024

rzvxa commented Aug 16, 2024 •

edited

Loading

graphite-app bot commented Aug 17, 2024 •

edited

Loading

Dunqing commented Aug 20, 2024

refactor(ast)!: Change order of fields in CallExpression #4859

refactor(ast)!: Change order of fields in CallExpression #4859

Conversation

cblh commented Aug 13, 2024

graphite-app bot commented Aug 13, 2024

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

codspeed-hq bot commented Aug 13, 2024 • edited Loading

CodSpeed Performance Report

Merging #4859 will not alter performance

Summary

cblh commented Aug 13, 2024

Dunqing commented Aug 13, 2024

rzvxa commented Aug 13, 2024 • edited Loading

overlookmotel commented Aug 14, 2024

overlookmotel commented Aug 15, 2024

rzvxa commented Aug 15, 2024

cblh commented Aug 16, 2024

rzvxa commented Aug 16, 2024

rzvxa commented Aug 16, 2024 • edited Loading

graphite-app bot commented Aug 17, 2024 • edited Loading

Merge activity

Dunqing commented Aug 20, 2024

codspeed-hq bot commented Aug 13, 2024 •

edited

Loading

rzvxa commented Aug 13, 2024 •

edited

Loading

rzvxa commented Aug 16, 2024 •

edited

Loading

graphite-app bot commented Aug 17, 2024 •

edited

Loading