nshki / chusaku

Annotate your Rails controllers with route info.
https://rubygems.org/gems/chusaku
MIT License
89 stars 5 forks source link

Add more detailed output with `--verbose` flag #25

Closed nshki closed 2 years ago

nshki commented 2 years ago

Discussion

24.

Overview

This PR adds a --verbose flag to Chusaku and outputs changes in detail when used with that flag.

Example output:

[test/mock/app/controllers/api/burritos_controller.rb:4]

Before:
```ruby
  # @route POST /api/burritos (burritos)

After:

[test/mock/app/controllers/api/burritos_controller.rb:5]

Before:

  def create; end

After:

  # @route POST /api/burritos (burritos)
  def create; end

[test/mock/app/controllers/api/tacos_controller.rb:4]

Before:

  def show; end

After:

  # @route GET / (root)
  # @route GET /api/tacos/:id (taco)
  def show; end

[test/mock/app/controllers/api/tacos_controller.rb:6]

Before:

  # This is an example of generated annotations that come with Rails 6
  # scaffolds. These should be replaced by Chusaku annotations.
  # POST /api/tacos
  # PATCH/PUT /api/tacos/1

After:

  # This is an example of generated annotations that come with Rails 6
  # scaffolds. These should be replaced by Chusaku annotations.

[test/mock/app/controllers/api/tacos_controller.rb:10]

Before:

  def create; end

After:

  # @route POST /api/tacos (tacos)
  def create; end

[test/mock/app/controllers/api/tacos_controller.rb:12]

Before:

  # Update all the tacos!
  # @route this should be deleted, it's not a valid route.
  # We should not see a duplicate @route in this comment block.
  # @route PUT /api/tacos/:id (taco)
  # But this should remain (@route), because it's just words.

After:

  # Update all the tacos!
  # We should not see a duplicate @route in this comment block.
  # But this should remain (@route), because it's just words.

[test/mock/app/controllers/api/tacos_controller.rb:17]

Before:

  def update; end

After:

  # @route PUT /api/tacos/:id (taco)
  # @route PATCH /api/tacos/:id (taco)
  def update; end

[test/mock/app/controllers/api/tacos_controller.rb:19]

Before:

  # This route doesn't exist, so it should be deleted.
  # @route DELETE /api/tacos/:id

After:

  # This route doesn't exist, so it should be deleted.

[test/mock/app/controllers/waterlilies_controller.rb:4]

Before:

  def show; end

After:

  # @route GET /waterlilies/:id (waterlilies)
  # @route GET /waterlilies/:id (waterlilies2)
  # @route GET /waterlilies/:id {blue: true} (waterlilies_blue)
  def show; end

[test/mock/app/controllers/waterlilies_controller.rb:6]

Before:

  def one_off; end

After:

  # @route GET /one-off
  def one_off; end

Chusaku has finished running.

nshki commented 2 years ago

@G-Rath here's a draft PR for you to check out. It doesn't include the GitHub Action setup quite yet, but wanted to see how you like the new --verbose output.

G-Rath commented 2 years ago

Looking good! I'm not sure how suitable it is for Github Actions, because you can't add to messages (so at best it'd just show the whole block without explaining what it's actually doing) but that might be fine and the best we can get anyway 🤔

nshki commented 2 years ago

@G-Rath I was reading over the documentation for Problem Matchers and originally thought we’d be able to just write some regex that captures all the changes to annotate them, but it seems like the limitations (annotation count limits) actually hinder this quite a bit especially for bigger codebases. 😢

As an alternative though, you can definitely set Chusaku to run in a GitHub Actions workflow and fail when there are pending annotations using the —exit-on-error-on-annotation and —dry-run flags. If you want the changes outputted, this new —verbose flag will give a nice overview in the workflow output.

G-Rath commented 2 years ago

@nshki I don't think those limitations actually factor into this all that much, since they're not per PR - so e.g. fixing some of what caused the current annotations (for any tool) will result in more showing up when you push up the fixes for those.

(golangci-lint is a good example of this flow - by default it only shows ~20 errors total rather than give you a possible massive list of them; so as you action those, you see new ones on the next run)

Additionally, long term I'd expect most changes to be only affecting a small number of routes (tbh I'd only expect 1-3) for existing projects that are having features and bug fixes applied; so this would mean you'd only have a small number of annotations that actually need changing.

Finally, those limits are GHA specific and could be changed in future - overall, I think tools/clis can focus on outputting information in a way that can be parsed by machines without worrying about specific UIs.

nshki commented 2 years ago

@G-Rath Great points! I was only considering the extreme case which would only really happen when Chusaku is introduced to a large codebase without actually running it in the same PR.

I think this verbose output is sufficient enough to get parsed for a GHA setup. I'll spend a little time testing Problem Matchers with a separate Rails project and will tweak the output if needed.

nshki commented 2 years ago

I've been testing the following GHA workflow in a private Rails repo:

.github/chusaku-annotations-matcher.json:

{
  "problemMatcher": [
    {
      "owner": "chusaku",
      "pattern": [
        {
          "regexp": "^\\[(.+)\\/(.+):(\\d+)\\]\\n[\\S|\\s]+?\\n---NEW\\n([\\S|\\s]*?(?=(\\n\\[.+:\\d+\\])|\\n\\n))",
          "fromPath": 1,
          "file": 2,
          "line": 3,
          "message": 4
        }
      ]
    }
  ]
}

.github/workflows/chusaku.yml:

name: Chusaku
on: [pull_request]
jobs:
  chusaku:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Set up Ruby
        uses: ruby/setup-ruby@v1
        with:
          ruby-version: 2.7.4
          bundler-cache: true
      - name: Run Chusaku
        run: |
          echo "::add-matcher::.github/chusaku-annotations-matcher.json"
          bundle exec chusaku --dry-run --verbose

I haven't been able to get proper annotations yet, so still debugging, but I've tested the regex via Rubular and have gotten the correct match groups there.

nshki commented 2 years ago

While I haven't cracked the GitHub Actions annotations portion, I'm going to go ahead and merge this PR since I think this will give people more parse-able output for any automation they'd like to run in conjunction with Chusaku.