More rules to be disabled to resolve conflict with rubocop

[tsukkee]

I found following rules also should be disabled to avoid conflict with rubocop formatting.

• Style/IfUnlessModifier

  • rubyfmt prefer normal if to if modifier when the line length is too long
  • Example: https://rubyfmt.run/#cHV0cyAidmVyeV9sb29vb29vb29vb29vb29vb29vb29vb29vb29uZ19zdHJpbmciIGlmIHZlcnlfbG9vb29vb29vb29vb29vb29vb29vb29uZ19jb25kaXRpb24/KGZvbywgYmFyLCBiYXosIGxvb29vb29vbmdfYXJndW1lbnRzKQo=

• Style/MethodCallWithoutArgsParentheses

  • rubyfmt will append () to the method calls when there is the local variable which has same name.
  • Example: https://rubyfmt.run/#Zm9vID0gc2VsZi5mb28=

[reese]

Style/MethodCallWithoutArgsParentheses makes sense to disable, but I'm not so sure about Style/IfUnlessModifier. That one is more of a style choice, yeah? rubyfmt will force to multiline if the line is too long, but the docs suggest that the lint is also aware of long lines. I think the Rubocop lint is actually linting against the opposite situation, when a multiline conditional would instead go on a single line, which I believe is more a matter of taste. (Though honestly I always turn that lint off anyways, so I might just be misreading the docs.)

[tsukkee]

Thank you for your response, and for maintaining this project!

I wasn't aware of the details of Style/IfUnlessModifier, but I now understand that both RuboCop and rubyfmt take line length into account.

This means that RuboCop and rubyfmt can produce different results depending on the RuboCop configuration.
Given that, I think we have two options:

• A. Ignore Style/IfUnlessModifier
• B. Keep Style/IfUnlessModifier enabled, but document how to avoid conflicts with it (for example, by recommending specific RuboCop settings)

Which approach do you think would be preferable?

[reese]

I think I would suggest a third approach, which is to put Style/IfUnlessModifier as one of the optional disables in this gem. It would keep it enabled by default but allows folks to disable it entirely if they want to disable a broader set of lints. That optional lint file is where other vague stylistic lints also live at the moment, and if you wanted to document some suggested configuration for keeping it enabled, you could also document it in the comments in that file as well.

[tsukkee]

Thank you! That sounds like a great approach, and I agree with it.

I'd be happy to open a PR for this. Were you planning to make the change yourself, or would you like me to put together a PR?

[reese]

You're more than welcome to submit a PR if you'd like