Skip to content

Commit a767940

Browse files
ElrendioElrendiobnjbvr
authored
Fix false positive for multi dep single use statements (#120)
Resolves #91 This also resolves some false detections of `not_my_dep::my_dep_name::stuff_in_my_dep` 😊 I couldn't find a way to avoid detection of `futures` in with the single line regexp ```rust pub use { async_trait::{mod1, dep2}, not_futures::{ futures::{futures_mod1, futures_mod2::{futures_mod21, futures_mod22}} }, reqwest, }; ``` This update does come at a small performance downside (that might be resolved by not instantiating a regexp per dependency?). On our repo of ~500k lines of machete goes from ~45s user time to ~48s of user time. --------- Co-authored-by: Elrendio <oscar.walter@ens.fr> Co-authored-by: Benjamin Bouvier <benjamin@bouvier.cc>
1 parent 16e3e93 commit a767940

File tree

1 file changed

+108
-8
lines changed

1 file changed

+108
-8
lines changed

src/search_unused.rs

+108-8
Original file line numberDiff line numberDiff line change
@@ -79,23 +79,40 @@ fn make_line_regexp(name: &str) -> String {
7979
// Breaking down this regular expression: given a line,
8080
// - `use (::)?(?i){name}(?-i)(::|;| as)`: matches `use foo;`, `use foo::bar`, `use foo as bar;`, with
8181
// an optional "::" in front of the crate's name.
82-
// - `\b(?i){name}(?-i)::`: matches `foo::X`, but not `barfoo::X`. `\b` means word boundary, so
83-
// putting it before the crate's name ensures there's no polluting prefix.
82+
// - `(?:[^:]|^|\W::)\b(?i){name}(?-i)::`: matches `foo::X`, but not `barfoo::X`. To ensure there's no polluting
83+
// prefix we add `(?:[^:]|^|\W::)\b`, meaning that the crate name must be prefixed by either:
84+
// * Not a `:` (therefore not a sub module)
85+
// * The start of a line
86+
// * Not a word character followed by `::` (to allow ::my_crate)
8487
// - `extern crate (?i){name}(?-i)( |;)`: matches `extern crate foo`, or `extern crate foo as bar`.
8588
// - `(?i){name}(?-i)` makes the match against the crate's name case insensitive
8689
format!(
87-
r#"use (::)?(?i){name}(?-i)(::|;| as)|\b(?i){name}(?-i)::|extern crate (?i){name}(?-i)( |;)"#
90+
r#"use (::)?(?i){name}(?-i)(::|;| as)|(?:[^:]|^|\W::)\b(?i){name}(?-i)::|extern crate (?i){name}(?-i)( |;)"#
8891
)
8992
}
9093

9194
fn make_multiline_regexp(name: &str) -> String {
9295
// Syntax documentation: https://docs.rs/regex/latest/regex/#syntax
9396
//
94-
// Breaking down this Terrible regular expression: tries to match compound `use as` statements,
95-
// as in `use { X as Y };`, with possibly multiple-lines in between. Will match the first `};`
96-
// that it finds, which *should* be the end of the use statement, but oh well.
97-
// `(?i){name}(?-i)` makes the match against the crate's name case insensitive.
98-
format!(r#"use \{{\s[^;]*(?i){name}(?-i)\s*as\s*[^;]*\}};"#)
97+
// Breaking down this Terrible regular expression: tries to match uses of the crate's name in
98+
// compound `use` statement accross multiple lines.
99+
//
100+
// It's split into 3 parts:
101+
// 1. Matches modules before the usage of the crate's name: `\s*(?:(::)?\w+{sub_modules_match}\s*,\s*)*`
102+
// 2. Matches the crate's name with optional sub-modules: `(::)?{name}{sub_modules_match}\s*`
103+
// 3. Matches modules after the usage of the crate's name: `(?:\s*,\s*(::)?\w+{sub_modules_match})*\s*,?\s*`
104+
//
105+
// In order to avoid false usage detection of `not_my_dep::my_dep` the regexp ensures that the
106+
// crate's name is at the top level of the use statement. However, it's not possible with
107+
// regexp to allow any number of matching `{` and `}` before the crate's usage (rust regexp
108+
// engine doesn't support recursion). Therefore, sub modules are authorized up to 4 levels
109+
// deep.
110+
111+
let sub_modules_match = r#"(?:::\w+)*(?:::\*|\s+as\s+\w+|::\{(?:[^{}]*(?:\{(?:[^{}]*(?:\{(?:[^{}]*(?:\{[^{}]*\})?[^{}]*)*\})?[^{}]*)*\})?[^{}]*)*\})?"#;
112+
113+
format!(
114+
r#"use \{{\s*(?:(::)?\w+{sub_modules_match}\s*,\s*)*(::)?{name}{sub_modules_match}\s*(?:\s*,\s*(::)?\w+{sub_modules_match})*\s*,?\s*\}};"#
115+
)
99116
}
100117

101118
/// Returns all the paths to the Rust source files for a crate contained at the given path.
@@ -666,6 +683,89 @@ pub use futures::future;
666683
"#
667684
)?);
668685

