Better errors (#152)

* Close #124 * Add `TiledError::InvalidEncodingFormat` * Rename `DecompressingError` to `IoError` * Add `InvalidPropertyValue` & `UnknownPropertyType` * Remove `Other` * Remove `ParseTileError` * Misc comment improvements * Revert `IoError` back to `DecompressingError` - Use `CouldNotOpenFile` for `Map` * Address PR comments
mapeditor · Feb 10, 2022 · dfe8458 · dfe8458
1 parent bb77240
commit dfe8458
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 43 deletions.
diff --git a/src/error.rs b/src/error.rs
@@ -1,13 +1,8 @@
 use std::{fmt, path::PathBuf};
 
-#[derive(Debug, Copy, Clone)]
-pub enum ParseTileError {
-    ColorError,
-    OrientationError,
-}
-
 /// Errors which occured when parsing the file
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum TiledError {
     /// A attribute was missing, had the wrong type of wasn't formated
     /// correctly.
@@ -31,7 +26,19 @@ pub enum TiledError {
     },
     /// There was an invalid tile in the map parsed.
     InvalidTileFound,
-    Other(String),
+    /// Unknown encoding or compression format or invalid combination of both (for tile layers)
+    InvalidEncodingFormat {
+        encoding: Option<String>,
+        compression: Option<String>,
+    },
+    /// There was an error parsing the value of a [`PropertyValue`].
+    /// 
+    /// [`PropertyValue`]: crate::PropertyValue
+    InvalidPropertyValue{description: String},
+    /// Found an unknown property value type while parsing a [`PropertyValue`].
+    /// 
+    /// [`PropertyValue`]: crate::PropertyValue
+    UnknownPropertyType{name: String},
 }
 
 impl fmt::Display for TiledError {
@@ -62,12 +69,26 @@ impl fmt::Display for TiledError {
                 )
             }
             TiledError::InvalidTileFound => write!(fmt, "Invalid tile found in map being parsed"),
-            TiledError::Other(s) => write!(fmt, "{}", s),
+            TiledError::InvalidEncodingFormat { encoding: None, compression: None } => 
+                write!(
+                    fmt,
+                    "Deprecated combination of encoding and compression"
+                ),
+            TiledError::InvalidEncodingFormat { encoding, compression } => 
+                write!(
+                    fmt,
+                    "Unknown encoding or compression format or invalid combination of both (for tile layers): {} encoding with {} compression",
+                    encoding.as_deref().unwrap_or("no"),
+                    compression.as_deref().unwrap_or("no")
+                ),
+            TiledError::InvalidPropertyValue{description} =>
+                write!(fmt, "Invalid property value: {}", description),
+            TiledError::UnknownPropertyType { name } =>
+                write!(fmt, "Unknown property value type '{}'", name),
         }
     }
 }
 
