Skip to content

fix: keep the first call's argument hug in short chains #686

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.

Sign up for GitHub

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

Merged
ryanatkn merged 1 commit into from
Aug 5, 2026
Merged

fix: keep the first call's argument hug in short chains #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions benches/js/diagnostics/corpus_stats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,10 @@ function main(): void {
if (files.length === 0) throw new Error(`No in-scope files found for ${title}`);

const overall = stats_of(files);
const per_lang = LANGUAGES.map(
(l) => ({ language: l, stats: stats_of(files.filter((f) => f.language === l)) })
).filter((r) => r.stats.files > 0);
const per_lang = LANGUAGES.map((l) => ({
language: l,
stats: stats_of(files.filter((f) => f.language === l))
})).filter((r) => r.stats.files > 0);
const groups = dir ? [] : group_by_entry(files, entry_paths);
const conc = dir ? concentration(files, root, largest_n) : [];
const big_files = files.filter((f) => f.bytes > big).sort((a, b) => b.bytes - a.bytes);
Expand Down
118 changes: 0 additions & 118 deletions crates/tsv_ts/src/printer/calls/arg_predicates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,109 +396,6 @@ pub fn is_simple_call_argument(expr: &Expression<'_>, depth: usize) -> bool {
}
}

/// Check if an expression contains a call expression with arguments (recursively).
///
/// Used to determine if a chain's first call has an argument that may need to
/// break independently. When the first call's arg contains a call WITH arguments,
/// that inner call might break, so we let each group format independently rather
/// than forcing expansion on the last call.
///
/// Empty calls like `a.b()` don't count because they won't break.
pub fn contains_call_expression(expr: &Expression<'_>) -> bool {
match expr {
// A call with arguments might break - return true
// Empty calls (no args) won't break - continue checking inside
Expression::CallExpression(call) => {
!call.arguments.is_empty() || contains_call_expression(call.callee)
}
Expression::NewExpression(new_expr) => {
!new_expr.arguments.is_empty() || contains_call_expression(new_expr.callee)
}

// Recurse into common wrapper types
Expression::MemberExpression(member) => {
contains_call_expression(member.object)
|| (member.computed && contains_call_expression(member.property))
}
Expression::TSAsExpression(e) => contains_call_expression(e.expression),
Expression::TSSatisfiesExpression(e) => contains_call_expression(e.expression),
Expression::TSTypeAssertion(e) => contains_call_expression(e.expression),
Expression::TSNonNullExpression(e) => contains_call_expression(e.expression),
Expression::TSInstantiationExpression(e) => contains_call_expression(e.expression),
Expression::AwaitExpression(e) => contains_call_expression(e.argument),
Expression::UnaryExpression(e) => contains_call_expression(e.argument),
Expression::UpdateExpression(e) => contains_call_expression(e.argument),
Expression::SpreadElement(e) => contains_call_expression(e.argument),
Expression::JsdocCast(cast) => contains_call_expression(cast.inner),
Expression::ParenthesizedExpression(paren) => contains_call_expression(paren.expression),

// Binary expressions (includes logical operators in internal AST)
Expression::BinaryExpression(e) => {
contains_call_expression(e.left) || contains_call_expression(e.right)
}
Expression::AssignmentExpression(e) => {
contains_call_expression(e.left) || contains_call_expression(e.right)
}

// Conditional expression
Expression::ConditionalExpression(e) => {
contains_call_expression(e.test)
|| contains_call_expression(e.consequent)
|| contains_call_expression(e.alternate)
}

// Sequence expression
Expression::SequenceExpression(e) => e.expressions.iter().any(contains_call_expression),

// Template literal expressions
Expression::TemplateLiteral(t) => t.expressions.iter().any(contains_call_expression),
Expression::TaggedTemplateExpression(t) => {
contains_call_expression(t.tag)
|| t.quasi.expressions.iter().any(contains_call_expression)
}

// Array/object literals
Expression::ArrayExpression(arr) => arr
.elements
.iter()
.any(|el| el.as_ref().is_some_and(contains_call_expression)),
Expression::ObjectExpression(obj) => obj.properties.iter().any(|prop| match prop {
internal::ObjectProperty::Property(p) => {
(p.computed && contains_call_expression(&p.key))
|| contains_call_expression(&p.value)
}
internal::ObjectProperty::SpreadElement(s) => contains_call_expression(s.argument),
}),

// Arrow/function expressions - check body for expression arrows
Expression::ArrowFunctionExpression(arr) => {
if let internal::ArrowFunctionBody::Expression(body) = &arr.body {
contains_call_expression(body)
} else {
false
}
}

// Simple expressions that don't contain calls
Expression::Identifier(_)
| Expression::Literal(_)
| Expression::RegexLiteral(_)
| Expression::ThisExpression(_)
| Expression::Super(_)
| Expression::MetaProperty(_)
| Expression::FunctionExpression(_)
| Expression::ClassExpression(_)
| Expression::YieldExpression(_)
| Expression::ImportExpression(_)
| Expression::ArrayPattern(_)
| Expression::ObjectPattern(_)
| Expression::AssignmentPattern(_)
| Expression::RestElement(_)
| Expression::PrivateIdentifier(_)
| Expression::TSParameterProperty(_) => false,
}
}

