troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4k stars 279 forks source link

Trailing comment in method body sometimes interferes with smell suppression comment #1709

Open owst opened 1 year ago

owst commented 1 year ago

Reek (6.1.4) does not report any smells for the following code:

class ExampleClass
  # :reek:LongParameterList
  private def some_method(first, second, third, fourth, fifth)
    p 1
  end
end

however, after adding a comment at the end of the method, the suppression seems to no longer work and reek reports the LongParameterList smell:

class ExampleClass
  # :reek:LongParameterList
  private def some_method(first, second, third, fourth, fifth)
    p 1
    # content of this comment doesn't seem to matter
  end
end

interestingly, removing the class seems to also cause the issue to stop (the smell is suppressed) despite the comment remaining:

# :reek:LongParameterList
private def some_method(first, second, third, fourth, fifth)
  p 1
  # content of this comment doesn't seem to matter
end

also it appears that adding another method before some_method causes the suppression to break again:

def foo
end

# :reek:LongParameterList
private def some_method(first, second, third, fourth, fifth)
  p 1
  # content of this comment doesn't seem to matter
end

Reproduction

To reproduce, set up files one.rb, two.rb, three.rb, four.rb with contents of the above four snippets. Then this is the output from reek, along with related ruby/gem versions:

$ ruby --version
ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-darwin21]
$ reek --version
reek 6.1.4
$ ruby-parse --version
ruby-parse based on parser version 3.2.2.0

$ reek --smell LongParameterList one.rb
Inspecting 1 file(s):
.

$ reek --smell LongParameterList two.rb
Inspecting 1 file(s):
S

two.rb -- 1 warning:
  [3]:LongParameterList: ExampleClass#some_method has 5 parameters [https://github.com/troessner/reek/blob/v6.1.4/docs/Long-Parameter-List.md]

$ reek --smell LongParameterList three.rb
Inspecting 1 file(s):
.

$ reek --smell LongParameterList four.rb
Inspecting 1 file(s):
S

four.rb -- 1 warning:
  [5]:LongParameterList: some_method has 5 parameters [https://github.com/troessner/reek/blob/v6.1.4/docs/Long-Parameter-List.md]
troessner commented 1 year ago

Hi Owen,

stellar bug report, thank you for that!