Fix handling of files with a version in the filename and no extension…

…, like `shfmt_v3.8.0_linux_arm64`

Changes.md

@@ -1,6 +1,8 @@
 ## 0.0.32 - 2024-06-01
 
 - Added support for plain `.tar` files with no compression.
+- Fix handling of files with a version in the filename and no extension, like
+  `shfmt_v3.8.0_linux_arm64`. This was fixed before but I broke it in the 0.0.31 release.
 
 ## 0.0.31 - 2024-06-01
 

src/extension.rs

@@ -1,6 +1,8 @@
 use anyhow::Result;
 use itertools::Itertools;
-use std::path::Path;
+use lazy_regex::regex;
+use log::debug;
+use std::{ffi::OsStr, path::Path};
 use strum::{EnumIter, IntoEnumIterator};
 use thiserror::Error;
 
@@ -56,21 +58,49 @@ impl Extension {
 
         // We need to try the longest extension first so that ".tar.gz"
         // matches before ".gz" and so on for other compression formats.
-        match Extension::iter()
+        if let Some(ext) = Extension::iter()
             .sorted_by(|a, b| Ord::cmp(&a.extension().len(), &b.extension().len()))
             .rev()
             .find(|e| path.ends_with(e.extension()))
         {
-            Some(ext) => Ok(Some(ext)),
-            None => Err(ExtensionError::UnknownExtension {
-                path: path.to_string(),
-                ext: ext_str.to_string_lossy().to_string(),
-            }
-            .into()),
+            return Ok(Some(ext));
         }
+
+        if extension_is_part_of_version(path, ext_str) {
+            debug!("the extension {ext_str:?} is part of the version, ignoring");
+            return Ok(None);
+        }
+
+        Err(ExtensionError::UnknownExtension {
+            path: path.to_string(),
+            ext: ext_str.to_string_lossy().to_string(),
+        }
+        .into())
     }
 }
 
+fn extension_is_part_of_version(path: &str, ext_str: &OsStr) -> bool {
+    let ext_str = ext_str.to_string_lossy().to_string();
+
+    let version_number_ext_re = regex!(r"^\.\d+");
+    if version_number_ext_re.is_match(&ext_str) {
+        return false;
+    }
+
+    // This matches something like "foo_3.2.1_linux_amd64" and captures ".1_".
+    let version_number_re = regex!(r"\d+\.(\d+[^.]+)");
+    let Some(caps) = version_number_re.captures(path) else {
+        return false;
+    };
+    let Some(dot_num) = caps.get(1) else {
+        return false;
+    };
+
+    // If the extension starts with the last part of the version then it's not
+    // a real extension.
+    ext_str == dot_num.as_str()
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -88,6 +118,7 @@ mod test {
     #[test_case("foo.xz", Ok(Some(Extension::Xz)))]
     #[test_case("foo.zip", Ok(Some(Extension::Zip)))]
     #[test_case("foo", Ok(None))]
+    #[test_case("foo_3.2.1_linux_amd64", Ok(None))]
     #[test_case("foo.bar", Err(ExtensionError::UnknownExtension { path: "foo.bar".to_string(), ext: "bar".to_string() }.into()))]
     fn from_path(path: &str, expect: Result<Option<Extension>>) {
         let ext = Extension::from_path(path);

src/picker.rs

@@ -13,7 +13,6 @@ use lazy_regex::{regex, Lazy};
 use log::debug;
 use platforms::{Arch, Endian, Platform, OS};
 use regex::Regex;
-use std::path::PathBuf;
 
 #[derive(Debug)]
 pub(crate) struct AssetPicker<'a> {
@@ -26,7 +25,19 @@ impl<'a> AssetPicker<'a> {
         Self { matching, platform }
     }
 
-    pub(crate) fn pick_asset(&self, mut assets: Vec<Asset>) -> Result<Asset> {
+    pub(crate) fn pick_asset(&self, assets: Vec<Asset>) -> Result<Asset> {
+        debug!("filtering out assets that do not have a valid extension");
+        let mut assets: Vec<_> = assets
+            .into_iter()
+            .filter(|a| {
+                if Extension::from_path(&a.name).is_err() {
+                    debug!("skipping asset with invalid extension: {}", a.name);
+                    return false;
+                }
+                true
+            })
+            .collect();
+
         if assets.len() == 1 {
             debug!("there is only one asset to pick");
             return Ok(assets.remove(0));
@@ -59,49 +70,13 @@ impl<'a> AssetPicker<'a> {
         let os_matcher = self.os_matcher();
         debug!("matching assets against OS using {}", os_matcher.as_str());
 
-        let version_re = regex!(r"(?:\d+\.)+(\d+.+?)\z");
-
         let mut matches: Vec<Asset> = vec![];
 
         // This could all be done much more simply with the iterator's .find()
         // method, but then there's no place to put all the debugging output.
         for asset in assets {
             debug!("matching OS against asset name = {}", asset.name);
 
-            if asset.name.contains('.') && Extension::from_path(&asset.name).is_err() {
-                // If the name is something like "mkcert-v1.4.4-darwin-amd46"
-                // then the naive approach will think that this has an
-                // extension of ".4-darwin-amd46" and reject it as an invalid
-                // extension.
-                //
-                // So we need to match the name against a regex for versions
-                // and check if the part after the version is the same as the
-                // file's "extension". If it is, it's not a real extension.
-                let mut contains_version = false;
-                if let Some(caps) = version_re.captures(&asset.name) {
-                    if let Some(ext) = caps.get(1).map(|m| m.as_str()) {
-                        let path = PathBuf::from(&asset.name);
-                        if ext
-                            == path
-                                .extension()
-                                .map(|o| o.to_str().unwrap_or_default())
-                                .unwrap_or_default()
-                        {
-                            contains_version = true;
-                            debug!(
-                                "it looks like this file name contains a version: {}",
-                                asset.name
-                            );
-                        }
-                    }
-                }
-
-                if !contains_version {
-                    debug!("it appears to have an invalid extension");
-                    continue;
-                }
-            }
-
             if os_matcher.is_match(&asset.name) {
                 debug!("matches our OS");
                 matches.push(asset);