Skip to content

Commit

Permalink
Allow patterns to be arrays
Browse files Browse the repository at this point in the history
  • Loading branch information
smoelius committed Aug 2, 2024
1 parent 6201092 commit 09b94f7
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 38 deletions.
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ libraries = [
]
```
For convenience, the `pattern` field can contain an array, in which case the pattern is considered to be the union of the array elements. Thus, the just given `workspace.metadata.dylint.libraries` example could alternatively be written as:
```toml
[workspace.metadata.dylint]
libraries = [
{ git = "https://github.com/trailofbits/dylint", pattern = [
"examples/general",
"examples/restriction/try_io_result",
] },
]
```
### Configurable libraries
Libraries can be configured by including a `dylint.toml` file in a linted workspace's root directory. The file should encode a [toml table] whose keys are library names. A library determines how its value in the table (if any) is interpreted.
Expand Down
12 changes: 12 additions & 0 deletions cargo-dylint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ libraries = [
]
```
For convenience, the `pattern` field can contain an array, in which case the pattern is considered to be the union of the array elements. Thus, the just given `workspace.metadata.dylint.libraries` example could alternatively be written as:
```toml
[workspace.metadata.dylint]
libraries = [
{ git = "https://github.com/trailofbits/dylint", pattern = [
"examples/general",
"examples/restriction/try_io_result",
] },
]
```
### Configurable libraries
Libraries can be configured by including a `dylint.toml` file in a linted workspace's root directory. The file should encode a [toml table] whose keys are library names. A library determines how its value in the table (if any) is interpreted.
Expand Down
13 changes: 13 additions & 0 deletions cargo-dylint/fixtures/array_pattern/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
name = "array_pattern"
version = "0.1.0"
edition = "2021"
publish = false

[workspace.metadata.dylint]
libraries = [
{ path = "../../../examples", pattern = [
"testing/clippy",
"restriction/question_mark_in_expression",
] },
]
1 change: 1 addition & 0 deletions cargo-dylint/fixtures/array_pattern/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

