rubocop: Discourage the use of rm_f and rm_rf in formulae and casks

[issyl0]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

• This cop checks for the use of FileUtils.rm_rf and suggests using FileUtils.rm_r because we should know if we couldn't delete a thing for some reason, not just force it.

[MikeMcQuaid]

Makes sense! Could we do this for rmtree too?

[Bo98]

Worth noting because we do include FileUtils that formulae will use rm_rf (self/no receiver).

However because we do that we can also replace rm_rf in formulae with something better as we're not really monkey patching anything there - the Formula class is our own API.

[MikeMcQuaid]

Worth noting because we do include FileUtils that formulae will use rm_rf (self/no receiver).

Yes, good point/catch.

However because we do that we can also replace rm_rf in formulae with something better as we're not really monkey patching anything there - the Formula class is our own API.

I think the existing behaviour of rm_rf makes sense: it's like rm -rf and there are cases (although few in Homebrew) where you do want to avoid a failure when the file doesn't exist. I'd sooner see us use a better call instead.

Point taken, though, that in other cases yeh this is a good case where we should just use our own APIs here.

[Bo98]

Point taken, though, that in other cases yeh this is a good case where we should just use our own APIs here.

Yeah at least we have that for FileUtils in formulae. The annoying one is Pathname#rmtree. The change in behaviour in newer Ruby is surprising and subtle. It's a shame there's not another Pathname method that can be used instead and instead needs to be FileUtils now.

Pretty much all existing rmtree usages are however likely safe to use as FilesUtils.rm_r as they were probably all written when rmtree did use that.

[issyl0]

Hmmm, autofixing stuff makes all of the tests fail. 🤔

[MikeMcQuaid]

@issyl0 I think this could be scoped just to formulae/casks, if that made things easier?

[issyl0]

In formulae and casks, now:

7091 files inspected, 282 offenses detected, 282 offenses autocorrectable

[Bo98]

Looking good!

[issyl0]

There appears to be a lack of consensus on the formula fixes PR as to if this is the right approach. Anyone else got any opinions there?

[issyl0]

External link https://marketplace.visualstudio.com/items?itemName=sharat.vscode-brewfile failed (status code 503) Yeah, this part of Azure is down it seems.