Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust Clippy Linter Settings: How to run Clippy and fmt on different crates and example directories with manifest files #4318

Open
ScottGibb opened this issue Nov 26, 2024 · 3 comments
Labels
question Further information is requested

Comments

@ScottGibb
Copy link

Thank you to all the devs that have worked on SuperLinter, really great tool!

Im wondering how I can apply Cargo Clippy to the following project:
https://github.com/dysonltd/tmag5273

Initially I created a linting workflow like so:

Clippy:
        name: Clippy
        runs-on: ubuntu-latest
        steps:
          - name: Checkout code
            uses: actions/checkout@v4
          - name: Clippy check on src
            run: cargo clippy --manifest-path Cargo.toml -- -D warnings
          - name: Clippy check on utils
            run: cargo clippy --manifest-path utils/Cargo.toml -- -D warnings
          - name: Clippy check on esp32 example
            run:  cd examples/esp32c3
                  cargo clippy --manifest-path ./Cargo.toml -- -D warnings
            
      Formatter:
        name: Cargo Formatter
        runs-on: ubuntu-latest
        steps:
          - name: Checkout code
            uses: actions/checkout@v4
          - name: Rustfmt check on src
            run: cargo fmt --manifest-path Cargo.toml -- --check
          - name: Rustfmt check on utils
            run: cargo fmt --manifest-path utils/Cargo.toml -- --check
          - name: Rustfmt check on esp32 example
            run: cargo fmt --manifest-path examples/esp32-c3/Cargo.toml -- --check

Is there a way I can apply the following settings to MegaLinter?

@ScottGibb ScottGibb added the question Further information is requested label Nov 26, 2024
@nvuillam
Copy link
Member

@ScottGibb thanks for your interest :)

Clippy is embedded in MegaLinter, but you can call it only once

According to your job logs, the call is cargo-clippy --all -fix

Is it equivalent to your calls ?

Note: you can add additional arguments using variable RUST_CLIPPY_ARGUMENTS in your .mega-linter.yml

For example:

RUST_CLIPPY_ARGUMENTS: ["-D", "warnings"]

About rustfmt, it is not embedded in MegaLinter yet but I think we could do that easily :)

Would you like to try a PR, or do you prefer we do it when we have the time ? :)

ps: it's MegaLinter, not SuperLinter (even if this project started from a SuperLinter PR, it's technically completely different ^^ )

@ScottGibb
Copy link
Author

ScottGibb commented Nov 27, 2024

Hi @nvuillam , thank you for the quick reply!

According to your job logs, the call is cargo-clippy --all -fix

Is it equivalent to your calls ?
Ive changed it quite a few times, with messing around with the linting. I believe I want the following command
cargo-clippy --workspace --all-targets -- -D warnings

Which I think turns my .mega-linter.yml into the following:

---
# Configuration file for MegaLinter
#
# See all available variables at https://megalinter.io/latest/config-file/ and in
# linters documentation

# all, none, or list of linter keys
APPLY_FIXES: all

# If you use ENABLE variable, all other languages/formats/tooling-formats will
# be disabled by default
# ENABLE:

# If you use ENABLE_LINTERS variable, all other linters will be disabled by default
ENABLE_LINTERS:
  - ACTION_ACTIONLINT
  - RUST_CLIPPY
  - MARKDOWN_MARKDOWNLINT
  - MARKDOWN_MARKDOWN_LINK_CHECK
  - MARKDOWN_MARKDOWN_TABLE_FORMATTER
  - YAML_YAMLLINT
  - YAML_PRETTIER
  - JSON_JSONLINT
  - JSON_PRETTIER

# DISABLE:

SHOW_ELAPSED_TIME: true

FILEIO_REPORTER: false

# Uncomment if you want MegaLinter to detect errors but not block CI to pass
# DISABLE_ERRORS: true
GITHUB_COMMENT_REPORTER: true
GITHUB_STATUS_REPORTER: true
VALIDATE_ALL_CODEBASE: true
OUTPUT_DETAIL: detailed

# Linter configuration files
MARKDOWN_MARKDOWNLINT_CONFIG_FILE: .markdownlint.json
YAML_YAMLLINT_CONFIG_FILE: .yamllint.yml

RUST_CLIPPY_ARGUMENTS:
  - "--workspace"
  - "--all-targets"
  - "--"
  - "-D"
  - "warnings"

One thing I did notice is that I would use cargo clippy not cargo-clippy and they do result in different outputs on my codebase, if i run them directly from the terminal.

cargo clippy

❯ cargo clippy --workspace  --all-targets -- -D warnings
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /Users/scott.gibb/Documents/Projects/rs-tmag5273-driver/examples/esp32-c3/Cargo.toml
workspace: /Users/scott.gibb/Documents/Projects/rs-tmag5273-driver/Cargo.toml
warning: /Users/scott.gibb/Documents/Projects/rs-tmag5273-driver/examples/esp32-c3/Cargo.toml: unused manifest key: dependencies.tmag5273.no-default-features
    Checking portable-atomic v1.10.0


