From 44dbcc2f1811e088040da3c5e440a54886e2a92d Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:52:56 +0000 Subject: [PATCH] refactor(oxlint/lsp): remove unused `Rope` references (#23341) With the changes from https://github.com/oxc-project/oxc/pull/23329, `Rope` does not need to be created and passed into the functions. This PR removes all references to `Rope`, because we never access it. --- Cargo.lock | 1 - apps/oxlint/Cargo.toml | 1 - apps/oxlint/src/lsp/error_with_position.rs | 184 +++++++-------------- apps/oxlint/src/lsp/server_linter.rs | 11 +- 4 files changed, 60 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c816bb461b..ef2661ce6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2892,7 +2892,6 @@ dependencies = [ "oxc_ast", "oxc_ast_visit", "oxc_config", - "oxc_data_structures", "oxc_diagnostics", "oxc_estree_tokens", "oxc_language_server", diff --git a/apps/oxlint/Cargo.toml b/apps/oxlint/Cargo.toml index 088d9f48c7..4675191a40 100644 --- a/apps/oxlint/Cargo.toml +++ b/apps/oxlint/Cargo.toml @@ -31,7 +31,6 @@ oxc_allocator = { workspace = true, features = ["fixed_size"] } oxc_ast = { workspace = true } oxc_ast_visit = { workspace = true, features = ["serialize"] } oxc_config = { workspace = true } -oxc_data_structures = { workspace = true, features = ["rope"] } oxc_diagnostics = { workspace = true } oxc_estree_tokens = { workspace = true } oxc_language_server = { workspace = true } diff --git a/apps/oxlint/src/lsp/error_with_position.rs b/apps/oxlint/src/lsp/error_with_position.rs index b860062a51..c0c0e5b32e 100644 --- a/apps/oxlint/src/lsp/error_with_position.rs +++ b/apps/oxlint/src/lsp/error_with_position.rs @@ -6,7 +6,6 @@ use tower_lsp_server::ls_types::{ NumberOrString, Position, Range, Uri, }; -use oxc_data_structures::rope::Rope; use oxc_diagnostics::{OxcCode, Severity}; use oxc_language_server::offset_to_position as lsp_offset_to_position; use oxc_linter::{ @@ -81,7 +80,6 @@ pub fn message_to_lsp_diagnostic( message: Message, uri: &Uri, source_text: &str, - rope: &Rope, rules_customization: Option<&RulesCustomization>, ) -> Option { let severity = if let Some(rules_customization) = rules_customization { @@ -105,8 +103,8 @@ pub fn message_to_lsp_diagnostic( .iter() .map(|span| { let offset = span.offset(); - let start_position = offset_to_position(rope, offset, source_text); - let end_position = offset_to_position(rope, offset + span.len(), source_text); + let start_position = offset_to_position(offset, source_text); + let end_position = offset_to_position(offset + span.len(), source_text); ls_types::DiagnosticRelatedInformation { location: ls_types::Location { @@ -122,8 +120,8 @@ pub fn message_to_lsp_diagnostic( ) }; - let start_position = offset_to_position(rope, message.span.start, source_text); - let end_position = offset_to_position(rope, message.span.end, source_text); + let start_position = offset_to_position(message.span.start, source_text); + let end_position = offset_to_position(message.span.end, source_text); let range = Range::new(start_position, end_position); let code = message.error.code.to_string(); @@ -183,7 +181,6 @@ pub fn message_to_lsp_diagnostic( } fixed_content.push(fix_to_fixed_content( &fix, - rope, source_text, FixedContentKind::LintRule(message.error.code.clone()), )); @@ -195,7 +192,6 @@ pub fn message_to_lsp_diagnostic( } fix_to_fixed_content( &fix, - rope, source_text, FixedContentKind::LintRule(message.error.code.clone()), ) @@ -222,7 +218,6 @@ pub fn message_to_lsp_diagnostic( &message.error.code, error_offset, section_offset, - rope, source_text, ); @@ -235,14 +230,9 @@ pub fn message_to_lsp_diagnostic( Some(DiagnosticReport { diagnostic, code_action }) } -fn fix_to_fixed_content( - fix: &Fix, - rope: &Rope, - source_text: &str, - fix_kind: FixedContentKind, -) -> FixedContent { - let start_position = offset_to_position(rope, fix.span.start, source_text); - let end_position = offset_to_position(rope, fix.span.end, source_text); +fn fix_to_fixed_content(fix: &Fix, source_text: &str, fix_kind: FixedContentKind) -> FixedContent { + let start_position = offset_to_position(fix.span.start, source_text); + let end_position = offset_to_position(fix.span.end, source_text); debug_assert!( fix.message.is_some(), @@ -304,7 +294,6 @@ pub fn create_unused_directives_report( directives: &DisableDirectives, severity: AllowWarnDeny, source_text: &str, - rope: &Rope, ) -> Vec { let mut reports = Vec::new(); let fix_message = "remove unused disable directive"; @@ -327,7 +316,6 @@ pub fn create_unused_directives_report( span, severity, source_text, - rope, Some(&Fix::delete(fix_span).with_message(fix_message)), )); } @@ -338,7 +326,6 @@ pub fn create_unused_directives_report( rule.name_span, severity, source_text, - rope, Some(&rule.create_fix(source_text, span).with_message(fix_message)), )); } @@ -359,7 +346,6 @@ pub fn create_unused_directives_report( *span, severity, source_text, - rope, // TODO: fixer // copy the structure of disable directives None, @@ -374,11 +360,10 @@ fn build_unused_disable_diagnostic_report( span: Span, severity: DiagnosticSeverity, source_text: &str, - rope: &Rope, fix: Option<&Fix>, ) -> DiagnosticReport { - let start_position = offset_to_position(rope, span.start, source_text); - let end_position = offset_to_position(rope, span.end, source_text); + let start_position = offset_to_position(span.start, source_text); + let end_position = offset_to_position(span.end, source_text); let range = Range::new(start_position, end_position); DiagnosticReport { @@ -397,7 +382,6 @@ fn build_unused_disable_diagnostic_report( range, fixed_content: vec![fix_to_fixed_content( fix, - rope, source_text, FixedContentKind::UnusedDirective, )], @@ -405,7 +389,7 @@ fn build_unused_disable_diagnostic_report( } } -pub fn offset_to_position(_rope: &Rope, offset: u32, source_text: &str) -> Position { +pub fn offset_to_position(offset: u32, source_text: &str) -> Position { lsp_offset_to_position(source_text, offset) } @@ -425,7 +409,6 @@ fn add_ignore_fixes( code: &OxcCode, error_offset: u32, section_offset: u32, - rope: &Rope, source_text: &str, ) { debug_assert!( @@ -441,17 +424,15 @@ fn add_ignore_fixes( &rule_name_with_plugin, error_offset, section_offset, - rope, source_text, )); - fixes.push(disable_for_this_section(&rule_name_with_plugin, section_offset, rope, source_text)); + fixes.push(disable_for_this_section(&rule_name_with_plugin, section_offset, source_text)); } fn disable_for_this_line( rule_name: &str, error_offset: u32, section_offset: u32, - rope: &Rope, source_text: &str, ) -> FixedContent { let bytes = source_text.as_bytes(); @@ -459,7 +440,7 @@ fn disable_for_this_line( // Reuse an inline disable-line comment on the same line when present. if let Some(existing_comment_end) = get_inline_disable_line_comment_end(error_offset, bytes) { - let position = offset_to_position(rope, existing_comment_end, source_text); + let position = offset_to_position(existing_comment_end, source_text); return FixedContent { message, code: format!(" {rule_name}"), @@ -490,7 +471,7 @@ fn disable_for_this_line( if let Some(existing_comment_end) = get_existing_disable_comment_end(insert_offset, DisableDirective::NextLine, bytes) { - let position = offset_to_position(rope, existing_comment_end, source_text); + let position = offset_to_position(existing_comment_end, source_text); return FixedContent { message, code: format!(" {rule_name}"), @@ -513,7 +494,7 @@ fn disable_for_this_line( }; let whitespace_string = String::from_utf8_lossy(whitespace_range); - let position = offset_to_position(rope, insert_offset, source_text); + let position = offset_to_position(insert_offset, source_text); FixedContent { message, code: format!( @@ -528,7 +509,6 @@ fn disable_for_this_line( fn disable_for_this_section( rule_name: &str, section_offset: u32, - rope: &Rope, source_text: &str, ) -> FixedContent { let bytes = source_text.as_bytes(); @@ -541,7 +521,7 @@ fn disable_for_this_section( if let Some(existing_comment_end) = get_existing_disable_comment_end(insert_offset, DisableDirective::Section, bytes) { - let position = offset_to_position(rope, existing_comment_end, source_text); + let position = offset_to_position(existing_comment_end, source_text); return FixedContent { message, code: format!(" {rule_name}"), @@ -552,7 +532,7 @@ fn disable_for_this_section( } let content = format!("{content_prefix}// oxlint-disable {rule_name}\n"); - let position = offset_to_position(rope, insert_offset, source_text); + let position = offset_to_position(insert_offset, source_text); FixedContent { message, @@ -801,7 +781,6 @@ fn get_inline_disable_line_comment_end(error_offset: u32, bytes: &[u8]) -> Optio #[cfg(test)] #[expect(clippy::cast_possible_truncation)] mod test { - use oxc_data_structures::rope::Rope; use oxc_diagnostics::OxcCode; use super::offset_to_position; @@ -846,17 +825,16 @@ mod test { #[test] #[should_panic(expected = "out of bounds")] fn out_of_bounds() { - offset_to_position(&Rope::from_str("foo"), 100, "foo"); + offset_to_position(100, "foo"); } #[test] fn add_ignore_fixes_uses_user_facing_plugin_names() { let source = "foo();"; - let rope = Rope::from_str(source); let code = OxcCode { scope: Some("jsx-a11y".into()), number: Some("alt-text".into()) }; let mut fixes = vec![]; - super::add_ignore_fixes(&mut fixes, &code, 0, 0, &rope, source); + super::add_ignore_fixes(&mut fixes, &code, 0, 0, source); assert_eq!(fixes[0].code, "// oxlint-disable-next-line jsx-a11y/alt-text\n"); assert_eq!(fixes[1].code, "// oxlint-disable jsx-a11y/alt-text\n"); @@ -865,8 +843,7 @@ mod test { #[test] fn disable_for_section_js_file() { let source = "console.log('hello');"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_section("no-console", 0, &rope, source); + let fix = super::disable_for_this_section("no-console", 0, source); assert_eq!(fix.code, "// oxlint-disable no-console\n"); assert_eq!(fix.range.start.line, 0); @@ -876,8 +853,7 @@ mod test { #[test] fn disable_for_section_after_lf() { let source = ""; - let rope = Rope::from_str(source); let section_offset = source.find(""; - let rope = Rope::from_str(source); let section_offset = source.find(""; - let rope = Rope::from_str(source); let section_offset = source.find("const x").unwrap() as u32; - let fix = super::disable_for_this_section("no-unused-vars", section_offset, &rope, source); + let fix = super::disable_for_this_section("no-unused-vars", section_offset, source); assert_eq!(fix.code, "\n// oxlint-disable no-unused-vars\n"); assert_eq!(fix.range.start.line, 1); @@ -971,10 +940,9 @@ mod test { let source = format!( "\n" ); - let rope = Rope::from_str(&source); let section_offset = source.find(existing).unwrap() as u32; - let fix = super::disable_for_this_section("no-console", section_offset, &rope, &source); + let fix = super::disable_for_this_section("no-console", section_offset, &source); assert_eq!(fix.code, " no-console"); assert_eq!(fix.range.start.line, 3); @@ -986,8 +954,7 @@ mod test { #[test] fn disable_for_this_line_single_line() { let source = "console.log('hello');"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 0, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 0, 0, source); assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 0); @@ -997,8 +964,7 @@ mod test { #[test] fn disable_for_this_line_with_spaces() { let source = " console.log('hello');"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 10, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 10, 0, source); assert_eq!(fix.code, " // oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 0); @@ -1008,8 +974,7 @@ mod test { #[test] fn disable_for_this_line_with_tabs() { let source = "\t\tconsole.log('hello');"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 10, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 10, 0, source); assert_eq!(fix.code, "\t\t// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 0); @@ -1019,8 +984,7 @@ mod test { #[test] fn disable_for_this_line_mixed_tabs_spaces() { let source = "\t \tconsole.log('hello');"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 12, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 12, 0, source); assert_eq!(fix.code, "\t \t// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 0); @@ -1030,8 +994,7 @@ mod test { #[test] fn disable_for_this_line_multiline_with_tabs() { let source = "function test() {\n\tconsole.log('hello');\n}"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 27, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 27, 0, source); assert_eq!(fix.code, "\t// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 1); @@ -1041,8 +1004,7 @@ mod test { #[test] fn disable_for_this_line_multiline_with_spaces() { let source = "function test() {\n console.log('hello');\n}"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 30, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 30, 0, source); assert_eq!(fix.code, " // oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 1); @@ -1052,8 +1014,7 @@ mod test { #[test] fn disable_for_this_line_complex_indentation() { let source = "function test() {\n\t \t console.log('hello');\n}"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 33, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 33, 0, source); assert_eq!(fix.code, "\t \t // oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 1); @@ -1063,8 +1024,7 @@ mod test { #[test] fn disable_for_this_line_no_indentation() { let source = "function test() {\nconsole.log('hello');\n}"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 26, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 26, 0, source); assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 1); @@ -1074,8 +1034,7 @@ mod test { #[test] fn disable_for_this_line_crlf_with_tabs() { let source = "function test() {\r\n\tconsole.log('hello');\r\n}"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 28, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 28, 0, source); assert_eq!(fix.code, "\t// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 1); @@ -1085,8 +1044,7 @@ mod test { #[test] fn disable_for_this_line_deeply_nested() { let source = "if (true) {\n\t\tif (nested) {\n\t\t\tconsole.log('deep');\n\t\t}\n}"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 40, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 40, 0, source); assert_eq!(fix.code, "\t\t\t// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 2); @@ -1096,8 +1054,7 @@ mod test { #[test] fn disable_for_this_line_at_start_of_file() { let source = "console.log('hello');"; - let rope = Rope::from_str(source); - let fix = super::disable_for_this_line("no-console", 0, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 0, 0, source); assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n"); assert_eq!(fix.range.start.line, 0); @@ -1108,9 +1065,8 @@ mod test { fn disable_for_this_line_whitespace_only_continuous() { // Test that only continuous whitespace from line start is captured let source = "function test() {\n \tcode \there\n}"; - let rope = Rope::from_str(source); // Error at position of 'code' (after " \t") - let fix = super::disable_for_this_line("no-console", 21, 0, &rope, source); + let fix = super::disable_for_this_line("no-console", 21, 0, source); // Should only capture " \t" at the beginning, not the spaces around "here" assert_eq!(fix.code, " \t// oxlint-disable-next-line no-console\n"); @@ -1122,11 +1078,9 @@ mod test { fn disable_for_this_line_with_section_offset() { // Test framework file with section offset (like Vue/Svelte) let source = ""; - let rope = Rope::from_str(source); let section_offset = 8; // At the \n after ""; - let rope = Rope::from_str(source); let section_offset = 8; // After ""; - let rope = Rope::from_str(source); let section_offset = 31; // At \n after ""; - let rope = Rope::from_str(source); let section_offset = 8; // At the \n after "