686+
// multi-dep single use statements
687+
assert!(test_one(
688+
"futures",
689+
r#"pub use {async_trait, futures, reqwest};"#
690+
)?);
691+
692+
// multi-dep single use statements with ::
693+
assert!(test_one(
694+
"futures",
695+
r#"pub use {async_trait, ::futures, reqwest};"#
696+
)?);
697+
698+
// No false usage detection of `not_my_dep::my_dep` on compound imports
699+
assert!(!test_one(
700+
"futures",
701+
r#"pub use {async_trait, not_futures::futures, reqwest};"#
702+
)?);
703+
704+
// No false usage detection of `not_my_dep::my_dep` on multiple lines
705+
assert!(!test_one(
706+
"futures",
707+
r#"
708+
pub use {
709+
async_trait,
710+
not_futures::futures,
711+
reqwest,
712+
};"#
713+
)?);
714+
715+
// No false usage detection on single line `not_my_dep::my_dep`
716+
assert!(!test_one(
717+
"futures",
718+
r#"use not_futures::futures::stuff_in_futures;"#
719+
)?);
720+
721+
// multi-dep single use statements with nesting
722+
assert!(test_one(
723+
"futures",
724+
r#"pub use {
725+
async_trait::{mod1, dep2},
726+
futures::{futures_mod1, futures_mod2::{futures_mod21, futures_mod22}},
727+
reqwest,
728+
};"#
729+
)?);
730+
731+
// multi-dep single use statements with star import and renaming
732+
assert!(test_one(
733+
"futures",
734+
r#"pub use {
735+
async_trait::sub_mod::*,
736+
futures as futures_renamed,
737+
reqwest,
738+
};"#
739+
)?);
740+
741+
// multi-dep single use statements with complex imports and renaming
742+
assert!(test_one(
743+
"futures",
744+
r#"pub use {
745+
other_dep::{
746+
star_mod::*,
747+
unnamed_import::{UnnamedTrait as _, other_mod},
748+
renamed_import as new_name,
749+
sub_import::{mod1, mod2},
750+
},
751+
futures as futures_renamed,
752+
reqwest,
753+
};"#
754+
)?);
755+
756+
// No false usage detection of `not_my_dep::my_dep` with nesting
757+
assert!(!test_one(
758+
"futures",
759+
r#"pub use {
760+
async_trait::{mod1, dep2},
761+
not_futures::futures::{futures_mod1, futures_mod2::{futures_mod21, futures_mod22}},
762+
reqwest,
763+
};"#
764+
)?);
765+
766+
// Detects top level usage
767+
assert!(test_one("futures", r#" ::futures::mod1"#)?);
768+
669769
Ok(())
670770
}
671771

0 commit comments

Comments
 (0)