WARNING: use --release
  We *strongly* recommend using release profile when building esp-hal.
  The dev profile can potentially be one or more orders of magnitude
  slower than release, and may cause issues with timing-senstive
  peripherals and/or devices.

    Checking tmag5273 v0.1.0 (/Users/scott.gibb/Documents/Projects/rs-tmag5273-driver)
error: `portable_atomic_unsafe_assume_single_core` cfg (`unsafe-assume-single-core` feature) is not compatible with target that supports atomic CAS;
       see also <https://github.com/taiki-e/portable-atomic/issues/148> for troubleshooting
   --> /Users/scott.gibb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.10.0/src/lib.rs:343:1
    |
343 | / compile_error!(
344 | |     "`portable_atomic_unsafe_assume_single_core` cfg (`unsafe-assume-single-core` feature) \
345 | |      is not compatible with target that supports atomic CAS;\n\
346 | |      see also <https://github.com/taiki-e/portable-atomic/issues/148> for troubleshooting"
347 | | );
    | |_^

error: could not compile `portable-atomic` (lib) due to 1 previous error

rs-tmag5273-driver on  add-static-analysis [!] is 📦 v0.1.0 via 🦀 v1.82.0 on ☁️  (eu-north-1) on

cargo-clippy

❯ cargo-clippy --workspace  --all-targets -- -D warnings
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /Users/scott.gibb/Documents/Projects/rs-tmag5273-driver/examples/esp32-c3/Cargo.toml
workspace: /Users/scott.gibb/Documents/Projects/rs-tmag5273-driver/Cargo.toml
warning: /Users/scott.gibb/Documents/Projects/rs-tmag5273-driver/examples/esp32-c3/Cargo.toml: unused manifest key: dependencies.tmag5273.no-default-features
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.07s

Don't suppose anyone knows the reason for this?

Would you like to try a PR, or do you prefer we do it when we have the time ? :)

I would be interested in doing so! But not sure when il find the time, perhaps we could create an issue on the repo and I can pick it up when theres time?

@ScottGibb
Copy link
Author

After experimenting, i've managed to create the following flow:

# MegaLinter GitHub Action configuration file
# More info at https://megalinter.io
---
name: MegaLinter

# Trigger mega-linter at every push. Action will also be visible from
# Pull Requests to main
on:
  # Comment this line to trigger action only on pull-requests
  # (not recommended if you don't pay for GH Actions)
  push:

  pull_request:
    branches:
      - main
      - master

# Comment env block if you do not want to apply fixes
env:
  # Apply linter fixes configuration
  #
  # When active, APPLY_FIXES must also be defined as environment variable
  # (in github/workflows/mega-linter.yml or other CI tool)
  APPLY_FIXES: all

  # Decide which event triggers application of fixes in a commit or a PR
  # (pull_request, push, all)
  APPLY_FIXES_EVENT: pull_request

  # If APPLY_FIXES is used, defines if the fixes are directly committed (commit)
  # or posted in a PR (pull_request)
  APPLY_FIXES_MODE: pull_request

concurrency:
  group: ${{ github.ref }}-${{ github.workflow }}
  cancel-in-progress: true

