From 32c1818b34936670c8207c31179318426988e155 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Sat, 25 Jan 2025 15:48:11 -0600 Subject: [PATCH 01/16] add test case --- .../src/rules/unicorn/switch_case_braces.rs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index bd2cb783643c1..5341216c51c0e 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -227,6 +227,34 @@ fn test() { "switch(foo) { default: doSomething(); }", Some(serde_json::json!(["avoid"])), ), + ( + " + const alpha = 7 + let beta = '' + let gamma = 0 + + switch (alpha) { + case 1: + beta = 'one' + gamma = 1 + break + } + ", + " + const alpha = 7 + let beta = '' + let gamma = 0 + + switch (alpha) { + case 1: { + beta = 'one' + gamma = 1 + break + } + } + ", + None, + ) ]; Tester::new(SwitchCaseBraces::NAME, SwitchCaseBraces::PLUGIN, pass, fail) From 4795658111fea5b2bf8bc496cb2b6a09df958d6d Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Sat, 25 Jan 2025 15:59:42 -0600 Subject: [PATCH 02/16] find error in code, print new line; still need to figure out better solution --- Cargo.toml | 2 +- .../src/rules/unicorn/switch_case_braces.rs | 87 +++++++++++-------- 2 files changed, 50 insertions(+), 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 974c802bf7723..d8737490df139 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -209,7 +209,7 @@ ignored = ["napi", "oxc_transform_napi", "oxc_parser_napi", "prettyplease"] [profile.dev] # Disabling debug info speeds up local and CI builds, # and we don't rely on it for debugging that much. -debug = false +debug = true [profile.dev.package] # Compile macros with some optimizations to make consuming crates build faster diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index 5341216c51c0e..ea4cbc4464af1 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -158,7 +158,15 @@ impl Rule for SwitchCaseBraces { formatter.print_ascii_byte(b'{'); let source_text = ctx.source_text(); + println!("source_text: {source_text}"); + for x in &case.consequent { + if let Statement::ExpressionStatement(stmt) = x { + formatter.print_str("\n"); + + println!("stmt: {stmt:?}"); + }; + formatter.print_str(x.span().source_text(source_text)); } @@ -167,6 +175,8 @@ impl Rule for SwitchCaseBraces { formatter.into_source_text() }; + println!("modified_code: {modified_code}"); + fixer.replace(case.span, modified_code) }, ); @@ -184,49 +194,50 @@ impl Rule for SwitchCaseBraces { fn test() { use crate::tester::Tester; - let pass = vec![ - "switch(something) { case 1: case 2: {console.log('something'); break;}}", - "switch(foo){ case 1: { break; } }", - "switch(foo){ case 1: { ; /* <-- not empty */} }", - "switch(foo){ case 1: { {} /* <-- not empty */} }", - "switch(foo){ case 1: { break; } }", - "switch(foo){ default: { doSomething(); } }", + let pass: Vec<&str> = vec![ + // "switch(something) { case 1: case 2: {console.log('something'); break;}}", + // "switch(foo){ case 1: { break; } }", + // "switch(foo){ case 1: { ; /* <-- not empty */} }", + // "switch(foo){ case 1: { {} /* <-- not empty */} }", + // "switch(foo){ case 1: { break; } }", + // "switch(foo){ default: { doSomething(); } }", ]; - let fail = vec![ - "switch(s){case'':/]/}", - "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", - "switch(something) { case 1: case 2: console.log('something'); break;}", - "switch(foo) { case 1: {} case 2: {} default: { doSomething(); } }", - "switch(foo) { case 1: { /* fallthrough */ } default: {}/* fallthrough */ case 3: { doSomething(); break; } }", - "switch(foo) { default: doSomething(); }", - "switch(foo) { case 1: { doSomething(); } break; /* <-- This should be between braces */ }", - "switch(foo) { default: label: {} }", - "switch(something) { case 1: case 2: { console.log('something'); break; } case 3: console.log('something else'); }", + let fail: Vec<&str> = vec![ + // "switch(s){case'':/]/}", + // "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", + // "switch(something) { case 1: case 2: console.log('something'); break;}", + // "switch(foo) { case 1: {} case 2: {} default: { doSomething(); } }", + // "switch(foo) { case 1: { /* fallthrough */ } default: {}/* fallthrough */ case 3: { doSomething(); break; } }", + // "switch(foo) { default: doSomething(); }", + // "switch(foo) { case 1: { doSomething(); } break; /* <-- This should be between braces */ }", + // "switch(foo) { default: label: {} }", + // "switch(something) { case 1: case 2: { console.log('something'); break; } case 3: console.log('something else'); }", ]; let fix = vec![ - ( - "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", - "switch(something) { case 1: case 2: {console.log('something'); break;}}", - None, - ), - ( - "switch(something) { case 1: {} case 2: console.log('something'); break;}", - "switch(something) { case 1: case 2: {console.log('something');break;}}", - None, - ), - ( - "switch(foo) { default: doSomething(); }", - "switch(foo) { default: {doSomething();} }", - None, - ), - ("switch(s){case'':/]/}", "switch(s){case '': {/]/}}", None), - ( - "switch(foo) { default: {doSomething();} }", - "switch(foo) { default: doSomething(); }", - Some(serde_json::json!(["avoid"])), - ), + // ( + // "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", + // "switch(something) { case 1: case 2: {console.log('something'); break;}}", + // None, + // ), + // ( + // "switch(something) { case 1: {} case 2: console.log('something'); break;}", + // "switch(something) { case 1: case 2: {console.log('something');break;}}", + // None, + // ), + // ( + // "switch(foo) { default: doSomething(); }", + // "switch(foo) { default: {doSomething();} }", + // None, + // ), + // ("switch(s){case'':/]/}", "switch(s){case '': {/]/}}", None), + // ( + // "switch(foo) { default: {doSomething();} }", + // "switch(foo) { default: doSomething(); }", + // Some(serde_json::json!(["avoid"])), + // ), + // Issue: https://github.com/oxc-project/oxc/issues/8491 ( " const alpha = 7 From 22855d9920d42a769b10a40866b11935d235fdff Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Sat, 25 Jan 2025 16:50:00 -0600 Subject: [PATCH 03/16] get closer to formatting --- .../src/rules/unicorn/switch_case_braces.rs | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index ea4cbc4464af1..146027e1d0211 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -160,12 +160,35 @@ impl Rule for SwitchCaseBraces { let source_text = ctx.source_text(); println!("source_text: {source_text}"); + let mut consequent_num = 0; + let case_test = case.test.as_ref().unwrap().span(); + let mut prev_span_end = case_test.end; + + println!("case_test: {:?}", case_test); + for x in &case.consequent { - if let Statement::ExpressionStatement(stmt) = x { + if matches!(x, Statement::ExpressionStatement(_) | Statement::BreakStatement(_)) { formatter.print_str("\n"); + let mut space_diff: usize = (x.span().start - prev_span_end).try_into().unwrap(); + + if consequent_num == 0 {space_diff -= 3;} else { + space_diff -= 1; + } + + let space = &" ".repeat(space_diff); + println!("space_diff: {space_diff}"); + formatter.print_str(space); + + + + prev_span_end = x.span().end; + + consequent_num += 1; - println!("stmt: {stmt:?}"); - }; + // TODO: figure out how to add correct spaces using spans. + // println!("stmt: {x:?}"); + // println!("stmt.span: {:?}", x.span()); + } formatter.print_str(x.span().source_text(source_text)); } From 8f9b13b8a89f89da2876147860fcf41a5376d2c8 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Sat, 25 Jan 2025 16:56:15 -0600 Subject: [PATCH 04/16] add todo comment --- crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index 146027e1d0211..ef72c032240eb 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -168,13 +168,15 @@ impl Rule for SwitchCaseBraces { for x in &case.consequent { if matches!(x, Statement::ExpressionStatement(_) | Statement::BreakStatement(_)) { - formatter.print_str("\n"); let mut space_diff: usize = (x.span().start - prev_span_end).try_into().unwrap(); if consequent_num == 0 {space_diff -= 3;} else { space_diff -= 1; } + formatter.print_str("\n"); + + let space = &" ".repeat(space_diff); println!("space_diff: {space_diff}"); formatter.print_str(space); @@ -195,6 +197,8 @@ impl Rule for SwitchCaseBraces { formatter.print_ascii_byte(b'}'); + // TODO: add space diff between discriminant and first case + formatter.into_source_text() }; From d676b149448d1834d55383cd3045d961e6e42be3 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Sun, 26 Jan 2025 00:29:04 -0600 Subject: [PATCH 05/16] add comment --- crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index ef72c032240eb..9ddb0c6037238 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -159,6 +159,8 @@ impl Rule for SwitchCaseBraces { let source_text = ctx.source_text(); println!("source_text: {source_text}"); + // better way to do this is to index into source text + // maybe create get_indent_str fn let mut consequent_num = 0; let case_test = case.test.as_ref().unwrap().span(); From d39a837660f55e48a0a59bf51eae678523c72dd2 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 17:42:04 -0600 Subject: [PATCH 06/16] get test to pass --- crates/oxc_linter/src/ast_util.rs | 18 +++ .../src/rules/unicorn/switch_case_braces.rs | 119 ++++++++---------- 2 files changed, 72 insertions(+), 65 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index be7dbf6295be6..bec04bb1db0e6 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -487,6 +487,24 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { } } +pub fn get_preceding_indent_str<'a>(source_text: &'a str, span: Span) -> Option<&'a str> { + // slice source text until start of given span, then get the preceding spaces from the last line of the source text. + let span_start = span.start as usize; + let preceding_source_text = &source_text[..span_start]; + + match preceding_source_text.lines().last() { + Some(line) => { + // only return if the line is whitespace. + if line.trim().is_empty() { + Some(line) + } else { + None + } + } + None => None, + } +} + pub fn could_be_error(ctx: &LintContext, expr: &Expression) -> bool { match expr.get_inner_expression() { Expression::NewExpression(_) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index 9ddb0c6037238..f4611a84a658d 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -3,7 +3,7 @@ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{ast_util::get_preceding_indent_str, context::LintContext, rule::Rule, AstNode}; #[derive(Clone, Copy)] enum Diagnostic { @@ -158,43 +158,29 @@ impl Rule for SwitchCaseBraces { formatter.print_ascii_byte(b'{'); let source_text = ctx.source_text(); - println!("source_text: {source_text}"); - // better way to do this is to index into source text - // maybe create get_indent_str fn - - let mut consequent_num = 0; - let case_test = case.test.as_ref().unwrap().span(); - let mut prev_span_end = case_test.end; - - println!("case_test: {:?}", case_test); for x in &case.consequent { if matches!(x, Statement::ExpressionStatement(_) | Statement::BreakStatement(_)) { - let mut space_diff: usize = (x.span().start - prev_span_end).try_into().unwrap(); + let indent_str = get_preceding_indent_str(source_text, x.span()); - if consequent_num == 0 {space_diff -= 3;} else { - space_diff -= 1; + if indent_str.is_some() { + formatter.print_ascii_byte(b'\n'); + formatter.print_str(indent_str.unwrap()); } + } - formatter.print_str("\n"); - - - let space = &" ".repeat(space_diff); - println!("space_diff: {space_diff}"); - formatter.print_str(space); - - - - prev_span_end = x.span().end; + formatter.print_str(x.span().source_text(source_text)); + } - consequent_num += 1; + let case_indent_str = get_preceding_indent_str(source_text, case.span()); - // TODO: figure out how to add correct spaces using spans. - // println!("stmt: {x:?}"); - // println!("stmt.span: {:?}", x.span()); - } + if case_indent_str.is_some() { + formatter.print_ascii_byte(b'\n'); + formatter.print_str(case_indent_str.unwrap()); - formatter.print_str(x.span().source_text(source_text)); + // indent + // let switch_indent_str = get_preceding_indent_str(source_text, switch.span()); + // formatter.print_str(switch_indent_str.unwrap_or("")); } formatter.print_ascii_byte(b'}'); @@ -204,6 +190,7 @@ impl Rule for SwitchCaseBraces { formatter.into_source_text() }; + println!("case.span: {:?}", case.span); println!("modified_code: {modified_code}"); fixer.replace(case.span, modified_code) @@ -219,53 +206,55 @@ impl Rule for SwitchCaseBraces { } } + + #[test] fn test() { use crate::tester::Tester; let pass: Vec<&str> = vec![ - // "switch(something) { case 1: case 2: {console.log('something'); break;}}", - // "switch(foo){ case 1: { break; } }", - // "switch(foo){ case 1: { ; /* <-- not empty */} }", - // "switch(foo){ case 1: { {} /* <-- not empty */} }", - // "switch(foo){ case 1: { break; } }", - // "switch(foo){ default: { doSomething(); } }", + "switch(something) { case 1: case 2: {console.log('something'); break;}}", + "switch(foo){ case 1: { break; } }", + "switch(foo){ case 1: { ; /* <-- not empty */} }", + "switch(foo){ case 1: { {} /* <-- not empty */} }", + "switch(foo){ case 1: { break; } }", + "switch(foo){ default: { doSomething(); } }", ]; let fail: Vec<&str> = vec![ - // "switch(s){case'':/]/}", - // "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", - // "switch(something) { case 1: case 2: console.log('something'); break;}", - // "switch(foo) { case 1: {} case 2: {} default: { doSomething(); } }", - // "switch(foo) { case 1: { /* fallthrough */ } default: {}/* fallthrough */ case 3: { doSomething(); break; } }", - // "switch(foo) { default: doSomething(); }", - // "switch(foo) { case 1: { doSomething(); } break; /* <-- This should be between braces */ }", - // "switch(foo) { default: label: {} }", - // "switch(something) { case 1: case 2: { console.log('something'); break; } case 3: console.log('something else'); }", + "switch(s){case'':/]/}", + "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", + "switch(something) { case 1: case 2: console.log('something'); break;}", + "switch(foo) { case 1: {} case 2: {} default: { doSomething(); } }", + "switch(foo) { case 1: { /* fallthrough */ } default: {}/* fallthrough */ case 3: { doSomething(); break; } }", + "switch(foo) { default: doSomething(); }", + "switch(foo) { case 1: { doSomething(); } break; /* <-- This should be between braces */ }", + "switch(foo) { default: label: {} }", + "switch(something) { case 1: case 2: { console.log('something'); break; } case 3: console.log('something else'); }", ]; let fix = vec![ - // ( - // "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", - // "switch(something) { case 1: case 2: {console.log('something'); break;}}", - // None, - // ), - // ( - // "switch(something) { case 1: {} case 2: console.log('something'); break;}", - // "switch(something) { case 1: case 2: {console.log('something');break;}}", - // None, - // ), - // ( - // "switch(foo) { default: doSomething(); }", - // "switch(foo) { default: {doSomething();} }", - // None, - // ), - // ("switch(s){case'':/]/}", "switch(s){case '': {/]/}}", None), - // ( - // "switch(foo) { default: {doSomething();} }", - // "switch(foo) { default: doSomething(); }", - // Some(serde_json::json!(["avoid"])), - // ), + ( + "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", + "switch(something) { case 1: case 2: {console.log('something'); break;}}", + None, + ), + ( + "switch(something) { case 1: {} case 2: console.log('something'); break;}", + "switch(something) { case 1: case 2: {console.log('something');break;}}", + None, + ), + ( + "switch(foo) { default: doSomething(); }", + "switch(foo) { default: {doSomething();} }", + None, + ), + ("switch(s){case'':/]/}", "switch(s){case '': {/]/}}", None), + ( + "switch(foo) { default: {doSomething();} }", + "switch(foo) { default: doSomething(); }", + Some(serde_json::json!(["avoid"])), + ), // Issue: https://github.com/oxc-project/oxc/issues/8491 ( " From 2f58869c7af5032a7357e1fa914207c75efcee2e Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 17:49:01 -0600 Subject: [PATCH 07/16] code cleanup --- .../src/rules/unicorn/switch_case_braces.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index f4611a84a658d..322cac3992270 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -161,8 +161,9 @@ impl Rule for SwitchCaseBraces { for x in &case.consequent { if matches!(x, Statement::ExpressionStatement(_) | Statement::BreakStatement(_)) { + // indent the statement in the case consequent, if needed let indent_str = get_preceding_indent_str(source_text, x.span()); - + if indent_str.is_some() { formatter.print_ascii_byte(b'\n'); formatter.print_str(indent_str.unwrap()); @@ -172,27 +173,18 @@ impl Rule for SwitchCaseBraces { formatter.print_str(x.span().source_text(source_text)); } + // indent the closing case bracket, if needed let case_indent_str = get_preceding_indent_str(source_text, case.span()); - if case_indent_str.is_some() { formatter.print_ascii_byte(b'\n'); formatter.print_str(case_indent_str.unwrap()); - - // indent - // let switch_indent_str = get_preceding_indent_str(source_text, switch.span()); - // formatter.print_str(switch_indent_str.unwrap_or("")); } formatter.print_ascii_byte(b'}'); - // TODO: add space diff between discriminant and first case - formatter.into_source_text() }; - println!("case.span: {:?}", case.span); - println!("modified_code: {modified_code}"); - fixer.replace(case.span, modified_code) }, ); @@ -255,7 +247,7 @@ fn test() { "switch(foo) { default: doSomething(); }", Some(serde_json::json!(["avoid"])), ), - // Issue: https://github.com/oxc-project/oxc/issues/8491 + // Issue: https://github.com/oxc-project/oxc/issues/8491 ( " const alpha = 7 @@ -283,7 +275,7 @@ fn test() { } ", None, - ) + ), ]; Tester::new(SwitchCaseBraces::NAME, SwitchCaseBraces::PLUGIN, pass, fail) From 3c5ee34a7daf751cda0116efbc54d1878ad8d3e5 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 17:53:19 -0600 Subject: [PATCH 08/16] address cargo suggestions --- crates/oxc_linter/src/ast_util.rs | 17 ++++------------- .../src/rules/unicorn/switch_case_braces.rs | 19 +++++++++---------- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index bec04bb1db0e6..fb5bbceb1f679 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -487,22 +487,13 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { } } -pub fn get_preceding_indent_str<'a>(source_text: &'a str, span: Span) -> Option<&'a str> { +pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> { // slice source text until start of given span, then get the preceding spaces from the last line of the source text. let span_start = span.start as usize; let preceding_source_text = &source_text[..span_start]; - - match preceding_source_text.lines().last() { - Some(line) => { - // only return if the line is whitespace. - if line.trim().is_empty() { - Some(line) - } else { - None - } - } - None => None, - } + + // only return last line if is whitespace + preceding_source_text.lines().last().filter(|&line| line.trim().is_empty()) } pub fn could_be_error(ctx: &LintContext, expr: &Expression) -> bool { diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index 322cac3992270..f3aadb559afa5 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -160,13 +160,15 @@ impl Rule for SwitchCaseBraces { let source_text = ctx.source_text(); for x in &case.consequent { - if matches!(x, Statement::ExpressionStatement(_) | Statement::BreakStatement(_)) { + if matches!( + x, + Statement::ExpressionStatement(_) + | Statement::BreakStatement(_) + ) { // indent the statement in the case consequent, if needed - let indent_str = get_preceding_indent_str(source_text, x.span()); - - if indent_str.is_some() { + if let Some(indent_str) = get_preceding_indent_str(source_text, x.span()) { formatter.print_ascii_byte(b'\n'); - formatter.print_str(indent_str.unwrap()); + formatter.print_str(indent_str); } } @@ -174,10 +176,9 @@ impl Rule for SwitchCaseBraces { } // indent the closing case bracket, if needed - let case_indent_str = get_preceding_indent_str(source_text, case.span()); - if case_indent_str.is_some() { + if let Some(case_indent_str) = get_preceding_indent_str(source_text, case.span()) { formatter.print_ascii_byte(b'\n'); - formatter.print_str(case_indent_str.unwrap()); + formatter.print_str(case_indent_str); } formatter.print_ascii_byte(b'}'); @@ -198,8 +199,6 @@ impl Rule for SwitchCaseBraces { } } - - #[test] fn test() { use crate::tester::Tester; From 7f50b815c2a155fb152860d5b464c16bdb22b366 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 17:54:16 -0600 Subject: [PATCH 09/16] run cargo fmt --- crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index f3aadb559afa5..c659cff2afa67 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -166,7 +166,9 @@ impl Rule for SwitchCaseBraces { | Statement::BreakStatement(_) ) { // indent the statement in the case consequent, if needed - if let Some(indent_str) = get_preceding_indent_str(source_text, x.span()) { + if let Some(indent_str) = + get_preceding_indent_str(source_text, x.span()) + { formatter.print_ascii_byte(b'\n'); formatter.print_str(indent_str); } @@ -176,7 +178,9 @@ impl Rule for SwitchCaseBraces { } // indent the closing case bracket, if needed - if let Some(case_indent_str) = get_preceding_indent_str(source_text, case.span()) { + if let Some(case_indent_str) = + get_preceding_indent_str(source_text, case.span()) + { formatter.print_ascii_byte(b'\n'); formatter.print_str(case_indent_str); } From b2a3f036b1a646f1664c842c694f68fb50a613a4 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:07:22 -0600 Subject: [PATCH 10/16] add comment --- crates/oxc_linter/src/ast_util.rs | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index fb5bbceb1f679..7005c1df5473b 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -486,9 +486,33 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { _ => false, } } - +/// Get the preceding indentation string before the start of a Span in a given source_text. Useful for maintaining the format of source code when applying a linting fix. +/// +/// Slice into a source text until the start of given span. +/// Then, get the preceding spaces from the last line of the source text. +/// If there are any non-whitespace characters preceding the span in the last line of source text, return None. +/// +/// Examples: +/// +/// 1. Given the following source text (with 2 preceding spaces): +/// +/// ```ts +/// const foo = 'bar' +/// ``` +/// +/// and the Span encapsulating the const foo = 'bar' expression statement, +/// this function will return " " (2 preceding spaces). +/// +/// 2. Given the following source text: +/// +/// ```ts +/// const fizz = 'buzz'; const foo = 'bar' +/// ``` +/// +/// and the Span encapsulating the "foo = 'bar'" expression statement, +/// this function will return None because there is non-whitespace before the statement, meaning it does not need to be indented before being written into a source code string. +/// pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> { - // slice source text until start of given span, then get the preceding spaces from the last line of the source text. let span_start = span.start as usize; let preceding_source_text = &source_text[..span_start]; From 6f4aaba7ecd6a7925f6a845c758a8b77b490e842 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:08:39 -0600 Subject: [PATCH 11/16] run cargo fmg --- crates/oxc_linter/src/ast_util.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 7005c1df5473b..62e11a8e37b24 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -487,31 +487,31 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { } } /// Get the preceding indentation string before the start of a Span in a given source_text. Useful for maintaining the format of source code when applying a linting fix. -/// -/// Slice into a source text until the start of given span. +/// +/// Slice into a source text until the start of given span. /// Then, get the preceding spaces from the last line of the source text. /// If there are any non-whitespace characters preceding the span in the last line of source text, return None. -/// +/// /// Examples: -/// +/// /// 1. Given the following source text (with 2 preceding spaces): -/// +/// /// ```ts /// const foo = 'bar' /// ``` -/// +/// /// and the Span encapsulating the const foo = 'bar' expression statement, /// this function will return " " (2 preceding spaces). -/// +/// /// 2. Given the following source text: -/// +/// /// ```ts /// const fizz = 'buzz'; const foo = 'bar' /// ``` -/// +/// /// and the Span encapsulating the "foo = 'bar'" expression statement, /// this function will return None because there is non-whitespace before the statement, meaning it does not need to be indented before being written into a source code string. -/// +/// pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> { let span_start = span.start as usize; let preceding_source_text = &source_text[..span_start]; From fc74455676622c582c97811273912516c9e7053f Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:14:42 -0600 Subject: [PATCH 12/16] unset debug = true in cargo toml --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d8737490df139..974c802bf7723 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -209,7 +209,7 @@ ignored = ["napi", "oxc_transform_napi", "oxc_parser_napi", "prettyplease"] [profile.dev] # Disabling debug info speeds up local and CI builds, # and we don't rely on it for debugging that much. -debug = true +debug = false [profile.dev.package] # Compile macros with some optimizations to make consuming crates build faster From 2ffd68ecc166a0ff0aa3e5f8aa4d7644b89d8745 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:19:07 -0600 Subject: [PATCH 13/16] update comment --- crates/oxc_linter/src/ast_util.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 62e11a8e37b24..a98b5d727440a 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -486,31 +486,34 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { _ => false, } } -/// Get the preceding indentation string before the start of a Span in a given source_text. Useful for maintaining the format of source code when applying a linting fix. +/// Get the preceding indentation string before the start of a Span in a given source_text string slice. Useful for maintaining the format of source code when applying a linting fix. /// -/// Slice into a source text until the start of given span. -/// Then, get the preceding spaces from the last line of the source text. -/// If there are any non-whitespace characters preceding the span in the last line of source text, return None. +/// Slice into source_text until the start of given Span. +/// Then, get the preceding spaces from the last line of the source_text. +/// If there are any non-whitespace characters preceding the Span in the last line of source_text, return None. /// /// Examples: /// -/// 1. Given the following source text (with 2 preceding spaces): +/// 1. Given the following source_text (with 2 preceding spaces): /// /// ```ts /// const foo = 'bar' /// ``` /// /// and the Span encapsulating the const foo = 'bar' expression statement, +/// /// this function will return " " (2 preceding spaces). /// -/// 2. Given the following source text: +/// 2. Given the following source_text: /// /// ```ts /// const fizz = 'buzz'; const foo = 'bar' /// ``` /// /// and the Span encapsulating the "foo = 'bar'" expression statement, -/// this function will return None because there is non-whitespace before the statement, meaning it does not need to be indented before being written into a source code string. +/// +/// this function will return None because there is non-whitespace before the statement, +/// meaning the line of source_text containing the span does not need to be indented before writing the span contents into a source code string. /// pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> { let span_start = span.start as usize; From b4433f3f3212413ffcd2da4280331f63bb8be98d Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:19:47 -0600 Subject: [PATCH 14/16] remove temp code --- crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs index c659cff2afa67..8be10789d06f6 100644 --- a/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs +++ b/crates/oxc_linter/src/rules/unicorn/switch_case_braces.rs @@ -207,7 +207,7 @@ impl Rule for SwitchCaseBraces { fn test() { use crate::tester::Tester; - let pass: Vec<&str> = vec![ + let pass = vec![ "switch(something) { case 1: case 2: {console.log('something'); break;}}", "switch(foo){ case 1: { break; } }", "switch(foo){ case 1: { ; /* <-- not empty */} }", @@ -216,7 +216,7 @@ fn test() { "switch(foo){ default: { doSomething(); } }", ]; - let fail: Vec<&str> = vec![ + let fail = vec![ "switch(s){case'':/]/}", "switch(something) { case 1: {} case 2: {console.log('something'); break;}}", "switch(something) { case 1: case 2: console.log('something'); break;}", From 30d1e2e19eb7dc595b0ed4a1ae79211af5025090 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:35:06 -0600 Subject: [PATCH 15/16] update comment --- crates/oxc_linter/src/ast_util.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index a98b5d727440a..fa3c1a01a694f 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -497,24 +497,23 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { /// 1. Given the following source_text (with 2 preceding spaces): /// /// ```ts -/// const foo = 'bar' +/// break /// ``` /// -/// and the Span encapsulating the const foo = 'bar' expression statement, +/// and the Span encapsulating the break statement, /// /// this function will return " " (2 preceding spaces). /// /// 2. Given the following source_text: /// /// ```ts -/// const fizz = 'buzz'; const foo = 'bar' +/// const foo = 'bar'; break; /// ``` /// -/// and the Span encapsulating the "foo = 'bar'" expression statement, +/// and the Span encapsulating the break statement, /// /// this function will return None because there is non-whitespace before the statement, -/// meaning the line of source_text containing the span does not need to be indented before writing the span contents into a source code string. -/// +/// meaning the line of source_text containing the Span is not indented on a new line. pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> { let span_start = span.start as usize; let preceding_source_text = &source_text[..span_start]; From f04acd3008d2856f3b178db2cb576fc64d98a1e2 Mon Sep 17 00:00:00 2001 From: Tyler Earls Date: Mon, 27 Jan 2025 18:36:13 -0600 Subject: [PATCH 16/16] run cargo fmt --- crates/oxc_linter/src/ast_util.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index fa3c1a01a694f..b5a6f5c4c65a1 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -501,7 +501,7 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { /// ``` /// /// and the Span encapsulating the break statement, -/// +/// /// this function will return " " (2 preceding spaces). /// /// 2. Given the following source_text: @@ -511,8 +511,8 @@ fn is_definitely_non_error_type(ty: &TSType) -> bool { /// ``` /// /// and the Span encapsulating the break statement, -/// -/// this function will return None because there is non-whitespace before the statement, +/// +/// this function will return None because there is non-whitespace before the statement, /// meaning the line of source_text containing the Span is not indented on a new line. pub fn get_preceding_indent_str(source_text: &str, span: Span) -> Option<&str> { let span_start = span.start as usize;