[ruby/syntax_suggest] Add comments and refactor AroundBlockScan methods

https://github.com/ruby/syntax_suggest/commit/cecd12292c

5 files changed, 127 insertions, 73 deletions

diff --git a/lib/syntax_suggest/around_block_scan.rb b/lib/syntax_suggest/around_block_scan.rb
index 4793c3b5e6..656246c0af 100644
--- a/lib/syntax_suggest/around_block_scan.rb
+++ b/lib/syntax_suggest/around_block_scan.rb
@@ -38,35 +38,64 @@ module SyntaxSuggest
       @before_array = []
       @stop_after_kw = false
 
-      @skip_hidden = false
-      @skip_empty = false
+      @force_add_hidden = false
+      @force_add_empty = false
     end
 
-    def skip(name)
-      case name
-      when :hidden?
-        @skip_hidden = true
-      when :empty?
-        @skip_empty = true
-      else
-        raise "Unsupported skip #{name}"
-      end
+    # When using this flag, `scan_while` will
+    # bypass the block it's given and always add a
+    # line that responds truthy to `CodeLine#hidden?`
+    #
+    # Lines are hidden when they've been evaluated by
+    # the parser as part of a block and found to contain
+    # valid code.
+    def force_add_hidden
+      @force_add_hidden = true
+      self
+    end
+
+    # When using this flag, `scan_while` will
+    # bypass the block it's given and always add a
+    # line that responds truthy to `CodeLine#empty?`
+    #
+    # Empty lines contain no code, only whitespace such
+    # as leading spaces a newline.
+    def force_add_empty
+      @force_add_empty = true
       self
     end
 
+    # Tells `scan_while` to look for mismatched keyword/end-s
+    #
+    # When scanning up, if we see more keywords then end-s it will
+    # stop. This might happen when scanning outside of a method body.
+    # the first scan line up would be a keyword and this setting would
+    # trigger a stop.
+    #
+    # When scanning down, stop if there are more end-s than keywords.
     def stop_after_kw
       @stop_after_kw = true
       self
     end
 
+    # Main work method
+    #
+    # The scan_while method takes a block that yields lines above and
+    # below the block. If the yield returns true, the @before_index
+    # or @after_index are modified to include the matched line.
+    #
+    # In addition to yielding individual lines, the internals of this
+    # object give a mini DSL to handle common situations such as
+    # stopping if we've found a keyword/end mis-match in one direction
+    # or the other.
     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_hidden && line.hidden?
-        next true if @skip_empty && line.empty?
+        next true if @force_add_hidden && line.hidden?
+        next true if @force_add_empty && line.empty?
 
         kw_count += 1 if line.is_kw?
         end_count += 1 if line.is_end?
@@ -86,8 +115,8 @@ module SyntaxSuggest
       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_empty && line.empty?
+        next true if @force_add_hidden && line.hidden?
+        next true if @force_add_empty && line.empty?
 
         kw_count += 1 if line.is_kw?
         end_count += 1 if line.is_end?
@@ -104,6 +133,33 @@ module SyntaxSuggest
       self
     end
 
+    # Shows surrounding kw/end pairs
+    #
+    # The purpose of showing these extra pairs is due to cases
+    # of ambiguity when only one visible line is matched.
+    #
+    # For example:
+    #
+    #     1  class Dog
+    #     2    def bark
+    #     4    def eat
+    #     5    end
+    #     6  end
+    #
+    # In this case either line 2 could be missing an `end` or
+    # line 4 was an extra line added by mistake (it happens).
+    #
+    # When we detect the above problem it shows the issue
+    # as only being on line 2
+    #
+    #     2    def bark
+    #
+    # Showing "neighbor" keyword pairs gives extra context:
+    #
+    #     2    def bark
+    #     4    def eat
+    #     5    end
+    #
     def capture_neighbor_context
       lines = []
       kw_count = 0
@@ -145,6 +201,20 @@ module SyntaxSuggest
       lines
     end
 
+    # Shows the context around code provided by "falling" indentation
+    #
+    # Converts:
+    #
+    #       it "foo" do
+    #
+    # into:
+    #
+    #   class OH
+    #     def hello
+    #       it "foo" do
+    #     end
+    #   end
+    #
     def on_falling_indent
       last_indent = @orig_indent
       before_lines.reverse_each do |line|
@@ -213,18 +283,31 @@ module SyntaxSuggest
       self
     end
 
+    # Finds code lines at the same or greater indentation and adds them
+    # to the block
     def scan_neighbors_not_empty
       scan_while { |line| line.not_empty? && line.indent >= @orig_indent }
     end
 
+    # Returns the next line to be scanned above the current block.
+    # Returns `nil` if at the top of the document already
     def next_up
       @code_lines[before_index.pred]
     end
 
+    # Returns the next line to be scanned below the current block.
+    # Returns `nil` if at the bottom of the document already
     def next_down
       @code_lines[after_index.next]
     end
 
+    # Scan blocks based on indentation of next line above/below block
+    #
+    # Determines indentaion of the next line above/below the current block.
+    #
+    # Normally this is called when a block has expanded to capture all "neighbors"
+    # at the same (or greater) indentation and needs to expand out. For example
+    # the `def/end` lines surrounding a method.
     def scan_adjacent_indent
       before_after_indent = []
       before_after_indent << (next_up&.indent || 0)
@@ -236,6 +319,16 @@ module SyntaxSuggest
       self
     end
 
