incorrect doc reference when methods are too close

[IgnacioFan]

While developing PR #2733, I accidentally found this very tiny bug! But there may be a different way to look at it. Anyway, just bring it up!

Description

If we have two methods like the example, the bar will share the doc from foo.

module Test
  # This method shows foo
  def foo; end
  def bar; end
end

class Something
  include Test

  def foobar
    bar
  end
end

But if we add one line between the two methods, the bar will act normally!

module Test
  # This method shows foo
  def foo
  end
  def bar; end
end

class Something
  include Test

  def foobar
    bar
  end
end

Additional Info

Here is where I found it over the conversation on PR #2733

[vinistock]

Thank you for creating the issue!

The mistake comes from this line. Essentially, we wanted to ensure that we didn't miss documentation for projects that add comments to their declaration with a blank line in between. Like this:

# Documentation for Foo

class Foo
  # Documentation for bar  

  def bar
  end
end

However, our check is a bit naive. If there's no comment immediately above the declaration, we check if there's a comment in the line above that one, without checking if the contents of the line in between is another declaration, which leads to the bug.

We should only consider comments as part of the documentation if the line in between is blank.

[IgnacioFan]

So, you mean whether a method will have a document, which depends on whether there is a comment above it. The comment can be above the method within one line break.

So, I think the solution could be to check whether the code above the method has # or not.

[vinistock]

So, I think the solution could be to check whether the code above the method has # or not.

Essentially, this line has to change to be

start_line -= 1 unless @comments_by_line.key?(start_line) || !source[start_line - 1].empty?

We don't have direct access to the source, but the parse result has a handle to it somewhere.

[IgnacioFan]

@vinistock Thanks, that's clear enough! I am gonna create a PR for this issue.

[IgnacioFan]

@vinistock, I tried out what you said. I didn't find a way to use parse_result as you do in the pseudo-code.
But I just came up with an idea: why not remove comments or mark them as visited from @comments_by_line while traversing nodes? This approach doesn't need to access the source. Alternatively, when collecting comments for each node, we just need to look up twice (first start_line is current line - 1 and second start_line is current line - 2) and break if comment returns nil

# Documentation for Foo <- will be removed or marked when visiting the Foo node

class Foo
  # ####################  <- will be removed or marked when visiting the bar node
  # Documentation for bar <- will be removed or marked when visiting the bar node
  # ####################  <- will be removed or marked when visiting the bar node
  #                       <- will be removed or marked when visiting the bar node
  def bar
  end

  # Documentation for baz <- will be removed or marked when visiting the baz node
  def baz; end
  def ban; end <- do nothing when visiting the ban node because there is no key or all marked as visited
end

[vinistock]

Good idea, I think deleting the comments from the hash is a good strategy 👍.