diff --git a/crates/rome_formatter/src/builders.rs b/crates/rome_formatter/src/builders.rs index 28833734e14..cbf70a7a75c 100644 --- a/crates/rome_formatter/src/builders.rs +++ b/crates/rome_formatter/src/builders.rs @@ -1653,17 +1653,17 @@ impl IfGroupBreaks<'_, Context> { /// &format_args![ /// text("["), /// soft_block_indent(&format_with(|f| { - /// f.fill(soft_line_break_or_space()) - /// .entry(&text("1,")) - /// .entry(&text("234568789,")) - /// .entry(&text("3456789,")) - /// .entry(&format_args!( + /// f.fill() + /// .entry(&soft_line_break_or_space(), &text("1,")) + /// .entry(&soft_line_break_or_space(), &text("234568789,")) + /// .entry(&soft_line_break_or_space(), &text("3456789,")) + /// .entry(&soft_line_break_or_space(), &format_args!( /// text("["), /// soft_block_indent(&text("4")), /// text("]"), /// if_group_breaks(&text(",")).with_group_id(Some(group_id)) /// )) - /// .finish() + /// .finish() /// })), /// text("]") /// ], @@ -2161,40 +2161,26 @@ pub fn get_lines_before(next_node: &SyntaxNode) -> usize { pub struct FillBuilder<'fmt, 'buf, Context> { result: FormatResult<()>, fmt: &'fmt mut Formatter<'buf, Context>, - - /// The separator to use to join the elements - separator: FormatElement, items: Vec, } impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { - pub(crate) fn new( - fmt: &'a mut Formatter<'buf, Context>, - separator: Separator, - ) -> Self - where - Separator: Format, - { - let mut buffer = VecBuffer::new(fmt.state_mut()); - let result = write!(buffer, [separator]); - let separator = buffer.into_element(); - + pub(crate) fn new(fmt: &'a mut Formatter<'buf, Context>) -> Self { Self { - result, + result: Ok(()), fmt, - separator, items: vec![], } } - /// Adds an iterator of entries to the fill output. - pub fn entries(&mut self, entries: I) -> &mut Self + /// Adds an iterator of entries to the fill output. Uses the passed `separator` to separate any two items. + pub fn entries(&mut self, separator: &dyn Format, entries: I) -> &mut Self where F: Format, I: IntoIterator, { for entry in entries { - self.entry(&entry); + self.entry(separator, &entry); } self @@ -2207,13 +2193,17 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { /// /// The usage of this method is highly discouraged and it's better to use /// other APIs on ways: for example progressively format the items based on their type. - pub fn flatten_entries(&mut self, entries: I) -> &mut Self + pub fn flatten_entries( + &mut self, + separator: &dyn Format, + entries: I, + ) -> &mut Self where F: Format, I: IntoIterator, { for entry in entries { - self.flatten_entry(&entry); + self.flatten_entry(separator, &entry); } self @@ -2221,12 +2211,28 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { /// Adds a new entry to the fill output. If the entry is a [FormatElement::List], /// then adds the list's entries to the fill output instead of the list itself. - pub fn flatten_entry(&mut self, entry: &dyn Format) -> &mut Self { + fn flatten_entry( + &mut self, + separator: &dyn Format, + entry: &dyn Format, + ) -> &mut Self { self.result = self.result.and_then(|_| { let mut buffer = VecBuffer::new(self.fmt.state_mut()); write!(buffer, [entry])?; - self.items.extend(buffer.into_vec()); + let entries = buffer.into_vec(); + self.items.reserve((entries.len() * 2).saturating_sub(1)); + + let mut buffer = VecBuffer::new(self.fmt.state_mut()); + for item in entries.into_iter() { + if !self.items.is_empty() { + write!(buffer, [separator])?; + + self.items.push(buffer.take_element()); + } + + self.items.push(item); + } Ok(()) }); @@ -2234,18 +2240,23 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { self } - /// Adds a new entry to the fill output. - pub fn entry(&mut self, entry: &dyn Format) -> &mut Self { + /// Adds a new entry to the fill output. The `separator` isn't written if this is the first element in the list. + pub fn entry( + &mut self, + separator: &dyn Format, + entry: &dyn Format, + ) -> &mut Self { self.result = self.result.and_then(|_| { let mut buffer = VecBuffer::new(self.fmt.state_mut()); - write!(buffer, [entry])?; - - let item = buffer.into_element(); - if !item.is_empty() { - self.items.push(item); + if !self.items.is_empty() { + write!(buffer, [separator])?; + self.items.push(buffer.take_element()); } + write!(buffer, [entry])?; + self.items.push(buffer.into_element()); + Ok(()) }); @@ -2260,10 +2271,9 @@ impl<'a, 'buf, Context> FillBuilder<'a, 'buf, Context> { match items.len() { 0 => Ok(()), 1 => self.fmt.write_element(items.pop().unwrap()), - _ => self.fmt.write_element(FormatElement::Fill(Fill { - content: items.into_boxed_slice(), - separator: Box::new(self.separator.clone()), - })), + _ => self + .fmt + .write_element(FormatElement::Fill(items.into_boxed_slice())), } }) } diff --git a/crates/rome_formatter/src/format_element.rs b/crates/rome_formatter/src/format_element.rs index fbb8c54bb69..5ede04a8129 100644 --- a/crates/rome_formatter/src/format_element.rs +++ b/crates/rome_formatter/src/format_element.rs @@ -67,8 +67,9 @@ pub enum FormatElement { List(List), /// Concatenates multiple elements together with a given separator printed in either - /// flat or expanded mode to fill the print width. See [crate::Formatter::fill]. - Fill(Fill), + /// flat or expanded mode to fill the print width. Expect that the content is a list of alternating + /// [element, separator] See [crate::Formatter::fill]. + Fill(Content), /// A text that should be printed as is, see [crate::text] for documentation and examples. Text(Text), @@ -253,26 +254,6 @@ impl Deref for List { } } -/// Fill is a list of [FormatElement]s along with a separator. -/// -/// The printer prints this list delimited by a separator, wrapping the list when it -/// reaches the specified `line_width`. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Fill { - pub(super) content: Content, - pub(super) separator: Box, -} - -impl Fill { - pub fn content(&self) -> &[FormatElement] { - &self.content - } - - pub fn separator(&self) -> &FormatElement { - &self.separator - } -} - #[derive(Clone, Debug, Eq, PartialEq)] pub struct Align { pub(super) content: Content, @@ -689,7 +670,7 @@ impl FormatElement { | FormatElement::ConditionalGroupContent(ConditionalGroupContent { content, .. }) | FormatElement::IndentIfGroupBreaks(IndentIfGroupBreaks { content, .. }) | FormatElement::Comment(content) - | FormatElement::Fill(Fill { content, .. }) + | FormatElement::Fill(content) | FormatElement::Verbatim(Verbatim { content, .. }) | FormatElement::Label(Label { content, .. }) | FormatElement::Indent(content) @@ -981,18 +962,8 @@ impl Format for FormatElement { ] ) } - FormatElement::Fill(fill) => { - write!( - f, - [ - text("fill("), - fill.separator.as_ref(), - text(","), - space(), - fill.content(), - text(")") - ] - ) + FormatElement::Fill(content) => { + write!(f, [text("fill("), content.as_ref(), text(")")]) } FormatElement::BestFitting(best_fitting) => { diff --git a/crates/rome_formatter/src/formatter.rs b/crates/rome_formatter/src/formatter.rs index 8d447d2309b..b522637e2a1 100644 --- a/crates/rome_formatter/src/formatter.rs +++ b/crates/rome_formatter/src/formatter.rs @@ -137,11 +137,11 @@ impl<'buf, Context> Formatter<'buf, Context> { /// use rome_formatter::{format, format_args}; /// /// let formatted = format!(SimpleFormatContext::default(), [format_with(|f| { - /// f.fill(soft_line_break_or_space()) - /// .entry(&text("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) - /// .entry(&text("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")) - /// .entry(&text("cccccccccccccccccccccccccccccc")) - /// .entry(&text("dddddddddddddddddddddddddddddd")) + /// f.fill() + /// .entry(&soft_line_break_or_space(), &text("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")) + /// .entry(&soft_line_break_or_space(), &text("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")) + /// .entry(&soft_line_break_or_space(), &text("cccccccccccccccccccccccccccccc")) + /// .entry(&soft_line_break_or_space(), &text("dddddddddddddddddddddddddddddd")) /// .finish() /// })]).unwrap(); /// @@ -163,7 +163,7 @@ impl<'buf, Context> Formatter<'buf, Context> { /// ]; /// /// let formatted = format!(SimpleFormatContext::default(), [format_with(|f| { - /// f.fill(soft_line_break()).entries(entries.iter()).finish() + /// f.fill().entries(&soft_line_break(), entries.iter()).finish() /// })]).unwrap(); /// /// assert_eq!( @@ -171,11 +171,8 @@ impl<'buf, Context> Formatter<'buf, Context> { /// formatted.print().as_code() /// ) /// ``` - pub fn fill<'a, Separator>(&'a mut self, separator: Separator) -> FillBuilder<'a, 'buf, Context> - where - Separator: Format, - { - FillBuilder::new(self, separator) + pub fn fill<'a>(&'a mut self) -> FillBuilder<'a, 'buf, Context> { + FillBuilder::new(self) } /// Formats `content` into an interned element without writing it to the formatter's buffer. diff --git a/crates/rome_formatter/src/printer/mod.rs b/crates/rome_formatter/src/printer/mod.rs index 7de80a02c3c..39b8cc8ce9a 100644 --- a/crates/rome_formatter/src/printer/mod.rs +++ b/crates/rome_formatter/src/printer/mod.rs @@ -6,7 +6,6 @@ use crate::format_element::{ Align, ConditionalGroupContent, DedentMode, Group, IndentIfGroupBreaks, LineMode, PrintMode, VerbatimKind, }; -use crate::intersperse::Intersperse; use crate::{FormatElement, GroupId, IndentStyle, Printed, SourceMarker, TextRange}; use rome_rowan::{TextLen, TextSize}; @@ -171,8 +170,8 @@ impl<'a> Printer<'a> { } } - FormatElement::Fill(fill) => { - self.print_fill(queue, fill.content(), fill.separator(), args); + FormatElement::Fill(content) => { + self.print_fill(queue, content, args); } FormatElement::List(list) => { @@ -391,7 +390,6 @@ impl<'a> Printer<'a> { &mut self, queue: &mut ElementCallQueue<'a>, content: &'a [FormatElement], - separator: &'a FormatElement, args: PrintElementArgs, ) { let empty_rest = ElementCallQueue::default(); @@ -424,50 +422,64 @@ impl<'a> Printer<'a> { ); // Process remaining items - for next_item in items { - // A line break in expanded mode is always necessary if the current item didn't fit. - // otherwise see if both contents fit on the line. - let current_and_next_fit = current_fits - && fits_on_line( - [separator, next_item], - args.with_print_mode(PrintMode::Flat), - &empty_rest, - self, - ); - - if current_and_next_fit { - // Print Space and next item on the same line - self.print_all( - queue, - &[separator, next_item], - args.with_print_mode(PrintMode::Flat), - ); - } else { - // Print the separator and then check again if the next item fits on the line now - self.print_all( - queue, - &[separator], - args.with_print_mode(PrintMode::Expanded), - ); - - let next_fits = fits_on_line( - [next_item], - args.with_print_mode(PrintMode::Flat), - &empty_rest, - self, - ); + loop { + match (items.next(), items.next()) { + (Some(separator), Some(next_item)) => { + // A line break in expanded mode is always necessary if the current item didn't fit. + // otherwise see if both contents fit on the line. + let current_and_next_fit = current_fits + && fits_on_line( + [separator, next_item], + args.with_print_mode(PrintMode::Flat), + &empty_rest, + self, + ); + + if current_and_next_fit { + // Print Space and next item on the same line + self.print_all( + queue, + &[separator, next_item], + args.with_print_mode(PrintMode::Flat), + ); + } else { + // Print the separator and then check again if the next item fits on the line now + self.print_all( + queue, + &[separator], + args.with_print_mode(PrintMode::Expanded), + ); + + let next_fits = fits_on_line( + [next_item], + args.with_print_mode(PrintMode::Flat), + &empty_rest, + self, + ); + + if next_fits { + self.print_all( + queue, + &[next_item], + args.with_print_mode(PrintMode::Flat), + ); + } else { + self.print_all( + queue, + &[next_item], + args.with_print_mode(PrintMode::Expanded), + ); + } - if next_fits { - self.print_all(queue, &[next_item], args.with_print_mode(PrintMode::Flat)); - } else { - self.print_all( - queue, - &[next_item], - args.with_print_mode(PrintMode::Expanded), - ); + current_fits = next_fits; + } + } + (Some(_), _) => { + // Don't print a trailing separator + } + (None, _) => { + break; } - - current_fits = next_fits; } } } @@ -952,10 +964,10 @@ fn fits_element_on_line<'a, 'rest>( FormatElement::List(list) => queue.extend(list.iter(), args), - FormatElement::Fill(fill) => queue.queue.0.extend( - Intersperse::new(fill.content().iter().rev(), fill.separator()) - .map(|t| PrintElementCall::new(t, args)), - ), + FormatElement::Fill(content) => queue + .queue + .0 + .extend(content.iter().rev().map(|t| PrintElementCall::new(t, args))), FormatElement::Text(token) => { let indent = std::mem::take(&mut state.pending_indent); @@ -1137,7 +1149,7 @@ mod tests { write!(&mut buffer, [root]).unwrap(); - Printer::new(options).print(&buffer.into_element()) + Printer::new(options).print(&dbg!(buffer.into_element())) } #[test] @@ -1329,25 +1341,43 @@ two lines`, let mut formatter = Formatter::new(&mut buffer); formatter - .fill(&soft_line_break_or_space()) + .fill() // These all fit on the same line together - .entry(&format_args!(text("1"), text(","))) - .entry(&format_args!(text("2"), text(","))) - .entry(&format_args!(text("3"), text(","))) + .entry( + &soft_line_break_or_space(), + &format_args!(text("1"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("2"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("3"), text(",")), + ) // This one fits on a line by itself, - .entry(&format_args!(text("723493294"), text(","))) + .entry( + &soft_line_break_or_space(), + &format_args!(text("723493294"), text(",")), + ) // fits without breaking - .entry(&group(&format_args!( - text("["), - soft_block_indent(&text("5")), - text("],") - ))) + .entry( + &soft_line_break_or_space(), + &group(&format_args!( + text("["), + soft_block_indent(&text("5")), + text("],") + )), + ) // this one must be printed in expanded mode to fit - .entry(&group(&format_args!( - text("["), - soft_block_indent(&text("123456789")), - text("]"), - ))) + .entry( + &soft_line_break_or_space(), + &group(&format_args!( + text("["), + soft_block_indent(&text("123456789")), + text("]"), + )), + ) .finish() .unwrap(); @@ -1368,10 +1398,19 @@ two lines`, group(&format_args![ text("["), soft_block_indent(&format_with(|f| { - f.fill(soft_line_break_or_space()) - .entry(&format_args!(text("1"), text(","))) - .entry(&format_args!(text("2"), text(","))) - .entry(&format_args!(text("3"), if_group_breaks(&text(",")))) + f.fill() + .entry( + &soft_line_break_or_space(), + &format_args!(text("1"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("2"), text(",")), + ) + .entry( + &soft_line_break_or_space(), + &format_args!(text("3"), if_group_breaks(&text(","))), + ) .finish() })), text("]") diff --git a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs index cc1a74b1eba..4dc02a0957d 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs @@ -1,16 +1,13 @@ -use crate::{semantic_services::Semantic, utils::batch::JsBatchMutation, JsRuleAction}; -use rome_analyze::{ - context::RuleContext, declare_rule, ActionCategory, Rule, RuleCategory, RuleDiagnostic, -}; +use crate::semantic_services::Semantic; +use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleCategory, RuleDiagnostic}; use rome_console::markup; -use rome_diagnostics::Applicability; use rome_js_semantic::{AllReferencesExtensions, SemanticScopeExtensions}; use rome_js_syntax::{ - JsCatchDeclaration, JsConstructorParameterList, JsConstructorParameters, JsFormalParameter, - JsFunctionDeclaration, JsIdentifierBinding, JsParameterList, JsParameters, JsSyntaxKind, + JsClassExpression, JsConstructorParameterList, JsConstructorParameters, JsFunctionDeclaration, + JsFunctionExpression, JsIdentifierBinding, JsParameterList, JsParameters, JsSyntaxKind, JsVariableDeclarator, }; -use rome_rowan::{AstNode, BatchMutationExt}; +use rome_rowan::AstNode; declare_rule! { /// Disallow unused variables. @@ -65,6 +62,20 @@ declare_rule! { /// }; /// foo(); /// ``` + /// + /// ```js + /// function foo(_unused) { + /// }; + /// foo(); + /// ``` + /// + /// ```jsx + /// import React from 'react'; + /// function foo() { + /// return
; + /// }; + /// foo(); + /// ``` pub(crate) NoUnusedVariables { version: "0.9.0", name: "noUnusedVariables", @@ -117,12 +128,28 @@ impl Rule for NoUnusedVariables { fn run(ctx: &RuleContext) -> Option { let binding = ctx.query(); - let model = ctx.model(); + let name = binding.name_token().ok()?; + let name = name.token_text_trimmed(); + let name = name.text(); + + // Old code import React but do not used directly + // only indirectly after transpiling JSX. + if name.starts_with('_') || name == "React" { + return None; + } + + // Ignore expressions + if binding.parent::().is_some() + || binding.parent::().is_some() + { + return None; + } if is_typescript_unused_ok(binding).is_some() { return None; } + let model = ctx.model(); if model.is_exported(binding) { return None; } @@ -198,43 +225,4 @@ impl Rule for NoUnusedVariables { Some(diag) } - - fn action(ctx: &RuleContext, _: &Self::State) -> Option { - let root = ctx.root(); - let binding = ctx.query(); - - let mut batch = root.begin(); - - // Try to remove node - let removed = if let Some(declaration) = binding.parent::() { - batch.remove_node(declaration); - true - } else if let Some(variable_declarator) = binding.parent::() { - batch.remove_js_variable_declarator(&variable_declarator) - } else if let Some(formal_parameter) = binding.parent::() { - batch.remove_js_formal_parameter(&formal_parameter) - } else if let Some(catch_declaration) = binding.parent::() { - batch.remove_node(catch_declaration); - true - } else { - false - }; - - if removed { - let symbol_type = match binding.syntax().parent().unwrap().kind() { - JsSyntaxKind::JS_FORMAL_PARAMETER => "parameter", - JsSyntaxKind::JS_FUNCTION_DECLARATION => "function", - _ => "variable", - }; - - Some(JsRuleAction { - category: ActionCategory::QuickFix, - applicability: Applicability::MaybeIncorrect, - message: markup! { "Remove this " {symbol_type} "." }.to_owned(), - mutation: batch, - }) - } else { - None - } - } } diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js index 39a0d7ca46e..977617aa07a 100644 --- a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js +++ b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js @@ -1,3 +1,5 @@ +import React from 'react'; + const a = 1; const b = 2, c = 3; @@ -47,4 +49,8 @@ let value; function Button() {} console.log(