From b6cd61bbec299970065469f9c568f91742d608b8 Mon Sep 17 00:00:00 2001 From: rolv Date: Wed, 23 Oct 2024 14:30:48 +0100 Subject: [PATCH] refactor: use `Cow<'source, str>` for text referenced by `ResponseWithContext` --- benches/benchmarks/check_texts.rs | 2 +- src/api/check.rs | 80 +++++++++++++++++++++---------- src/api/server.rs | 21 ++++---- tests/match_positions.rs | 2 +- 4 files changed, 66 insertions(+), 39 deletions(-) diff --git a/benches/benchmarks/check_texts.rs b/benches/benchmarks/check_texts.rs index ae9a246..ec97216 100644 --- a/benches/benchmarks/check_texts.rs +++ b/benches/benchmarks/check_texts.rs @@ -52,7 +52,7 @@ async fn check_text_split(text: &str) -> Response { async { let req = Request::default().with_text(Cow::Owned(line.to_string())); let resp = request_until_success(&req, &client).await; - check::ResponseWithContext::new(req.get_text().into_owned(), resp) + check::ResponseWithContext::new(req.get_text(), resp) } })) .await; diff --git a/src/api/check.rs b/src/api/check.rs index 27dd891..da2874a 100644 --- a/src/api/check.rs +++ b/src/api/check.rs @@ -1,6 +1,6 @@ //! Structures for `check` requests and responses. -use std::{borrow::Cow, mem, ops::Deref}; +use std::{borrow::Cow, marker::PhantomData, mem, ops::Deref}; #[cfg(feature = "annotate")] use annotate_snippets::{ @@ -950,28 +950,30 @@ impl Response { /// /// This structure exists to keep a link between a check response /// and the original text that was checked. -#[derive(Debug, Clone, PartialEq)] -pub struct ResponseWithContext { +#[derive(Debug, Clone, PartialEq, IntoStatic)] +pub struct ResponseWithContext<'source> { /// Original text that was checked by LT. - pub text: String, + pub text: Cow<'source, str>, /// Check response. pub response: Response, /// Text's length. pub text_length: usize, } -impl Deref for ResponseWithContext { +impl<'source> Deref for ResponseWithContext<'source> { type Target = Response; fn deref(&self) -> &Self::Target { &self.response } } -impl ResponseWithContext { +impl<'source> ResponseWithContext<'source> { /// Bind a check response with its original text. #[must_use] - pub fn new(text: String, response: Response) -> Self { + pub fn new(text: Cow<'source, str>, response: Response) -> Self { let text_length = text.chars().count(); + + // Add more context to response Self { text, response, @@ -980,7 +982,7 @@ impl ResponseWithContext { } /// Return an iterator over matches. - pub fn iter_matches(&self) -> std::slice::Iter<'_, Match> { + pub fn iter_matches(&'source self) -> std::slice::Iter<'source, Match> { self.response.iter_matches() } @@ -992,7 +994,7 @@ impl ResponseWithContext { /// Return an iterator over matches and corresponding line number and line /// offset. #[must_use] - pub fn iter_match_positions(&self) -> MatchPositions<'_, std::slice::Iter<'_, Match>> { + pub fn iter_match_positions(&self) -> MatchPositions<'_, '_, std::slice::Iter<'_, Match>> { self.into() } @@ -1024,43 +1026,58 @@ impl ResponseWithContext { self.response.matches.append(&mut other.response.matches); - self.text.push_str(other.text.as_str()); - self.text_length += other.text_length; + self.text = Cow::Owned(self.text.into_owned() + &other.text); + self.text_length = other.text_length; self } } -impl From for Response { - #[allow(clippy::needless_borrow)] - fn from(mut resp: ResponseWithContext) -> Self { - let iter: MatchPositions<'_, std::slice::IterMut<'_, Match>> = (&mut resp).into(); - - for (line_number, line_offset, m) in iter { +impl<'source> From> for Response { + fn from(mut resp: ResponseWithContext<'source>) -> Self { + for (line_number, line_offset, m) in MatchPositions::new(&resp.text, &mut resp.response) { m.more_context = Some(MoreContext { line_number, line_offset, }); } + resp.response } } /// Iterator over matches and their corresponding line number and line offset. #[derive(Clone, Debug)] -pub struct MatchPositions<'source, T> { +pub struct MatchPositions<'source, 'response, T: Iterator + 'response> { text_chars: std::str::Chars<'source>, matches: T, line_number: usize, line_offset: usize, offset: usize, + _marker: PhantomData<&'response ()>, } -impl<'source> From<&'source ResponseWithContext> - for MatchPositions<'source, std::slice::Iter<'source, Match>> +impl<'source, 'response> MatchPositions<'source, 'response, std::slice::IterMut<'response, Match>> { + fn new(text: &'source str, response: &'response mut Response) -> Self { + MatchPositions { + _marker: Default::default(), + text_chars: text.chars(), + matches: response.iter_matches_mut(), + line_number: 1, + line_offset: 0, + offset: 0, + } + } +} + +impl<'source, 'response> From<&'source ResponseWithContext<'source>> + for MatchPositions<'source, 'response, std::slice::Iter<'response, Match>> +where + 'source: 'response, { fn from(response: &'source ResponseWithContext) -> Self { MatchPositions { + _marker: Default::default(), text_chars: response.text.chars(), matches: response.iter_matches(), line_number: 1, @@ -1070,11 +1087,14 @@ impl<'source> From<&'source ResponseWithContext> } } -impl<'source> From<&'source mut ResponseWithContext> - for MatchPositions<'source, std::slice::IterMut<'source, Match>> +impl<'source, 'response> From<&'source mut ResponseWithContext<'source>> + for MatchPositions<'source, 'response, std::slice::IterMut<'response, Match>> +where + 'source: 'response, { fn from(response: &'source mut ResponseWithContext) -> Self { MatchPositions { + _marker: Default::default(), text_chars: response.text.chars(), matches: response.response.iter_matches_mut(), line_number: 1, @@ -1084,8 +1104,8 @@ impl<'source> From<&'source mut ResponseWithContext> } } -impl<'source, T> MatchPositions<'source, T> { - /// Set the line number to a give value. +impl<'source, 'response, T: Iterator + 'response> MatchPositions<'source, 'response, T> { + /// Set the line number to a given value. /// /// By default, the first line number is 1. pub fn set_line_number(mut self, line_number: usize) -> Self { @@ -1114,7 +1134,11 @@ impl<'source, T> MatchPositions<'source, T> { } } -impl<'source> Iterator for MatchPositions<'source, std::slice::Iter<'source, Match>> { +impl<'source, 'response> Iterator + for MatchPositions<'source, 'response, std::slice::Iter<'response, Match>> +where + 'response: 'source, +{ type Item = (usize, usize, &'source Match); fn next(&mut self) -> Option { @@ -1127,7 +1151,11 @@ impl<'source> Iterator for MatchPositions<'source, std::slice::Iter<'source, Mat } } -impl<'source> Iterator for MatchPositions<'source, std::slice::IterMut<'source, Match>> { +impl<'source, 'response> Iterator + for MatchPositions<'source, 'response, std::slice::IterMut<'response, Match>> +where + 'response: 'source, +{ type Item = (usize, usize, &'source mut Match); fn next(&mut self) -> Option { diff --git a/src/api/server.rs b/src/api/server.rs index 6e51b44..6ec979d 100644 --- a/src/api/server.rs +++ b/src/api/server.rs @@ -413,7 +413,9 @@ impl ServerClient { pub async fn check_multiple_and_join<'source>( &self, requests: Vec>, - ) -> Result { + ) -> Result> { + use std::borrow::Cow; + let mut tasks = Vec::with_capacity(requests.len()); requests @@ -424,16 +426,13 @@ impl ServerClient { tasks.push(tokio::spawn(async move { let response = server_client.check(&request).await?; - let text = request - .text - .ok_or_else(|| { - Error::InvalidRequest( - "missing text field; cannot join requests with data annotations" - .to_string(), - ) - })? - .into_owned(); - Result::<(String, Response)>::Ok((text, response)) + let text = request.text.ok_or_else(|| { + Error::InvalidRequest( + "missing text field; cannot join requests with data annotations" + .to_string(), + ) + })?; + Result::<(Cow<'static, str>, Response)>::Ok((text, response)) })); }); diff --git a/tests/match_positions.rs b/tests/match_positions.rs index 13752f1..2abf4c7 100644 --- a/tests/match_positions.rs +++ b/tests/match_positions.rs @@ -10,7 +10,7 @@ macro_rules! test_match_positions { let client = ServerClient::from_env_or_default(); let req = check::Request::default().with_text(Cow::Borrowed($text)); let resp = client.check(&req).await.unwrap(); - let resp = check::ResponseWithContext::new(req.get_text().into_owned(), resp); + let resp = check::ResponseWithContext::new(req.get_text(), resp); let expected = vec![$(($x, $y)),*]; let got = resp.iter_match_positions();