Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
owst opened this issue Apr 6, 2023 · 1 comment
Assignees

Comments

@owst
Copy link

owst commented Apr 6, 2023

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]
@mvz mvz self-assigned this Apr 7, 2023
@troessner
Copy link
Owner

Hi Owen,

stellar bug report, thank you for that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants