Skip to content

conditionals: add support for nested indentation #450

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
48 changes: 36 additions & 12 deletions deployment/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -318,18 +318,6 @@
"description": "Maintains the line breaks as written by the programmer."
}]
},
"conditionalExpression.linePerExpression": {
"description": "Whether to force a line per expression when spanning multiple lines.",
"type": "boolean",
"default": true,
"oneOf": [{
"const": true,
"description": "Formats with each part on a new line."
}, {
"const": false,
"description": "Maintains the line breaks as written by the programmer."
}]
},
"memberExpression.linePerExpression": {
"description": "Whether to force a line per expression when spanning multiple lines.",
"type": "boolean",
Expand Down Expand Up @@ -793,6 +781,42 @@
"binaryExpression.linePerExpression": {
"$ref": "#/definitions/binaryExpression.linePerExpression"
},
"conditionalExpression.linePerExpression": {
"description": "Whether to force a line per expression when spanning multiple lines.",
"type": "boolean",
"default": true,
"oneOf": [{
"const": true,
"description": "Formats with each part on a new line."
}, {
"const": false,
"description": "Maintains the line breaks as written by the programmer."
}]
},
"conditionalExpression.useNestedIndentation": {
"description": "Whether to use nested indentation within conditional expressions.",
"type": "boolean",
"default": true,
"oneOf": [{
"const": true,
"description": "Uses nested indentation in the true and false expressions."
}, {
"const": false,
"description": "Don't use nested indentation."
}]
},
"conditionalType.useNestedIndentation": {
"description": "Whether to use nested indentation within conditional types.",
"type": "boolean",
"default": true,
"oneOf": [{
"const": true,
"description": "Uses nested indentation in the true and false expressions."
}, {
"const": false,
"description": "Don't use nested indentation."
}]
},
"jsx.bracketPosition": {
"$ref": "#/definitions/jsx.bracketPosition"
},
Expand Down
24 changes: 23 additions & 1 deletion src/configuration/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,14 @@ impl ConfigurationBuilder {
self.insert("conditionalExpression.linePerExpression", value.into())
}

/// Whether to force a line per expression when spanning multiple lines.
///
/// * `true` - Formats with each part on a new line.
/// * `false` (default) - Maintains the line breaks as written by the programmer.
pub fn conditional_type_line_per_expression(&mut self, value: bool) -> &mut Self {
self.insert("conditionalType.linePerExpression", value.into())
}

/// Whether to force a line per expression when spanning multiple lines.
///
/// * `true` - Formats with each part on a new line.
Expand Down Expand Up @@ -747,6 +755,16 @@ impl ConfigurationBuilder {
self.insert("whileStatement.preferHanging", value.into())
}

/* situational indentation */

pub fn conditional_expression_use_nested_indentation(&mut self, value: bool) -> &mut Self {
self.insert("conditionalExpression.useNestedIndentation", value.into())
}

pub fn conditional_type_use_nested_indentation(&mut self, value: bool) -> &mut Self {
self.insert("conditionalType.useNestedIndentation", value.into())
}

/* force single line */

pub fn export_declaration_force_single_line(&mut self, value: bool) -> &mut Self {
Expand Down Expand Up @@ -1078,6 +1096,7 @@ mod tests {
.arrow_function_use_parentheses(UseParentheses::Maintain)
.binary_expression_line_per_expression(false)
.conditional_expression_line_per_expression(true)
.conditional_type_line_per_expression(true)
.member_expression_line_per_expression(false)
.type_literal_separator_kind(SemiColonOrComma::Comma)
.type_literal_separator_kind_single_line(SemiColonOrComma::Comma)
Expand Down Expand Up @@ -1138,6 +1157,9 @@ mod tests {
.union_and_intersection_type_prefer_hanging(true)
.variable_statement_prefer_hanging(true)
.while_statement_prefer_hanging(true)
/* situational indentation */
.conditional_expression_use_nested_indentation(false)
.conditional_type_use_nested_indentation(false)
/* member spacing */
.enum_declaration_member_spacing(MemberSpacing::Maintain)
/* next control flow position */
Expand Down Expand Up @@ -1246,7 +1268,7 @@ mod tests {
.while_statement_space_around(true);

let inner_config = config.get_inner_config();
assert_eq!(inner_config.len(), 175);
assert_eq!(inner_config.len(), 178);
let diagnostics = resolve_config(inner_config, &resolve_global_config(ConfigKeyMap::new(), &Default::default()).config).diagnostics;
assert_eq!(diagnostics.len(), 0);
}
Expand Down
4 changes: 4 additions & 0 deletions src/configuration/resolve_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub fn resolve_config(config: ConfigKeyMap, global_config: &GlobalConfiguration)
arrow_function_use_parentheses: get_value(&mut config, "arrowFunction.useParentheses", UseParentheses::Maintain, &mut diagnostics),
binary_expression_line_per_expression: get_value(&mut config, "binaryExpression.linePerExpression", false, &mut diagnostics),
conditional_expression_line_per_expression: get_value(&mut config, "conditionalExpression.linePerExpression", true, &mut diagnostics),
conditional_type_line_per_expression: get_value(&mut config, "conditionalType.linePerExpression", false, &mut diagnostics),
jsx_quote_style: get_value(&mut config, "jsx.quoteStyle", quote_style.to_jsx_quote_style(), &mut diagnostics),
jsx_multi_line_parens: get_value(&mut config, "jsx.multiLineParens", JsxMultiLineParens::Prefer, &mut diagnostics),
jsx_force_new_lines_surrounding_content: get_value(&mut config, "jsx.forceNewLinesSurroundingContent", false, &mut diagnostics),
Expand Down Expand Up @@ -169,6 +170,9 @@ pub fn resolve_config(config: ConfigKeyMap, global_config: &GlobalConfiguration)
union_and_intersection_type_prefer_hanging: get_value(&mut config, "unionAndIntersectionType.preferHanging", prefer_hanging, &mut diagnostics),
variable_statement_prefer_hanging: get_value(&mut config, "variableStatement.preferHanging", prefer_hanging, &mut diagnostics),
while_statement_prefer_hanging: get_value(&mut config, "whileStatement.preferHanging", prefer_hanging, &mut diagnostics),
/* situational indentation */
conditional_expression_use_nested_indentation: get_value(&mut config, "conditionalExpression.useNestedIndentation", false, &mut diagnostics),
conditional_type_use_nested_indentation: get_value(&mut config, "conditionalType.useNestedIndentation", false, &mut diagnostics),
/* member spacing */
enum_declaration_member_spacing: get_value(&mut config, "enumDeclaration.memberSpacing", MemberSpacing::Maintain, &mut diagnostics),
/* next control flow position */
Expand Down
7 changes: 7 additions & 0 deletions src/configuration/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ pub struct Configuration {
pub binary_expression_line_per_expression: bool,
#[serde(rename = "conditionalExpression.linePerExpression")]
pub conditional_expression_line_per_expression: bool,
#[serde(rename = "conditionalType.linePerExpression")]
pub conditional_type_line_per_expression: bool,
#[serde(rename = "jsx.quoteStyle")]
pub jsx_quote_style: JsxQuoteStyle,
#[serde(rename = "jsx.multiLineParens")]
Expand Down Expand Up @@ -405,6 +407,11 @@ pub struct Configuration {
pub variable_statement_prefer_hanging: bool,
#[serde(rename = "whileStatement.preferHanging")]
pub while_statement_prefer_hanging: bool,
/* situational indentation */
#[serde(rename = "conditionalExpression.useNestedIndentation")]
pub conditional_expression_use_nested_indentation: bool,
#[serde(rename = "conditionalType.useNestedIndentation")]
pub conditional_type_use_nested_indentation: bool,
/* member spacing */
#[serde(rename = "enumDeclaration.memberSpacing")]
pub enum_declaration_member_spacing: MemberSpacing,
Expand Down
46 changes: 35 additions & 11 deletions src/generation/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2172,9 +2172,14 @@ fn gen_conditional_expr<'a>(node: &'a CondExpr, context: &mut Context<'a>) -> Pr
let colon_token = context.token_finder.get_first_operator_after(&node.cons, ":").unwrap();
let line_per_expression = context.config.conditional_expression_line_per_expression;
let has_newline_test_cons = node_helpers::get_use_new_lines_for_nodes(&node.test, &node.cons, context.program);
let has_newline_const_alt = node_helpers::get_use_new_lines_for_nodes(&node.cons, &node.alt, context.program);
Copy link
Author

Choose a reason for hiding this comment

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

This is just a typo fix, const -> cons

let mut force_test_cons_newline = !context.config.conditional_expression_prefer_single_line && has_newline_test_cons;
let mut force_cons_alt_newline = !context.config.conditional_expression_prefer_single_line && has_newline_const_alt;
let has_newline_cons_alt = node_helpers::get_use_new_lines_for_nodes(&node.cons, &node.alt, context.program);
let use_new_lines_for_nested_conditional = context.config.conditional_expression_use_nested_indentation && {
node.parent().kind() == NodeKind::CondExpr || node.cons.kind() == NodeKind::CondExpr || node.alt.kind() == NodeKind::CondExpr
};
let mut force_test_cons_newline =
has_newline_test_cons && (!context.config.conditional_expression_prefer_single_line || use_new_lines_for_nested_conditional);
let mut force_cons_alt_newline =
has_newline_cons_alt && (!context.config.conditional_expression_prefer_single_line || use_new_lines_for_nested_conditional);
if line_per_expression && (force_test_cons_newline || force_cons_alt_newline) {
// for line per expression, if one is true then both should be true
force_test_cons_newline = true;
Expand Down Expand Up @@ -2282,7 +2287,7 @@ fn gen_conditional_expr<'a>(node: &'a CondExpr, context: &mut Context<'a>) -> Pr
items
};

if top_most_data.is_top_most {
if top_most_data.is_top_most || use_new_lines_for_nested_conditional {
items.push_condition(conditions::indent_if_start_of_line(cons_and_alt_items));
} else {
items.push_condition(indent_if_sol_and_same_indent_as_top_most(cons_and_alt_items, top_most_data.il));
Expand Down Expand Up @@ -5212,10 +5217,18 @@ fn gen_array_type<'a>(node: &'a TsArrayType, context: &mut Context<'a>) -> Print
}

fn gen_conditional_type<'a>(node: &'a TsConditionalType, context: &mut Context<'a>) -> PrintItems {
let use_new_lines =
!context.config.conditional_type_prefer_single_line && node_helpers::get_use_new_lines_for_nodes(&node.true_type, &node.false_type, context.program);
let top_most_data = get_top_most_data(node, context);
let is_parent_conditional_type = node.parent().kind() == NodeKind::TsConditionalType;
let line_per_expression = context.config.conditional_type_line_per_expression;
let use_new_lines_for_nested_conditional = context.config.conditional_type_use_nested_indentation && {
is_parent_conditional_type || node.true_type.kind() == NodeKind::TsConditionalType || node.false_type.kind() == NodeKind::TsConditionalType
};
let prefer_single_line = context.config.conditional_type_prefer_single_line;
let has_newline_extends_true = node_helpers::get_use_new_lines_for_nodes(&node.extends_type, &node.true_type, context.program);
let has_newline_true_false = node_helpers::get_use_new_lines_for_nodes(&node.true_type, &node.false_type, context.program);
let force_newline_extends_true = has_newline_extends_true && (!prefer_single_line || use_new_lines_for_nested_conditional);
let force_newline_true_false = has_newline_true_false && (!prefer_single_line || use_new_lines_for_nested_conditional);

let mut items = PrintItems::new();
let before_false_ln = LineNumber::new("beforeFalse");
let question_token = context.token_finder.get_first_operator_after(&node.extends_type, "?").unwrap();
Expand Down Expand Up @@ -5243,10 +5256,17 @@ fn gen_conditional_type<'a>(node: &'a TsConditionalType, context: &mut Context<'
items
})));

if question_comment_items.previous_lines.is_empty() {
items.push_signal(Signal::SpaceOrNewLine);
} else {
if !question_comment_items.previous_lines.is_empty() {
items.extend(question_comment_items.previous_lines);
} else if line_per_expression {
items.push_condition(conditions::new_line_if_multiple_lines_space_or_new_line_otherwise(
top_most_data.ln,
Some(before_false_ln),
));
} else if force_newline_extends_true {
items.push_signal(Signal::NewLine);
} else {
items.push_signal(Signal::SpaceOrNewLine);
}

items.push_condition({
Expand Down Expand Up @@ -5275,7 +5295,7 @@ fn gen_conditional_type<'a>(node: &'a TsConditionalType, context: &mut Context<'
items.extend(colon_comment_items.previous_lines);

// false type
if use_new_lines {
if force_newline_true_false {
items.push_signal(Signal::NewLine);
} else {
items.push_condition(conditions::new_line_if_multiple_lines_space_or_new_line_otherwise(
Expand Down Expand Up @@ -5303,7 +5323,11 @@ fn gen_conditional_type<'a>(node: &'a TsConditionalType, context: &mut Context<'
items
};

if is_parent_conditional_type {
// Unless `use_nested_indentation` is enabled, we keep the indentation the same. e.g.:
// type A<T> = T extends string ? 'string'
// : T extends number ? 'number'
// : T extends boolean ? 'boolean';
if !use_new_lines_for_nested_conditional && is_parent_conditional_type {
items.extend(false_type_generated);
} else {
items.push_condition(conditions::indent_if_start_of_line(false_type_generated));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
~~ lineWidth: 50, conditionalExpression.useNestedIndentation: true ~~
== should handle nested indentation on a basic ternary / conditional expression ==
const value = is_prod
? do1()
: is_laptop
? do2()
: do3();

[expect]
const value = is_prod
? do1()
: is_laptop
? do2()
: do3();

== should keep on single line if fits on a single line ==
const value = a ? b ? c : d : e;

[expect]
const value = a ? b ? c : d : e;

== should go multi-line when exceeding the line width ==
const value = testing ? this ? out : testing : exceedsWidth;

[expect]
const value = testing
? this ? out : testing
: exceedsWidth;

== should both go multi-line when exceeding the line width twice ==
const value = testing ? this ? out : testingTestingTestingTestingTesting : exceedsWidth;

[expect]
const value = testing
? this
? out
: testingTestingTestingTestingTesting
: exceedsWidth;
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
~~ lineWidth: 80, conditionalType.useNestedIndentation: true, conditionalType.linePerExpression: true ~~
== should handle nested indentation with extends and mapped types ==
export type R<S> = S extends Record<string, T<any, any>> ? { [K in keyof S]: ReturnType<S[K][1]> } : S extends T<infer _T, any>
? _T extends Array<infer I>
? I
: _T : S extends SZ<infer _T, any> ? _T : S extends DZ<infer _T>
? _T : never;

[expect]
export type R<S> = S extends Record<string, T<any, any>>
? { [K in keyof S]: ReturnType<S[K][1]> }
: S extends T<infer _T, any>
? _T extends Array<infer I>
? I
: _T
: S extends SZ<infer _T, any>
? _T
: S extends DZ<infer _T>
? _T
: never;

== should keep on single line if fits on a single line ==
type Test = A extends B ? C extends D ? c : d : e;

[expect]
type Test = A extends B ? C extends D ? c : d : e;
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
~~ lineWidth: 80, conditionalType.useNestedIndentation: true ~~
== should handle nested indentation with extends and mapped types ==
export type R<S> = S extends Record<string, T<any, any>> ? { [K in keyof S]: ReturnType<S[K][1]> } : S extends T<infer _T, any>
? _T extends Array<infer I>
? I
: _T : S extends SZ<infer _T, any> ? _T : S extends DZ<infer _T>
? _T : never;

[expect]
export type R<S> = S extends Record<string, T<any, any>>
? { [K in keyof S]: ReturnType<S[K][1]> }
: S extends T<infer _T, any>
? _T extends Array<infer I>
? I
: _T
: S extends SZ<infer _T, any> ? _T
: S extends DZ<infer _T>
? _T
: never;

== should keep on single line if fits on a single line ==
type Test = A extends B ? C extends D ? c : d : e;

[expect]
type Test = A extends B ? C extends D ? c : d : e;