kaleidawave / ezno

A JavaScript compiler and TypeScript checker written in Rust with a focus on static analysis and runtime performance
https://kaleidawave.github.io/posts/introducing-ezno/
MIT License
2.44k stars 45 forks source link

feat(ci): auto-fix formatting and linting on new PRs #108

Open maxpatiiuk opened 8 months ago

maxpatiiuk commented 8 months ago

This is in response to your Cool things in '23 post:

I also really want a GitHub action/bot to apply clippy --fix and rustfmt format on the current PR. I don't know how I feel asking the author to do the sort of bookkeeping in a project if simple checks fail. It would be nice if I could comment @my-bot try fix lints and formatting and it runs it and commits the fixes (rather than me having to checkout and do it manually). Is this even a good idea, does it change who authored it when it appears in git blame`? If you know a simple solution feel free to PR it.

I do not know know about rust-tooling, but in JavaScript-land there exists lint-staged to run pre-configured linters on files that were changed between main and the head of a branch the PR is on.

There is also husky for running lint-staged as a pre-commit hook in Git (so that devs get feedback even before commit is made, even if they don't have rustfmt in their IDEs)

I am not opening a PR for this as I am not sure if you would be open to having npm-based tools added in this repository, but here is how the above could be configured:

Create lint-staged.config.js:

const autoFix = !process.env.NO_AUTO_FIX;

module.exports = {
  "*.{rs}": [`clippy ${autoFix ? "--fix" : ""}`, 'rustfmt format'],
};

Install dependencies:

npm install --save-dev lint-staged husky

Add this to scripts section in package.json:

    "lint": "lint-staged --config lint-staged.config.js",

Create GitHub Action workflow lint.yml:

name: Lint code

on:
  push:
  pull_request:

jobs:
  linting:
    if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
    steps:

      - uses: actions/checkout@v3
        with:
          fetch-depth: 0
          # Fetch a branch, not detached HEAD (to allow committing changes)
          ref: ${{ github.event_name == 'pull_request' && github.head_ref || github.ref_name }}

      - uses: actions/setup-node@v3

      - name: Install dependencies
        run: npm ci

      - name: Lint packages
        run: |
          if [ ${{ github.event_name }} == 'pull_request' ]; then
            npx lint --diff="origin/${{ github.base_ref }}...origin/${{ github.head_ref }}"
          else
            npx lint --diff "HEAD~1...HEAD"
          fi

      - name: Commit linted files (if made any changes)
        if: github.event_name == 'pull_request'
        # Creates a new commit. Amending an existing commit is a bad idea
        # because:
        # - Would require original committer to force pull. May cause merge
        #   conflicts
        # - Changes that require linting might have been made over several
        #   commits. If you amend the last commit, than the fixes for all
        #   commits would be in the last commit.
        run: |
          # Check if any files were changed by autofix
          git add .
          if git diff-index --quiet HEAD --; then
            echo "Linters did not detect any issues. Good job!"
          else
            # Set committer to the person who committed the last commit on this branch:
            git config --local committer.name "$( git log -1 --pretty=format:'%cn' )"
            git config --local committer.email "$( git log -1 --pretty=format:'%ce' )"
            # Author commit as a GitHub Action. Commits authored by GitHub
            # Action do not trigger GitHub Action workflows. This avoids a
            # cycle of Action triggering itself in a loop.
            git config --global author.email "github-actions"
            git config --global author.name "41898282+github-actions[bot]@users.noreply.github.com"

            git commit \
              --message "chore(lint): lint code with clippy and rustfmt" \
              --message "Triggered by ${{ github.event.pull_request.head.sha }} on branch ${{ github.head_ref }}" \
              --no-verify
            git push --no-verify
          fi

Alternatively, instead of using npm's lint-staged, there is a GitHub action dorny/paths-filter Example usage in https://github.com/specify/specify7/blob/production/.github/workflows/test.yml

kaleidawave commented 8 months ago

Yay someone read my post!

Interesting that lint-staged can work on Git diffs. Unfortunately it seems that in Rust land, clippy lints the whole project (probably because it needs global information).

The code under name: Commit linted files (if made any changes) looks useful for applying the changes. This is cool that it keeps the author

git config --local committer.name "$( git log -1 --pretty=format:'%cn' )"
git config --local committer.email "$( git log -1 --pretty=format:'%ce' )"

I wonder if it is only possible to trigger only on a comment. Or maybe just doing it automatically might be fine...? Will think about it 👍