Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ci/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions ci/crates/github-repo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,8 @@ workspace = true

[dependencies]
anyhow.workspace = true
serde.workspace = true

[dev-dependencies]
serde_json.workspace = true
serde-saphyr.workspace = true
167 changes: 140 additions & 27 deletions ci/crates/github-repo/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
//! Shared GitHub repository URL classification.
//! Shared tool source and GitHub repository URL classification.

use anyhow::{Context, Result, ensure};
use serde::{Deserialize, Serialize, Serializer};

/// A repository parsed from a GitHub HTTP(S) URL, borrowing its owner and name.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct GithubRepo<'a> {
owner: &'a str,
name: &'a str,
/// A repository parsed from a GitHub HTTP(S) URL.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct GithubRepo {
owner: String,
name: String,
original_url: String,
}

impl<'a> TryFrom<&'a str> for GithubRepo<'a> {
impl GithubRepo {
/// Returns the original URL without normalizing its spelling or trailing slashes.
#[must_use]
pub fn as_str(&self) -> &str {
&self.original_url
}
}

impl TryFrom<&str> for GithubRepo {
type Error = anyhow::Error;

/// Parses an HTTP(S) GitHub repository URL, ignoring trailing slashes.
Expand All @@ -18,8 +28,8 @@ impl<'a> TryFrom<&'a str> for GithubRepo<'a> {
///
/// Returns an error for an unsupported prefix, missing owner or repository,
/// or a repository path containing a subpath.
fn try_from(url: &'a str) -> Result<Self> {
let url = url.trim_end_matches('/');
fn try_from(original_url: &str) -> Result<Self> {
let url = original_url.trim_end_matches('/');
let path = url
.strip_prefix("https://github.com/")
.or_else(|| url.strip_prefix("http://github.com/"))
Expand All @@ -29,44 +39,86 @@ impl<'a> TryFrom<&'a str> for GithubRepo<'a> {
!owner.is_empty() && !name.is_empty() && !name.contains('/'),
"Expected a repository URL with no subpath"
);
Ok(Self { owner, name })
Ok(Self {
owner: owner.to_owned(),
name: name.to_owned(),
original_url: original_url.to_owned(),
})
}
}

impl std::fmt::Display for GithubRepo<'_> {
impl std::fmt::Display for GithubRepo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}/{}", self.owner, self.name)
}
}

/// A tool's source URL, classified without rejecting unsupported or malformed URLs.
///
/// Serialized as the original plain string, not as a tagged enum.
#[derive(Debug, Clone, Deserialize, PartialEq, Eq)]
#[serde(from = "String")]
pub enum ToolSource {
/// A supported GitHub repository URL.
Github(GithubRepo),
/// Any other source string, retained verbatim for manual review.
Other(String),
}

impl ToolSource {
/// Returns the original source string without URL normalization.
#[must_use]
pub fn as_str(&self) -> &str {
match self {
Self::Github(repo) => repo.as_str(),
Self::Other(source) => source,
}
}
}

impl From<String> for ToolSource {
fn from(source: String) -> Self {
GithubRepo::try_from(source.as_str()).map_or(Self::Other(source), Self::Github)
}
}

impl From<&str> for ToolSource {
fn from(source: &str) -> Self {
GithubRepo::try_from(source).map_or_else(|_| Self::Other(source.to_owned()), Self::Github)
}
}

impl std::fmt::Display for ToolSource {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
}
}

impl Serialize for ToolSource {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
serializer.serialize_str(self.as_str())
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn parses_plain_github_url() -> Result<()> {
let repo = GithubRepo::try_from("https://github.com/owner/repo")?;
assert_eq!(
repo,
GithubRepo {
owner: "owner",
name: "repo"
}
);
assert_eq!(repo.owner, "owner");
assert_eq!(repo.name, "repo");
assert_eq!(repo.as_str(), "https://github.com/owner/repo");
assert_eq!(repo.to_string(), "owner/repo");
Ok(())
}

#[test]
fn parses_trailing_slash() -> Result<()> {
let repo = GithubRepo::try_from("https://github.com/owner/repo/")?;
assert_eq!(
repo,
GithubRepo {
owner: "owner",
name: "repo"
}
);
assert_eq!(repo.to_string(), "owner/repo");
assert_eq!(repo.as_str(), "https://github.com/owner/repo/");
Ok(())
}

Expand Down Expand Up @@ -113,6 +165,66 @@ mod tests {
}
}

