Skip to content

Commit

Permalink
[ruby/syntax_suggest] Fix
Browse files Browse the repository at this point in the history
ruby/syntax_suggest#187 Handle if/else with
empty/comment
line

Reported in #187 this code:

```
class Foo
  def foo
    if cond?
      foo
    else
      # comment
    end
  end

  # ...

  def bar
    if @recv
    end_is_missing_here
  end
end
```

Triggers an incorrect suggestion:

```
Unmatched keyword, missing `end' ?
   1  class Foo
   2    def foo
>  3      if cond?
>  5      else
   8    end
  16  end
```

Part of the issue is that while scanning we're using newlines to determine when to stop and pause. This is useful for determining logically smaller chunks to evaluate but in this case it causes us to pause before grabbing the "end" that is right below the newline. This problem is similar to ruby/syntax_suggest#179.

However in the case of expanding same indentation "neighbors" I want to always grab all empty values at the end of the scan. I don't want to do that when changing indentation levels as it affects scan results.

There may be some way to normalize this behavior between the two, but the tests really don't like that change.

To fix this issue for expanding against different indentation I needed a way to first try and grab any additional newlines with the ability to rollback that guess. In #192 I experimented with decoupling scanning from the AroundBlockScan logic. It also added the ability to take a snapshot of the current scanner and rollback to prior changes.

With this ability in place now we:

- Grab extra empties before looking at the above/below line for the matching keyword/end statement
- If there's a match, grab it
- If there's no match, discard the newlines we picked up in the evaluation

That solves the issue.

ruby/syntax_suggest@513646b912
  • Loading branch information
schneems authored and hsbt committed May 23, 2023
1 parent aaf815c commit c638ffa
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 9 deletions.
18 changes: 13 additions & 5 deletions lib/syntax_suggest/around_block_scan.rb
Expand Up @@ -181,7 +181,7 @@ def capture_before_after_kws
lines << line if line.is_kw? || line.is_end?
}
)
@scanner.try_rollback
@scanner.stash_changes
lines
end

Expand Down Expand Up @@ -240,7 +240,7 @@ def on_falling_indent
true
}
)
@scanner.try_rollback
@scanner.stash_changes
self
end

Expand Down Expand Up @@ -275,26 +275,34 @@ def lookahead_balance_one_line

return self if kw_count == end_count # nothing to balance

@scanner.commit_if_changed
scan_while { |line| line.empty? }

# More ends than keywords, check if we can balance expanding up
next_up = @scanner.next_up
next_down = @scanner.next_down
if (end_count - kw_count) == 1 && next_up
if next_up.is_kw? && next_up.indent >= @orig_indent
if next_up.is_kw? && next_up.indent >= @target_indent
@scanner.scan(
up: ->(line, _, _) { line == next_up },
down: ->(line, _, _) { false }
)
@scanner.commit_if_changed
end

# More keywords than ends, check if we can balance by expanding down
elsif (kw_count - end_count) == 1 && next_down
if next_down.is_end? && next_down.indent >= @orig_indent
if next_down.is_end? && next_down.indent >= @target_indent
@scanner.scan(
up: ->(line, _, _) { false },
down: ->(line) { line == next_down }
down: ->(line, _, _) { line == next_down }
)
@scanner.commit_if_changed
end
end

@scanner.stash_changes

self
end

Expand Down
7 changes: 5 additions & 2 deletions lib/syntax_suggest/block_expand.rb
Expand Up @@ -61,11 +61,14 @@ def call(block)
# they can expand to capture more code up and down). It does this conservatively
# as there's no undo (currently).
def expand_indent(block)
AroundBlockScan.new(code_lines: @code_lines, block: block)
now = AroundBlockScan.new(code_lines: @code_lines, block: block)
.force_add_hidden
.stop_after_kw
.scan_adjacent_indent
.code_block

now.lookahead_balance_one_line

now.code_block
end

# A neighbor is code that is at or above the current indent line.
Expand Down
11 changes: 9 additions & 2 deletions lib/syntax_suggest/scan_history.rb
Expand Up @@ -17,13 +17,20 @@ def initialize(code_lines:, block:)
def commit_if_changed
if changed?
@history << CodeBlock.new(lines: @code_lines[before_index..after_index])
refresh_index
end

self
end

def try_rollback
# Discards any changes that have not been committed
def stash_changes
refresh_index
end

# Discard changes that have not been committed and revert the last commit
#
# Cannot revert the first commit
def revert_last_commit
if @history.length > 1
@history.pop
refresh_index
Expand Down
31 changes: 31 additions & 0 deletions spec/syntax_suggest/integration/syntax_suggest_spec.rb
Expand Up @@ -204,5 +204,36 @@ def bark
> 4 end
EOM
end

it "empty else" do
source = <<~'EOM'
class Foo
def foo
if cond?
foo
else
end
end
# ...
def bar
if @recv
end_is_missing_here
end
end
EOM

io = StringIO.new
SyntaxSuggest.call(
io: io,
source: source
)
out = io.string
expect(out).to include(<<~EOM)
end_is_missing_here
EOM
end
end
end

0 comments on commit c638ffa

Please sign in to comment.