Add hint when if or unless is followed by do

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## HEAD (unreleased)
 
+- Added: Support for detecting `if ... do` (accidental `do` after an `if` or `unless`)  ()
+
 ## 2.0.2
 
 - Fix: Separate multiple parser errors by newline. (https://github.com/ruby/syntax_suggest/pull/232)

lib/syntax_suggest/explain_syntax.rb

@@ -111,7 +111,48 @@ def errors
         return GetParseErrors.errors(@code_lines.map(&:original).join).uniq
       end
 
-      missing.map { |miss| why(miss) }
+      out = missing.map { |miss| why(miss) }
+      out.concat(keyword_do_block_hints)
+      out
+    end
+
+    # Keywords that do not accept a `do` block
+    # Note: `while` and `until` DO accept `do`, e.g. `while true do; end`
+    KEYWORD_NO_DO_BLOCK = %w[if unless].freeze
+
+    # Detects lines with a conditional keyword followed by `do`
+    # (e.g., `if ... do` instead of `it ... do` (common in rspec syntax)
+    #
+    # Only triggers when `do` appears after `if`/`unless` but before
+    # an `end` that would close it. For example:
+    #
+    #   `if x do; end; end` - triggers (do before if's end)
+    #   `if x; end; foo do; end` - does not trigger (do after if's end)
+    #
+    # Returns an array of hint messages
+    private def keyword_do_block_hints
+      hints = []
+
+      @code_lines.each do |line|
+        # Stack of unclosed if/unless keywords
+        kw_stack = []
+
+        line.lex.each do |lex|
+          next unless lex.type == :on_kw
+
+          if lex.is_kw? && KEYWORD_NO_DO_BLOCK.include?(lex.token)
+            kw_stack << lex.token
+          elsif lex.token == "end" && kw_stack.any?
+            # An `end` closes the most recent keyword
+            kw_stack.pop
+          elsif lex.token == "do" && kw_stack.any?
+            # `do` appeared while there's still an unclosed if/unless
+            hints << "Both `#{kw_stack.last}` and `do` require an `end`."
+          end
+        end
+      end
+
+      hints.uniq
     end
   end
 end

spec/integration/syntax_suggest_spec.rb

@@ -209,6 +209,32 @@ def bark
       EOM
     end
 
+    it "if .. do" do
+      source = <<~EOM
+        describe "something" do
+          if "does something" do
+            print "foo"
+          end
+        end
+      EOM
+
+      io = StringIO.new
+      SyntaxSuggest.call(
+        io: io,
+        source: source
+      )
+      out = io.string
+      expect(out).to include(<<~EOM)
+        Unmatched keyword, missing `end' ?
+        Both `if` and `do` require an `end`.
+
+          1  describe "something" do
+        > 2    if "does something" do
+        > 4    end
+          5  end
+      EOM
+    end
+
     it "empty else" do
       source = <<~EOM
         class Foo

spec/unit/explain_syntax_spec.rb

@@ -177,6 +177,121 @@ def meow
       expect(explain.errors).to eq([explain.why("end")])
     end
 
+    # https://github.com/ruby/syntax_suggest/issues/206
+    it "explains `if` with `do` error" do
+      source = <<~EOM
+        describe "something" do
+          if "does something" do
+            print "foo"
+          end
+        end
+      EOM
+
+      explain = ExplainSyntax.new(
+        code_lines: CodeLine.from_source(source)
+      ).call
+
+      expect(explain.missing).to eq(["end"])
+      expect(explain.errors).to eq([
+        explain.why("end"),
+        "Both `if` and `do` require an `end`."
+      ])
+    end
+
+    it "shows hint for multiple `if`/`unless` with `do` on separate lines" do
+      source = <<~EOM
+        describe "something" do
+          unless "does something" do
+            print "bar"
+          end
+          if "does something" do
+            print "foo"
+          end
+        end
+      EOM
+
+      explain = ExplainSyntax.new(
+        code_lines: CodeLine.from_source(source)
+      ).call
+
+      expect(explain.missing).to eq(["end"])
+      expect(explain.errors).to eq([
+        explain.why("end"),
+        "Both `unless` and `do` require an `end`.",
+        "Both `if` and `do` require an `end`."
+      ])
+    end
+
+    it "shows hint for innermost unclosed `if` when nested before `do`" do
+      source = <<~EOM
+        describe "something" do
+          unless if "do something" do; end
+            print "bar"
+          end
+        end
+      EOM
+
+      explain = ExplainSyntax.new(
+        code_lines: CodeLine.from_source(source)
+      ).call
+
+      expect(explain.missing).to eq(["end"])
+      expect(explain.errors).to eq([
+        explain.why("end"),
+        "Both `if` and `do` require an `end`."
+      ])
+    end
+
+    it "shows hint for `unless` when inner `if` is closed before `do`" do
+      source = <<~EOM
+        describe "something" do
+          unless if "do something"; end; do
+            print "bar"
+          end
+        end
+      EOM
+
+      explain = ExplainSyntax.new(
+        code_lines: CodeLine.from_source(source)
+      ).call
+
+      expect(explain.missing).to eq(["end"])
+      expect(explain.errors).to eq([
+        explain.why("end"),
+        "Both `unless` and `do` require an `end`."
+      ])
+    end
+
+    it "does not show hint if the `do` is after a method call that might accept a block" do
+      source = <<~EOM
+        [1,2,3].map { |x| x * 2 if x > 1 }.each do |y|
+          puts y
+      EOM
+
+      explain = ExplainSyntax.new(
+        code_lines: CodeLine.from_source(source)
+      ).call
+
+      expect(explain.missing).to eq(["end"])
+      expect(explain.errors).to eq([
+        explain.why("end")
+      ])
+
+      source = <<~EOM
+        [1,2,3].map { |x| if x > 1; x * 2; end }.each do |y|
+          puts y
+      EOM
+
+      explain = ExplainSyntax.new(
+        code_lines: CodeLine.from_source(source)
+      ).call
+
+      expect(explain.missing).to eq(["end"])
+      expect(explain.errors).to eq([
+        explain.why("end")
+      ])
+    end
+
     it "falls back to ripper on unknown errors" do
       source = <<~EOM
         class Cat