Skip to content

Commit

Permalink
[nextest-filtering] Remove use of RefCell
Browse files Browse the repository at this point in the history
Because parsers now take `&mut I` and all cloning is gone,
we don't need `RefCell` anymore.
  • Loading branch information
epage committed Jan 10, 2024
1 parent 71e149b commit bad429b
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 29 deletions.
11 changes: 5 additions & 6 deletions nextest-filtering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: MIT OR Apache-2.0

use miette::{Diagnostic, SourceSpan};
use std::cell::RefCell;
use thiserror::Error;

/// A set of errors that occurred while parsing a filter expression.
Expand Down Expand Up @@ -150,19 +149,19 @@ pub enum GlobConstructError {
RegexError(String),
}

#[derive(Debug, Clone)]
#[derive(Debug)]

Check warning on line 152 in nextest-filtering/src/errors.rs

View check run for this annotation

Codecov / codecov/patch

nextest-filtering/src/errors.rs#L152

Added line #L152 was not covered by tests
pub(crate) struct State<'a> {
// A `RefCell` is required here because the state must implement `Clone` to work with nom.
errors: &'a RefCell<Vec<ParseSingleError>>,
errors: &'a mut Vec<ParseSingleError>,
}

impl<'a> State<'a> {
pub fn new(errors: &'a RefCell<Vec<ParseSingleError>>) -> Self {
pub fn new(errors: &'a mut Vec<ParseSingleError>) -> Self {
Self { errors }
}

pub fn report_error(&self, error: ParseSingleError) {
self.errors.borrow_mut().push(error);
pub fn report_error(&mut self, error: ParseSingleError) {
self.errors.push(error);
}
}

Expand Down
8 changes: 3 additions & 5 deletions nextest-filtering/src/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use guppy::{
use miette::SourceSpan;
use nextest_metadata::{RustBinaryId, RustTestBinaryKind};
use recursion::{Collapsible, CollapsibleExt, MappableFrame, PartiallyApplied};
use std::{cell::RefCell, collections::HashSet, fmt};
use std::{collections::HashSet, fmt};

/// Matcher for name
///
Expand Down Expand Up @@ -227,11 +227,9 @@ impl FilteringSet {
impl FilteringExpr {
/// Parse a filtering expression
pub fn parse(input: String, graph: &PackageGraph) -> Result<Self, FilterExpressionParseErrors> {
let errors = RefCell::new(Vec::new());
match parse(new_span(&input, &errors)) {
let mut errors = Vec::new();
match parse(new_span(&input, &mut errors)) {
Ok(parsed_expr) => {
let errors = errors.into_inner();

if !errors.is_empty() {
return Err(FilterExpressionParseErrors::new(input.clone(), errors));
}
Expand Down
35 changes: 17 additions & 18 deletions nextest-filtering/src/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use guppy::graph::cargo::BuildPlatform;
use miette::SourceSpan;
use std::{cell::RefCell, fmt};
use std::fmt;
use winnow::{
ascii::line_ending,
combinator::{alt, delimited, eof, fold_repeat, peek, preceded, repeat, terminated},
Expand Down Expand Up @@ -43,7 +43,7 @@ impl<'a> ToSourceSpan for Span<'a> {
}

Check warning on line 43 in nextest-filtering/src/parsing.rs

View check run for this annotation

Codecov / codecov/patch

nextest-filtering/src/parsing.rs#L42-L43

Added lines #L42 - L43 were not covered by tests
}

pub(crate) fn new_span<'a>(input: &'a str, errors: &'a RefCell<Vec<ParseSingleError>>) -> Span<'a> {
pub(crate) fn new_span<'a>(input: &'a str, errors: &'a mut Vec<ParseSingleError>) -> Span<'a> {
Span {
input: winnow::Located::new(input),
state: State::new(errors),
Expand Down Expand Up @@ -115,11 +115,11 @@ pub enum ParsedExpr<S = SourceSpan> {

impl ParsedExpr {
pub fn parse(input: &str) -> Result<Self, Vec<ParseSingleError>> {
let errors = RefCell::new(Vec::new());
let span = new_span(input, &errors);
let mut errors = Vec::new();
let span = new_span(input, &mut errors);
match parse(span).unwrap() {
ExprResult::Valid(expr) => Ok(expr),
ExprResult::Error => Err(errors.into_inner()),
ExprResult::Error => Err(errors),

Check warning on line 122 in nextest-filtering/src/parsing.rs

View check run for this annotation

Codecov / codecov/patch

nextest-filtering/src/parsing.rs#L122

Added line #L122 was not covered by tests
}
}

Expand Down Expand Up @@ -904,12 +904,11 @@ pub(crate) fn parse(input: Span<'_>) -> Result<ExprResult, winnow::error::ErrMod
#[cfg(test)]
mod tests {
use super::*;
use std::cell::RefCell;

#[track_caller]
fn parse_regex(input: &str) -> NameMatcher {
let errors = RefCell::new(Vec::new());
let span = new_span(input, &errors);
let mut errors = Vec::new();
let span = new_span(input, &mut errors);
parse_regex_matcher.parse_peek(span).unwrap().1.unwrap()
}

Expand Down Expand Up @@ -943,8 +942,8 @@ mod tests {

#[track_caller]
fn parse_glob(input: &str) -> NameMatcher {
let errors = RefCell::new(Vec::new());
let span = new_span(input, &errors);
let mut errors = Vec::new();
let span = new_span(input, &mut errors);
let matcher = parse_glob_matcher
.parse_peek(span)
.unwrap_or_else(|error| {
Expand All @@ -957,7 +956,7 @@ mod tests {
(reported errors: {errors:?})"
)
});
if errors.borrow().len() > 0 {
if errors.len() > 0 {
panic!("for input {input}, parse_glob_matcher reported errors: {errors:?}");
}

Expand Down Expand Up @@ -996,8 +995,8 @@ mod tests {

#[track_caller]
fn parse_set(input: &str) -> SetDef {
let errors = RefCell::new(Vec::new());
let span = new_span(input, &errors);
let mut errors = Vec::new();
let span = new_span(input, &mut errors);
parse_set_def.parse_peek(span).unwrap().1.unwrap()
}

Expand Down Expand Up @@ -1425,19 +1424,19 @@ mod tests {
Ok((n1, n2))
}

let errors = RefCell::new(Vec::new());
let mut span = new_span("something(aa, bb)", &errors);
let mut errors = Vec::new();
let mut span = new_span("something(aa, bb)", &mut errors);
if parse_future_syntax.parse_next(&mut span).is_err() {
panic!("Failed to parse comma separated matchers");
}
}

#[track_caller]
fn parse_err(input: &str) -> Vec<ParseSingleError> {
let errors = RefCell::new(Vec::new());
let span = new_span(input, &errors);
let mut errors = Vec::new();
let span = new_span(input, &mut errors);
super::parse(span).unwrap();
errors.into_inner()
errors
}

macro_rules! assert_error {
Expand Down

0 comments on commit bad429b

Please sign in to comment.