#[test]
fn source_variants_preserve_raw_strings_through_serde() -> Result<()> {
for (raw, github) in [
("https://github.com/Owner/Repo.git", true),
("http://github.com/Owner/Repo///", true),
("https://github.com/owner/repo?tab=readme", true),
("https://github.com/owner/repo#readme", true),
("https://github.com/owner/repo%2Ftree", true),
("https://github.com/owner/repo ", true),
("https://gitlab.com/owner/repo/", false),
("https://github.com/owner/repo/tree/main", false),
("https://github.com/owner/", false),
("https://github.com//repo", false),
("https://GitHub.com/owner/repo", false),
(" https://github.com/owner/repo", false),
("git@github.com:owner/repo.git", false),
("not a URL", false),
("", false),
] {
let encoded = serde_json::to_string(raw)?;
for source in [
ToolSource::from(raw),
ToolSource::from(raw.to_owned()),
serde_json::from_str::<ToolSource>(&encoded)?,
serde_saphyr::from_str::<ToolSource>(&encoded)?,
] {
assert_eq!(matches!(source, ToolSource::Github(_)), github, "{raw}");
assert_eq!(source.as_str(), raw);
assert_eq!(source.to_string(), raw);
assert_eq!(serde_json::to_string(&source)?, encoded);
}
}
Ok(())
}

#[test]
fn source_deserialization_matches_plain_string_acceptance() {
for raw in ["null", "42", "true", "[]", r#"{"Github":"owner/repo"}"#] {
assert!(serde_json::from_str::<ToolSource>(raw).is_err(), "{raw}");
assert_eq!(
serde_saphyr::from_str::<ToolSource>(raw).ok(),
serde_saphyr::from_str::<String>(raw)
.ok()
.map(ToolSource::from),
"{raw}"
);
}
}

#[test]
fn repository_owns_its_url() -> Result<()> {
let repo = {
let url = String::from("http://github.com/Owner/Repo///");
GithubRepo::try_from(url.as_str())?
};
assert_eq!(repo.to_string(), "Owner/Repo");
assert_eq!(repo.as_str(), "http://github.com/Owner/Repo///");
Ok(())
}

