chore: fix pedantic lint

[ognis1205]

Description

This PR fixes a subset of existing clippy::pedantic lints in the codebase.

The changes are intentionally scoped to a limited number of clippy lints to keep the review manageable and to allow reviewers to evaluate each fix incrementally, commit by commit.

• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#assigning_clones
• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#unnecessary_debug_formatting
• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#semicolon_if_nothing_returned
• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#inefficient_to_string
• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#items_after_statements
• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#wildcard_imports
• https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#elidable_lifetime_names

See also:

• https://doc.rust-lang.org/stable/cargo/reference/manifest.html#the-lints-section

Motivation and Context

Applying clippy::pedantic on the existing codebase currently produces a large number of warnings, which makes it difficult for contributors to identify actionable issues.

This PR reduces that noise by incrementally fixing selected pedantic lints, so that future contributors can more easily surface meaningful feedback from clippy::pedantic during development.

How Has This Been Tested?

• cargo clippy --all-targets -- -W clippy::pedantic
• cargo test

Screenshots / Logs (if applicable)

N/A

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (no code change)
• Refactor (refactoring production code)
• Other

Checklist:

• My code follows the code style of this project.
• I have updated the documentation accordingly (if applicable).
• I have formatted the code with rustfmt.
  • cargo +nightly fmt --all
• I checked the lints with clippy.
  • cargo clippy --tests --verbose -- -D warnings
• I have added tests to cover my changes.
• All new and existing tests passed.
  • cargo test

[codecov-commenter]

Codecov Report

❌ Patch coverage is 7.14286% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.62%. Comparing base (91e65f5) to head (95f941f).

Files with missing lines	Patch %	Lines
git-cliff/src/lib.rs	0.00%	11 Missing ⚠️
git-cliff-core/src/remote/azure_devops.rs	0.00%	2 Missing ⚠️
git-cliff-core/src/remote/bitbucket.rs	0.00%	2 Missing ⚠️
git-cliff-core/src/remote/gitea.rs	0.00%	2 Missing ⚠️
git-cliff-core/src/remote/github.rs	0.00%	2 Missing ⚠️
git-cliff-core/src/remote/gitlab.rs	0.00%	2 Missing ⚠️
git-cliff-core/src/repo.rs	50.00%	2 Missing ⚠️
git-cliff-core/src/commit.rs	0.00%	1 Missing ⚠️
git-cliff-core/src/config.rs	0.00%	1 Missing ⚠️
git-cliff/src/bin/mangen.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1350      +/-   ##
==========================================
+ Coverage   44.14%   47.62%   +3.48%     
==========================================
  Files          24       24              
  Lines        2286     2117     -169     
==========================================
- Hits         1009     1008       -1     
+ Misses       1277     1109     -168     

Flag	Coverage Δ
unit-tests	47.62% <7.15%> (+3.48%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[orhun]

This looks pretty good!

Can't we ignore/allow #[allow(clippy::unnecessary_debug_formatting)] on the workspace level? I remember some cargo feature like that. Adding that as a cfg statement in code seems a bit too much.

Unrelated, we should pass the GITHUB_TOKEN to the "Run profiler" job. It's failing due to rate limit.

[ognis1205]

Can't we ignore/allow #[allow(clippy::unnecessary_debug_formatting)] on the workspace level? I remember some cargo feature like that. Adding that as a cfg statement in code seems a bit too much.

I re-checked the lint configuration and behavior around git-cliff project once more.

According to the rustc lint level source documentation (I couldn't find this clearly documented on the Cargo side), the precedence is defined as:

• Attributes: #[allow(...)], #![deny(...)], etc.
• Command-line options: --cap-lints, --force-warn, -A, -W, -D, -F

When looking at how this plays out in Cargo for git-cliff with the changes introduced in this commit, the effective precedence seems to be:

• Attributes: #[allow(...)], #![deny(...)], etc.
• Command-line options: --cap-lints, --force-warn, -A, -W, -D, -F
• Cargo.toml

Specifically, -W clippy::pedantic passed on the command line ends up overriding the lint configuration in Cargo.toml, so the latter doesn’t really take effect.

I do agree that managing this at the workspace / Cargo.toml level would be cleaner if possible, but with the current precedence rules, I'm not seeing a concrete way to do that without either:

• changing the CI clippy invocation, or
• falling back to attributes in code.

If you have a specific approach in mind that I'm missing, I'd be very happy to try it.

Unrelated, we should pass the GITHUB_TOKEN to the "Run profiler" job. It's failing due to rate limit.

I will file another PR for this!

[orhun]

Oh what I mean was adding this entry to Cargo.toml:

[workspace.lints.clippy]
unnecessary_debug_formatting = "allow"

[ognis1205]

Oh what I mean was adding this entry to Cargo.toml:

[workspace.lints.clippy]
unnecessary_debug_formatting = "allow"

I believe with that approach the CLI flags would still take precedence.

When running clippy with -W clippy::pedantic either in CI or manually, it implicitly enables -W clippy::unnecessary-debug-formatting, which means the [workspace.lints.clippy] setting in Cargo.toml doesn’t take effect.

In that case we would still get warnings like:

warning: unnecessary `Debug` formatting in `debug!` args
   --> git-cliff-core/src/config.rs:522:62
    |
522 |                 log::debug!("Using configuration file from: {supported_path:?}");
    |                                                              ^^^^^^^^^^^^^^
    |
    = help: use `Display` formatting and change this to `supported_path.display()`
    = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#unnecessary_debug_formatting
    = note: `-W clippy::unnecessary-debug-formatting` implied by `-W clippy::pedantic`
    = help: to override `-W clippy::pedantic` add `#[allow(clippy::unnecessary_debug_formatting)]`

So unless we adjust the CI clippy invocation, the Cargo.toml setting wouldn't take effect.

[ognis1205]

As a side note, due to the changes introduced in this commit, I believe the current clippy invocation does not behave as originally intended.

[orhun]

Oh okay, didn't realize enabling the pedantic lint would override that configuration. I was under the impression that config would take precedence over the CLI...

Then let's just adjust the CLI for this

[ognis1205]

Oh okay, didn't realize enabling the pedantic lint would override that configuration. I was under the impression that config would take precedence over the CLI...

I had the same impression, so this is a bit confusing indeed.

Then let's just adjust the CLI for this

For

cargo clippy --tests --verbose -- -D warnings

everything is already passing today, so I don't think we need to add any special configuration in Cargo.toml for this.

On the other hand,

cargo clippy --all-targets --verbose -- -W clippy::pedantic

does currently emit a few warnings, but I think they're all addressable ones, including those covered by this PR.

So as a policy, how about not adding any explicit lint configuration to Cargo.toml and continuing to control this via the CLI instead? If that sounds reasonable, I'm happy to follow up with a separate PR.

[orhun]

Yup, that sounds reasonable. We should also make a one-line note before the CLI invocation so that the reason why we are doing that is clear 👍🏼

I'd be fine with doing that in this PR