[Fix #4882] Use IndentationWidth of Layout/Tab for other cops

[AlexWayfer]

• Metrics/LineLength
• Style/IfUnlessModifier

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Run rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

[bbatsov]

I agree with the overall spirit of the change, but I wonder if this is the best way to share the common config between the few cops. Many we can pull up some config values under CommonSettings or something like this instead?

[AlexWayfer]

I agree with the overall spirit of the change, but I wonder if this is the best way to share the common config between the few cops. Many we can pull up some config values under CommonSettings or something like this instead?

Why don't you like Layout/Tab => IndentationWidth?

[bbatsov]

Because from a user perspective I find it extremely odd that one cop would be configuring the behavior of a few others. That's never exactly transparent, and it's often quite confusing. We have such dependencies in the code even now, but it's usually for cops where the dependencies between they are quite obvious - I'd normally never go looking for a base indentation width setting in some cop converting tabs into spaces.

[bbatsov]

Hmm, I didn't read the ticket before commenting - seeing it I'm even more puzzled. :D I guess you want to teach other cops what to do when tabs are present and this cop for tabs vs spaces is disabled, right? Do you plan to mix tabs and spaces?

[AlexWayfer]

Hmm, I didn't read the ticket before commenting - seeing it I'm even more puzzled. :D I guess you want to teach other cops what to do when tabs are present and this cop for tabs vs spaces is disabled, right?

Yes. I thought about:

Layout/Indentaiton:
  EnforcedStyle: spaces # or 'tabs'
  Width: 2 # or Size:

But I think Layout/Tab is OK for these purposes.

Do you plan to mix tabs and spaces?

No. Only for alignment:

→if foo &&
→...bar
→→job
→end

[bbatsov]

Got it. I'm on the fence about this. @jonas054 What do you think?

[jonas054]

👍 I think the change is good.

We can assume that most people use only spaces for indentation and alignment, so it won't cause any problems for them. And the change does provide some rudimentary support in RuboCop for people who like the Smart Tabs idea (tabs for indentation and spaces for alignment).

Down the road, we might like to add EnforcedStyle: smart_tabs to Layout/Tab so it can check that there are no tabs after space, and a default EnforcedStyle: no_tabs acting like today.

[bbatsov]

Down the road, we might like to add EnforcedStyle: smart_tabs to Layout/Tab so it can check that there are no tabs after space, and a default EnforcedStyle: no_tabs acting like today.

That's a great idea!

[bbatsov]

@AlexWayfer The documentation of the affected cops has to be updated to mention the new behaviour and the changelog has to be moved to the current release.

[AlexWayfer]

and the changelog has to be moved to the current release

Done.

@AlexWayfer The documentation of the affected cops has to be updated to mention the new behaviour

I didn't find any information about docs changing in the CONTRIBUTION.md (should I edit anything in the manual/ dir?), but I changed comments above the code of the affected cops.

[jonas054]

@AlexWayfer It's not mentioned in CONTRIBUTION.md, but in the check list of the PR you have "Run rake default or rake parallel." If there are any changes in cop documentation, you will get changes in generated manual files, that you need to commit.

There are currently failures in the build, BTW.

[AlexWayfer]

@AlexWayfer It's not mentioned in CONTRIBUTION.md, but in the check list of the PR you have "Run rake default or rake parallel." If there are any changes in cop documentation, you will get changes in generated manual files, that you need to commit.

Thank you!

There are currently failures in the build, BTW.

That's strange. OK, I fixed.

---

Wat?

[AlexWayfer]

Oh, I found #5966.

[koic]

@AlexWayfer I opened a PR #6003 to fix for Layout/ClosingHeredocIndentation false positive.

[AlexWayfer]

@AlexWayfer I opened a PR #6003 to fix for Layout/ClosingHeredocIndentation false positive.

Thank you, I'll wait merging for rebase.

[AlexWayfer]

I was on vacation, now I've rebased this branch and tests passed.

(thanks, @koic)