From c5d4c5484c8bc81a5fe5bf47665b1795caa59e30 Mon Sep 17 00:00:00 2001 From: Thomas Zahner Date: Tue, 10 Dec 2024 15:13:55 +0100 Subject: [PATCH 1/2] Address warning: it is more idiomatic to use `Option<&T>` instead of `&Option` --- lychee-bin/src/commands/check.rs | 8 ++-- lychee-lib/src/collector.rs | 11 +++-- lychee-lib/src/types/file.rs | 1 - lychee-lib/src/utils/path.rs | 20 ++++---- lychee-lib/src/utils/request.rs | 80 ++++++++++++++++---------------- 5 files changed, 61 insertions(+), 59 deletions(-) diff --git a/lychee-bin/src/commands/check.rs b/lychee-bin/src/commands/check.rs index 3551c4c9c8..5c0614b179 100644 --- a/lychee-bin/src/commands/check.rs +++ b/lychee-bin/src/commands/check.rs @@ -192,7 +192,7 @@ async fn progress_bar_task( while let Some(response) = recv_resp.recv().await { show_progress( &mut io::stderr(), - &pb, + pb.as_ref(), &response, formatter.as_ref(), &verbose, @@ -331,7 +331,7 @@ fn ignore_cache(uri: &Uri, status: &Status, cache_exclude_status: &HashSet) fn show_progress( output: &mut dyn Write, - progress_bar: &Option, + progress_bar: Option<&ProgressBar>, response: &Response, formatter: &dyn ResponseFormatter, verbose: &Verbosity, @@ -401,7 +401,7 @@ mod tests { let formatter = get_response_formatter(&options::OutputMode::Plain); show_progress( &mut buf, - &None, + None, &response, formatter.as_ref(), &Verbosity::default(), @@ -423,7 +423,7 @@ mod tests { let formatter = get_response_formatter(&options::OutputMode::Plain); show_progress( &mut buf, - &None, + None, &response, formatter.as_ref(), &Verbosity::debug(), diff --git a/lychee-lib/src/collector.rs b/lychee-lib/src/collector.rs index 955bdd24e7..ae3b004f65 100644 --- a/lychee-lib/src/collector.rs +++ b/lychee-lib/src/collector.rs @@ -124,7 +124,12 @@ impl Collector { let content = content?; let extractor = Extractor::new(self.use_html5ever, self.include_verbatim); let uris: Vec = extractor.extract(&content); - let requests = request::create(uris, &content, &base, &basic_auth_extractor); + let requests = request::create( + uris, + &content, + base.as_ref(), + basic_auth_extractor.as_ref(), + ); Result::Ok(stream::iter(requests.into_iter().map(Ok))) } }) @@ -493,8 +498,8 @@ mod tests { source: InputSource::String( r#" Index - About - Another + About + Another "# .into(), ), diff --git a/lychee-lib/src/types/file.rs b/lychee-lib/src/types/file.rs index 7cdeff6c80..d2f8631c6c 100644 --- a/lychee-lib/src/types/file.rs +++ b/lychee-lib/src/types/file.rs @@ -54,7 +54,6 @@ impl> From

for FileType { } /// Helper function to check if a path is likely a URL. - fn is_url(path: &Path) -> bool { path.to_str() .and_then(|s| Url::parse(s).ok()) diff --git a/lychee-lib/src/utils/path.rs b/lychee-lib/src/utils/path.rs index bb4847ed9a..0a7cdc62da 100644 --- a/lychee-lib/src/utils/path.rs +++ b/lychee-lib/src/utils/path.rs @@ -10,8 +10,8 @@ static CURRENT_DIR: Lazy = Lazy::new(|| env::current_dir().expect("cannot get current dir from environment")); /// Returns the base if it is a valid `PathBuf` -fn get_base_dir(base: &Option) -> Option { - base.as_ref().and_then(Base::dir) +fn get_base_dir(base: Option<&Base>) -> Option { + base.and_then(Base::dir) } /// Create an absolute path out of a `PathBuf`. @@ -40,7 +40,7 @@ fn dirname(src: &'_ Path) -> Option<&'_ Path> { /// Resolve `dst` that was linked to from within `src` /// /// Returns Ok(None) in case of an absolute local link without a `base_url` -pub(crate) fn resolve(src: &Path, dst: &Path, base: &Option) -> Result> { +pub(crate) fn resolve(src: &Path, dst: &Path, base: Option<&Base>) -> Result> { let resolved = match dst { relative if dst.is_relative() => { // Find `dst` in the parent directory of `src` @@ -110,7 +110,7 @@ mod test_path { let dummy = PathBuf::from("index.html"); let abs_path = PathBuf::from("./foo.html"); assert_eq!( - resolve(&dummy, &abs_path, &None)?, + resolve(&dummy, &abs_path, None)?, Some(env::current_dir().unwrap().join("foo.html")) ); Ok(()) @@ -123,7 +123,7 @@ mod test_path { let dummy = PathBuf::from("./index.html"); let abs_path = PathBuf::from("./foo.html"); assert_eq!( - resolve(&dummy, &abs_path, &None)?, + resolve(&dummy, &abs_path, None)?, Some(env::current_dir().unwrap().join("foo.html")) ); Ok(()) @@ -136,7 +136,7 @@ mod test_path { let abs_index = PathBuf::from("/path/to/index.html"); let abs_path = PathBuf::from("./foo.html"); assert_eq!( - resolve(&abs_index, &abs_path, &None)?, + resolve(&abs_index, &abs_path, None)?, Some(PathBuf::from("/path/to/foo.html")) ); Ok(()) @@ -149,9 +149,9 @@ mod test_path { fn test_resolve_absolute_from_base_dir() -> Result<()> { let dummy = PathBuf::new(); let abs_path = PathBuf::from("/foo.html"); - let base = Some(Base::Local(PathBuf::from("/some/absolute/base/dir"))); + let base = Base::Local(PathBuf::from("/some/absolute/base/dir")); assert_eq!( - resolve(&dummy, &abs_path, &base)?, + resolve(&dummy, &abs_path, Some(&base))?, Some(PathBuf::from("/some/absolute/base/dir/foo.html")) ); Ok(()) @@ -163,9 +163,9 @@ mod test_path { fn test_resolve_absolute_from_absolute() -> Result<()> { let abs_index = PathBuf::from("/path/to/index.html"); let abs_path = PathBuf::from("/other/path/to/foo.html"); - let base = Some(Base::Local(PathBuf::from("/some/absolute/base/dir"))); + let base = Base::Local(PathBuf::from("/some/absolute/base/dir")); assert_eq!( - resolve(&abs_index, &abs_path, &base)?, + resolve(&abs_index, &abs_path, Some(&base))?, Some(PathBuf::from( "/some/absolute/base/dir/other/path/to/foo.html" )) diff --git a/lychee-lib/src/utils/request.rs b/lychee-lib/src/utils/request.rs index 7867e50f09..12cde17e89 100644 --- a/lychee-lib/src/utils/request.rs +++ b/lychee-lib/src/utils/request.rs @@ -13,20 +13,12 @@ use crate::{ Base, BasicAuthCredentials, ErrorKind, Request, Result, Uri, }; -/// Extract basic auth credentials for a given URL. -fn extract_credentials( - extractor: &Option, - uri: &Uri, -) -> Option { - extractor.as_ref().and_then(|ext| ext.matches(uri)) -} - /// Create a request from a raw URI. fn create_request( raw_uri: &RawUri, source: &InputSource, - base: &Option, - extractor: &Option, + base: Option<&Base>, + extractor: Option<&BasicAuthExtractor>, ) -> Result { let uri = try_parse_into_uri(raw_uri, source, base)?; let source = truncate_source(source); @@ -37,6 +29,14 @@ fn create_request( Ok(Request::new(uri, source, element, attribute, credentials)) } +/// Extract basic auth credentials for a given URL. +fn extract_credentials( + extractor: Option<&BasicAuthExtractor>, + uri: &Uri, +) -> Option { + extractor.and_then(|ext| ext.matches(uri)) +} + /// Try to parse the raw URI into a `Uri`. /// /// If the raw URI is not a valid URI, create a URI by joining the base URL with the text. @@ -48,7 +48,7 @@ fn create_request( /// to create a valid URI. /// - If a URI cannot be created from the file path. /// - If the source is not a file path (i.e. the URI type is not supported). -fn try_parse_into_uri(raw_uri: &RawUri, source: &InputSource, base: &Option) -> Result { +fn try_parse_into_uri(raw_uri: &RawUri, source: &InputSource, base: Option<&Base>) -> Result { let text = raw_uri.text.clone(); let uri = match Uri::try_from(raw_uri.clone()) { Ok(uri) => uri, @@ -81,7 +81,7 @@ pub(crate) fn is_anchor(text: &str) -> bool { fn create_uri_from_file_path( file_path: &Path, link_text: &str, - base: &Option, + base: Option<&Base>, ) -> Result { let target_path = if is_anchor(link_text) { // For anchors, we need to append the anchor to the file name. @@ -126,16 +126,15 @@ fn truncate_source(source: &InputSource) -> InputSource { pub(crate) fn create( uris: Vec, input_content: &InputContent, - base: &Option, - extractor: &Option, + base: Option<&Base>, + extractor: Option<&BasicAuthExtractor>, ) -> HashSet { - let base = base - .clone() - .or_else(|| Base::from_source(&input_content.source)); + let base_fallback = Base::from_source(&input_content.source); + let base = base.or_else(|| base_fallback.as_ref()); uris.into_iter() .filter_map(|raw_uri| { - match create_request(&raw_uri, &input_content.source, &base, extractor) { + match create_request(&raw_uri, &input_content.source, base, extractor) { Ok(request) => Some(request), Err(e) => { warn!("Error creating request: {:?}", e); @@ -160,7 +159,7 @@ pub(crate) fn create( fn resolve_and_create_url( src_path: &Path, dest_path: &str, - base_uri: &Option, + base_uri: Option<&Base>, ) -> Result { let (dest_path, fragment) = url::remove_get_params_and_separate_fragment(dest_path); @@ -195,7 +194,7 @@ mod tests { #[test] fn test_create_uri_from_path() { let result = - resolve_and_create_url(&PathBuf::from("/README.md"), "test+encoding", &None).unwrap(); + resolve_and_create_url(&PathBuf::from("/README.md"), "test+encoding", None).unwrap(); assert_eq!(result.as_str(), "file:///test+encoding"); } @@ -209,14 +208,14 @@ mod tests { #[test] fn test_relative_url_resolution() { - let base = Some(Base::try_from("https://example.com/path/page.html").unwrap()); + let base = Base::try_from("https://example.com/path/page.html").unwrap(); let input = create_input( r#"Relative Link"#, FileType::Html, ); let uris = vec![RawUri::from("relative.html")]; - let requests = create(uris, &input, &base, &None); + let requests = create(uris, &input, Some(&base), None); assert_eq!(requests.len(), 1); assert!(requests @@ -226,14 +225,14 @@ mod tests { #[test] fn test_absolute_url_resolution() { - let base = Some(Base::try_from("https://example.com/path/page.html").unwrap()); + let base = Base::try_from("https://example.com/path/page.html").unwrap(); let input = create_input( r#"Absolute Link"#, FileType::Html, ); let uris = vec![RawUri::from("https://another.com/page")]; - let requests = create(uris, &input, &base, &None); + let requests = create(uris, &input, Some(&base), None); assert_eq!(requests.len(), 1); assert!(requests @@ -243,14 +242,14 @@ mod tests { #[test] fn test_root_relative_url_resolution() { - let base = Some(Base::try_from("https://example.com/path/page.html").unwrap()); + let base = Base::try_from("https://example.com/path/page.html").unwrap(); let input = create_input( r#"Root Relative Link"#, FileType::Html, ); let uris = vec![RawUri::from("/root-relative")]; - let requests = create(uris, &input, &base, &None); + let requests = create(uris, &input, Some(&base), None); assert_eq!(requests.len(), 1); assert!(requests @@ -260,14 +259,14 @@ mod tests { #[test] fn test_parent_directory_url_resolution() { - let base = Some(Base::try_from("https://example.com/path/page.html").unwrap()); + let base = Base::try_from("https://example.com/path/page.html").unwrap(); let input = create_input( r#"Parent Directory Link"#, FileType::Html, ); let uris = vec![RawUri::from("../parent")]; - let requests = create(uris, &input, &base, &None); + let requests = create(uris, &input, Some(&base), None); assert_eq!(requests.len(), 1); assert!(requests @@ -277,11 +276,11 @@ mod tests { #[test] fn test_fragment_url_resolution() { - let base = Some(Base::try_from("https://example.com/path/page.html").unwrap()); + let base = Base::try_from("https://example.com/path/page.html").unwrap(); let input = create_input(r##"Fragment Link"##, FileType::Html); let uris = vec![RawUri::from("#fragment")]; - let requests = create(uris, &input, &base, &None); + let requests = create(uris, &input, Some(&base), None); assert_eq!(requests.len(), 1); assert!(requests @@ -291,14 +290,13 @@ mod tests { #[test] fn test_no_base_url_resolution() { - let base = None; let input = create_input( r#"Absolute Link"#, FileType::Html, ); let uris = vec![RawUri::from("https://example.com/page")]; - let requests = create(uris, &input, &base, &None); + let requests = create(uris, &input, None, None); assert_eq!(requests.len(), 1); assert!(requests @@ -308,11 +306,11 @@ mod tests { #[test] fn test_create_request_from_relative_file_path() { - let base = Some(Base::Local(PathBuf::from("/tmp/lychee"))); + let base = Base::Local(PathBuf::from("/tmp/lychee")); let input_source = InputSource::FsPath(PathBuf::from("page.html")); let actual = - create_request(&RawUri::from("file.html"), &input_source, &base, &None).unwrap(); + create_request(&RawUri::from("file.html"), &input_source, Some(&base), None).unwrap(); assert_eq!( actual, @@ -330,15 +328,15 @@ mod tests { #[test] fn test_create_request_from_absolute_file_path() { - let base = Some(Base::Local(PathBuf::from("/tmp/lychee"))); + let base = Base::Local(PathBuf::from("/tmp/lychee")); let input_source = InputSource::FsPath(PathBuf::from("/tmp/lychee/page.html")); // Use an absolute path that's outside the base directory let actual = create_request( &RawUri::from("/usr/local/share/doc/example.html"), &input_source, - &base, - &None, + Some(&base), + None, ) .unwrap(); @@ -358,28 +356,28 @@ mod tests { #[test] fn test_parse_relative_path_into_uri() { - let base = Some(Base::Local(PathBuf::from("/tmp/lychee"))); + let base = Base::Local(PathBuf::from("/tmp/lychee")); let input = create_input( r#"Relative Link"#, FileType::Html, ); let raw_uri = RawUri::from("relative.html"); - let uri = try_parse_into_uri(&raw_uri, &input.source, &base).unwrap(); + let uri = try_parse_into_uri(&raw_uri, &input.source, Some(&base)).unwrap(); assert_eq!(uri.url.as_str(), "file:///tmp/lychee/relative.html"); } #[test] fn test_parse_absolute_path_into_uri() { - let base = Some(Base::Local(PathBuf::from("/tmp/lychee"))); + let base = Base::Local(PathBuf::from("/tmp/lychee")); let input = create_input( r#"Absolute Link"#, FileType::Html, ); let raw_uri = RawUri::from("absolute.html"); - let uri = try_parse_into_uri(&raw_uri, &input.source, &base).unwrap(); + let uri = try_parse_into_uri(&raw_uri, &input.source, Some(&base)).unwrap(); assert_eq!(uri.url.as_str(), "file:///tmp/lychee/absolute.html"); } From 423b73c22b20cf3f74f3aa171c1dba20997d65fb Mon Sep 17 00:00:00 2001 From: Thomas Zahner Date: Tue, 10 Dec 2024 15:28:03 +0100 Subject: [PATCH 2/2] Document test_utils module --- lychee-lib/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lychee-lib/src/lib.rs b/lychee-lib/src/lib.rs index 93df6d0db8..712460c1be 100644 --- a/lychee-lib/src/lib.rs +++ b/lychee-lib/src/lib.rs @@ -73,6 +73,7 @@ pub mod filter; #[cfg(test)] #[macro_use] +/// Utility functions and macros to help testing lychee pub mod test_utils; #[cfg(test)]