Skip to content

Commit

Permalink
refactor(traverse): speed up tests (#4538)
Browse files Browse the repository at this point in the history
Closes #4537.

`oxc_traverse`'s tests to ensure code which would be undefined behavior will not compile were using `trybuild`. This is a thorough way to run these tests, but requires running a lengthy compilation each time tests run.

Implement these tests as `compile_fail` doc tests instead. This is not quite as thorough - now only testing that they don't compile, rather than also *why* they don't compile - but acceptable given the outsized cost of doing it "properly".
  • Loading branch information
overlookmotel committed Jul 29, 2024
1 parent d6974d4 commit e6a8af6
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 178 deletions.
76 changes: 0 additions & 76 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ textwrap = "0.16.1"
tokio = "1.38.0"
tower-lsp = "0.20.0"
tracing-subscriber = "0.3.18"
trybuild = "1.0.96"
tsify = "0.4.5"
unicode-id-start = "1" # Relaxed version so the user can decide which unicode version to use.
unicode-width = "0.1.13"
Expand Down
7 changes: 3 additions & 4 deletions crates/oxc_traverse/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ include = ["/src"]
workspace = true

[lib]
test = false
test = false
# Doctests must be enabled for this crate as they are used to run tests
# which check the soundness of its code
doctest = true

[dependencies]
Expand All @@ -29,6 +31,3 @@ oxc_syntax = { workspace = true }

compact_str = { workspace = true }
memoffset = { workspace = true }

[dev-dependencies]
trybuild = { workspace = true }
81 changes: 81 additions & 0 deletions crates/oxc_traverse/src/compile_fail_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#![cfg(doctest)]

//! Tests to ensure lifetimes prevent references to escape visitor functions.
//! If they could, it'd allow aliasing, which would be undefined behavior.
//!
//! These tests were originally implemented using `trybuild` crate,
//! but it disproportionately hurt time taken to run tests.
//! So using `compile_fail` doc tests instead.
//! <https://github.com/oxc-project/oxc/issues/4537>

/**
```compile_fail
use oxc_ast::ast::IdentifierReference;
use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
struct Trans<'a, 'b> {
ancestor: Option<&'b Ancestor<'a>>,
}
impl<'a, 'b> Traverse<'a> for Trans<'a, 'b> {
fn enter_identifier_reference(
&mut self,
_node: &mut IdentifierReference<'a>,
ctx: &mut TraverseCtx<'a>,
) {
self.ancestor = Some(ctx.parent());
}
}
```
*/
const CANNOT_HOLD_ONTO_ANCESTOR: () = ();

/**
```compile_fail
use oxc_ast::ast::IdentifierReference;
use oxc_traverse::{ancestor::ProgramWithoutDirectives, Ancestor, Traverse, TraverseCtx};
struct Trans<'a, 'b> {
program: Option<&'b ProgramWithoutDirectives<'a>>,
}
impl<'a, 'b> Traverse<'a> for Trans<'a, 'b> {
fn enter_identifier_reference(
&mut self,
_node: &mut IdentifierReference<'a>,
ctx: &mut TraverseCtx<'a>,
) {
if let Ancestor::ProgramDirectives(program) = ctx.parent() {
self.program = Some(program);
}
}
}
```
*/
const CANNOT_HOLD_ONTO_ANCESTOR_NODE: () = ();

/**
```compile_fail
use oxc_ast::ast::{IdentifierReference, Statement};
use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
struct Trans<'a, 'b> {
stmt: Option<&'b Statement<'a>>,
}
impl<'a, 'b> Traverse<'a> for Trans<'a, 'b> {
fn enter_identifier_reference(
&mut self,
_node: &mut IdentifierReference<'a>,
ctx: &mut TraverseCtx<'a>,
) {
if let Ancestor::ProgramDirectives(program) = ctx.parent() {
let body = program.body();
let stmt = &body[0];
self.stmt = Some(stmt);
}
}
}
```
*/
const CANNOT_HOLD_ONTO_AST_NODE: () = ();
2 changes: 2 additions & 0 deletions crates/oxc_traverse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ mod traverse;
pub use traverse::Traverse;
mod walk;

mod compile_fail_tests;

/// Traverse AST with a [`Traverse`] impl.
///
/// This allows:
Expand Down
5 changes: 0 additions & 5 deletions crates/oxc_traverse/tests/compile_fail.rs

This file was deleted.

18 changes: 0 additions & 18 deletions crates/oxc_traverse/tests/compile_fail/ancestor_lifetime1.rs

This file was deleted.

11 changes: 0 additions & 11 deletions crates/oxc_traverse/tests/compile_fail/ancestor_lifetime1.stderr

This file was deleted.

20 changes: 0 additions & 20 deletions crates/oxc_traverse/tests/compile_fail/ancestor_lifetime2.rs

This file was deleted.

11 changes: 0 additions & 11 deletions crates/oxc_traverse/tests/compile_fail/ancestor_lifetime2.stderr

This file was deleted.

21 changes: 0 additions & 21 deletions crates/oxc_traverse/tests/compile_fail/ancestor_lifetime3.rs

This file was deleted.

11 changes: 0 additions & 11 deletions crates/oxc_traverse/tests/compile_fail/ancestor_lifetime3.stderr

This file was deleted.

0 comments on commit e6a8af6

Please sign in to comment.