diff --git a/crates/rome_cli/tests/snapshots/main_check/maximum_diagnostics.snap b/crates/rome_cli/tests/snapshots/main_check/maximum_diagnostics.snap index ca5e6343173..3a3442ab47a 100644 --- a/crates/rome_cli/tests/snapshots/main_check/maximum_diagnostics.snap +++ b/crates/rome_cli/tests/snapshots/main_check/maximum_diagnostics.snap @@ -37,6 +37,7 @@ check.js:2:1 lint/correctness/useBlockStatements FIXABLE ━━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;)·{}for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -57,6 +58,7 @@ check.js:2:1 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + while·(true);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -77,6 +79,7 @@ check.js:2:13 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;)·{}for(;true;);for(;true;);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -97,6 +100,7 @@ check.js:2:13 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);while·(true);for(;true;);for(;true;);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -117,6 +121,7 @@ check.js:2:25 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;)·{}for(;true;);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -137,6 +142,7 @@ check.js:2:25 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);while·(true);for(;true;);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -157,6 +163,7 @@ check.js:2:37 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;);for(;true;)·{}for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -177,6 +184,7 @@ check.js:2:37 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;);while·(true);for(;true;);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -197,6 +205,7 @@ check.js:2:49 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;);for(;true;);for(;true;)·{}for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -217,6 +226,7 @@ check.js:2:49 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;);for(;true;);while·(true);for(;true;); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -237,6 +247,7 @@ check.js:2:61 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;)·{} 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -257,6 +268,7 @@ check.js:2:61 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 2 │ + for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);while·(true); 3 3 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -278,6 +290,7 @@ check.js:3:1 lint/correctness/useBlockStatements FIXABLE ━━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;)·{}for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -300,6 +313,7 @@ check.js:3:1 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + while·(true);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); @@ -322,6 +336,7 @@ check.js:3:13 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;);for(;true;)·{}for(;true;);for(;true;);for(;true;);for(;true;); @@ -344,6 +359,7 @@ check.js:3:13 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;);while·(true);for(;true;);for(;true;);for(;true;);for(;true;); @@ -366,6 +382,7 @@ check.js:3:25 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;);for(;true;);for(;true;)·{}for(;true;);for(;true;);for(;true;); @@ -388,6 +405,7 @@ check.js:3:25 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;);for(;true;);while·(true);for(;true;);for(;true;);for(;true;); @@ -410,6 +428,7 @@ check.js:3:37 lint/correctness/useBlockStatements FIXABLE ━━━━━━ i Suggested fix: Wrap the statement with a `JsBlockStatement` + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;);for(;true;);for(;true;);for(;true;)·{}for(;true;);for(;true;); @@ -432,6 +451,7 @@ check.js:3:37 lint/correctness/useWhile FIXABLE ━━━━━━━━━━ i Suggested fix: Use a while loop + 1 1 │ 2 2 │ for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ - for(;true;);for(;true;);for(;true;);for(;true;);for(;true;);for(;true;); 3 │ + for(;true;);for(;true;);for(;true;);while·(true);for(;true;);for(;true;); diff --git a/crates/rome_cli/tests/snapshots/main_ci/ci_does_not_run_linter.snap b/crates/rome_cli/tests/snapshots/main_ci/ci_does_not_run_linter.snap index fa71e410e99..aa4277438c3 100644 --- a/crates/rome_cli/tests/snapshots/main_ci/ci_does_not_run_linter.snap +++ b/crates/rome_cli/tests/snapshots/main_ci/ci_does_not_run_linter.snap @@ -35,8 +35,13 @@ file.js format ━━━━━━━━━━━━━━━━━━━━━ × File content differs from formatting output - 2 │ → return·{·something·}; - │ ++ + + 1 │ - + 2 1 │ function f() { + 3 │ - return·{·something·} + 2 │ + → return·{·something·}; + 4 3 │ } + 5 4 │ + ``` diff --git a/crates/rome_diagnostics/src/v2/display/diff.rs b/crates/rome_diagnostics/src/v2/display/diff.rs index 8b9a4366521..69880e7b9c6 100644 --- a/crates/rome_diagnostics/src/v2/display/diff.rs +++ b/crates/rome_diagnostics/src/v2/display/diff.rs @@ -24,9 +24,11 @@ pub(super) fn print_diff(fmt: &mut fmt::Formatter<'_>, diff: &TextEdit) -> io::R process_diff_ops( diff, - &mut modified_lines, - &mut inserted_lines, - &mut before_line_to_after, + PushToLineState { + modified_lines: &mut modified_lines, + inserted_lines: &mut inserted_lines, + before_line_to_after: &mut before_line_to_after, + }, &mut after_line, &mut before_line, ); @@ -41,7 +43,19 @@ pub(super) fn print_diff(fmt: &mut fmt::Formatter<'_>, diff: &TextEdit) -> io::R return None; } - Some((key, inserted_lines.get(key)?)) + let line = inserted_lines.get(key)?; + let mut has_non_empty = false; + + for (_, text) in &line.diffs { + has_non_empty = has_non_empty || !text.is_empty(); + } + + // Disallow fully empty lines from being displayed in short mode + if has_non_empty { + Some((key, line)) + } else { + None + } }); if let Some((key, entry)) = modified_line { @@ -83,11 +97,9 @@ pub(super) fn print_diff(fmt: &mut fmt::Formatter<'_>, diff: &TextEdit) -> io::R /// to line numbers in the new revision /// - `after_line` counts the number of lines in the new revision of the document /// - `before_line` counts the number of lines in the old revision of the document -fn process_diff_ops<'diff>( +fn process_diff_ops<'a, 'diff>( diff: &'diff TextEdit, - modified_lines: &mut BTreeSet, - inserted_lines: &mut BTreeMap>, - before_line_to_after: &mut BTreeMap, + mut state: PushToLineState<'a, 'diff>, after_line: &mut OneIndexed, before_line: &mut OneIndexed, ) { @@ -103,16 +115,15 @@ fn process_diff_ops<'diff>( *before_line = before_line.saturating_add(1); } - before_line_to_after.insert(*before_line, *after_line); + state.before_line_to_after.insert(*before_line, *after_line); push_to_line( - modified_lines, - inserted_lines, - before_line_to_after, + &mut state, *before_line, *after_line, ChangeTag::Equal, "", + false, ); } @@ -123,43 +134,36 @@ fn process_diff_ops<'diff>( let tag = op.tag(); let text = op.text(diff); - // Doesn't contain a newline - if !text.contains('\n') { - push_to_line( - modified_lines, - inserted_lines, - before_line_to_after, - *before_line, - *after_line, - tag, - text, - ); - continue; - } + let parts_count = text.split('\n').count(); + let last_part = match parts_count.checked_sub(1) { + Some(last_part) => last_part, + None => { + // Doesn't contain a newline + push_to_line(&mut state, *before_line, *after_line, tag, text, false); + continue; + } + }; // Get all the lines - let mut parts = text.split('\n'); + let mut parts = text.split('\n').enumerate(); // Deconstruct each text chunk let current_line = parts.next(); // The first chunk belongs to the current line - if let Some(current_line) = current_line { - if !current_line.is_empty() { - push_to_line( - modified_lines, - inserted_lines, - before_line_to_after, - *before_line, - *after_line, - tag, - current_line, - ); - } + if let Some((part_index, current_line)) = current_line { + push_to_line( + &mut state, + *before_line, + *after_line, + tag, + current_line, + part_index < last_part, + ); } // Create unique lines for each other chunk - for new_line in parts { + for (part_index, new_line) in parts { match tag { ChangeTag::Equal => { *after_line = after_line.saturating_add(1); @@ -174,16 +178,15 @@ fn process_diff_ops<'diff>( } } - before_line_to_after.insert(*before_line, *after_line); + state.before_line_to_after.insert(*before_line, *after_line); push_to_line( - modified_lines, - inserted_lines, - before_line_to_after, + &mut state, *before_line, *after_line, tag, new_line, + part_index < last_part, ); } } @@ -238,25 +241,36 @@ impl<'a> GroupDiffsLine<'a> { } } -fn push_to_line<'a>( - modified_lines: &mut BTreeSet, - inserted_lines: &mut BTreeMap>, - before_line_to_after: &mut BTreeMap, +struct PushToLineState<'a, 'b> { + modified_lines: &'a mut BTreeSet, + inserted_lines: &'a mut BTreeMap>, + before_line_to_after: &'a mut BTreeMap, +} + +fn push_to_line<'a, 'b>( + state: &mut PushToLineState<'a, 'b>, before_line: OneIndexed, after_line: OneIndexed, tag: ChangeTag, - text: &'a str, + text: &'b str, + allow_empty: bool, ) { + let PushToLineState { + modified_lines, + inserted_lines, + before_line_to_after, + } = state; + match tag { ChangeTag::Insert => { GroupDiffsLine::insert(inserted_lines, LineKey::after(after_line), tag, text); - if !text.is_empty() { + if allow_empty || !text.is_empty() { modified_lines.insert(LineKey::after(after_line)); } } ChangeTag::Delete => { GroupDiffsLine::insert(inserted_lines, LineKey::before(before_line), tag, text); - if !text.is_empty() { + if allow_empty || !text.is_empty() { modified_lines.insert(LineKey::before(before_line)); } } @@ -785,4 +799,164 @@ function name(args) { "\nactual:\n{output:#?}\nexpected:\n{expected:#?}", ); } + + #[test] + fn remove_single_line() { + const SOURCE_LEFT: &str = "declare module \"test\" { + interface A { + + prop: string; + } +} +"; + + const SOURCE_RIGHT: &str = "declare module \"test\" { + interface A { + prop: string; + } +} +"; + + let diff = TextEdit::from_unicode_words(SOURCE_LEFT, SOURCE_RIGHT); + + let mut output = MarkupBuf::default(); + print_diff(&mut fmt::Formatter::new(&mut output), &diff).unwrap(); + + let expected = markup! { + " ""1"" ""1 │ "" declare module \"test\" {\n" + " ""2"" ""2 │ "" \tinterface A {\n" + " ""3"" "" │ ""-"" \n" + " ""4"" ""3 │ "" \t\tprop: string;\n" + " ""5"" ""4 │ "" \t}\n" + "\n" + } + .to_owned(); + + assert_eq!( + output, expected, + "\nactual:\n{output:#?}\nexpected:\n{expected:#?}", + ); + } + + #[test] + fn remove_many_lines() { + const SOURCE_LEFT: &str = "declare module \"test\" { + interface A { + + + + prop: string; + } +} +"; + + const SOURCE_RIGHT: &str = "declare module \"test\" { + interface A { + prop: string; + } +} +"; + + let diff = TextEdit::from_unicode_words(SOURCE_LEFT, SOURCE_RIGHT); + + let mut output = MarkupBuf::default(); + print_diff(&mut fmt::Formatter::new(&mut output), &diff).unwrap(); + + let expected = markup! { + " ""1"" ""1 │ "" declare module \"test\" {\n" + " ""2"" ""2 │ "" \tinterface A {\n" + " ""3"" "" │ ""-"" \n" + " ""4"" "" │ ""-"" \n" + " ""5"" "" │ ""-"" \n" + " ""6"" ""3 │ "" \t\tprop: string;\n" + " ""7"" ""4 │ "" \t}\n" + "\n" + } + .to_owned(); + + assert_eq!( + output, expected, + "\nactual:\n{output:#?}\nexpected:\n{expected:#?}", + ); + } + + #[test] + fn insert_single_line() { + const SOURCE_LEFT: &str = "declare module \"test\" { + interface A { + prop: string; + } +} +"; + + const SOURCE_RIGHT: &str = "declare module \"test\" { + interface A { + + prop: string; + } +} +"; + + let diff = TextEdit::from_unicode_words(SOURCE_LEFT, SOURCE_RIGHT); + + let mut output = MarkupBuf::default(); + print_diff(&mut fmt::Formatter::new(&mut output), &diff).unwrap(); + + let expected = markup! { + " ""1"" ""1 │ "" declare module \"test\" {\n" + " ""2"" ""2 │ "" \tinterface A {\n" + " ""3 │ ""+"" \n" + " ""3"" ""4 │ "" \t\tprop: string;\n" + " ""4"" ""5 │ "" \t}\n" + "\n" + } + .to_owned(); + + assert_eq!( + output, expected, + "\nactual:\n{output:#?}\nexpected:\n{expected:#?}", + ); + } + + #[test] + fn insert_many_lines() { + const SOURCE_LEFT: &str = "declare module \"test\" { + interface A { + prop: string; + } +} +"; + + const SOURCE_RIGHT: &str = "declare module \"test\" { + interface A { + + + + prop: string; + } +} +"; + + let diff = TextEdit::from_unicode_words(SOURCE_LEFT, SOURCE_RIGHT); + + let mut output = MarkupBuf::default(); + print_diff(&mut fmt::Formatter::new(&mut output), &diff).unwrap(); + + let expected = markup! { + " ""1"" ""1 │ "" declare module \"test\" {\n" + " ""2"" ""2 │ "" \tinterface A {\n" + " ""3 │ ""+"" \n" + " ""4 │ ""+"" \n" + " ""5 │ ""+"" \n" + " ""3"" ""6 │ "" \t\tprop: string;\n" + " ""4"" ""7 │ "" \t}\n" + "\n" + } + .to_owned(); + + assert_eq!( + output, expected, + "\nactual:\n{output:#?}\nexpected:\n{expected:#?}", + ); + } } diff --git a/crates/rome_js_analyze/tests/specs/correctness/noDebugger.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noDebugger.js.snap index 03fbb704b3d..1963d65206e 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noDebugger.js.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noDebugger.js.snap @@ -55,8 +55,13 @@ noDebugger.js:5:1 lint/correctness/noDebugger FIXABLE ━━━━━━━━ i Suggested fix: Remove debugger statement - 5 │ debugger; - │ --------- + 3 3 │ if (foo) debugger + 4 4 │ + 5 │ - debugger; + 6 │ - + 7 5 │ function test() { + 8 6 │ let a = 3; + ``` diff --git a/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap b/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap index 20fec132c12..08adf2a1f52 100644 --- a/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap +++ b/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap @@ -53,6 +53,7 @@ noShoutyConstants.js:1:7 lint/style/noShoutyConstants FIXABLE ━━━━━ 1 │ - const·FOO·=·"FOO"; 2 │ - console.log(FOO,·FOO2); + 1 │ + 2 │ + console.log("FOO",·FOO2); 3 3 │ 4 4 │ const FOO2 = "FOO2", a = "FOO3", FOO4 = "FOO4"; diff --git a/website/src/docs/lint/rules/noShoutyConstants.md b/website/src/docs/lint/rules/noShoutyConstants.md index d3a93de32da..92a9936bda0 100644 --- a/website/src/docs/lint/rules/noShoutyConstants.md +++ b/website/src/docs/lint/rules/noShoutyConstants.md @@ -42,6 +42,7 @@ console.log(FOO); 1 │ - const·FOO·=·"FOO"; 2 │ - console.log(FOO); + 1 │ + 2 │ + console.log("FOO"); 3 3 │