From ad8b2bd91e61c859df5bd5c6a85961b964608705 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 19 Jan 2022 20:13:14 -0600 Subject: [PATCH 1/6] Fix AroundBlockScan Previously we were accidentally not using the persisted @before_index and instead using the "original" before index. This commit fixes that problem and adds a test for the behavior. --- CHANGELOG.md | 2 ++ lib/dead_end/around_block_scan.rb | 4 ++++ spec/integration/dead_end_spec.rb | 2 -- spec/unit/capture_code_context_spec.rb | 30 +++++++++++++++++++++++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a77e2b4..f4ca4c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## HEAD (unreleased) +- Fixed internal class AroundBlockScan, minor changes in outputs (https://github.com/zombocom/dead_end/pull/131) + ## 3.1.1 - Fix case where Ripper lexing identified incorrect code as a keyword (https://github.com/zombocom/dead_end/pull/122) diff --git a/lib/dead_end/around_block_scan.rb b/lib/dead_end/around_block_scan.rb index ddb8198..388e95c 100644 --- a/lib/dead_end/around_block_scan.rb +++ b/lib/dead_end/around_block_scan.rb @@ -193,6 +193,10 @@ def after_index @code_lines[0...@orig_before_index] || [] end + private def before_lines + now = @code_lines[0...before_index] || [] + end + private def after_lines @code_lines[after_index.next..-1] || [] end diff --git a/spec/integration/dead_end_spec.rb b/spec/integration/dead_end_spec.rb index 8ad9913..bd2746a 100644 --- a/spec/integration/dead_end_spec.rb +++ b/spec/integration/dead_end_spec.rb @@ -29,7 +29,6 @@ module DeadEnd 6 class SyntaxTree < Ripper 170 def self.parse(source) 174 end - 176 private ❯ 754 def on_args_add(arguments, argument) ❯ 776 class ArgsAddBlock ❯ 810 end @@ -119,7 +118,6 @@ module DeadEnd 7 REQUIRED_BY = {} 9 attr_reader :name 10 attr_writer :cost - 11 attr_accessor :parent ❯ 13 def initialize(name) ❯ 18 def self.reset! ❯ 25 end diff --git a/spec/unit/capture_code_context_spec.rb b/spec/unit/capture_code_context_spec.rb index f001e0e..0eb7300 100644 --- a/spec/unit/capture_code_context_spec.rb +++ b/spec/unit/capture_code_context_spec.rb @@ -4,6 +4,35 @@ module DeadEnd RSpec.describe CaptureCodeContext do + it "capture_before_after_kws" do + source = <<~'EOM' + def sit + end + + def bark + + def eat + end + EOM + + code_lines = CodeLine.from_source(source) + + block = CodeBlock.new(lines: code_lines[0]) + + display = CaptureCodeContext.new( + blocks: [block], + code_lines: code_lines + ) + lines = display.call + expect(lines.join).to eq(<<~EOM) + def sit + end + def bark + def eat + end + EOM + end + it "handles ambiguous end" do source = <<~'EOM' def call # 1 @@ -94,7 +123,6 @@ def call expect(lines.join).to eq(<<~EOM) class Rexe VERSION = '1.5.1' - PROJECT_URL = 'https://github.com/keithrbennett/rexe' class Lookups def format_requires end From d22a5104355a2be154a45e9e391d3130fed3cb59 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 19 Jan 2022 20:36:29 -0600 Subject: [PATCH 2/6] Preserve AroundBlockScan when position when false When scanning returns nothing, the block position should not change. --- lib/dead_end/around_block_scan.rb | 18 +++++++++++------- spec/unit/around_block_scan_spec.rb | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/lib/dead_end/around_block_scan.rb b/lib/dead_end/around_block_scan.rb index 388e95c..03e3613 100644 --- a/lib/dead_end/around_block_scan.rb +++ b/lib/dead_end/around_block_scan.rb @@ -54,7 +54,7 @@ def scan_while(&block) kw_count = 0 end_count = 0 - @before_index = before_lines.reverse_each.take_while do |line| + index = before_lines.reverse_each.take_while do |line| next false if stop_next next true if @skip_array.detect { |meth| line.send(meth) } @@ -67,10 +67,14 @@ def scan_while(&block) block.call(line) end.last&.index + if index && index < before_index + @before_index = index + end + stop_next = false kw_count = 0 end_count = 0 - @after_index = after_lines.take_while do |line| + index = after_lines.take_while do |line| next false if stop_next next true if @skip_array.detect { |meth| line.send(meth) } @@ -82,6 +86,10 @@ def scan_while(&block) block.call(line) end.last&.index + + if index && index > after_index + @after_index = index + end self end @@ -190,11 +198,7 @@ def after_index end private def before_lines - @code_lines[0...@orig_before_index] || [] - end - - private def before_lines - now = @code_lines[0...before_index] || [] + @code_lines[0...before_index] || [] end private def after_lines diff --git a/spec/unit/around_block_scan_spec.rb b/spec/unit/around_block_scan_spec.rb index 781a4f1..8016b7f 100644 --- a/spec/unit/around_block_scan_spec.rb +++ b/spec/unit/around_block_scan_spec.rb @@ -4,6 +4,23 @@ module DeadEnd RSpec.describe AroundBlockScan do + it "continues scan from last location even if scan is false" do + source = <<~'EOM' + print 'omg' + print 'lol' + print 'haha' + EOM + code_lines = CodeLine.from_source(source) + block = CodeBlock.new(lines: code_lines[1]) + expand = AroundBlockScan.new(code_lines: code_lines, block: block) + .scan_neighbors + + expect(expand.code_block.to_s).to eq(source) + expand.scan_while { |line| false } + + expect(expand.code_block.to_s).to eq(source) + end + it "scan_adjacent_indent works on first or last line" do source_string = <<~EOM def foo From 25b0df428e1f529c4a7de66516e8148fb7514ec3 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 19 Jan 2022 20:40:44 -0600 Subject: [PATCH 3/6] Refactor BlockExpand to always grab empties This refactor removes the need to allocate an extra AroundBlockScan. Not a big hotspot, but it's not needed either. --- lib/dead_end/block_expand.rb | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/dead_end/block_expand.rb b/lib/dead_end/block_expand.rb index de47e6a..a2bdf4b 100644 --- a/lib/dead_end/block_expand.rb +++ b/lib/dead_end/block_expand.rb @@ -36,7 +36,7 @@ def initialize(code_lines:) end def call(block) - if (next_block = expand_neighbors(block, grab_empty: true)) + if (next_block = expand_neighbors(block)) return next_block end @@ -51,24 +51,18 @@ def expand_indent(block) .code_block end - def expand_neighbors(block, grab_empty: true) - scan = AroundBlockScan.new(code_lines: @code_lines, block: block) + def expand_neighbors(block) + expanded = AroundBlockScan.new(code_lines: @code_lines, block: block) .skip(:hidden?) .stop_after_kw .scan_neighbors + .scan_while { |line| line.empty? || line.hidden? } + .code_block - # Slurp up empties - if grab_empty - scan = AroundBlockScan.new(code_lines: @code_lines, block: scan.code_block) - .scan_while { |line| line.empty? || line.hidden? } - end - - new_block = scan.code_block - - if block.lines == new_block.lines + if block.lines == expanded.lines nil else - new_block + expanded end end end From a140e4d134255e482b758c8bafef281e1979d2d0 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 19 Jan 2022 20:50:35 -0600 Subject: [PATCH 4/6] Remove Kernel#send calls Calling `send` and iterating over an array is not great. We're only dealing with 2 cases, we can explicitly handle both of them. --- lib/dead_end/around_block_scan.rb | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/dead_end/around_block_scan.rb b/lib/dead_end/around_block_scan.rb index 03e3613..2436cb7 100644 --- a/lib/dead_end/around_block_scan.rb +++ b/lib/dead_end/around_block_scan.rb @@ -37,10 +37,20 @@ def initialize(code_lines:, block:) @after_array = [] @before_array = [] @stop_after_kw = false + + @skip_hidden = false + @skip_empty = false end def skip(name) - @skip_array << name + case name + when :hidden? + @skip_hidden = true + when :empty? + @skip_empty = true + else + raise "Unsupported skip #{name}" + end self end @@ -49,14 +59,15 @@ def stop_after_kw self end - def scan_while(&block) + def scan_while stop_next = false kw_count = 0 end_count = 0 index = before_lines.reverse_each.take_while do |line| next false if stop_next - next true if @skip_array.detect { |meth| line.send(meth) } + next true if @skip_hidden && line.hidden? + next true if @skip_empty && line.empty? kw_count += 1 if line.is_kw? end_count += 1 if line.is_end? @@ -64,7 +75,7 @@ def scan_while(&block) stop_next = true end - block.call(line) + yield line end.last&.index if index && index < before_index @@ -76,7 +87,8 @@ def scan_while(&block) end_count = 0 index = after_lines.take_while do |line| next false if stop_next - next true if @skip_array.detect { |meth| line.send(meth) } + next true if @skip_hidden && line.hidden? + next true if @skip_empty && line.empty? kw_count += 1 if line.is_kw? end_count += 1 if line.is_end? @@ -84,7 +96,7 @@ def scan_while(&block) stop_next = true end - block.call(line) + yield line end.last&.index if index && index > after_index From 51b0a4472854b5d1d379daba45e43f98b81cb72a Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 19 Jan 2022 20:52:10 -0600 Subject: [PATCH 5/6] Don't duplicate logic We're already skipping `hidden?` we don't need to call the method again. --- lib/dead_end/block_expand.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dead_end/block_expand.rb b/lib/dead_end/block_expand.rb index a2bdf4b..8275fd8 100644 --- a/lib/dead_end/block_expand.rb +++ b/lib/dead_end/block_expand.rb @@ -56,7 +56,7 @@ def expand_neighbors(block) .skip(:hidden?) .stop_after_kw .scan_neighbors - .scan_while { |line| line.empty? || line.hidden? } + .scan_while { |line| line.empty? } # Slurp up empties .code_block if block.lines == expanded.lines From b579f5f377a2b59ec3b49c7effc6419b0bc1debf Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 19 Jan 2022 20:57:26 -0600 Subject: [PATCH 6/6] Don't allocate CodeBlock when not needed Experimenting with this approach (not a hotspot). --- lib/dead_end/around_block_scan.rb | 8 ++++++-- lib/dead_end/block_expand.rb | 13 +++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/dead_end/around_block_scan.rb b/lib/dead_end/around_block_scan.rb index 2436cb7..8c12c68 100644 --- a/lib/dead_end/around_block_scan.rb +++ b/lib/dead_end/around_block_scan.rb @@ -87,7 +87,7 @@ def scan_while end_count = 0 index = after_lines.take_while do |line| next false if stop_next - next true if @skip_hidden && line.hidden? + next true if @skip_hidden && line.hidden? next true if @skip_empty && line.empty? kw_count += 1 if line.is_kw? @@ -198,7 +198,11 @@ def start_at_next_line end def code_block - CodeBlock.new(lines: @code_lines[before_index..after_index]) + CodeBlock.new(lines: lines) + end + + def lines + @code_lines[before_index..after_index] end def before_index diff --git a/lib/dead_end/block_expand.rb b/lib/dead_end/block_expand.rb index 8275fd8..7f3396f 100644 --- a/lib/dead_end/block_expand.rb +++ b/lib/dead_end/block_expand.rb @@ -52,18 +52,23 @@ def expand_indent(block) end def expand_neighbors(block) - expanded = AroundBlockScan.new(code_lines: @code_lines, block: block) + expanded_lines = AroundBlockScan.new(code_lines: @code_lines, block: block) .skip(:hidden?) .stop_after_kw .scan_neighbors .scan_while { |line| line.empty? } # Slurp up empties - .code_block + .lines - if block.lines == expanded.lines + if block.lines == expanded_lines nil else - expanded + CodeBlock.new(lines: expanded_lines) end end + + # Managable rspec errors + def inspect + "#" + end end end