+    # TODO: Doc or delete
+    #
+    # I don't remember why this is needed, but it's called in code_context.
+    # It's related to the implementation of `capture_neighbor_context` somehow
+    # and that display improvement is only triggered when there's one visible line
+    #
+    # I think the primary purpose is to not include the current line in the
+    # logic evaluation of `capture_neighbor_context`. If that's true, then
+    # we should fix that method to handle this logic instead of only using
+    # it in one place and together.
     def start_at_next_line
       before_index
       after_index
@@ -244,26 +337,39 @@ module SyntaxSuggest
       self
     end
 
+    # Return the currently matched lines as a `CodeBlock`
+    #
+    # When a `CodeBlock` is created it will gather metadata about
+    # itself, so this is not a free conversion. Avoid allocating
+    # more CodeBlock's than needed
     def code_block
       CodeBlock.new(lines: lines)
     end
 
+    # Returns the lines matched by the current scan as an
+    # array of CodeLines
     def lines
       @code_lines[before_index..after_index]
     end
 
+    # Gives the index of the first line currently scanned
     def before_index
       @before_index ||= @orig_before_index
     end
 
+    # Gives the index of the last line currently scanned
     def after_index
       @after_index ||= @orig_after_index
     end
 
+    # Returns an array of all the CodeLines that exist before
+    # the currently scanned block
     private def before_lines
       @code_lines[0...before_index] || []
     end
 
+    # Returns an array of all the CodeLines that exist after
+    # the currently scanned block
     private def after_lines
       @code_lines[after_index.next..-1] || []
     end
diff --git a/lib/syntax_suggest/block_expand.rb b/lib/syntax_suggest/block_expand.rb
index 8142e74869..8431d15edd 100644
--- a/lib/syntax_suggest/block_expand.rb
+++ b/lib/syntax_suggest/block_expand.rb
@@ -62,7 +62,7 @@ module SyntaxSuggest
     # as there's no undo (currently).
     def expand_indent(block)
       AroundBlockScan.new(code_lines: @code_lines, block: block)
-        .skip(:hidden?)
+        .force_add_hidden
         .stop_after_kw
         .scan_adjacent_indent
         .code_block
@@ -126,7 +126,7 @@ module SyntaxSuggest
     # We try to resolve this edge case with `lookahead_balance_one_line` below.
     def expand_neighbors(block)
       neighbors = AroundBlockScan.new(code_lines: @code_lines, block: block)
-        .skip(:hidden?)
+        .force_add_hidden
         .stop_after_kw
         .scan_neighbors_not_empty
 
diff --git a/lib/syntax_suggest/parse_blocks_from_indent_line.rb b/lib/syntax_suggest/parse_blocks_from_indent_line.rb
index d1071732fe..241ed6acb4 100644
--- a/lib/syntax_suggest/parse_blocks_from_indent_line.rb
+++ b/lib/syntax_suggest/parse_blocks_from_indent_line.rb
@@ -36,8 +36,8 @@ module SyntaxSuggest
     # Builds blocks from bottom up
     def each_neighbor_block(target_line)
       scan = AroundBlockScan.new(code_lines: code_lines, block: CodeBlock.new(lines: target_line))
-        .skip(:empty?)
-        .skip(:hidden?)
+        .force_add_empty
+        .force_add_hidden
         .scan_while { |line| line.indent >= target_line.indent }
 
       neighbors = scan.code_block.lines
diff --git a/spec/syntax_suggest/integration/syntax_suggest_spec.rb b/spec/syntax_suggest/integration/syntax_suggest_spec.rb
index e96173717d..bb50fafce7 100644
--- a/spec/syntax_suggest/integration/syntax_suggest_spec.rb
+++ b/spec/syntax_suggest/integration/syntax_suggest_spec.rb
@@ -207,57 +207,5 @@ module SyntaxSuggest
         > 4  end
       EOM
     end
-
-    it "comment inside of a method" do
-      source = <<~'EOM'
-        class Dog
-          def bark
-            # todo
-          end
-
-          def sit
-            print "sit"
-          end
-        end
-        end # extra end
-      EOM
-
-      io = StringIO.new
-      SyntaxSuggest.call(
-        io: io,
-        source: source
-      )
-      out = io.string
-      expect(out).to include(<<~EOM)
-        >  1  class Dog
-        >  9  end
-        > 10  end # extra end
-      EOM
-    end
-
-    it "space inside of a method" do
-      source = <<~'EOM'
-        class Dog       #  1
-          def bark      #  2
-
-          end           #  4
-
-          def sit       #  6
-            print "sit" #  7
-          end           #  8
-        end             #  9
-        end # extra end
-      EOM
-
-      io = StringIO.new
-      SyntaxSuggest.call(
-        io: io,
-        source: source
-      )
-      out = io.string
-      expect(out).to include(<<~EOM)
-        > 10  end # extra end
-      EOM
-    end
   end
 end
diff --git a/spec/syntax_suggest/unit/around_block_scan_spec.rb b/spec/syntax_suggest/unit/around_block_scan_spec.rb
index be1c3a4780..88d973e151 100644
--- a/spec/syntax_suggest/unit/around_block_scan_spec.rb
+++ b/spec/syntax_suggest/unit/around_block_scan_spec.rb
@@ -149,8 +149,8 @@ module SyntaxSuggest
 
       block = CodeBlock.new(lines: code_lines[3])
       expand = AroundBlockScan.new(code_lines: code_lines, block: block)
-      expand.skip(:empty?)
-      expand.skip(:hidden?)
+      expand.force_add_empty
+      expand.force_add_hidden
       expand.scan_neighbors_not_empty
 
       expect(expand.code_block.to_s).to eq(<<~EOM.indent(4))