jobs:
  megalinter:
    name: MegaLinter
    runs-on: ubuntu-latest

    # Give the default GITHUB_TOKEN write permission to commit and push, comment
    # issues, and post new Pull Requests; remove the ones you do not need
    permissions:
      contents: write
      issues: write
      pull-requests: write

    steps:
      # Git Checkout
      - name: Checkout Code
        uses: actions/checkout@v4
        with:
          token: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}

          # If you use VALIDATE_ALL_CODEBASE = true, you can remove this line to
          # improve performance
          fetch-depth: 0

      # MegaLinter
      - name: MegaLinter

        # You can override MegaLinter flavor used to have faster performances
        # More info at https://megalinter.io/latest/flavors/
        uses: oxsecurity/megalinter/flavors/rust@v8

        id: ml

        # All available variables are described in documentation
        # https://megalinter.io/latest/config-file/
        env:
          # Validates all source when push on main, else just the git diff with
          # main. Override with true if you always want to lint all sources
          #
          # To validate the entire codebase, set to:
          # VALIDATE_ALL_CODEBASE: true
          #
          # To validate only diff with main, set to:
          # VALIDATE_ALL_CODEBASE: >-
          #   ${{
          #     github.event_name == 'push' &&
          #     github.ref == 'refs/heads/main'
          #   }}
          VALIDATE_ALL_CODEBASE: true

          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

          # Uncomment to use ApiReporter (Grafana)
          # API_REPORTER: true
          # API_REPORTER_URL: ${{ secrets.API_REPORTER_URL }}
          # API_REPORTER_BASIC_AUTH_USERNAME: ${{ secrets.API_REPORTER_BASIC_AUTH_USERNAME }}
          # API_REPORTER_BASIC_AUTH_PASSWORD: ${{ secrets.API_REPORTER_BASIC_AUTH_PASSWORD }}
          # API_REPORTER_METRICS_URL: ${{ secrets.API_REPORTER_METRICS_URL }}
          # API_REPORTER_METRICS_BASIC_AUTH_USERNAME: ${{ secrets.API_REPORTER_METRICS_BASIC_AUTH_USERNAME }}
          # API_REPORTER_METRICS_BASIC_AUTH_PASSWORD: ${{ secrets.API_REPORTER_METRICS_BASIC_AUTH_PASSWORD }}
          # API_REPORTER_DEBUG: false

          # ADD YOUR CUSTOM ENV VARIABLES HERE TO OVERRIDE VALUES OF
          # .mega-linter.yml AT THE ROOT OF YOUR REPOSITORY

      # Upload MegaLinter artifacts
      - name: Archive production artifacts
        uses: actions/upload-artifact@v4
        if: success() || failure()
        with:
          name: MegaLinter reports
          path: |
            megalinter-reports
            mega-linter.log
      # Create pull request if applicable
      # (for now works only on PR from same repository, not from forks)
      - name: Create Pull Request with applied fixes
        uses: peter-evans/create-pull-request@v6
        id: cpr
        if: >-
          steps.ml.outputs.has_updated_sources == 1 &&
          (
            env.APPLY_FIXES_EVENT == 'all' ||
            env.APPLY_FIXES_EVENT == github.event_name
          ) &&
          env.APPLY_FIXES_MODE == 'pull_request' &&
          (
            github.event_name == 'push' ||
            github.event.pull_request.head.repo.full_name == github.repository
          ) &&
          !contains(github.event.head_commit.message, 'skip fix')
        with:
          token: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}
          commit-message: "[MegaLinter] Apply linters automatic fixes"
          title: "[MegaLinter] Apply linters automatic fixes"
          labels: bot

      - name: Create PR output
        if: >-
          steps.ml.outputs.has_updated_sources == 1 &&
          (
            env.APPLY_FIXES_EVENT == 'all' ||
            env.APPLY_FIXES_EVENT == github.event_name
          ) &&
          env.APPLY_FIXES_MODE == 'pull_request' &&
          (
            github.event_name == 'push' ||
            github.event.pull_request.head.repo.full_name == github.repository
          ) &&
          !contains(github.event.head_commit.message, 'skip fix')
        run: |
          echo "PR Number - ${{ steps.cpr.outputs.pull-request-number }}"
          echo "PR URL - ${{ steps.cpr.outputs.pull-request-url }}"
      # Push new commit if applicable
      # (for now works only on PR from same repository, not from forks)
      - name: Prepare commit
        if: >-
          steps.ml.outputs.has_updated_sources == 1 &&
          (
            env.APPLY_FIXES_EVENT == 'all' ||
            env.APPLY_FIXES_EVENT == github.event_name
          ) &&
          env.APPLY_FIXES_MODE == 'commit' &&
          github.ref != 'refs/heads/main' &&
          (
            github.event_name == 'push' ||
            github.event.pull_request.head.repo.full_name == github.repository
          ) &&
          !contains(github.event.head_commit.message, 'skip fix')
        run: sudo chown -Rc $UID .git/

      - name: Commit and push applied linter fixes
        uses: stefanzweifel/git-auto-commit-action@v5
        if: >-
          steps.ml.outputs.has_updated_sources == 1 &&
          (
            env.APPLY_FIXES_EVENT == 'all' ||
            env.APPLY_FIXES_EVENT == github.event_name
          ) &&
          env.APPLY_FIXES_MODE == 'commit' &&
          github.ref != 'refs/heads/main' &&
          (
            github.event_name == 'push' ||
            github.event.pull_request.head.repo.full_name == github.repository
          ) &&
          !contains(github.event.head_commit.message, 'skip fix')
        with:
          branch: >-
            ${{
              github.event.pull_request.head.ref ||
              github.head_ref ||
              github.ref
            }}
          commit_message: "[MegaLinter] Apply linters fixes"
          commit_user_name: megalinter-bot
          commit_user_email: nicolas.vuillamy@ox.security

  cargo_fmt:
    name: Cargo Formatter
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
      - name: Rustfmt check on src
        run: cargo fmt --manifest-path Cargo.toml -- --check
      - name: Rustfmt check on utils
        run: cargo fmt --manifest-path utils/Cargo.toml -- --check
      - name: Rustfmt check on esp32 example
        run: cargo fmt --manifest-path examples/esp32-c3/Cargo.toml -- --check

  cargo_clippy:
    name: Cargo Clippy
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4
      - name: Clippy check on esp32 example
        run: |
          cd examples/esp32-c3
          cargo clippy -- -D warnings

What I found is that I couldn't seem to get either cargo clippy or megalinter to correctly lint my esp32-c3 example. So I ended up adding an extra stage. I think the issue lies somewhere in cargo clippy and how it scans the folders. Im putting in this question for completeness and if anyone in the community has the same problem or any ideas :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants