Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle expansion failure #66

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
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
10 changes: 8 additions & 2 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,17 @@ where
.output()
.map_err(|e| Error::CargoExpandExecutionError(e.to_string()))?;

if !cargo_expand.status.success() {
let has_errors = {
let msg = b"error: ";
cargo_expand.stderr.windows(msg.len()).any(|w| w == msg)
};

// Handle compilation or macro expansion errors
if !cargo_expand.status.success() || (!cargo_expand.stdout.is_empty() && has_errors) {
return Ok((false, cargo_expand.stderr));
}

Ok((true, cargo_expand.stdout))
Ok((cargo_expand.status.success(), cargo_expand.stdout))
}

/// Builds dependencies for macro expansion and pipes `cargo` output to `STDOUT`.
Expand Down
217 changes: 172 additions & 45 deletions src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,26 @@ pub fn expand(path: impl AsRef<Path>) {
path,
ExpansionBehavior::RegenerateFiles,
Option::<Vec<String>>::None,
false,
);
}

/// Attempts to expand macros in files that match glob pattern and expects the expansion to fail.
///
/// # Refresh behavior
///
/// If no matching `.expanded.rs` files present, they will be created and result (error) of expansion
/// will be written into them.
///
/// # Panics
///
/// Will panic if matching `.expanded.rs` file is present, but has different expanded code in it.
pub fn expand_fail(path: impl AsRef<Path>) {
run_tests(
path,
ExpansionBehavior::RegenerateFiles,
Option::<Vec<String>>::None,
true,
);
}

Expand All @@ -68,15 +88,26 @@ where
I: IntoIterator<Item = S> + Clone,
S: AsRef<OsStr>,
{
run_tests(path, ExpansionBehavior::RegenerateFiles, Some(args));
run_tests(path, ExpansionBehavior::RegenerateFiles, Some(args), false);
}

/// Same as [`expand_fail`] but allows to pass additional arguments to `cargo-expand`.
///
/// [`expand_fail`]: expand/fn.expand_fail.html
pub fn expand_args_fail<I, S>(path: impl AsRef<Path>, args: I)
where
I: IntoIterator<Item = S> + Clone,
S: AsRef<OsStr>,
{
run_tests(path, ExpansionBehavior::RegenerateFiles, Some(args), true);
}

/// Attempts to expand macros in files that match glob pattern.
/// More strict version of [`expand`] function.
///
/// # Refresh behavior
///
/// If no matching `.expanded.rs` files present, it considered a failed test.
/// If no matching `.expanded.rs` files present, it's considered a failed test.
///
/// # Panics
///
Expand All @@ -89,6 +120,29 @@ pub fn expand_without_refresh(path: impl AsRef<Path>) {
path,
ExpansionBehavior::ExpectFiles,
Option::<Vec<String>>::None,
false,
);
}

/// Attempts to expand macros in files that match glob pattern and expects a failure.
/// More strict version of [`expand_fail`] function.
///
/// # Refresh behavior
///
/// If no matching `.expanded.rs` files present, it's considered a failed test.
///
/// # Panics
///
/// Will panic if no matching `.expanded.rs` file is present. Otherwise it will exhibit the same
/// behavior as in [`expand_fail`].
///
/// [`expand_fail`]: expand/fn.expand_fail.html
pub fn expand_without_refresh_fail(path: impl AsRef<Path>) {
run_tests(
path,
ExpansionBehavior::ExpectFiles,
Option::<Vec<String>>::None,
true,
);
}

Expand All @@ -100,7 +154,18 @@ where
I: IntoIterator<Item = S> + Clone,
S: AsRef<OsStr>,
{
run_tests(path, ExpansionBehavior::ExpectFiles, Some(args));
run_tests(path, ExpansionBehavior::ExpectFiles, Some(args), false);
}

/// Same as [`expand_without_refresh_fail`] but allows to pass additional arguments to `cargo-expand` and expects a failure.
///
/// [`expand_without_refresh_fail`]: expand/fn.expand_without_refresh_fail.html
pub fn expand_without_refresh_args_fail<I, S>(path: impl AsRef<Path>, args: I)
where
I: IntoIterator<Item = S> + Clone,
S: AsRef<OsStr>,
{
run_tests(path, ExpansionBehavior::ExpectFiles, Some(args), true);
}

#[derive(Debug, Copy, Clone)]
Expand All @@ -109,8 +174,12 @@ enum ExpansionBehavior {
ExpectFiles,
}