/// Check if arguments form a "function composition" pattern that forces expansion.
///
/// Matches Prettier's `isFunctionCompositionArgs` logic:
Expand Down Expand Up @@ -594,21 +491,6 @@ mod tests {
assert!(!is_simple_call_argument(&parse_expr(&arena, "[...x]"), 2));
}

#[test]
fn contains_call_expression_recursion() {
let arena = Bump::new();
// An empty call does not count, but we recurse into the callee.
assert!(!contains_call_expression(&parse_expr(&arena, "a.b()")));
// A call WITH arguments counts.
assert!(contains_call_expression(&parse_expr(&arena, "a.b(x)")));
// Recurse through a binary expression.
assert!(contains_call_expression(&parse_expr(&arena, "a + f(x)")));
// A computed member recurses into the property.
assert!(contains_call_expression(&parse_expr(&arena, "a[f(x)]")));
// No call anywhere.
assert!(!contains_call_expression(&parse_expr(&arena, "a + b")));
}

#[test]
fn function_composition_args_detection() {
let arena = Bump::new();
Expand Down
47 changes: 7 additions & 40 deletions crates/tsv_ts/src/printer/chain/builder/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use super::printing::{
use super::types::{ChainGroup, ChainNode, ChainNodeRefVec};
use crate::ast::internal::Expression;
use crate::printer::Printer;
use crate::printer::calls::arg_predicates::contains_call_expression;
use smallvec::smallvec;
use tsv_lang::Span;
use tsv_lang::doc::{DocBuf, arena::DocId};
Expand Down Expand Up @@ -360,19 +359,6 @@ fn build_short_chain_doc<'a>(
);
}

// Check if chain ends with member (for callback arg breaking preference)
let chain_ends_with_member = ends_with_member(rest_groups, first_groups);

// When chain ends with member and first groups have calls, prefer expanding
// first groups' call args over breaking the chain.
if first_has_calls && chain_ends_with_member {
let first_expanded_doc = build_first_groups_expanded_doc(first_groups, printer);
let mut state_first_expanded_parts: DocBuf = smallvec![first_expanded_doc];
state_first_expanded_parts.extend(rest_docs.iter().copied());
let state_first_expanded = d.concat(&state_first_expanded_parts);
return d.conditional_group(&[on_line, state_first_expanded]);
}

// Prettier's short chain behavior (member-chain.js lines 351-360):
// For chains with groups.length <= cutoff, just return group(oneLine).
if !first_has_calls {
Expand Down Expand Up @@ -412,32 +398,13 @@ fn build_short_chain_doc<'a>(
return d.group(on_line);
}

// Check for nested calls in first call's args
let first_call_arg_contains_call = first_groups
.iter()
.flat_map(|g| g.nodes.iter())
.filter_map(ChainNode::as_call_expression)
.any(|call| call.arguments.iter().any(contains_call_expression));

if !first_call_arg_contains_call {
// Prettier: group(printedGroups.flat()) for short chains (member-chain.js:351-359).
// group() lets hardlines in the first call (e.g., multiline array) render
// naturally while the second call's inner group handles its own arg layout.
return d.group(on_line);
}

// When first call's arg contains calls, try both expansion directions
let rest_expanded = build_rest_expanded_docs(rest_groups, printer);
let mut state_last_expanded_parts: DocBuf = smallvec![first_doc];
state_last_expanded_parts.extend(rest_expanded);
let state_last_expanded = d.concat(&state_last_expanded_parts);

let first_expanded_doc = build_first_groups_expanded_doc(first_groups, printer);
let mut state_first_expanded_parts: DocBuf = smallvec![first_expanded_doc];
state_first_expanded_parts.extend(rest_docs.iter().copied());
let state_first_expanded = d.concat(&state_first_expanded_parts);

d.conditional_group(&[on_line, state_last_expanded, state_first_expanded])
// Prettier: group(printedGroups.flat()) for short chains (member-chain.js:351-359).
// group() lets hardlines in the first call (e.g., multiline array) render
// naturally while each call's inner args group handles its own layout — including
// the last-argument hug of a first call whose argument breaks (`X.map((x) => ({`).
// A chain-level conditional_group here would measure the whole line flat and
// pre-empt that inner hug, force-expanding the first call's argument list instead.
d.group(on_line)
}

/// Whether the chain is `base_call(args).a.b...` — a bare base call followed by ONLY
Expand Down
Loading