Don't error if an executable has no version info

Instead, treat it as having no version (so true if comparator is !=, <
or <=).

.travis.yml

@@ -33,6 +33,10 @@ before_script:
   - 7z x loot_api-0.13.8-0-g47797cc_dev-win64.7z
   - mv loot_api-0.13.8-0-g47797cc_dev-win64 loot_api_win64
 
+  - wget https://github.com/loot/loot-api-python/releases/download/4.0.2/loot_api_python-4.0.2-0-gd356ac2_master-python2.7-win32.7z
+  - 7z x loot_api_python-4.0.2-0-gd356ac2_master-python2.7-win32.7z
+  - mv loot_api_python-4.0.2-0-gd356ac2_master-python2.7-win32 loot_api_python
+
   - wget https://7-zip.org/a/7z1805-extra.7z
   - 7z x 7z1805-extra.7z -o7z
 

appveyor.yml

@@ -30,6 +30,10 @@ install:
   - 7z x loot_api_win64.7z
   - mv loot_api-0.13.8-0-g47797cc_dev-win64 loot_api_win64
 
+  - ps: (New-Object Net.WebClient).DownloadFile('https://github.com/loot/loot-api-python/releases/download/4.0.2/loot_api_python-4.0.2-0-gd356ac2_master-python2.7-win32.7z', "$PWD/loot_api_python.7z")
+  - 7z x loot_api_python.7z
+  - mv loot_api_python-4.0.2-0-gd356ac2_master-python2.7-win32 loot_api_python
+
   - ps: (New-Object Net.WebClient).DownloadFile('https://7-zip.org/a/7z1805-extra.7z', "$PWD/7z.7z")
   - ps: 7z x "$PWD/7z.7z" -o7z
 

src/function/eval.rs

