Skip to content

Commit

Permalink
Warn if extension is passed with a leading dot (#636)
Browse files Browse the repository at this point in the history
If someone passes `.snap` as an extension, they'll get files of
`foo..snap`, which is probably not what they want.

We could strip the leading `.`

Also some light refactoring
  • Loading branch information
max-sixty authored Oct 4, 2024
1 parent 71f7713 commit 379fbe4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 22 deletions.
44 changes: 24 additions & 20 deletions cargo-insta/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,12 @@ fn handle_target_args<'a>(
// Empty if none are selected, implying cargo default
packages: &'a [String],
) -> Result<LocationInfo<'a>, Box<dyn Error>> {
let exts: Vec<&str> = target_args.extensions.iter().map(|x| x.as_str()).collect();
let mut cmd = cargo_metadata::MetadataCommand::new();

// if a workspace root is provided we first check if it points to a `Cargo.toml`. If it
// does we instead treat it as manifest path. If both are provided we fail with an error
// as this would indicate an error.
let (workspace_root, manifest_path) = match (
// if a workspace root is provided we first check if it points to a
// `Cargo.toml`. If it does we instead treat it as manifest path. If both
// are provided we fail with an error.
match (
target_args.workspace_root.as_deref(),
target_args.manifest_path.as_deref(),
) {
Expand All @@ -387,32 +387,27 @@ fn handle_target_args<'a>(
"both manifest-path and workspace-root provided.".to_string(),
))
}
(None, Some(manifest)) => (None, Some(Cow::Borrowed(manifest))),
(None, Some(manifest)) => {
cmd.manifest_path(manifest);
}
(Some(root), None) => {
// TODO: should we do this ourselves? Probably fine, but are we
// adding anything by not just deferring to cargo?
let assumed_manifest = root.join("Cargo.toml");
if assumed_manifest.is_file() {
(None, Some(Cow::Owned(assumed_manifest)))
cmd.manifest_path(assumed_manifest);
} else {
(Some(root), None)
cmd.current_dir(root);
}
}
(None, None) => (None, None),
(None, None) => {}
};

let mut cmd = cargo_metadata::MetadataCommand::new();

// If a manifest path is provided, set it in the command
if let Some(manifest_path) = manifest_path {
cmd.manifest_path(manifest_path);
}
if let Some(workspace_root) = workspace_root {
cmd.current_dir(workspace_root);
}
let metadata = cmd.no_deps().exec()?;
let workspace_root = metadata.workspace_root.as_std_path().to_path_buf();
let tool_config = ToolConfig::from_workspace(&workspace_root)?;

// If `--all` is passed, or there's no root package, we include all
// If `--workspace` is passed, or there's no root package, we include all
// packages. If packages are specified, we filter from all packages.
// Otherwise we use just the root package.
//
Expand All @@ -435,7 +430,16 @@ fn handle_target_args<'a>(
Ok(LocationInfo {
workspace_root,
packages,
exts,
exts: target_args
.extensions
.iter()
.map(|x| {
if let Some(no_period) = x.strip_prefix(".") {
eprintln!("`{}` supplied as an extenstion. This will use `foo.{}` as file names; likely you want `{}` instead.", x, x, no_period)
};
x.as_str()
})
.collect(),
find_flags: get_find_flags(&tool_config, target_args),
tool_config,
})
Expand Down
8 changes: 6 additions & 2 deletions insta/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ pub fn get_tool_config(workspace_dir: &Path) -> Arc<ToolConfig> {
.lock()
.unwrap()
.entry(workspace_dir.to_path_buf())
.or_insert_with(|| ToolConfig::from_workspace(workspace_dir).unwrap().into())
.or_insert_with(|| {
ToolConfig::from_workspace(workspace_dir)
.unwrap_or_else(|e| panic!("Error building config from {:?}: {}", workspace_dir, e))
.into()
})
.clone()
}

Expand Down Expand Up @@ -96,7 +100,7 @@ impl std::error::Error for Error {
}

/// Represents a tool configuration.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct ToolConfig {
force_pass: bool,
require_full_match: bool,
Expand Down

0 comments on commit 379fbe4

Please sign in to comment.