add more failing tests and make them pass

Cargo.lock

@@ -1106,6 +1106,7 @@ dependencies = [
  "libc",
  "linked-hash-map",
  "lz4_flex",
+ "memchr",
  "num_cpus",
  "once_cell",
  "parse-display",

Cargo.toml

@@ -62,6 +62,7 @@ glob = "0.3.2"
 infer = "0.16.0"
 insta = { version = "1.40.0", features = ["filters"] }
 itertools = "0.14.0"
+memchr = "2.7.4"
 parse-display = "0.9.1"
 pretty_assertions = "1.4.1"
 proptest = "1.5.0"

src/commands/mod.rs

@@ -81,7 +81,7 @@ pub fn run(
                     let parsed_formats = parse_format_flag(&formats)?;
                     (Some(formats), parsed_formats)
                 }
-                None => (None, extension::extensions_from_path(&output_path)),
+                None => (None, extension::extensions_from_path(&output_path)?),
             };
 
             check::check_invalid_compression_with_non_archive_format(
@@ -167,7 +167,7 @@ pub fn run(
                 }
             } else {
                 for path in files.iter() {
-                    let (pathbase, mut file_formats) = extension::separate_known_extensions_from_name(path);
+                    let (pathbase, mut file_formats) = extension::separate_known_extensions_from_name(path)?;
 
                     if let ControlFlow::Break(_) = check::check_mime_type(path, &mut file_formats, question_policy)? {
                         return Ok(());
@@ -229,7 +229,7 @@ pub fn run(
                 }
             } else {
                 for path in files.iter() {
-                    let mut file_formats = extension::extensions_from_path(path);
+                    let mut file_formats = extension::extensions_from_path(path)?;
 
                     if let ControlFlow::Break(_) = check::check_mime_type(path, &mut file_formats, question_policy)? {
                         return Ok(());

src/extension.rs

@@ -5,7 +5,10 @@ use std::{ffi::OsStr, fmt, path::Path};
 use bstr::ByteSlice;
 use CompressionFormat::*;
 
-use crate::{error::Error, utils::logger::warning};
+use crate::{
+    error::{Error, FinalError, Result},
+    utils::logger::warning,
+};
 
 pub const SUPPORTED_EXTENSIONS: &[&str] = &[
     "tar",
@@ -188,17 +191,38 @@ pub fn parse_format_flag(input: &OsStr) -> crate::Result<Vec<Extension>> {
 /// Extracts extensions from a path.
 ///
 /// Returns both the remaining path and the list of extension objects.
-pub fn separate_known_extensions_from_name(path: &Path) -> (&Path, Vec<Extension>) {
+pub fn separate_known_extensions_from_name(path: &Path) -> Result<(&Path, Vec<Extension>)> {
     let mut extensions = vec![];
 
     let Some(mut name) = path.file_name().and_then(<[u8] as ByteSlice>::from_os_str) else {
-        return (path, extensions);
+        return Ok((path, extensions));
     };
 
-    while let Some((new_name, extension)) = split_extension_at_end(&name) {
+    while let Some((new_name, extension)) = split_extension_at_end(name) {
         name = new_name;
         extensions.insert(0, extension);
         if extensions[0].is_archive() {
+            if let Some((_, misplaced_extension)) = split_extension_at_end(name) {
+                return Err(FinalError::with_title("File extensions are invalid for operation")
+                    .detail(format!(
+                        "The archive extension '.{}' must come before any non-archive extensions, like '.{}'",
+                        extensions[0].display_text, misplaced_extension.display_text
+                    ))
+                    .detail(format!(
+                        "File: '{path:?}' contains '.{}' and '.{}'",
+                        misplaced_extension.display_text, extensions[0].display_text,
+                    ))
+                    .detail(format!("'.{}' is an archive format", extensions[0].display_text))
+                    .detail(format!(
+                        "'.{}' isn't an archive format",
+                        misplaced_extension.display_text
+                    ))
+                    .hint("You can use `--format` to specify what format to use, examples:")
+                    .hint("  ouch compress 1 2 file --format zip")
+                    .hint("  ouch decompress file --format gz")
+                    .hint("  ouch list archive --format zip")
+                    .into());
+            }
             break;
         }
     }
@@ -212,13 +236,12 @@ pub fn separate_known_extensions_from_name(path: &Path) -> (&Path, Vec<Extension
         }
     }
 
-    (name.to_path().unwrap(), extensions)
+    Ok((name.to_path().unwrap(), extensions))
 }
 
 /// Extracts extensions from a path, return only the list of extension objects
-pub fn extensions_from_path(path: &Path) -> Vec<Extension> {
+pub fn extensions_from_path(path: &Path) -> Result<Vec<Extension>> {
-    let (_, extensions) = separate_known_extensions_from_name(path);
+    separate_known_extensions_from_name(path).map(|(_, extensions)| extensions)
-    extensions
 }
 
 /// Panics if formats has an empty list of compression formats
@@ -278,8 +301,8 @@ mod tests {
     fn test_extensions_from_path() {
         let path = Path::new("bolovo.tar.gz");
 
-        let extensions: Vec<Extension> = extensions_from_path(path);
+        let extensions = extensions_from_path(path).unwrap();
-        let formats: Vec<CompressionFormat> = flatten_compression_formats(&extensions);
+        let formats = flatten_compression_formats(&extensions);
 
         assert_eq!(formats, vec![Tar, Gzip]);
     }
@@ -288,30 +311,30 @@ mod tests {
     /// Test extension parsing for input/output files
     fn test_separate_known_extensions_from_name() {
         assert_eq!(
-            separate_known_extensions_from_name("file".as_ref()),
+            separate_known_extensions_from_name("file".as_ref()).unwrap(),
             ("file".as_ref(), vec![])
         );
         assert_eq!(
-            separate_known_extensions_from_name("tar".as_ref()),
+            separate_known_extensions_from_name("tar".as_ref()).unwrap(),
             ("tar".as_ref(), vec![])
         );
         assert_eq!(
-            separate_known_extensions_from_name(".tar".as_ref()),
+            separate_known_extensions_from_name(".tar".as_ref()).unwrap(),
             (".tar".as_ref(), vec![])
         );
         assert_eq!(
-            separate_known_extensions_from_name("file.tar".as_ref()),
+            separate_known_extensions_from_name("file.tar".as_ref()).unwrap(),
             ("file".as_ref(), vec![Extension::new(&[Tar], "tar")])
         );
         assert_eq!(
-            separate_known_extensions_from_name("file.tar.gz".as_ref()),
+            separate_known_extensions_from_name("file.tar.gz".as_ref()).unwrap(),
             (
                 "file".as_ref(),
                 vec![Extension::new(&[Tar], "tar"), Extension::new(&[Gzip], "gz")]
             )
         );
         assert_eq!(
-            separate_known_extensions_from_name(".tar.gz".as_ref()),
+            separate_known_extensions_from_name(".tar.gz".as_ref()).unwrap(),
             (".tar".as_ref(), vec![Extension::new(&[Gzip], "gz")])
         );
     }
@@ -370,16 +393,7 @@ mod tests {
 
     #[test]
     fn test_extension_parsing_with_multiple_archive_formats() {
-        assert_eq!(
+        assert!(separate_known_extensions_from_name("file.tar.zip".as_ref()).is_err());
-            separate_known_extensions_from_name("file.tar.zip".as_ref()),
+        assert!(separate_known_extensions_from_name("file.7z.zst.zip.lz4".as_ref()).is_err());
-            ("file.tar".as_ref(), vec![Extension::new(&[Zip], "zip")])
-        );
-        assert_eq!(
-            separate_known_extensions_from_name("file.7z.zst.zip.lz4".as_ref()),
-            (
-                "file.7z.zst".as_ref(),
-                vec![Extension::new(&[Zip], "zip"), Extension::new(&[Lz4], "lz4")]
-            )
-        );
     }
 }

tests/integration.rs

@@ -7,8 +7,10 @@ use std::{
     path::{Path, PathBuf},
 };
 
+use bstr::ByteSlice;
 use fs_err as fs;
 use itertools::Itertools;
+use memchr::memmem;
 use parse_display::Display;
 use pretty_assertions::assert_eq;
 use proptest::sample::size_range;
@@ -213,7 +215,7 @@ fn multiple_files(
     let before = &dir.join("before");
     let before_dir = &before.join("dir");
     fs::create_dir_all(before_dir).unwrap();
-    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
     let after = &dir.join("after");
     create_random_files(before_dir, depth, &mut SmallRng::from_entropy());
     ouch!("-A", "c", before_dir, archive);
@@ -240,7 +242,7 @@ fn multiple_files_with_conflict_and_choice_to_overwrite(
     fs::create_dir_all(after_dir).unwrap();
     create_random_files(after_dir, depth, &mut SmallRng::from_entropy());
 
-    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
     ouch!("-A", "c", before_dir, archive);
 
     crate::utils::cargo_bin()
@@ -281,7 +283,7 @@ fn multiple_files_with_conflict_and_choice_to_not_overwrite(
     fs::write(after_dir.join("something.txt"), "Some content").unwrap();
     fs::copy(after_dir.join("something.txt"), after_backup_dir.join("something.txt")).unwrap();
 
-    let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
     ouch!("-A", "c", before_dir, archive);
 
     crate::utils::cargo_bin()
@@ -392,7 +394,7 @@ fn smart_unpack_with_single_file(
         })
         .collect::<Vec<_>>();
 
-    let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
 
     crate::utils::cargo_bin()
         .arg("compress")
@@ -443,7 +445,7 @@ fn smart_unpack_with_multiple_files(
         .map(|entry| entry.unwrap().path())
         .collect::<Vec<PathBuf>>();
 
-    let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
 
     let output_path = root_path.join("archive");
     assert!(!output_path.exists());
@@ -492,7 +494,7 @@ fn no_smart_unpack_with_single_file(
         .map(|entry| entry.unwrap().path())
         .collect::<Vec<PathBuf>>();
 
-    let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
 
     let output_path = root_path.join("archive");
     assert!(!output_path.exists());
@@ -542,7 +544,7 @@ fn no_smart_unpack_with_multiple_files(
         .map(|entry| entry.unwrap().path())
         .collect::<Vec<PathBuf>>();
 
-    let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
 
     let output_path = root_path.join("archive");
     assert!(!output_path.exists());
@@ -590,7 +592,7 @@ fn multiple_files_with_disabled_smart_unpack_by_dir(
     let dest_files_path = root_path.join("dest_files");
     fs::create_dir_all(&dest_files_path).unwrap();
 
-    let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
 
     crate::utils::cargo_bin()
         .arg("compress")
@@ -707,7 +709,7 @@ fn symlink_pack_and_unpack(
 
     files_path.push(symlink_path);
 
-    let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions)));
+    let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions)));
 
     crate::utils::cargo_bin()
         .arg("compress")
@@ -894,22 +896,23 @@ fn reading_nested_archives_with_two_archive_extensions_adjacent() {
                 .success();
         }
 
-        crate::utils::cargo_bin()
+        let output = crate::utils::cargo_bin()
             .args(["list", &compressed_path, "--yes"])
             .assert()
-            .success();
+            .failure()
+            .get_output()
+            .clone();
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some());
 
-        crate::utils::cargo_bin()
+        let output = crate::utils::cargo_bin()
             .args(["decompress", &compressed_path, "--dir", &in_dir("out"), "--yes"])
             .assert()
-            .success();
+            .failure()
-
+            .get_output()
-        let decompressed_files = glob::glob(&format!("{}/*", in_dir("out")))
+            .clone();
-            .unwrap()
+        let stderr = output.stderr.to_str().unwrap();
-            .collect::<Result<Vec<_>, _>>()
+        assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some());
-            .unwrap();
-        let decompression_expected = format!("{}/out/{}", temp_dir.path().display(), files[1]);
-        assert_eq!(decompressed_files, [Path::new(&decompression_expected)]);
     }
 }
 
@@ -941,22 +944,124 @@ fn reading_nested_archives_with_two_archive_extensions_interleaved() {
                 .success();
         }
 
-        // // TODO: uncomment after fixing the 7z BadSignature error [4, 34, 77, 24, 96, 64]
+        let output = crate::utils::cargo_bin()
-        // crate::utils::cargo_bin()
+            .args(["list", &compressed_path, "--yes"])
-        //     .args(["list", &compressed_path, "--yes"])
+            .assert()
-        //     .assert()
+            .failure()
-        //     .success();
+            .get_output()
+            .clone();
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some());
 
-        crate::utils::cargo_bin()
+        let output = crate::utils::cargo_bin()
             .args(["decompress", &compressed_path, "--dir", &in_dir("out"), "--yes"])
             .assert()
-            .success();
+            .failure()
+            .get_output()
+            .clone();
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some());
+    }
+}
 
-        let decompressed_files = glob::glob(&format!("{}/*", in_dir("out")))
+#[test]
-            .unwrap()
+fn compressing_archive_with_two_archive_formats() {
-            .collect::<Result<Vec<_>, _>>()
+    let archive_formats = ["tar", "zip", "7z"].into_iter();
-            .unwrap();
+
-        let decompression_expected = format!("{}/out/{}", temp_dir.path().display(), files[2]);
+    for (first_archive, second_archive) in archive_formats.clone().cartesian_product(archive_formats.rev()) {
-        assert_eq!(decompressed_files, [Path::new(&decompression_expected)]);
+        let temp_dir = tempdir().unwrap();
+        let dir = temp_dir.path().display().to_string();
+
+        let output = crate::utils::cargo_bin()
+            .args([
+                "compress",
+                "README.md",
+                &format!("{dir}/out.{first_archive}.{second_archive}"),
+                "--yes",
+            ])
+            .assert()
+            .failure()
+            .get_output()
+            .clone();
+
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some());
+
+        let output = crate::utils::cargo_bin()
+            .args([
+                "compress",
+                "README.md",
+                &format!("{dir}/out.{first_archive}.{second_archive}"),
+                "--yes",
+                "--format",
+                &format!("{first_archive}.{second_archive}"),
+            ])
+            .assert()
+            .failure()
+            .get_output()
+            .clone();
+
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(
+            stderr.as_bytes(),
+            b"can only be used at the start of the file extension",
+        )
+        .is_some());
+
+        crate::utils::cargo_bin()
+            .args([
+                "compress",
+                "README.md",
+                &format!("{dir}/out.{first_archive}.{second_archive}"),
+                "--yes",
+                "--format",
+                first_archive,
+            ])
+            .assert()
+            .success();
+    }
+}
+
+#[test]
+fn fail_when_compressing_archive_as_the_second_extension() {
+    for archive_format in ["tar", "zip", "7z"] {
+        let temp_dir = tempdir().unwrap();
+        let dir = temp_dir.path().display().to_string();
+
+        let output = crate::utils::cargo_bin()
+            .args([
+                "compress",
+                "README.md",
+                &format!("{dir}/out.zst.{archive_format}"),
+                "--yes",
+            ])
+            .assert()
+            .failure()
+            .get_output()
+            .clone();
+
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some());
+
+        let output = crate::utils::cargo_bin()
+            .args([
+                "compress",
+                "README.md",
+                &format!("{dir}/out_file"),
+                "--yes",
+                "--format",
+                &format!("zst.{archive_format}"),
+            ])
+            .assert()
+            .failure()
+            .get_output()
+            .clone();
+
+        let stderr = output.stderr.to_str().unwrap();
+        assert!(memmem::find(
+            stderr.as_bytes(),
+            format!("'{archive_format}' can only be used at the start of the file extension").as_bytes(),
+        )
+        .is_some());
     }
 }