From 1fa642bb4a27a3a1dbbcd278f722fb8a7ce45467 Mon Sep 17 00:00:00 2001
From: kyu08 <49891479+kyu08@users.noreply.github.com>
Date: Fri, 15 Dec 2023 23:37:06 +0900
Subject: [PATCH] Add error handling when the path passed to include directive
 could not be found #150

---
 src/models/makefile.rs | 13 ++++++++-----
 src/models/target.rs   |  5 ++++-
 src/models/util.rs     | 16 +++++++---------
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/models/makefile.rs b/src/models/makefile.rs
index 9fab3695..7259253d 100644
--- a/src/models/makefile.rs
+++ b/src/models/makefile.rs
@@ -14,7 +14,7 @@ pub struct Makefile {
 impl Makefile {
     pub fn create_makefile() -> Result<Makefile> {
         let Some(makefile_name) = Makefile::specify_makefile_name(".".to_string()) else { return Err(anyhow!("makefile not found.\n")) };
-        Ok(Makefile::new(Path::new(&makefile_name).to_path_buf()))
+        Makefile::new(Path::new(&makefile_name).to_path_buf())
     }
 
     pub fn to_include_files_string(&self) -> Vec<String> {
@@ -39,20 +39,21 @@ impl Makefile {
 
     // I gave up writing tests using temp_dir because it was too difficult (it was necessary to change the implementation to some extent).
     // It is not difficult to ensure that it works with manual tests, so I will not do it for now.
-    fn new(path: PathBuf) -> Makefile {
+    fn new(path: PathBuf) -> Result<Makefile> {
         // If the file path does not exist, the make command cannot be executed in the first place,
         // so it is not handled here.
-        let file_content = util::path_to_content(path.clone());
+        let file_content = util::path_to_content(path.clone())?;
         let include_files = content_to_include_file_paths(file_content.clone())
             .iter()
             .map(|included_file_path| Makefile::new(included_file_path.clone()))
+            .filter_map(Result::ok)
             .collect();
 
-        Makefile {
+        Ok(Makefile {
             path,
             include_files,
             targets: Targets::new(file_content),
-        }
+        })
     }
 
     fn specify_makefile_name(target_path: String) -> Option<String> {
@@ -125,6 +126,7 @@ pub fn content_to_include_file_paths(file_content: String) -> Vec<PathBuf> {
 /// Pattern like `include foo *.mk $(bar)` is not handled for now.
 /// Additional search is not executed if file is not found based on current directory.
 fn line_to_including_file_paths(line: String) -> Option<Vec<PathBuf>> {
+    // TODO: ファイルが見つからなかったら無視する。今はたぶんunwrapしているのでpanicしている
     // not to allow tab character, ` ` is used instead of `\s`
     let regex = Regex::new(r"^ *(include|-include|sinclude).*$").unwrap();
     regex.find(line.as_str()).map(|line| {
@@ -135,6 +137,7 @@ fn line_to_including_file_paths(line: String) -> Option<Vec<PathBuf>> {
 
         let mut directive_and_file_names: Vec<PathBuf> = line_excluding_comment
             .split_whitespace()
+            // TODO: ${}みたいな変数をfilterする
             .map(|e| Path::new(e).to_path_buf())
             .collect();
 
diff --git a/src/models/target.rs b/src/models/target.rs
index 4cd61983..c2893b18 100644
--- a/src/models/target.rs
+++ b/src/models/target.rs
@@ -21,7 +21,10 @@ impl Targets {
 }
 
 pub fn target_line_number(path: PathBuf, target_to_search: String) -> Option<u32> {
-    let content = util::path_to_content(path);
+    let content = match util::path_to_content(path) {
+        Ok(c) => c,
+        Err(_) => return None,
+    };
     for (index, line) in content.lines().enumerate() {
         if let Some(t) = line_to_target(line.to_string()) {
             if t == target_to_search {
diff --git a/src/models/util.rs b/src/models/util.rs
index 0079fb78..66459531 100644
--- a/src/models/util.rs
+++ b/src/models/util.rs
@@ -1,12 +1,10 @@
-use std::{fs::File, io::Read, path::PathBuf};
+use anyhow::{anyhow, Result};
 
-pub fn path_to_content(path: PathBuf) -> String {
-    let mut content = String::new();
+use std::{fs::read_to_string, path::PathBuf};
 
-    // Not handle cases where files are not found because make command cannot be
-    // executed in the first place if Makefile or included files are not found.
-    let mut f = File::open(path).unwrap();
-    f.read_to_string(&mut content).unwrap();
-
-    content
+pub fn path_to_content(path: PathBuf) -> Result<String> {
+    match read_to_string(path.as_path()) {
+        Ok(c) => Ok(c),
+        Err(e) => Err(anyhow!(e)),
+    }
 }