FC006 - not detecting 4-digit numbers

[miketheman]

Consider:

file '/tmp/file' do
  mode 0644
end

According to the docs, this rule should be triggered when not a 5-digit octal mode. Are the docs wrong, or is FC wrong?

[acrmp]

Hi Mike,

Thanks for raising this. For FC006 we allow the mode to be specified as four unquoted numbers, but only where the first number is a zero.

See #9 for the background detail on this. I think we left this case out of the rule documentation to try to avoid making the message too confusing.

Thanks,

Andrew.

[miketheman]

Thanks for the explanation.

That tells me that this is a docs bug, and should be addressed. :)

-M

[acrmp]

Ha. The existing docs do over-simplify but with the intent of making the rule to follow easier to remember - just quote the mode or specify at least five digits.

If you would like to submit a pull request to add "four digits are ok but only if the first digit is a leading zero" then that's totally cool, but let's try not to make it confusing.

[sethvargo]

@acrmp I would personally prefer if Foodcritic picked a way and enforced it. Part of being a linter is enforcing consistency, even if it's not "wrong" to do it the other way.

[acrmp]

Does anyone else have strong feelings on this?

In my opinion quoting is preferred but I'm not keen to raise FC006 which is a correctness warning against numeric modes specified to five digits.

[miketheman]

My feeling is:

If it can go wrong, it will, and it will leave a new user scratching their head in wonderment.

If you can make logic work for:

• quoted string
• 5 digit octal mode
• 4 digit with a leading 0

then that covers it.
If not, then first 2 are my vote.

[miketheman]

Oh, and it should probably be more "style" than "correctness" if it's a stylistic choice and won't break something,

[acrmp]

@miketheman I'm not quite following you.

• FC006 as implemented currently checks for file modes that are not any of the three cases you specified above
• It's specified as a correctness rule because file modes specified as 4 digit without a leading zero will cause incorrect permissions to be set
• Seth is advocating making one of the approaches the recommended approach and warning against the others. We could add another rule tagged with style for this.

[miketheman]

Then I believe this goes back to my original question. The docs surrounding this are misrepresenting the desired behavior.

The rule is of correctness, so if the conditions are all there for testing the scenarios and report if there's something wrong, then cool.

The docs read "... either quote the octal number or ensure it is specified to five digits..." - so the docs might need an update to reflect allowing 4-digit, leading 0 permissions masks.

The reason I think we should go "string or 5-digit" is Ruby's File.chmod is expecting an integer, and the chmod expresses all examples in 5-digit mode (with leading 0).

While it is correct-enough to have 4-digit leading 0, then maybe we need a style rule for this, as well as a doc update.

Basically, I think it's time for FC to become somewhat opinionated about certain styles, and "recommend" them.

The term "best practices" that everyone seems to ask for keeps coming up, and style is part of that, as it's not correctness.

[miketheman]

Closing due to inactivity.