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

Style/ClassAndModuleChildren does not handle comments properly #9767

Closed
dvandersluis opened this issue May 3, 2021 · 4 comments · Fixed by #9784
Closed

Style/ClassAndModuleChildren does not handle comments properly #9767

dvandersluis opened this issue May 3, 2021 · 4 comments · Fixed by #9784
Labels

Comments

@dvandersluis
Copy link
Member

EnforcedStyle: compact

class A # class A comment
  class B # class B comment
  end
end

autocorrects to

class A::B # class B comment
end

The class A comment disappears.


EnforcedStyle: nested

class A::B # class A comment
end

autocorrects to

module A
  class B # class A comment
  end
end

The comment moves to the inner class which may not be desired.

RuboCop version

master

@dvandersluis
Copy link
Member Author

I think in both cases, we should probably just move all comments to above the outer class/module.

@tejasbubane
Copy link
Contributor

tejasbubane commented May 7, 2021

@dvandersluis I feel the existing behaviour for EnforcedStyle: nested is correct. See the test I added in linked PR. Style/InlineComment should take care of moving it up.

@dvandersluis
Copy link
Member Author

@tejasbubane I actually have never even noticed that Style/InlineComment exists! It's disabled by default.

But that's not actually what I was getting at. My point was that there's no way of knowing if the inline comment was for module A or class B in the original case, so putting it on top is probably "safer".

@tejasbubane
Copy link
Contributor

tejasbubane commented May 8, 2021

@dvandersluis

My point was that there's no way of knowing if the inline comment was for module A or class B in the original case, so putting it on top is probably "safer".

class A::B # some comment

Since this is definition of B, doesn't that mean the comment is for B and A is just a namespace wrapper?

My point is EnforcedStyle: compact for this cop actually eats up the comment which is definitely a bug.
EnforcedStyle: nested on the other hand, leaves the comment as is and I feel it is not the job of this cop to move it up.

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

Successfully merging a pull request may close this issue.

2 participants