Add lex based banner

CHANGELOG.md

@@ -1,10 +1,11 @@
 ## HEAD (unreleased)
 
+- Fix bug causing poor results (fix #95, fix #88) ()
 - [Breaking] Remove previously deprecated `require "dead_end/fyi"` interface (https://github.com/zombocom/dead_end/pull/94)
 - DeadEnd is now fired on EVERY syntax error (https://github.com/zombocom/dead_end/pull/94)
-- Output format changes
-  - The "banner" is removed in favor of original parse error messages (https://github.com/zombocom/dead_end/pull/94)
+- Output format changes:
   - Parse errors emitted per-block rather than for the whole document (https://github.com/zombocom/dead_end/pull/94)
+  - The "banner" is now based on lexical analysis rather than parser regex (fix #68, fix #87) ()
 
 ## 2.0.2
 

README.md

@@ -2,19 +2,14 @@
 
 An error in your code forces you to stop. DeadEnd helps you find those errors to get you back on your way faster.
 
-    DeadEnd: Unmatched `end` detected
-
-    This code has an unmatched `end`. Ensure that all `end` lines
-    in your code have a matching syntax keyword  (`def`,  `do`, etc.)
-    and that you don't have any extra `end` lines.
-
-    file: path/to/dog.rb
-    simplified:
+```
+Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?
 
-       3  class Dog
-    ❯  5    defbark
-    ❯  7    end
-      12  end
+  1  class Dog
+❯ 2    defbark
+❯ 4    end
+  5  end
+```
 
 ## Installation in your codebase
 
@@ -52,34 +47,99 @@ This gives you the CLI command `$ dead_end` for more info run `$ dead_end --help
 
 ## What syntax errors does it handle?
 
+Dead end will fire against all syntax errors and can isolate any syntax error. In addition, dead_end attempts to produce human readable descriptions of what needs to be done to resolve the issue. For example:
+
 - Missing `end`:
 
+<!--
 ```ruby
 class Dog
   def bark
     puts "bark"
-
-  def woof
-    puts "woof"
-  end
 end
-# => scratch.rb:8: syntax error, unexpected end-of-input, expecting `end'
 ```
+-->
 
-- Unexpected `end`
+```
+Unmatched keyword, missing `end' ?
 
+❯ 1  class Dog
+❯ 2    def bark
+❯ 4  end
+```
+
+- Missing keyword
+<!--
 ```ruby
 class Dog
   def speak
-    @sounds.each |sound| # Note the missing `do` here
+    @sounds.each |sound|
       puts sound
     end
   end
 end
-# => scratch.rb:7: syntax error, unexpected `end', expecting end-of-input
 ```
+-->
 
-As well as unmatched `|` and unmatched `}`. These errors can be time consuming to debug because Ruby often only tells you the last line in the file. The command `ruby -wc path/to/file.rb` can narrow it down a little bit, but this library does a better job.
+```
+Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?
+
+  1  class Dog
+  2    def speak
+❯ 3      @sounds.each |sound|
+❯ 5      end
+  6    end
+  7  end
+```
+
+- Missing pair characters (like `{}`, `[]`, `()` , or `|<var>|`)
+<!--
+
+```ruby
+class Dog
+  def speak(sound
+    puts sound
+  end
+end
+```
+-->
+
+```
+Unmatched `(', missing `)' ?
+
+  1  class Dog
+❯ 2    def speak(sound
+❯ 4    end
+  5  end
+```
+
+- Any ambiguous or unknown errors will be annotated by the original ripper error output:
+
+<!--
+class Dog
+  def meals_last_month
+    puts 3 *
+  end
+end
+-->
+
+```
+syntax error, unexpected end-of-input
+
+  1  class Dog
+  2    def meals_last_month
+❯ 3      puts 3 *
+  4    end
+  5  end
+```
+
+## How is it better than `ruby -wc`?
+
+Ruby allows you to syntax check a file with warnings using `ruby -wc`. This emits a parser error instead of a human focused error. Ruby's parse errors attempt to narrow down the location and can tell you if there is a glaring indentation error involving `end`.
+
+The `dead_end` algorithm doesn't just guess at the location of syntax errors, it re-parses the document to prove that it captured them.
+
+This library focuses on the human side of syntax errors. It cares less about why the document could not be parsed (computer problem) and more on what the programmer needs (human problem) to fix the problem.
 
 ## Sounds cool, but why isn't this baked into Ruby directly?
 
@@ -105,6 +165,14 @@ After checking out the repo, run `bin/setup` to install dependencies. Then, run
 
 To install this gem onto your local machine, run `bundle exec rake install`. To release a new version, update the version number in `version.rb`, and then run `bundle exec rake release`, which will create a git tag for the version, push git commits and tags, and push the `.gem` file to [rubygems.org](https://rubygems.org).
 
+### How to debug changes to output display
+
+You can see changes to output against a variety of invalid code by running specs and using the `DEBUG_DISPLAY=1` environment variable. For example:
+
+```
+$ DEBUG_DISPLAY=1 be rspec spec/ --format=failures
+```
+
 ## Contributing
 
 Bug reports and pull requests are welcome on GitHub at https://github.com/zombocom/dead_end. This project is intended to be a safe, welcoming space for collaboration, and contributors are expected to adhere to the [code of conduct](https://github.com/zombocom/dead_end/blob/master/CODE_OF_CONDUCT.md).

lib/dead_end.rb

@@ -28,8 +28,9 @@ def self.handle_error(e)
     raise e
   end
 
-  def self.call(source:, filename:, terminal: DEFAULT_VALUE, record_dir: nil, timeout: TIMEOUT_DEFAULT, io: $stderr)
+  def self.call(source:, filename: DEFAULT_VALUE, terminal: DEFAULT_VALUE, record_dir: nil, timeout: TIMEOUT_DEFAULT, io: $stderr)
     search = nil
+    filename = nil if filename == DEFAULT_VALUE
     Timeout.timeout(timeout) do
       record_dir ||= ENV["DEBUG"] ? "tmp" : nil
       search = CodeSearch.new(source, record_dir: record_dir).call
@@ -141,4 +142,6 @@ def self.valid?(source)
 require_relative "dead_end/display_invalid_blocks"
 require_relative "dead_end/parse_blocks_from_indent_line"
 
+require_relative "dead_end/explain_syntax"
+
 require_relative "dead_end/auto"

lib/dead_end/code_block.rb

@@ -70,8 +70,24 @@ def invalid?
     end
 
     def valid?
-      return @valid if @valid != UNSET
-      @valid = DeadEnd.valid?(to_s)
+      if @valid == UNSET
+        # Performance optimization
+        #
+        # If all the lines were previously hidden
+        # and we expand to capture additional empty
+        # lines then the result cannot be invalid
+        #
+        # That means there's no reason to re-check all
+        # lines with ripper (which is expensive).
+        # Benchmark in commit message
+        @valid = if lines.all? { |l| l.hidden? || l.empty? }
+          true
+        else
+          DeadEnd.valid?(lines.map(&:original).join)
+        end
+      else
+        @valid
+      end
     end
 
     def to_s

lib/dead_end/code_frontier.rb

@@ -54,18 +54,42 @@ def initialize(code_lines:)
       @code_lines = code_lines
       @frontier = []
       @unvisited_lines = @code_lines.sort_by(&:indent_index)
+      @has_run = false
+      @check_next = true
     end
 
     def count
       @frontier.count
     end
 
+    # Performance optimization
+    #
+    # Parsing with ripper is expensive
+    # If we know we don't have any blocks with invalid
+    # syntax, then we know we cannot have found
+    # the incorrect syntax yet.
+    #
+    # When an invalid block is added onto the frontier
+    # check document state
+    private def can_skip_check?
+      check_next = @check_next
+      @check_next = false
+
+      if check_next
+        false
+      else
+        true
+      end
+    end
+
     # Returns true if the document is valid with all lines
     # removed. By default it checks all blocks in present in
     # the frontier array, but can be used for arbitrary arrays
     # of codeblocks as well
-    def holds_all_syntax_errors?(block_array = @frontier)
-      without_lines = block_array.map do |block|
+    def holds_all_syntax_errors?(block_array = @frontier, can_cache: true)
+      return false if can_cache && can_skip_check?
+
+      without_lines = block_array.flat_map do |block|
         block.lines
       end
 
@@ -120,6 +144,8 @@ def <<(block)
       @frontier.reject! { |b|
         b.starts_at >= block.starts_at && b.ends_at <= block.ends_at
       }
+
+      @check_next = true if block.invalid?
       @frontier << block
       @frontier.sort!
 
@@ -142,7 +168,7 @@ def self.combination(array)
     # the smallest possible set of blocks that contain all the syntax errors
     def detect_invalid_blocks
       self.class.combination(@frontier.select(&:invalid?)).detect do |block_array|
-        holds_all_syntax_errors?(block_array)
+        holds_all_syntax_errors?(block_array, can_cache: false)
       end || []
     end
   end

lib/dead_end/display_invalid_blocks.rb

@@ -27,8 +27,10 @@ def call
         return self
       end
 
-      @io.puts("--> #{filename}") if filename
-      @io.puts
+      if filename
+        @io.puts("--> #{filename}")
+        @io.puts
+      end
       @blocks.each do |block|
         display_block(block)
       end
@@ -37,29 +39,35 @@ def call
     end
 
     private def display_block(block)
+      # Build explanation
+      explain = ExplainSyntax.new(
+        code_lines: block.lines
+      ).call
+
+      # Enhance code output
+      # Also handles several ambiguious cases
       lines = CaptureCodeContext.new(
         blocks: block,
         code_lines: @code_lines
       ).call
 
+      # Build code output
       document = DisplayCodeWithLineNumbers.new(
         lines: lines,
         terminal: @terminal,
         highlight_lines: block.lines
       ).call
 
-      RipperErrors.new(block.lines.map(&:original).join).call.errors.each do |e|
+      # Output syntax error explanation
+      explain.errors.each do |e|
         @io.puts e
       end
       @io.puts
 
+      # Output code
       @io.puts(document)
     end
 
-    private def banner
-      Banner.new(invalid_obj: @invalid_obj).call
-    end
-
     private def code_with_context
       lines = CaptureCodeContext.new(
         blocks: @blocks,