From cc74bbbaffabc37fc77a6ab272dfdad3aac0e9ec Mon Sep 17 00:00:00 2001 From: Reese Williams Date: Sat, 3 Dec 2022 12:05:10 -0800 Subject: [PATCH] Generically handle all method visibility modifiers (#369) * Fixtures * Properly handle end spacing for all method-modifiers * Make index a constant and add some more explanatory comments Co-authored-by: Reese Williams --- fixtures/small/private_def_actual.rb | 9 ------- fixtures/small/private_def_expected.rb | 9 ------- fixtures/small/visibility_modifier_actual.rb | 23 ++++++++++++++++++ .../small/visibility_modifier_expected.rb | 24 +++++++++++++++++++ librubyfmt/src/intermediary.rs | 7 ++++++ librubyfmt/src/line_tokens.rs | 13 ---------- librubyfmt/src/render_queue_writer.rs | 21 ++++++++++++---- 7 files changed, 71 insertions(+), 35 deletions(-) delete mode 100644 fixtures/small/private_def_actual.rb delete mode 100644 fixtures/small/private_def_expected.rb create mode 100644 fixtures/small/visibility_modifier_actual.rb create mode 100644 fixtures/small/visibility_modifier_expected.rb diff --git a/fixtures/small/private_def_actual.rb b/fixtures/small/private_def_actual.rb deleted file mode 100644 index 4e9e8848..00000000 --- a/fixtures/small/private_def_actual.rb +++ /dev/null @@ -1,9 +0,0 @@ -class Foo - private def bar - 42 - end - - private def baz(a, b, c) - a + b + c - end -end diff --git a/fixtures/small/private_def_expected.rb b/fixtures/small/private_def_expected.rb deleted file mode 100644 index 4e9e8848..00000000 --- a/fixtures/small/private_def_expected.rb +++ /dev/null @@ -1,9 +0,0 @@ -class Foo - private def bar - 42 - end - - private def baz(a, b, c) - a + b + c - end -end diff --git a/fixtures/small/visibility_modifier_actual.rb b/fixtures/small/visibility_modifier_actual.rb new file mode 100644 index 00000000..575cc113 --- /dev/null +++ b/fixtures/small/visibility_modifier_actual.rb @@ -0,0 +1,23 @@ +class Foo + private def bar + 42 + end + + private def baz(a, b, c) + a + b + c + end +end + +module WhiteTeaBowl + sig do + params(walking_this: Path) + end + module_function def i_choose_one; end + + sig do + void + end + patch_of def sunlight + "after another" + end +end diff --git a/fixtures/small/visibility_modifier_expected.rb b/fixtures/small/visibility_modifier_expected.rb new file mode 100644 index 00000000..f4626893 --- /dev/null +++ b/fixtures/small/visibility_modifier_expected.rb @@ -0,0 +1,24 @@ +class Foo + private def bar + 42 + end + + private def baz(a, b, c) + a + b + c + end +end + +module WhiteTeaBowl + sig do + params(walking_this: Path) + end + module_function def i_choose_one + end + + sig do + void + end + patch_of def sunlight + "after another" + end +end diff --git a/librubyfmt/src/intermediary.rs b/librubyfmt/src/intermediary.rs index 7370de2a..43169cfd 100644 --- a/librubyfmt/src/intermediary.rs +++ b/librubyfmt/src/intermediary.rs @@ -222,6 +222,13 @@ impl Intermediary { } } + pub fn insert_blankline_from_end(&mut self, index_from_end: usize) { + self.tokens.insert( + self.tokens.len() - index_from_end, + ConcreteLineToken::HardNewLine, + ) + } + pub fn insert_trailing_blankline(&mut self, _bl: BlanklineReason) { if self.index_of_last_hard_newline <= 2 { self.tokens.insert( diff --git a/librubyfmt/src/line_tokens.rs b/librubyfmt/src/line_tokens.rs index 65020db2..119df23e 100644 --- a/librubyfmt/src/line_tokens.rs +++ b/librubyfmt/src/line_tokens.rs @@ -116,19 +116,6 @@ impl ConcreteLineToken { } } - pub fn is_method_visibility_modifier(&self) -> bool { - match self { - Self::DirectPart { part } => { - part == "public" - || part == "private" - || part == "protected" - || part == "public_class_method" - || part == "private_class_method" - } - _ => false, - } - } - pub fn is_single_line_breakable_garbage(&self) -> bool { match self { Self::DirectPart { part } => part == &"".to_string(), diff --git a/librubyfmt/src/render_queue_writer.rs b/librubyfmt/src/render_queue_writer.rs index 68b6c652..bf1412f6 100644 --- a/librubyfmt/src/render_queue_writer.rs +++ b/librubyfmt/src/render_queue_writer.rs @@ -63,16 +63,29 @@ impl RenderQueueWriter { } if let Some( - [&ConcreteLineToken::End, &ConcreteLineToken::AfterCallChain, &ConcreteLineToken::HardNewLine, &ConcreteLineToken::Indent { .. }, x], - ) = accum.last::<5>() + [&ConcreteLineToken::End, &ConcreteLineToken::AfterCallChain, &ConcreteLineToken::HardNewLine, &ConcreteLineToken::Indent { .. }, x, maybe_space, maybe_def], + ) = accum.last::<7>() { match x { ConcreteLineToken::DefKeyword => {} _ => { if x.is_in_need_of_a_trailing_blankline() - && !x.is_method_visibility_modifier() + && !matches!( + (maybe_space, maybe_def), + (ConcreteLineToken::Space, ConcreteLineToken::DefKeyword) + ) { - accum.insert_trailing_blankline(BlanklineReason::ComesAfterEnd); + // If we're here, the last few tokens must look like this: + // | token | index_from_end | + // | End | 6 | + // |. AfterCallChain | 5 | + // | HardNewline | 4 | <-- insert after this token + // |. Indent | 3 . | + // | (ArbitraryToken) | 2 | + // | (ArbitraryToken) | 1 | + // | (ArbitraryToken) | 0 | + const LAST_NEWLINE_INDEX_FROM_END: usize = 4; + accum.insert_blankline_from_end(LAST_NEWLINE_INDEX_FROM_END); } } }