@@ -172,13 +172,13 @@ fn get_version(state: &State, file_path: &Path) -> Result<Option<Version>, Error
     if state.game_type.is_plugin_filename(file_path) {
         Ok(None)
     } else {
-        Version::read_file_version(file_path).map(Some)
+        Version::read_file_version(file_path)
     }
 }
 
 fn get_product_version(file_path: &Path) -> Result<Option<Version>, Error> {
     if file_path.exists() {
-        Version::read_product_version(file_path).map(Some)
+        Version::read_product_version(file_path)
     } else {
         Ok(None)
     }

src/version.rs

@@ -30,10 +30,9 @@ pub struct Version {
 }
 
 impl Version {
-    pub fn read_file_version(file_path: &Path) -> Result<Self, Error> {
+    pub fn read_file_version(file_path: &Path) -> Result<Option<Self>, Error> {
         Self::read_version(file_path, |v| {
-            v.fixed()
+            v.fixed().map(|f| {
-                .map(|f| {
                 format!(
                     "{}.{}.{}.{}",
                     f.dwFileVersion.Major,
@@ -42,28 +41,28 @@ impl Version {
                     f.dwFileVersion.Build
                 )
             })
-                .unwrap_or_else(String::new)
         })
     }
 
-    pub fn read_product_version(file_path: &Path) -> Result<Self, Error> {
+    pub fn read_product_version(file_path: &Path) -> Result<Option<Self>, Error> {
         Self::read_version(file_path, |v| {
             v.query_value(&"ProductVersion")
                 .map(String::from_utf16_lossy)
-                .unwrap_or_else(String::new)
         })
     }
 
-    fn read_version<F: Fn(&VersionInfo) -> String>(
+    fn read_version<F: Fn(&VersionInfo) -> Option<String>>(
         file_path: &Path,
         formatter: F,
-    ) -> Result<Self, Error> {
+    ) -> Result<Option<Self>, Error> {
         let file_map =
             FileMap::open(file_path).map_err(|e| Error::IoError(file_path.to_path_buf(), e))?;
-        let version_info = get_pe_version_info(file_map.as_ref())
-            .map_err(|e| Error::PeParsingError(file_path.to_path_buf(), e.into()))?;
 
-        Ok(Version::from(formatter(&version_info).as_str()))
+        match get_pe_version_info(file_map.as_ref()) {
+            Ok(v) => Ok(formatter(&v).map(Version::from)),
+            Err(FindError::NotFound) => Ok(None),
+            Err(e) => Err(Error::PeParsingError(file_path.to_path_buf(), e.into())),
+        }
     }
 }
 
@@ -126,9 +125,9 @@ fn split_version_string(string: &str) -> (&str, &str) {
     }
 }
 
-impl<'a> From<&'a str> for Version {
+impl<T: AsRef<str>> From<T> for Version {
-    fn from(string: &'a str) -> Self {
+    fn from(string: T) -> Self {
-        let (release, pre_release) = split_version_string(trim_metadata(string));
+        let (release, pre_release) = split_version_string(trim_metadata(string.as_ref()));
 
         Version {
             release_ids: release
@@ -197,7 +196,9 @@ mod tests {
         #[test]
         fn version_read_file_version_should_read_the_file_version_field_of_a_32_bit_executable() {
             let version =
-                Version::read_file_version(Path::new("tests/loot_api_win32/loot_api.dll")).unwrap();
+                Version::read_file_version(Path::new("tests/loot_api_win32/loot_api.dll"))
+                    .unwrap()
+                    .unwrap();
 
             assert_eq!(
                 version.release_ids,
@@ -214,7 +215,9 @@ mod tests {
         #[test]
         fn version_read_file_version_should_read_the_file_version_field_of_a_64_bit_executable() {
             let version =
-                Version::read_file_version(Path::new("tests/loot_api_win64/loot_api.dll")).unwrap();
+                Version::read_file_version(Path::new("tests/loot_api_win64/loot_api.dll"))
+                    .unwrap()
+                    .unwrap();
 
             assert_eq!(
                 version.release_ids,
@@ -244,10 +247,21 @@ mod tests {
             assert_eq!("An error was encountered while reading the version fields of \"Cargo.toml\": bad magic", error.to_string());
         }
 
+        #[test]
+        fn version_read_file_version_should_return_none_if_there_is_no_version_info() {
+            let version =
+                Version::read_file_version(Path::new("tests/loot_api_python/loot_api.pyd"))
+                    .unwrap();
+
+            assert!(version.is_none());
+        }
+
         #[test]
         fn version_read_product_version_should_read_the_file_version_field_of_a_32_bit_executable()
         {
-            let version = Version::read_product_version(Path::new("tests/7z/7za.exe")).unwrap();
+            let version = Version::read_product_version(Path::new("tests/7z/7za.exe"))
+                .unwrap()
+                .unwrap();
 
             assert_eq!(
                 version.release_ids,
@@ -259,7 +273,9 @@ mod tests {
         #[test]
         fn version_read_product_version_should_read_the_file_version_field_of_a_64_bit_executable()
         {
-            let version = Version::read_product_version(Path::new("tests/7z/x64/7za.exe")).unwrap();
+            let version = Version::read_product_version(Path::new("tests/7z/x64/7za.exe"))
+                .unwrap()
+                .unwrap();
 
             assert_eq!(
                 version.release_ids,
@@ -278,7 +294,7 @@ mod tests {
             dll_bytes[0x23B10] = 0x19;
             std::fs::write(&dll_path, dll_bytes).unwrap();
 
-            let version = Version::read_product_version(&dll_path).unwrap();
+            let version = Version::read_product_version(&dll_path).unwrap().unwrap();
 
             assert_eq!(
                 version.release_ids,
@@ -302,6 +318,15 @@ mod tests {
 
             assert_eq!("An error was encountered while reading the version fields of \"Cargo.toml\": bad magic", error.to_string());
         }
+
+        #[test]
+        fn version_read_product_version_should_return_none_if_there_is_no_version_info() {
+            let version =
+                Version::read_product_version(Path::new("tests/loot_api_python/loot_api.pyd"))
+                    .unwrap();
+
+            assert!(version.is_none());
+        }
     }
 
     mod empty {