-// This is a skeleton implementation, which should probably be extended in the future.
 impl std::error::Error for TiledError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {

diff --git a/src/layers/tile/util.rs b/src/layers/tile/util.rs
@@ -10,18 +10,18 @@ pub(crate) fn parse_data_line(
     parser: &mut impl Iterator<Item = XmlEventResult>,
     tilesets: &[MapTilesetGid],
 ) -> Result<Vec<Option<LayerTileData>>, TiledError> {
-    match (encoding, compression) {
-        (None, None) => {
-            return Err(TiledError::Other(
-                "XML format is currently not supported".to_string(),
-            ))
+    match (encoding.as_deref(), compression.as_deref()) {
+        (Some("base64"), None) => {
+            return parse_base64(parser).map(|v| convert_to_tiles(&v, tilesets))
         }
-        (Some(e), None) => match e.as_ref() {
-            "base64" => return parse_base64(parser).map(|v| convert_to_tiles(&v, tilesets)),
-            "csv" => return decode_csv(parser, tilesets),
-            e => return Err(TiledError::Other(format!("Unknown encoding format {}", e))),
-        },
-        (Some(e), Some(c)) => match (e.as_ref(), c.as_ref()) {
+        (Some("csv"), None) => return decode_csv(parser, tilesets),
+        (Some(_), None) => {
+            return Err(TiledError::InvalidEncodingFormat {
+                encoding,
+                compression,
+            })
+        }
+        (Some(e), Some(c)) => match (e, c) {
             ("base64", "zlib") => {
                 return parse_base64(parser)
                     .and_then(decode_zlib)
@@ -38,14 +38,19 @@ pub(crate) fn parse_data_line(
                     .and_then(decode_zstd)
                     .map(|v| convert_to_tiles(&v, tilesets))
             }
-            (e, c) => {
-                return Err(TiledError::Other(format!(
-                    "Unknown combination of {} encoding and {} compression",
-                    e, c
-                )))
+            _ => {
+                return Err(TiledError::InvalidEncodingFormat {
+                    encoding,
+                    compression,
+                })
             }
         },
-        _ => return Err(TiledError::Other("Missing encoding format".to_string())),
+        _ => {
+            return Err(TiledError::InvalidEncodingFormat {
+                encoding,
+                compression,
+            })
+        }
     };
 }
 

diff --git a/src/map.rs b/src/map.rs
@@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt, fs::File, io::Read, path::Path, rc::Rc, str
 use xml::{attribute::OwnedAttribute, reader::XmlEvent, EventReader};
 
 use crate::{
-    error::{ParseTileError, TiledError},
+    error::TiledError,
     layers::{LayerData, LayerTag},
     properties::{parse_properties, Color, Properties},
     tileset::Tileset,
@@ -90,8 +90,10 @@ impl Map {
         path: impl AsRef<Path>,
         cache: &mut impl ResourceCache,
     ) -> Result<Self, TiledError> {
-        let reader = File::open(path.as_ref())
-            .map_err(|_| TiledError::Other(format!("Map file not found: {:?}", path.as_ref())))?;
+        let reader = File::open(path.as_ref()).map_err(|err| TiledError::CouldNotOpenFile {
+            path: path.as_ref().to_owned(),
+            err,
+        })?;
         Self::parse_reader(reader, path.as_ref(), cache)
     }
 }
@@ -268,15 +270,15 @@ pub enum Orientation {
 }
 
 impl FromStr for Orientation {
-    type Err = ParseTileError;
+    type Err = ();
 
-    fn from_str(s: &str) -> Result<Orientation, ParseTileError> {
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "orthogonal" => Ok(Orientation::Orthogonal),
             "isometric" => Ok(Orientation::Isometric),
             "staggered" => Ok(Orientation::Staggered),
             "hexagonal" => Ok(Orientation::Hexagonal),
-            _ => Err(ParseTileError::OrientationError),
+            _ => Err(()),
         }
     }
 }

diff --git a/src/properties.rs b/src/properties.rs
@@ -74,32 +74,39 @@ impl PropertyValue {
         match property_type.as_str() {
             "bool" => match value.parse() {
                 Ok(val) => Ok(PropertyValue::BoolValue(val)),
-                Err(err) => Err(TiledError::Other(err.to_string())),
+                Err(err) => Err(TiledError::InvalidPropertyValue {
+                    description: err.to_string(),
+                }),
             },
             "float" => match value.parse() {
                 Ok(val) => Ok(PropertyValue::FloatValue(val)),
-                Err(err) => Err(TiledError::Other(err.to_string())),
+                Err(err) => Err(TiledError::InvalidPropertyValue {
+                    description: err.to_string(),
+                }),
             },
             "int" => match value.parse() {
                 Ok(val) => Ok(PropertyValue::IntValue(val)),
-                Err(err) => Err(TiledError::Other(err.to_string())),
+                Err(err) => Err(TiledError::InvalidPropertyValue {
+                    description: err.to_string(),
+                }),
             },
             "color" if value.len() > 1 => match u32::from_str_radix(&value[1..], 16) {
                 Ok(color) => Ok(PropertyValue::ColorValue(color)),
-                Err(_) => Err(TiledError::Other(format!(
-                    "Improperly formatted color property"
-                ))),
+                Err(err) => Err(TiledError::InvalidPropertyValue {
+                    description: err.to_string(),
+                }),
             },
             "string" => Ok(PropertyValue::StringValue(value)),
             "object" => match value.parse() {
                 Ok(val) => Ok(PropertyValue::ObjectValue(val)),
-                Err(err) => Err(TiledError::Other(err.to_string())),
+                Err(err) => Err(TiledError::InvalidPropertyValue {
+                    description: err.to_string(),
+                }),
             },
             "file" => Ok(PropertyValue::FileValue(value)),
-            _ => Err(TiledError::Other(format!(
-                "Unknown property type \"{}\"",
-                property_type
-            ))),
+            _ => Err(TiledError::UnknownPropertyType {
+                name: property_type,
+            }),
         }
     }
 }