Skip to content

Commit

Permalink
This is a difficult problem
Browse files Browse the repository at this point in the history
The algorithm builds a pretty good looking tree but then it sees this:

```
  1  class Blerg
❯ 2    Foo.call do |lol
❯ 3      print lol
❯ 4    end # one
  5    print lol
  6    class Foo
  7    end # two
  8  end # three
```

It thinks that there are two invalid nodes here

```
❯ 2    Foo.call do |lol
```

and

```
❯ 4    end # one
```

While it's obvious to you and me that they belong together the search algorithm calls a `multiple_invalid_parents` and continues to explore both which gives us the weird output:


```
       +Unmatched `|', missing `|' ?
       +Unmatched keyword, missing `end' ?
       +
       +  1  class Blerg
       +❯ 2    Foo.call do |lol
       +  4    end # one
       +  6    class Foo
       +  8  end # three
       +Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?
       +
       +  1  class Blerg
       +  2    Foo.call do |lol
       +❯ 4    end # one
       +  6    class Foo
       +  7    end # two
       +  8  end # three
```

Interesting enough if we combined these two it would be the perfect output, which makes me think it's not a different problem than the "fix it in post" ones I mentioned on the last commit, but rather might be a variation on them.

Sometimes we only match the `end` but not the source line. Other times we match the end AND the source line if they both have a syntax error on them. 

A harder case is replacing the mismatched pipe with something mismatched up like

```
        class Blerg
          Foo.call }
            print haha
            print lol
          end # one
          print lol
          class Foo
          end # two
        end # three
```

Gives us:

```
❯ 1  class Blerg
  9  end # three
Unmatched `}', missing `{' ?

  1  class Blerg
❯ 2    Foo.call }
  5    end # one
  7    class Foo
  9  end # three
Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?

  1  class Blerg
❯ 5    end # one
  7    class Foo
  8    end # two
  9  end # three
Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?

  1  class Blerg
❯ 9  end # three
```

So it's like double the problem from before. `class Blerg/end # three` belong together and `Foo.call }/end #one` belong together. Technically one of these is not a good match `class Blerg/end # three`. 

We could do something naive that might work long term or try to get more clever. 

It seems like we could add another grouping round after the search round. We could try to intelligently pair nodes together and do fancy stuff like re-check parsability of the daocument. That would be the advanced path.

The "easy" path would be to shove all these groups together at the very end so that the output might look like this:

```
❯ 1  class Blerg
❯ 2    Foo.call }
❯ 5    end # one
❯ 9  end # three
```

Which is honestly pretty good. Not ideal, but good. The main downside is we would eventually need a way to split off ACTUAL multiple syntax errors and report on them. I don't remember at this point if that was a feature of the old algorithm or not. If not, then it's no regression so maybe it's fine to start there.

So that's good. Maybe the search algorithm is good enough and it's just up to touching up the post operations. Here's the list from last time of the failure:

### Needs investigation 0

```
  1  class Blerg
❯ 2    Foo.call do |a
  5    class Foo
  6    end # two
  7  end # three
Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?

  1  class Blerg
❯ 3    end # one
  5    class Foo
  6    end # two
  7  end # three
  handles mismatched | (FAILED - 1)

Failures:

  1) Integration tests that don't spawn a process (like using the cli) handles mismatched |
     Failure/Error: raise("this should be one failure, not two")

     RuntimeError:
       this should be one failure, not two
     # ./spec/integration/dead_end_spec.rb:603:in `block (2 levels) in <module:DeadEnd>'
```

### Needs investigation 1

Expected:

```
          7  context "test" do
        ❯ 8    it "should" do
          9  end
```

Actual

```
       +Unmatched keyword, missing `end' ?
       +
       +  1  context "foo bar" do
       +❯ 7  context "test" do
       +  9  end

     # ./spec/integration/dead_end_spec.rb:418:in `block (2 levels) in <module:DeadEnd>'
```


### Needs investigation 2


Expected:

```
    it "finds a naked end" do
      source = <<~'EOM'
        def foo
          end # one
        end # two
      EOM

      io = StringIO.new
      DeadEnd.call(
        io: io,
        source: source
      )

      expect(io.string).to include(<<~'EOM')
        ❯ end # one
      EOM
    end
```

Actual:


```
       +Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?
       +
       +  1  def foo
       +❯ 3  end # two
```



## Fix it in post 1 

```
       +Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?
       +
       +   1  describe "things" do
       +   2    it "blerg" do
       +   3    end
       +❯  6    end
       +   8    it "zlerg" do
       +   9    end
       +  10  end

     # ./spec/integration/dead_end_spec.rb:227:in `block (2 levels) in <module:DeadEnd>'
```

Several of these repeated


## Fix it in post 2

Subtly different:


```
       +Unmatched `end', missing keyword (`do', `def`, `if`, etc.) ?
       +
       +   2  RSpec.describe AclassNameHere, type: :worker do
       +   3    describe "thing" do
       +  13    end # line 16 accidental end, but valid block
       +❯ 23    end # mismatched due to 16
       +  24  end

     # ./spec/integration/dead_end_spec.rb:481:in `block (2 levels) in <module:DeadEnd>'
```


Next time I want to look at "### Needs investigation 2" to see how it compares/contrasts to the other ones.
  • Loading branch information
schneems committed Jul 26, 2022
1 parent c3b30da commit 4dfa1b2
Showing 1 changed file with 25 additions and 2 deletions.
27 changes: 25 additions & 2 deletions spec/integration/dead_end_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -562,12 +562,36 @@ def foo
EOM
end

it "is harder" do
source = <<~EOM
class Blerg
Foo.call }
print haha
print lol
end # one
print lol
class Foo
end # two
end # three
EOM

io = StringIO.new
DeadEnd.call(
io: io,
source: source
)

puts io.string
raise "not implemented"
end

it "handles mismatched |" do
source = <<~EOM
class Blerg
Foo.call do |a
print lol
end # one
puts lol
print lol
class Foo
end # two
end # three
Expand Down Expand Up @@ -599,6 +623,5 @@ class Foo

raise("this should be one failure, not two")
end

end
end

0 comments on commit 4dfa1b2

Please sign in to comment.