Skip to content

feat(prettier): fill printing CallExpression implementation#11299

Merged
graphite-app[bot] merged 1 commit intomainfrom
05-26-feat_prettier_fill_printing_callexpression_implementation
May 30, 2025
Merged

feat(prettier): fill printing CallExpression implementation#11299
graphite-app[bot] merged 1 commit intomainfrom
05-26-feat_prettier_fill_printing_callexpression_implementation

Conversation

@Dunqing
Copy link
Copy Markdown
Member

@Dunqing Dunqing commented May 26, 2025

Printing a call expression correctly is mostly done, except for printing comments. Not many tests have passed because there is a problem with ParentStack. Some places use ParentStack to check the current parent, which may be incorrect. The issue is that some printing implementations do not call push/pop, meaning the recorded parent node is not accurate—it only reflects the last pushed record. I’ve attempted to solve this problem in #11339.

@github-actions github-actions bot added A-formatter Area - Formatter C-enhancement Category - New feature or request labels May 26, 2025
Copy link
Copy Markdown
Member Author

Dunqing commented May 26, 2025


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.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented May 27, 2025

CodSpeed Instrumentation Performance Report

Merging #11299 will degrade performances by 11.62%

Comparing 05-26-feat_prettier_fill_printing_callexpression_implementation (50a98cf) with main (eae51ca)

Summary

❌ 3 regressions
✅ 35 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
formatter[cal.com.tsx] 142.7 ms 150.4 ms -5.08%
formatter[react.development.js] 9.2 ms 9.5 ms -3.92%
mangler[cal.com.tsx] 3.1 ms 3.5 ms -11.62%

@Dunqing Dunqing force-pushed the 05-26-feat_prettier_fill_printing_callexpression_implementation branch 2 times, most recently from 87bbf78 to 3eb38cc Compare May 29, 2025 06:03
@Dunqing Dunqing marked this pull request as ready for review May 30, 2025 06:40
Copilot AI review requested due to automatic review settings May 30, 2025 06:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the missing pieces for printing CallExpression in the formatter, addresses parent-stack accuracy, and updates conformance snapshots.

  • Adds a dedicated call_arguments module and removes the blanket Vec<Argument> formatter
  • Introduces current() on ParentStack and uses it in a new is_long_curried_call helper
  • Supplies an overload for arrow functions with custom options and wires up member-chain utilities
  • Updates TypeScript and JavaScript conformance snapshots to reflect new output

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/oxc_formatter/src/write/mod.rs Added call_arguments submodule and related imports
crates/oxc_formatter/src/write/arrow_function_expression.rs Added new_with_options constructor for arrow funcs
crates/oxc_formatter/src/utils/mod.rs Imported ParentStack; added is_long_curried_call
crates/oxc_formatter/src/utils/member_chain/simple_argument.rs Introduced SimpleArgument logic for call heuristics
crates/oxc_formatter/src/formatter/parent_stack.rs Added current accessor alongside parent
crates/oxc_formatter/src/formatter/formatter.rs Exposed current_kind helper
tasks/prettier_conformance/snapshots/prettier.ts.snap.md Updated TS conformance snapshot
tasks/prettier_conformance/snapshots/prettier.js.snap.md Updated JS conformance snapshot
Comments suppressed due to low confidence (1)

crates/oxc_formatter/src/write/arrow_function_expression.rs:166

  • The new new_with_options constructor isn’t covered by any unit or integration tests yet. Add tests to verify formatting variations when custom options are provided.
pub fn new_with_options(

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label May 30, 2025
Copy link
Copy Markdown
Member

Boshen commented May 30, 2025

Merge activity

Printing a call expression correctly is mostly done, except for printing comments. Not many tests have passed because there is a problem with `ParentStack`. Some places use `ParentStack` to check the current parent, which may be incorrect. The issue is that some printing implementations do not call `push`/`pop`, meaning the recorded parent node is not accurate—it only reflects the last pushed record. I’ve attempted to solve this problem in #11339.
@graphite-app graphite-app bot force-pushed the 05-26-feat_prettier_fill_printing_callexpression_implementation branch from 1d29850 to 50a98cf Compare May 30, 2025 14:33
@graphite-app graphite-app bot merged commit 50a98cf into main May 30, 2025
24 of 25 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 30, 2025
@graphite-app graphite-app bot deleted the 05-26-feat_prettier_fill_printing_callexpression_implementation branch May 30, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-formatter Area - Formatter C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants