Safely check values instead of unwrapping

oblador · Oct 19, 2024 · 827d2b3 · 827d2b3
1 parent f9fed4e
commit 827d2b3
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 44 deletions.
diff --git a/src/affected.rs b/src/affected.rs
@@ -5,7 +5,7 @@ use std::{
 };
 
 use oxc_resolver::{ResolveError, Resolver};
-use oxc_span::{SourceType, VALID_EXTENSIONS};
+use oxc_span::SourceType;
 
 use crate::imports;
 
@@ -70,48 +70,45 @@ pub fn collect_affected(
             continue;
         }
 
-        let source_type = SourceType::from_path(absolute_path.clone()).unwrap();
-        let source_text: String = fs::read_to_string(&absolute_path).unwrap();
+        let Ok(source_type) = SourceType::from_path(absolute_path.clone()) else {
+            continue;
+        };
+        let Ok(source_text) = fs::read_to_string(&absolute_path) else {
+            errors.push(format!("Cannot read file: {}", absolute_path.display()));
+            continue;
+        };
         let result = imports::collect_imports(source_type, source_text.as_str());
         errors.extend(result.errors);
-        let parent_path = absolute_path.parent().unwrap();
-        for import_path in result.imports_paths.iter() {
-            match resolver.resolve(parent_path, import_path.as_str()) {
-                Err(e) => match e {
-                    ResolveError::Builtin(_) => {} // Skip builtins
-                    _ => {
-                        errors.push(e.to_string());
-                    }
-                },
-                Ok(resolution) => {
-                    let import = current_dir.join(resolution.path());
-                    if affected.contains(&import) {
-                        extend_affected(&mut affected, &absolute_path, &dependents_map);
-                    } else if dependents_map.contains_key(&import) {
-                        let dependents: &mut HashSet<PathBuf> =
-                            dependents_map.get_mut(&import).unwrap();
-                        dependents.insert(absolute_path.clone());
-                    } else {
-                        dependents_map.insert(
-                            import.clone(),
-                            HashSet::from_iter(vec![absolute_path.clone()]),
-                        );
-
-                        // Skip node_modules
-                        if import.components().any(|c| {
-                            module_paths.contains(c.to_owned().as_os_str().to_str().unwrap())
-                        }) {
-                            continue;
+        if let Some(parent_path) = absolute_path.parent() {
+            for import_path in result.imports_paths.iter() {
+                match resolver.resolve(parent_path, import_path.as_str()) {
+                    Err(e) => match e {
+                        ResolveError::Builtin(_) => {} // Skip builtins
+                        _ => {
+                            errors.push(e.to_string());
                         }
+                    },
+                    Ok(resolution) => {
+                        let import = current_dir.join(resolution.path());
+                        if affected.contains(&import) {
+                            extend_affected(&mut affected, &absolute_path, &dependents_map);
+                        } else if dependents_map.contains_key(&import) {
+                            if let Some(dependents) = dependents_map.get_mut(&import) {
+                                dependents.insert(absolute_path.clone());
+                            }
+                        } else {
+                            dependents_map.insert(
+                                import.clone(),
+                                HashSet::from_iter(vec![absolute_path.clone()]),
+                            );
 
-                        // Skip non-source files
-                        match import.extension() {
-                            None => {}
-                            Some(ext) => {
-                                if VALID_EXTENSIONS.contains(&ext.to_str().unwrap()) {
-                                    unvisited.push(import);
-                                }
+                            // Skip node_modules
+                            if import.components().any(|c| {
+                                module_paths.contains(c.to_owned().as_os_str().to_str().unwrap())
+                            }) {
+                                continue;
                             }
+                            unvisited.push(import);
                         }
                     }
                 }

diff --git a/src/imports.rs b/src/imports.rs
@@ -49,9 +49,10 @@ impl<'a> Visit<'a> for CollectImports {
                 self.import_paths.insert(literal.value.to_string());
             }
             oxc_ast::ast::Expression::TemplateLiteral(literal) => {
-                if literal.expressions.len() == 0 && literal.quasis.len() == 1 {
-                    self.import_paths
-                        .insert(literal.quasis.first().unwrap().value.raw.to_string());
+                if literal.expressions.len() == 0 {
+                    if let Some(first) = literal.quasis.first() {
+                        self.import_paths.insert(first.value.raw.to_string());
+                    }
                 } else {
                     self.errors
                         .push("Import call must not have dynamic template literals".to_string());
@@ -66,9 +67,8 @@ impl<'a> Visit<'a> for CollectImports {
     }
 
     fn visit_export_named_declaration(&mut self, it: &oxc_ast::ast::ExportNamedDeclaration<'a>) {
-        if it.source.is_some() {
-            self.import_paths
-                .insert(it.source.as_ref().unwrap().value.to_string());
+        if let Some(source) = &it.source {
+            self.import_paths.insert(source.value.to_string());
         }
         walk::walk_export_named_declaration(self, it);
     }