Add define block skip (#241)

* Add define block skip

* Support nested block definition

* Include `override define` as block definition

* Add some unittests

* Move get_line_type below `impl Targets` block

* Use first method instead of index based access

* Ensure proper formatting

* Add unittests for get_line_type

---------

Co-authored-by: kyu08 <[email protected]>

flake.nix

@@ -16,7 +16,7 @@
       {
         devShells.default = pkgs.mkShell {
           inputsFrom = [ packages.fzf-make ];
-          packages = with pkgs; [ clippy typos ];
+          packages = with pkgs; [ rustfmt clippy typos ];
         };
 
         formatter = pkgs.nixpkgs-fmt;

src/model/target.rs

@@ -10,21 +10,68 @@ pub struct Targets(pub Vec<String>);
 impl Targets {
     pub fn new(content: String) -> Targets {
         let mut result: Vec<String> = Vec::new();
+        let mut define_block_depth = 0;
+
         for line in content.lines() {
-            if let Some(t) = line_to_target(line.to_string()) {
-                result.push(t);
+            match get_line_type(line) {
+                LineType::DefineStart => {
+                    define_block_depth += 1;
+                }
+                LineType::DefineEnd => {
+                    define_block_depth -= 1;
+                }
+                LineType::Normal => {
+                    if define_block_depth == 0 {
+                        if let Some(t) = line_to_target(line.to_string()) {
+                            result.push(t);
+                        }
+                    }
+                }
             }
         }
 
         Targets(result)
     }
 }
 
+const DEFINE_BLOCK_START: &str = "define";
+const DEFINE_BLOCK_END: &str = "endef";
+const OVERRIDE: &str = "override";
+
+#[derive(Debug, PartialEq)]
+enum LineType {
+    Normal,
+    DefineStart,
+    DefineEnd,
+}
+
+fn get_line_type(line: &str) -> LineType {
+    let words: Vec<&str> = line.split_whitespace().collect();
+
+    if words.is_empty() {
+        return LineType::Normal;
+    }
+
+    if words.len() >= 2 && words[0] == OVERRIDE && words[1] == DEFINE_BLOCK_START {
+        return LineType::DefineStart;
+    }
+
+    match words.first() {
+        Some(&w) => match w {
+            DEFINE_BLOCK_START => LineType::DefineStart,
+            DEFINE_BLOCK_END => LineType::DefineEnd,
+            _ => LineType::Normal,
+        },
+        None => LineType::Normal,
+    }
+}
+
 pub fn target_line_number(path: PathBuf, target_to_search: String) -> Option<u32> {
     let content = match file_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 {
@@ -107,6 +154,45 @@ build:
                 contents: "echo hello",
                 expect: Targets(vec![]),
             },
+            Case {
+                title: "trap script as a define block",
+                contents: "\
+.PHONY: all
+
+all: my_script
+define script-block
+#!/bin/bash
+
+echo \"this is a trap: not good\"
+endef
+my_script:
+	$(file >my_script,$(script-block))\n",
+                expect: Targets(vec!["all".to_string(), "my_script".to_string()]),
+            },
+            Case {
+                title: "nested define",
+                contents: "\
+define lvl-1
+a:
+    define lvl2
+a:
+
+    endef
+a:
+endef
+                ",
+                expect: Targets(vec![]),
+            },
+            Case {
+                title: "override define",
+                contents: "\
+override define foo
+not-good:
+
+endef
+                ",
+                expect: Targets(vec![]),
+            },
         ];
 
         for case in cases {
@@ -119,6 +205,57 @@ build:
         }
     }
 
+    #[test]
+    fn get_line_type_test() {
+        struct Case {
+            title: &'static str,
+            line: &'static str,
+            expect: LineType,
+        }
+
+        let cases = vec![
+            Case {
+                title: "empty line",
+                line: "",
+                expect: LineType::Normal,
+            },
+            Case {
+                title: "override define",
+                line: "override define",
+                expect: LineType::DefineStart,
+            },
+            Case {
+                title: "define",
+                line: "define",
+                expect: LineType::DefineStart,
+            },
+            Case {
+                title: "endef",
+                line: "endef",
+                expect: LineType::DefineEnd,
+            },
+            Case {
+                title: "define whitespace",
+                line: "  define   foo",
+                expect: LineType::DefineStart,
+            },
+            Case {
+                title: "endef whitespace",
+                line: "  endef  ",
+                expect: LineType::DefineEnd,
+            },
+        ];
+
+        for case in cases {
+            assert_eq!(
+                case.expect,
+                get_line_type(case.line),
+                "\nFailed: 🚨{:?}🚨\n",
+                case.title,
+            );
+        }
+    }
+
     #[test]
     fn line_to_target_test() {
         struct Case {