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

Run Cargo clippy and fmt in CI #392

Merged
merged 2 commits into from
Mar 18, 2025
Merged

Run Cargo clippy and fmt in CI #392

merged 2 commits into from
Mar 18, 2025

Conversation

edmorley
Copy link
Member

Since we should always be running these in CI for all projects that use Rust.

The job was copied from that run here:
https://github.com/heroku/heroku-java-metrics-agent/blob/86dd6bf68d3cafdecbc0b281f2a4f54a00c41371/.github/workflows/ci.yml#L82-L98

GUS-W-18062768.

@edmorley edmorley self-assigned this Mar 18, 2025
@edmorley
Copy link
Member Author

Clippy is currently failing with:

error: docs for function which may panic missing `# Panics` section
  --> src/lib.rs:37:1
   |
37 | pub fn heroku_jvm_application_deployer_jar_path() -> PathBuf {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: first possible panic found here
  --> src/lib.rs:42:22
   |
42 |     for dir_entry in std::fs::read_dir(maven_target_dir).unwrap() {
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc
   = note: `-D clippy::missing-panics-doc` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::missing_panics_doc)]`

error: case-sensitive file extension comparison
  --> src/lib.rs:46:72
   |
46 |         if file_name.starts_with("heroku-jvm-application-deployer") && file_name.ends_with(".jar") {
   |                                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: consider using a case-insensitive comparison instead
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons
   = note: `-D clippy::case-sensitive-file-extension-comparisons` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::case_sensitive_file_extension_comparisons)]`
help: use std::path::Path
   |
46 ~         if file_name.starts_with("heroku-jvm-application-deployer") && std::path::Path::new(&file_name)
47 +             .extension()
48 ~             .map_or(false, |ext| ext.eq_ignore_ascii_case("jar")) {
   |

error: docs for function which may panic missing `# Panics` section
  --> src/lib.rs:55:1
   |
55 | pub fn http_get_expect_200(url: &str) -> String {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: first possible panic found here
  --> src/lib.rs:56:5
   |
56 | /     thread_sleep_backoff(10, Duration::from_secs(1), Duration::from_secs(30), || {
57 | |         ureq::get(url).call()
58 | |     })
59 | |     .expect("")
60 | |     .into_string()
61 | |     .expect("")
   | |_______________^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc

error: docs for function which may panic missing `# Panics` section
  --> src/lib.rs:65:1
   |
65 | pub fn create_heroku_app(path: &Path, space: Option<&str>) -> HerokuAppCreateResult {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: first possible panic found here
  --> src/lib.rs:79:5
   |
79 |     serde_json::from_slice::<HerokuAppCreateResult>(&output.stdout).unwrap()
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc

error: docs for function which may panic missing `# Panics` section
   --> src/lib.rs:114:1
    |
114 | pub fn run_command(command: &mut Command, error: &str, suppress_output: bool) -> Output {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
note: first possible panic found here
   --> src/lib.rs:115:18
    |
115 |       let output = if suppress_output {
    |  __________________^
116 | |         command.output()
117 | |     } else {
118 | |         command.output_and_write_streams(stdout(), stderr())
119 | |     }
120 | |     .expect("");
    | |_______________^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc

error: docs for function which may panic missing `# Panics` section
   --> src/lib.rs:137:1
    |
137 | pub fn prepare_fixture(name: &str) -> PathBuf {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
note: first possible panic found here
   --> src/lib.rs:138:20
    |
138 |     let temp_dir = tempfile::tempdir().expect("").into_path();
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc

Verified

This commit was signed with the committer’s verified signature.
edmorley Ed Morley
Since we should always be running these in CI for all projects
that use Rust.

The job was copied from that run here:
https://github.com/heroku/heroku-java-metrics-agent/blob/86dd6bf68d3cafdecbc0b281f2a4f54a00c41371/.github/workflows/ci.yml#L82-L98

GUS-W-18062768.

Verified

This commit was signed with the committer’s verified signature.
edmorley Ed Morley
@edmorley edmorley force-pushed the edmorley/ci-rust-lint branch from 104ce47 to d24d7d3 Compare March 18, 2025 13:15
@edmorley edmorley marked this pull request as ready for review March 18, 2025 13:17
@edmorley edmorley requested review from Malax and a team as code owners March 18, 2025 13:17
@edmorley edmorley enabled auto-merge (squash) March 18, 2025 13:17
@edmorley edmorley merged commit e7e78f2 into main Mar 18, 2025
5 of 6 checks passed
@edmorley edmorley deleted the edmorley/ci-rust-lint branch March 18, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants