Fix brew style path checking

[samford]

• 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?

---

brew style tap support was broken in #17357, so now something like brew style homebrew/core exits without checking anything. This happens because the new file-handling logic doesn't do anything with a tap path. Previously, a tap path would be added to ruby_files but now it isn't added to any of the arrays of files to check.

This fixes the issue by adding some logic to add the path to the ruby_files array if it's a tap. [That's the general idea but feel free to suggest an alternative approach if there's a better way to address this.]

---

While working on a fix, I was wondering if we had a way to check whether a path is a tap (e.g., path.tap?) but I didn't see anything from a quick search (though I may have missed something). This also adds a #tap? method to extend/pathname.rb, so we can reuse this logic.

If it's unlikely that we will ever use this again/elsewhere, I can just inline this code where it's used.

[samford]

It's late over here but I'll create PRs in the morning for the other tap style fixes (so this can pass CI).

[MikeMcQuaid]

Thanks @samford! I think brew style . was broken as well so I've pushed a (perhaps too wide) fix for that too plus removing the monkeypatch on Pathname (because of #17471)

[MikeMcQuaid]

Still working on some additional fixes here! Sorry to hijack your PR @samford 😅

[samford]

Still working on some additional fixes here! Sorry to hijack your PR @samford 😅

No worries, I'm only interested in having the bugs fixed and I'm not married to any particular implementation. You were working on this before, so I'm sure you know more about what's intended here 👍

I was about to push a fix for the brew typecheck errors, if that's good with you (I don't want to get in the way of any changes you're working on).

[MikeMcQuaid]

I was about to push a fix for the brew typecheck errors, if that's good with you (I don't want to get in the way of any changes you're working on).

@samford hold off, thanks, I have a more comprehensive fix I'm working on locally 👍🏻

[MikeMcQuaid]

Now that actionlint is actually working: have some additional fixes needed here. @samford if you have bandwidth: would love help with these!

[MikeMcQuaid]

Ah, I think the documentation CI will unfortunately inevitably be failing until this code reaches master so may be worth extracting that part to a separate PR.

[samford]

I opened a PR for the shellcheck issues in homebrew/services' tests.yml file (Homebrew/homebrew-services#677) but I'm not sure what's an appropriate fix for this actionlint issue in homebrew/test-bot's tests.yml:

../../../../linuxbrew/.linuxbrew/Homebrew/Library/Taps/homebrew/homebrew-test-bot/.github/workflows/tests.yml:46:16: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type {image: string; options: string} [expression]
   |
46 |     container: ${{ matrix.container }}
   |                ^~~

[Bo98]

Seems like a false positive unfortunately (rhysd/actionlint#77).

We already ignore it in Homebrew/actions: https://github.com/Homebrew/actions/blob/20fa46653804ab48a53e4b54e365b35bca71ccf4/.github/workflows/actionlint.yml#L42-L45

[samford]

Seems like a false positive unfortunately

Thanks for confirming. I came across that issue when I was searching but I wasn't sure if it applied to our situation as well.

For what it's worth, the build.yml workflow in homebrew/portable-ruby has the same issue:

homebrew-portable-ruby/.github/workflows/build.yml:27:16: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type {image: string; options: string} [expression]
   |
27 |     container: ${{matrix.container}}
   |                ^~~~~~~~~~~~~~~~~~~~~

---

At this point, I've created PRs for all the issues I've seen while running brew style locally on official taps (excluding the aforementioned actionlint issue). I may be forgetting something but I think the only thing I haven't addressed are the handler.sh issues mentioned in Homebrew/homebrew-command-not-found#159. It's probably best for someone who's familiar with that file and/or better at shell scripting to take that on.

[MikeMcQuaid]

Thanks @samford!

[MikeMcQuaid]

Thanks @samford!