17 changes: 17 additions & 0 deletions cargo-dylint/tests/library_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,23 @@ fn initialize() {
// underlying problem, rather than try to fix the tests themselves.
static MUTEX: Mutex<()> = Mutex::new(());

#[test]
fn array_pattern() {
let _lock = MUTEX.lock().unwrap();

let assert = std::process::Command::cargo_bin("cargo-dylint")
.unwrap()
.current_dir("fixtures/array_pattern")
.args(["dylint", "list"])
.assert()
.success();
let stdout = std::str::from_utf8(&assert.get_output().stdout).unwrap();
let lines = stdout.lines().collect::<Vec<_>>();
assert_eq!(2, lines.len());
assert!(lines[0].starts_with("clippy "));
assert!(lines[1].starts_with("question_mark_in_expression "));
}

#[test]
fn invalid_pattern() {
let _lock = MUTEX.lock().unwrap();
Expand Down
12 changes: 12 additions & 0 deletions dylint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ libraries = [
]
```
For convenience, the `pattern` field can contain an array, in which case the pattern is considered to be the union of the array elements. Thus, the just given `workspace.metadata.dylint.libraries` example could alternatively be written as:
```toml
[workspace.metadata.dylint]
libraries = [
{ git = "https://github.com/trailofbits/dylint", pattern = [
"examples/general",
"examples/restriction/try_io_result",
] },
]
```
### Configurable libraries
Libraries can be configured by including a `dylint.toml` file in a linted workspace's root directory. The file should encode a [toml table] whose keys are library names. A library determines how its value in the table (if any) is interpreted.
Expand Down
85 changes: 47 additions & 38 deletions dylint/src/library_packages/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{error::warn, opts};
use anyhow::{anyhow, bail, ensure, Context, Result};
use cargo_metadata::{Error, Metadata, MetadataCommand, Package as MetadataPackage};
use cargo_util_schemas::manifest::TomlDetailedDependency;
use cargo_util_schemas::manifest::{StringOrVec, TomlDetailedDependency};
use dylint_internal::{config, env, library_filename, rustup::SanitizeEnvironment, CommandExt};
use glob::glob;
use if_chain::if_chain;
Expand Down Expand Up @@ -76,7 +76,7 @@ impl Package {

#[derive(Debug, Deserialize)]
struct Library {
pattern: Option<String>,
pattern: Option<StringOrVec>,
#[serde(flatten)]
details: TomlDetailedDependency,
}
Expand Down Expand Up @@ -116,7 +116,10 @@ pub fn from_opts(opts: &opts::Dylint) -> Result<Vec<Package>> {

let library = Library {
details,
pattern: lib_sel.pattern.clone(),
pattern: lib_sel
.pattern
.as_ref()
.map(|pattern| StringOrVec(vec![pattern.clone()])),
};

library_packages(opts, metadata, &[library])
Expand Down Expand Up @@ -291,45 +294,51 @@ fn library_package(
let (source_id, dependency_root) =
dependency_source_id_and_root(opts, metadata, gctx, details)?;

let pattern = if let Some(pattern) = &library.pattern {
dependency_root.join(pattern)
let patterns = if let Some(StringOrVec(patterns)) = &library.pattern {
patterns.clone()
} else {
#[allow(clippy::redundant_clone)]
dependency_root.clone()
vec![String::new()]
};

let entries = glob(&pattern.to_string_lossy())?;

let paths = entries
.map(|entry| {
entry.map_err(Into::into).and_then(|path| {
// smoelius: Because `dependency_root` might not be absolute, `path` might not be
// absolute. So `path` must be normalized.
let path_buf = cargo_util::paths::normalize_path(&path);
if let Some(pattern) = &library.pattern {
// smoelius: Use `cargo_util::paths::normalize_path` instead of `canonicalize`
// so as not to "taint" the path with a path prefix on Windows.
//
// This problem keeps coming up. For example, it recently came up in:
// https://github.com/trailofbits/dylint/pull/944
let dependency_root = cargo_util::paths::normalize_path(&dependency_root);
ensure!(
path_buf.starts_with(&dependency_root),
"Pattern `{pattern}` could refer to `{}`, which is outside of `{}`",
path_buf.to_string_lossy(),
dependency_root.to_string_lossy()
);
}
Ok(path_buf)
})
})
.collect::<std::result::Result<Vec<_>, _>>()?;
let mut paths = Vec::new();
for pattern in patterns {
let results = glob(&dependency_root.join(&pattern).to_string_lossy())?;

let mut matched = false;
for result in results {
let path = result?;

// smoelius: Because `dependency_root` might not be absolute, `path` might not be
// absolute. So `path` must be normalized.
let path_buf = cargo_util::paths::normalize_path(&path);

// smoelius: If `library.pattern` is set, verify the `path` that it matched is in
// `dependency_root`.
//
// Note that even if `library.pattern` is not set, the current loop must still be
// traversed. Recall, `dependency_root` could be a `glob` pattern. In such a
// case, the paths that it matches must be pushed onto `paths`.
if library.pattern.is_some() {
// smoelius: Use `cargo_util::paths::normalize_path` instead of `canonicalize`
// so as not to "taint" the path with a path prefix on Windows.
//
// This problem keeps coming up. For example, it recently came up in:
// https://github.com/trailofbits/dylint/pull/944
let dependency_root = cargo_util::paths::normalize_path(&dependency_root);
ensure!(
path_buf.starts_with(&dependency_root),
"Pattern `{pattern}` could refer to `{}`, which is outside of `{}`",
path_buf.to_string_lossy(),
dependency_root.to_string_lossy()
);
}

ensure!(
!paths.is_empty(),
"No paths matched `{}`",
pattern.to_string_lossy()
);
paths.push(path_buf);
matched = true;
}

ensure!(matched, "No paths matched `{}`", pattern);
}

// smoelius: Collecting the package ids before building reveals missing/unparsable `Cargo.toml`
// files sooner.
Expand Down

0 comments on commit 09b94f7

Please sign in to comment.