fn run_tests<I, S>(path: impl AsRef<Path>, expansion_behavior: ExpansionBehavior, args: Option<I>)
where
fn run_tests<I, S>(
path: impl AsRef<Path>,
expansion_behavior: ExpansionBehavior,
args: Option<I>,
expect_fail: bool,
) where
I: IntoIterator<Item = S> + Clone,
S: AsRef<OsStr>,
{
Expand All @@ -132,33 +201,41 @@ where
let expanded_path = test.test.with_extension(EXPANDED_RS_SUFFIX);

match test.run(&project, expansion_behavior, &args) {
Ok(outcome) => match outcome {
ExpansionOutcome::Same => {
let _ = writeln!(std::io::stdout(), "{} - ok", path);
}
Ok(ExpansionOutcome { error, outcome }) => {
if let Some(msg) = error {
if !expect_fail {
message_expansion_error(msg);
failures += 1;

ExpansionOutcome::Different(a, b) => {
message_different(&path.to_string(), &a, &b);
failures += 1;
continue;
}
}

ExpansionOutcome::Update(_) => {
let _ = writeln!(std::io::stderr(), "{} - refreshed", expanded_path.display());
}
match outcome {
ExpansionOutcomeKind::Same => {
let _ = writeln!(std::io::stdout(), "{} - ok", path);
}

ExpansionOutcome::ExpandError(msg) => {
message_expansion_error(msg);
failures += 1;
}
ExpansionOutcome::NoExpandedFileFound => {
let _ = writeln!(
std::io::stderr(),
"{} is expected but not found",
expanded_path.display()
);
failures += 1;
ExpansionOutcomeKind::Different(a, b) => {
message_different(&path.to_string(), &a, &b);
failures += 1;
}

ExpansionOutcomeKind::Update(_) => {
let _ =
writeln!(std::io::stderr(), "{} - refreshed", expanded_path.display());
}

ExpansionOutcomeKind::NoExpandedFileFound => {
let _ = writeln!(
std::io::stderr(),
"{} is expected but not found",
expanded_path.display()
);
failures += 1;
}
}
},
}

Err(e) => {
eprintln!("Error: {:#?}", e);
Expand Down Expand Up @@ -301,11 +378,22 @@ fn make_config() -> Config {
}

#[derive(Debug)]
enum ExpansionOutcome {
struct ExpansionOutcome {
error: Option<Vec<u8>>,
outcome: ExpansionOutcomeKind,
}

impl ExpansionOutcome {
pub fn new(error: Option<Vec<u8>>, outcome: ExpansionOutcomeKind) -> Self {
Self { error, outcome }
}
}

#[derive(Debug)]
enum ExpansionOutcomeKind {
Same,
Different(Vec<u8>, Vec<u8>),
Update(Vec<u8>),
ExpandError(Vec<u8>),
NoExpandedFileFound,
}

Expand All @@ -328,9 +416,11 @@ impl ExpandedTest {
{
let (success, output_bytes) = cargo::expand(project, &self.name, args)?;

if !success {
return Ok(ExpansionOutcome::ExpandError(output_bytes));
}
let error = if success {
None
} else {
Some(output_bytes.clone())
};

let file_stem = self
.test
Expand All @@ -342,17 +432,27 @@ impl ExpandedTest {
expanded.pop();
let expanded = &expanded.join(format!("{}.{}", file_stem, EXPANDED_RS_SUFFIX));

let output = normalize_expansion(&output_bytes);
let output = if success {
normalize_expansion(&output_bytes, CARGO_EXPAND_SKIP_LINES_COUNT, project)
} else {
normalize_expansion(&output_bytes, CARGO_EXPAND_ERROR_SKIP_LINES_COUNT, project)
};

if !expanded.exists() {
if let ExpansionBehavior::ExpectFiles = expansion_behavior {
return Ok(ExpansionOutcome::NoExpandedFileFound);
return Ok(ExpansionOutcome::new(
error,
ExpansionOutcomeKind::NoExpandedFileFound,
));
}

// Write a .expanded.rs file contents with an newline character at the end
std::fs::write(expanded, &format!("{}\n", output))?;
std::fs::write(expanded, output)?;

return Ok(ExpansionOutcome::Update(output_bytes));
return Ok(ExpansionOutcome::new(
error,
ExpansionOutcomeKind::Update(output_bytes),
));
}

let expected_expansion_bytes = std::fs::read(expanded)?;
Expand All @@ -362,33 +462,60 @@ impl ExpandedTest {

if !same && project.overwrite {
if let ExpansionBehavior::ExpectFiles = expansion_behavior {
return Ok(ExpansionOutcome::NoExpandedFileFound);
return Ok(ExpansionOutcome::new(
error,
ExpansionOutcomeKind::NoExpandedFileFound,
));
}

// Write a .expanded.rs file contents with an newline character at the end
std::fs::write(expanded, &format!("{}\n", output))?;
std::fs::write(expanded, output)?;

return Ok(ExpansionOutcome::Update(output_bytes));
return Ok(ExpansionOutcome::new(
error,
ExpansionOutcomeKind::Update(output_bytes),
));
}

Ok(if same {
ExpansionOutcome::Same
ExpansionOutcome::new(error, ExpansionOutcomeKind::Same)
} else {
let output_bytes = output.into_bytes(); // Use normalized text for a message
ExpansionOutcome::Different(expected_expansion_bytes, output_bytes)
ExpansionOutcome::new(
error,
ExpansionOutcomeKind::Different(expected_expansion_bytes, output_bytes),
)
})
}
}

// `cargo expand` does always produce some fixed amount of lines that should be ignored
const CARGO_EXPAND_SKIP_LINES_COUNT: usize = 5;
const CARGO_EXPAND_ERROR_SKIP_LINES_COUNT: usize = 1;

/// Removes specified number of lines and removes some unnecessary or non-determenistic cargo output
fn normalize_expansion(input: &[u8], num_lines_to_skip: usize, project: &Project) -> String {
// These prefixes are non-deterministic and project-dependent
// These prefixes or the whole line shall be removed
let project_path_prefix = format!(" --> {}/", project.source_dir.to_string_lossy());
let proj_name_prefix = format!(" Checking {} v0.0.0", project.name);
let blocking_prefix = " Blocking waiting for file lock on package cache";

fn normalize_expansion(input: &[u8]) -> String {
let code = String::from_utf8_lossy(input);
code.lines()
.skip(CARGO_EXPAND_SKIP_LINES_COUNT)
let lines = code
.lines()
.skip(num_lines_to_skip)
.filter(|line| !line.starts_with(&proj_name_prefix))
.map(|line| line.strip_prefix(&project_path_prefix).unwrap_or(line))
.map(|line| line.strip_prefix(&blocking_prefix).unwrap_or(line))
.collect::<Vec<_>>()
.join("\n")
.join("\n");

if !lines.ends_with("\n\n") {
return format!("{}\n", lines);
} else {
lines
}
}

fn expand_globs(path: impl AsRef<Path>) -> Vec<ExpandedTest> {
Expand Down
2 changes: 1 addition & 1 deletion src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn try_find() -> Result<Vec<String>, Ignored> {
}

fn is_lower_hex_digit(byte: u8) -> bool {
byte >= b'0' && byte <= b'9' || byte >= b'a' && byte <= b'f'
(b'0'..=b'9').contains(&byte) || (b'a'..=b'f').contains(&byte)
}

fn from_json<'de, T, D>(deserializer: D) -> Result<T, D::Error>
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,9 @@ mod rustflags;

pub use expand::expand;
pub use expand::expand_args;
pub use expand::expand_args_fail;
pub use expand::expand_fail;
pub use expand::expand_without_refresh;
pub use expand::expand_without_refresh_args;
pub use expand::expand_without_refresh_args_fail;
pub use expand::expand_without_refresh_fail;
4 changes: 2 additions & 2 deletions src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use diff::Result;

/// Prints the difference of the two snippets of expanded code.
pub(crate) fn message_different(name: &str, a: &[u8], b: &[u8]) {
let a = String::from_utf8_lossy(&a);
let b = String::from_utf8_lossy(&b);
let a = String::from_utf8_lossy(a);
let b = String::from_utf8_lossy(b);

let changes = diff::lines(&a, &b);

Expand Down
15 changes: 15 additions & 0 deletions test-procmacro-project/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,18 @@ pub fn my_attribute(_args: TokenStream, input: TokenStream) -> TokenStream {

tokens.into()
}

#[proc_macro]
pub fn my_macro_panics(_input: TokenStream) -> TokenStream {
panic!("test")
}

#[proc_macro_derive(MyDerivePanics)]
pub fn my_derive_panics(_input: TokenStream) -> TokenStream {
panic!("test")
}

#[proc_macro_attribute]
pub fn my_attribute_panics(_args: TokenStream, _input: TokenStream) -> TokenStream {
panic!("test")
}
Loading