hive-forge: lint unlabeled --scope rejects an unknown scope
Refs #4468 A typo'd or invented --scope matched no label at all, and because nothing carries a label in a scope that does not exist, every open item came back reported as missing it -- the failure direction reads as a finding instead of an error. Validate --scope the same way list --label already validates --label: reuse labels::repo_labels (the paginated label fetch) and error with the bad scope plus the available ones, using the same with_suggestions near-miss helper list --label's message uses.
This commit is contained in:
parent
d9d6d37951
commit
ded0379f97
1 changed files with 82 additions and 1 deletions
|
|
@ -10,7 +10,7 @@
|
|||
//! - `assignments [--user NAME]`
|
||||
//! - `unlabeled --scope NAME [--type issues|pulls|all] [--state open|closed|all]`
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::{BTreeMap, BTreeSet};
|
||||
|
||||
use anyhow::{Result, bail};
|
||||
use clap::{Args as ClapArgs, Subcommand, ValueEnum};
|
||||
|
|
@ -22,6 +22,7 @@ use serde_json::{Value, json};
|
|||
use time::OffsetDateTime;
|
||||
|
||||
use crate::client::Client;
|
||||
use crate::verbs::labels;
|
||||
use crate::verbs::{print_json, rfc3339};
|
||||
|
||||
/// Safety cap on paginated walks: 20 pages × 50 items = 1000.
|
||||
|
|
@ -444,6 +445,7 @@ fn run_unlabeled(client: &Client, args: UnlabeledArgs) -> Result<()> {
|
|||
if args.scope.trim().is_empty() {
|
||||
bail!("hive-forge lint unlabeled: --scope must not be empty");
|
||||
}
|
||||
ensure_scope_resolves(client, &args.scope)?;
|
||||
let prefix = format!("{}/", args.scope);
|
||||
let items = fetch_issues(client, args.r#type.query_type(), args.state.issue_state())?;
|
||||
let filtered: Vec<Value> = items
|
||||
|
|
@ -456,6 +458,49 @@ fn run_unlabeled(client: &Client, args: UnlabeledArgs) -> Result<()> {
|
|||
})
|
||||
}
|
||||
|
||||
/// Check `--scope` names a real scope — one an exclusive label on the
|
||||
/// repo actually uses — before the pagination walk even starts.
|
||||
///
|
||||
/// Reuses `labels::repo_labels` (the same paginated fetch `list --label`
|
||||
/// validates against) rather than a second fetch-and-check helper. The
|
||||
/// scope taxonomy isn't fetched anywhere else, so this derives it here:
|
||||
/// the distinct prefixes before `/` on the repo's *exclusive* labels,
|
||||
/// matching `has_scoped_label`'s own definition of what counts as
|
||||
/// scoped. Left unvalidated, a typo'd scope matches nothing, and
|
||||
/// "matches nothing" is exactly what `unlabeled` reports as "missing
|
||||
/// this label" — so every open item comes back flagged, the wrong
|
||||
/// direction for a filter that fails.
|
||||
fn ensure_scope_resolves(client: &Client, scope: &str) -> Result<()> {
|
||||
let all = labels::repo_labels(client)?;
|
||||
resolve_scope(&all, scope)
|
||||
}
|
||||
|
||||
/// The scope-checking half of `ensure_scope_resolves`, split out so it's
|
||||
/// testable against a fixture `Label` slice without a live `Client`
|
||||
/// (mirrors `labels::resolve_ids`, which does the same split for the
|
||||
/// same reason).
|
||||
fn resolve_scope(all: &[forgejo_api::structs::Label], scope: &str) -> Result<()> {
|
||||
let scopes: BTreeSet<&str> = all
|
||||
.iter()
|
||||
.filter(|l| l.exclusive == Some(true))
|
||||
.filter_map(|l| l.name.as_deref())
|
||||
.filter_map(|n| n.split_once('/').map(|(s, _)| s))
|
||||
.collect();
|
||||
if !scopes.contains(scope) {
|
||||
let available: Vec<&str> = scopes.into_iter().collect();
|
||||
bail!(
|
||||
"unresolved label scope: {} — available scopes: {}",
|
||||
crate::verbs::with_suggestions(&[scope], &available),
|
||||
if available.is_empty() {
|
||||
"(none)".to_owned()
|
||||
} else {
|
||||
available.join(", ")
|
||||
}
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// True if `it` carries an *exclusive* scoped label whose name starts
|
||||
/// with `prefix` (e.g. `"type/"`). Exclusivity is Forgejo's actual
|
||||
/// scoped-label marker — a plain label that merely happens to contain
|
||||
|
|
@ -543,6 +588,42 @@ mod tests {
|
|||
serde_json::from_value(v).expect("fixture is a valid PullRequest")
|
||||
}
|
||||
|
||||
fn label(name: &str, exclusive: bool) -> forgejo_api::structs::Label {
|
||||
serde_json::from_value(json!({ "name": name, "exclusive": exclusive }))
|
||||
.expect("fixture is a valid Label")
|
||||
}
|
||||
|
||||
/// A real scope in the repo's exclusive-label taxonomy resolves with
|
||||
/// no error — the unchanged behaviour a fix must not break.
|
||||
#[test]
|
||||
fn a_real_scope_resolves() {
|
||||
let all = vec![label("type/bug", true), label("area/ops", true)];
|
||||
assert!(resolve_scope(&all, "type").is_ok());
|
||||
}
|
||||
|
||||
/// A scope with no exclusive label using it errors, naming the typo
|
||||
/// and every scope that does exist — the bug this fixes: an unknown
|
||||
/// `--scope` used to match nothing and so report every open item as
|
||||
/// "missing" it instead of failing.
|
||||
#[test]
|
||||
fn an_unknown_scope_errors_listing_available_scopes() {
|
||||
let all = vec![label("type/bug", true), label("area/ops", true)];
|
||||
let err = resolve_scope(&all, "zzqqxx").unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(msg.contains("zzqqxx"), "missing typo'd scope: {msg}");
|
||||
assert!(msg.contains("type"), "missing available scope: {msg}");
|
||||
assert!(msg.contains("area"), "missing available scope: {msg}");
|
||||
}
|
||||
|
||||
/// A non-exclusive label with a slash in its name isn't a real scoped
|
||||
/// label (matches `has_scoped_label`'s own definition), so it must not
|
||||
/// count as an available scope either.
|
||||
#[test]
|
||||
fn a_non_exclusive_lookalike_label_is_not_a_scope() {
|
||||
let all = vec![label("area/ops", false)];
|
||||
assert!(resolve_scope(&all, "area").is_err());
|
||||
}
|
||||
|
||||
/// `exclusive` is Forgejo's real scoped-label marker. A label that
|
||||
/// merely *looks* scoped (a `/` in a plain label's name) must not
|
||||
/// count, or `lint unlabeled` silently stops reporting an item that
|
||||
|
|
|
|||
Loading…
Reference in a new issue