diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1bc8dcb..ec41e31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,6 @@ jobs: - name: Check formatting run: cargo fmt --check - name: Run clippy - run: cargo clippy --all-targets --all-features -- -D warnings -A clippy::pedantic + run: cargo clippy --all-targets --all-features -- -D warnings -W clippy::pedantic -W clippy::nursery - name: Run audit run: cargo install cargo-audit && cargo audit diff --git a/src/client/blame.rs b/src/client/blame.rs index 7b04bd7..45a7352 100644 --- a/src/client/blame.rs +++ b/src/client/blame.rs @@ -69,18 +69,20 @@ fn is_file_author(response_data: &blame::ResponseData, login: &str) -> bool { // println!("\n\nRanges: {:?}\n\n", v); - let authors = match v { - Some(ranges) => ranges - .iter() - .filter_map(|range| range.commit.authors.nodes.as_ref()) - .flatten() - .filter_map(|node| { - node.as_ref() - .and_then(|n| n.user.as_ref().map(|user| user.login.as_str())) - }) - .collect(), - _ => vec![], - }; + let authors = v.map_or_else( + Vec::new, + |ranges| { + ranges + .iter() + .filter_map(|range| range.commit.authors.nodes.as_ref()) + .flatten() + .filter_map(|node| { + node.as_ref() + .and_then(|n| n.user.as_ref().map(|user| user.login.as_str())) + }) + .collect() + }, + ); // println!("\n\n Authors: {:?}\n\n", authors); let login_str: String = login.to_string(); @@ -119,11 +121,8 @@ async fn girhub_blame( } Err(anyhow!("Errors fetching the authors of {path} {error_str}",)) } else { - match response_body.data { - Some(data) => Ok(data), - None => Err(anyhow!( - "Missing response data fetching the authors of {path}" - )), - } + response_body + .data + .map_or_else(|| Err(anyhow!("Missing response data fetching the authors of {path}")), Ok) } } diff --git a/src/client/mod.rs b/src/client/mod.rs index b88217c..f188b1f 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -92,18 +92,23 @@ pub async fn fetch_scored_prs( Ok(list_prs.into_iter().flatten().collect::>()) } +#[allow(clippy::future_not_send)] pub async fn call( github_api_token: &str, q: &QueryBody, ) -> Result { - let client = reqwest::Client::builder().user_agent(AGENT).build()?; - let res = client - .post("https://api.github.com/graphql") - .json(&q) - .bearer_auth(github_api_token) - .send() - .await?; - Ok(res) + #[allow(clippy::future_not_send)] + async { + let client = reqwest::Client::builder().user_agent(AGENT).build()?; + let res = client + .post("https://api.github.com/graphql") + .json(&q) + .bearer_auth(github_api_token) + .send() + .await?; + Ok(res) + } + .await } async fn query( @@ -157,10 +162,9 @@ fn get_response_data( } Err(anyhow!("Errors executing the query {error_str}")) } else { - match response_body.data { - Some(data) => Ok(data), - None => Err(anyhow!("Missing response data executing the query")), - } + response_body + .data + .map_or_else(|| Err(anyhow!("Missing response data executing the query")), Ok) } } @@ -169,21 +173,17 @@ fn limited_batch_size(batch_size: u8) -> i64 { } fn last_item_cursor(response_data: &repo_view::ResponseData, batch_size: i64) -> Option { - match &response_data.search.edges { - Some(items) => - { - #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] - if items.len() < usize::try_from(batch_size).unwrap_or(usize::MAX) { - None - } else { - match items.last() { - Some(Some(item)) => Some(item.cursor.clone()), - _ => None, - } + response_data.search.edges.as_ref().and_then(|items| { + #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] + if items.len() < usize::try_from(batch_size).unwrap_or(usize::MAX) { + None + } else { + match items.last() { + Some(Some(item)) => Some(item.cursor.clone()), + _ => None, } } - None => None, - } + }) } fn github_query(username: &str, options: &PrArgs) -> String { @@ -200,7 +200,7 @@ fn github_query(username: &str, options: &PrArgs) -> String { ) } -fn query_drafts(include_drafts: bool) -> &'static str { +const fn query_drafts(include_drafts: bool) -> &'static str { if include_drafts { "" } else { @@ -244,11 +244,7 @@ fn query_repos(repos: &[String]) -> String { } fn query_org(org: Option<&String>) -> String { - if let Some(org) = org { - format!("org:{org} ") - } else { - String::new() - } + org.map_or_else(String::new, |org| format!("org:{org} ")) } async fn ranked_prs( @@ -348,49 +344,50 @@ fn regex_match( or: bool, pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest, ) -> bool { - match regex { - Some(re) => re.is_match(&pr.title), - None => or, - } + regex.map_or(or, |re| re.is_match(&pr.title)) } -fn is_empty(pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest) -> bool { +const fn is_empty(pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest) -> bool { pr.additions == 0 && pr.deletions == 0 } -fn has_conflicts(pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest) -> bool { +const fn has_conflicts(pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest) -> bool { matches!(pr.mergeable, repo_view::MergeableState::CONFLICTING) } fn pr_files(pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest) -> Vec { - match &pr.files { - Some(files) => files - .nodes - .iter() - .flatten() - .flatten() - .map(|f| f.path.clone()) - .collect(), - None => vec![], - } -} - -fn pr_labels(labels: Option<&repo_view::RepoViewSearchEdgesNodeOnPullRequestLabels>) -> Labels { - match labels { - Some(labels) => Labels( - labels + pr.files + .as_ref() + .map_or_else(Vec::new, |files| { + files .nodes .iter() .flatten() .flatten() - .map(|l| Label { - name: l.name.clone(), - color: l.color.clone(), - }) - .collect(), - ), - None => Labels(vec![]), - } + .map(|f| f.path.clone()) + .collect() + }) +} + +fn pr_labels(labels: Option<&repo_view::RepoViewSearchEdgesNodeOnPullRequestLabels>) -> Labels { + labels + .map_or_else( + || Labels(vec![]), + |labels| { + Labels( + labels + .nodes + .iter() + .flatten() + .flatten() + .map(|l| Label { + name: l.name.clone(), + color: l.color.clone(), + }) + .collect(), + ) + }, + ) } async fn pr_stats( @@ -445,15 +442,12 @@ async fn pr_stats( } fn author(pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest) -> String { - match &pr.author { - Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestAuthor { login, on: _ }) => { - login.clone() - } - _ => String::new(), - } + pr.author + .as_ref() + .map_or_else(String::new, |repo_view::RepoViewSearchEdgesNodeOnPullRequestAuthor { login, on: _ }| login.clone()) } -fn include_by_tests_state(state: &TestsState, options: &PrArgs) -> bool { +const fn include_by_tests_state(state: &TestsState, options: &PrArgs) -> bool { match state { TestsState::Success => !options.exclude_tests_success, TestsState::Failure => options.include_tests_failure, @@ -495,49 +489,41 @@ fn commit_status_state( status: &repo_view::RepoViewSearchEdgesNodeOnPullRequestCommitsNodesCommitStatusCheckRollup, tests_re: Option<&Regex>, ) -> TestsState { - match tests_re { - Some(tests_re) => { + tests_re + .map_or_else(|| tests_state(&status.state), |tests_re| { commit_tests_state_from_contexts(status.contexts.nodes.as_ref(), tests_re) - } - None => tests_state(&status.state), - } + }) } fn commit_tests_state_from_contexts( contexts_nodes: Option<&Vec>>, tests_re: &Regex, ) -> TestsState { - match contexts_nodes { - Some(nodes) => { - let states: Vec = nodes.iter().filter_map(|node| - match node { - Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestCommitsNodesCommitStatusCheckRollupContextsNodes::StatusContext(status_context)) => { - if tests_re.is_match(&status_context.context) { - Some(tests_state(&status_context.state)) - } else { - None - } - }, - _ => None - }).collect(); - match states { - v if v.iter().any(|state| matches!(state, TestsState::Failure)) => { - TestsState::Failure - } - v if v.iter().any(|state| matches!(state, TestsState::Pending)) => { - TestsState::Pending - } - v if v.iter().all(|state| matches!(state, TestsState::Success)) => { - TestsState::Success - } - _ => TestsState::None, - } + contexts_nodes.map_or(TestsState::None, |nodes| { + let states: Vec = nodes.iter().filter_map(|node| + match node { + Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestCommitsNodesCommitStatusCheckRollupContextsNodes::StatusContext(status_context)) => { + if tests_re.is_match(&status_context.context) { + Some(tests_state(&status_context.state)) + } else { + None + } + }, + _ => None + }).collect(); + if states.iter().any(|state| matches!(state, TestsState::Failure)) { + TestsState::Failure + } else if states.iter().any(|state| matches!(state, TestsState::Pending)) { + TestsState::Pending + } else if states.iter().all(|state| matches!(state, TestsState::Success)) { + TestsState::Success + } else { + TestsState::None } - None => TestsState::None, - } + }) } -fn tests_state(state: &repo_view::StatusState) -> TestsState { +const fn tests_state(state: &repo_view::StatusState) -> TestsState { match state { repo_view::StatusState::SUCCESS => TestsState::Success, repo_view::StatusState::PENDING | repo_view::StatusState::EXPECTED => TestsState::Pending, @@ -610,7 +596,7 @@ fn pr_num_approvals(review_states: &[&repo_view::PullRequestReviewState]) -> i64 } #[allow(clippy::cast_possible_wrap)] -fn pr_num_reviewers(review_states: &[&repo_view::PullRequestReviewState]) -> i64 { +const fn pr_num_reviewers(review_states: &[&repo_view::PullRequestReviewState]) -> i64 { review_states.len() as i64 } @@ -618,8 +604,9 @@ fn parse_date(date: Option<&String>) -> Option> { date.and_then(|s| s.parse::>().ok()) } +#[allow(clippy::missing_const_for_fn, clippy::single_option_map)] fn age(date_time: Option>) -> Option { - date_time.map(|date_time| (Utc::now() - date_time).num_minutes()) + date_time.map(|dt| (Utc::now() - dt).num_minutes()) } fn pr_based_on_main_branch(base_branch_name: &str) -> bool { @@ -630,29 +617,31 @@ fn review_requested( requests: Option<&repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequests>, username: &str, ) -> ReviewRequested { - match requests { - Some(requests) => { - requests.nodes.iter().flatten().flatten().find(|r| - match &r.requested_reviewer { - Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::User(reviewer)) => -reviewer.login == username, - Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::Team(team)) => - team.members.nodes.iter().flatten().flatten().any(|member| member.login == username), - Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::Mannequin) => false, // Just ignore Mannequins - None => panic!("Something is wrong with the GitHub token! Have you added the read:org scope?"), - }) - .map_or( - ReviewRequested::NotRequested, - |r| - if r.as_code_owner { - ReviewRequested::RequestedAsCodeOwner - } else { - ReviewRequested::RequestedNotAsCodeOwner - } - ) - } - None => ReviewRequested::NotRequested, - } + requests + .map_or(ReviewRequested::NotRequested, |r| { + r.nodes + .as_ref() + .map_or(ReviewRequested::NotRequested, |nodes| { + nodes + .iter() + .flatten() + .find(|r| match &r.requested_reviewer { + Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::User(reviewer)) => reviewer.login == username, + Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::Team(team)) => { + team.members.nodes.iter().flatten().flatten().any(|member| member.login == username) + } + Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::Mannequin) => false, + None => panic!("Something is wrong with the GitHub token! Have you added the read:org scope?"), + }) + .map_or(ReviewRequested::NotRequested, |r| { + if r.as_code_owner { + ReviewRequested::RequestedAsCodeOwner + } else { + ReviewRequested::RequestedNotAsCodeOwner + } + }) + }) + }) } #[cfg(test)] diff --git a/src/table.rs b/src/table.rs index 7fb91c3..7f6a696 100644 --- a/src/table.rs +++ b/src/table.rs @@ -78,7 +78,7 @@ fn pr_row(spr: &ScoredPr, debug: bool) -> Vec { const YES: &str = "yes"; const NO: &str = "no"; -fn show_bool(value: bool) -> &'static str { +const fn show_bool(value: bool) -> &'static str { if value { YES } else { @@ -86,7 +86,7 @@ fn show_bool(value: bool) -> &'static str { } } -fn tests_result_label(tests_result: &TestsState) -> &'static str { +const fn tests_result_label(tests_result: &TestsState) -> &'static str { match tests_result { TestsState::Success => "OK", TestsState::Pending => "..", @@ -104,8 +104,9 @@ fn show_files(files: &Files) -> String { } fn show_duration(minutes: Option) -> String { - match minutes { - Some(min) => { + minutes.map_or_else( + || String::from("-"), + |min| { let d = min / 60 / 24; let h = (min - d * 24 * 60) / 60; let m = min - d * 24 * 60 - h * 60; @@ -121,16 +122,14 @@ fn show_duration(minutes: Option) -> String { } else { String::new() }, - if d == 0 && m > 0 { + if m > 0 { format!("{m}m ") } else { String::new() } ) - } - // d.format("%d-%m-%Y %H:%M").to_string(), - None => String::from("-"), - } + }, + ) } fn build_table() -> Table { diff --git a/src/types.rs b/src/types.rs index 6f7764f..0b3ae6f 100644 --- a/src/types.rs +++ b/src/types.rs @@ -71,14 +71,14 @@ pub struct Score { } impl Score { - pub fn from_pr(required_approvals: u8, pr: &Pr) -> Score { + pub fn from_pr(required_approvals: u8, pr: &Pr) -> Self { let tests_result_i = match pr.tests_result { TestsState::Pending => 1, TestsState::Failure => 2, - TestsState::Success | TestsState::None => 0, // a repo without CI is treated as successful + TestsState::Success | TestsState::None => 0, }; #[allow(clippy::cast_precision_loss, clippy::cast_lossless)] - Score { + Self { age: pr.last_commit_age_min.unwrap_or(0) as f64 / 60.0 * 2.0, tests_result: f64::from(tests_result_i - 1) * -200.0, open_conversations: pr.open_conversations as f64 * -30.0, @@ -150,8 +150,8 @@ pub enum ReviewState { impl std::fmt::Display for ReviewState { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { let text = match self { - ReviewState::Dismissed => "Dismissed", - ReviewState::WithAddressedConversations => "With addressed conversations", + Self::Dismissed => "Dismissed", + Self::WithAddressedConversations => "With addressed conversations", }; write!(f, "{text}") }