#[test]
fn rejects_subpath() {
assert!(GithubRepo::try_from("https://github.com/owner/repo/tree/main/subdir").is_err());
Expand All @@ -138,8 +250,9 @@ mod tests {
assert_eq!(
GithubRepo::try_from(url).ok(),
Some(GithubRepo {
owner: "owner",
name: "repo"
owner: "owner".into(),
name: "repo".into(),
original_url: url.into(),
})
);
}
Expand Down
27 changes: 26 additions & 1 deletion ci/crates/pr-check/src/input.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Catalog input paths and tool loading.

use anyhow::{Context, Result, ensure};
use github_repo::ToolSource;
use serde::Deserialize;
use std::fmt;
use std::path::{Component, PathBuf};
Expand Down Expand Up @@ -40,7 +41,7 @@ impl fmt::Display for ToolPath {
#[derive(Debug, Deserialize)]
pub struct ToolEntry {
pub name: String,
pub source: Option<String>,
pub source: Option<ToolSource>,
pub homepage: Option<String>,
}

Expand Down Expand Up @@ -104,6 +105,30 @@ mod tests {
Ok(())
}

#[test]
fn tool_sources_are_classified_during_deserialization() -> Result<()> {
let tool: ToolEntry =
serde_saphyr::from_str("name: Example\nsource: http://github.com/Owner/Repo///\n")?;
let Some(ToolSource::Github(repo)) = tool.source else {
anyhow::bail!("Expected a GitHub repository");
};
assert_eq!(repo.to_string(), "Owner/Repo");
assert_eq!(repo.as_str(), "http://github.com/Owner/Repo///");
for source in [
"https://gitlab.com/owner/repo",
"https://github.com/owner/repo/tree/main",
"",
] {
let yaml = format!("name: Example\nsource: '{source}'\n");
let tool: ToolEntry = serde_saphyr::from_str(&yaml)?;
assert!(matches!(tool.source, Some(ToolSource::Other(ref value)) if value == source));
}
for yaml in ["name: Example\n", "name: Example\nsource: null\n"] {
assert!(serde_saphyr::from_str::<ToolEntry>(yaml)?.source.is_none());
}
Ok(())
}

#[test]
fn tool_yaml_keeps_optional_fields_and_ignores_unrelated_metadata() -> Result<()> {
let tool: ToolEntry =
Expand Down
12 changes: 4 additions & 8 deletions ci/crates/pr-check/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use anyhow::{Context, Result, bail};
use chrono::{DateTime, Utc};
use github_repo::GithubRepo;
use github_repo::{GithubRepo, ToolSource};
use serde::{Deserialize, de::DeserializeOwned};

use crate::checks::{Check, DomainAge};
Expand Down Expand Up @@ -67,7 +67,7 @@ impl GithubClient {
/// # Errors
///
/// Returns an error if the API call fails.
async fn repo_info(&self, repo: GithubRepo<'_>) -> Result<Option<RepoInfo>> {
async fn repo_info(&self, repo: &GithubRepo) -> Result<Option<RepoInfo>> {
let url = format!("https://api.github.com/repos/{repo}");
self.get::<RepoInfo>(&url).await
}
Expand All @@ -77,7 +77,7 @@ impl GithubClient {
/// # Errors
///
/// Returns an error if the API call fails.
async fn contributor_count(&self, repo: GithubRepo<'_>) -> Result<Option<usize>> {
async fn contributor_count(&self, repo: &GithubRepo) -> Result<Option<usize>> {
let url = format!("https://api.github.com/repos/{repo}/contributors?per_page=100&anon=0");
Ok(self
.get::<Vec<Contributor>>(&url)
Expand All @@ -95,11 +95,7 @@ impl GithubClient {
pub async fn check_tool(client: &GithubClient, tool: &ToolEntry) -> Result<ToolReport> {
let source = &tool.source;

let repo = source
.as_deref()
.and_then(|url| GithubRepo::try_from(url).ok());

if let Some(repo) = repo {
if let Some(ToolSource::Github(repo)) = source {
let repo_result = client.repo_info(repo).await;
let contributors_result = client.contributor_count(repo).await;

Expand Down
23 changes: 22 additions & 1 deletion ci/crates/pr-check/src/report.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Report status, Markdown rendering, and workflow exit-code contract.

use askama::Template;
use github_repo::ToolSource;
use std::process::ExitCode;

// Identifies the report as output from the contribution checker.
Expand Down Expand Up @@ -42,7 +43,7 @@ impl CheckResult {
#[derive(Debug)]
pub struct ToolReport {
pub name: String,
pub source: Option<String>,
pub source: Option<ToolSource>,
pub stars: CheckResult,
pub contributors: CheckResult,
pub age: CheckResult,
Expand Down Expand Up @@ -140,6 +141,26 @@ mod tests {
}
}

#[test]
fn comments_preserve_source_text_for_each_variant() {
for source in [
"http://github.com/Owner/Repo///",
"https://gitlab.com/owner/repo",
"",
] {
let report = ToolReport {
source: Some(source.into()),
..passing_report()
};
let reports = Reports::from_iter([report]);
assert!(
Comment::from(&reports)
.to_string()
.contains(&format!("Source: {source}\n"))
);
}
}

#[test]
fn collect_and_extend_preserve_input_order() {
let report = |name: &str| ToolReport {
Expand Down
Loading
Loading