wip(core): add tests and update parse_line

Author: omarabid

core/src/commands.rs

@@ -19,7 +19,17 @@ pub fn codeowners_parse(
 
     let codeowners_files = crate::common::find_codeowners_files(path)?;
 
-    dbg!(codeowners_files);
+    dbg!(&codeowners_files);
+
+    let parsed_codeowners = codeowners_files
+        .iter()
+        .filter_map(|file| {
+            let parsed = crate::common::parse_codeowners(file).ok()?;
+            Some((file, parsed))
+        })
+        .collect::<Vec<_>>();
+
+    dbg!(&parsed_codeowners);
 
     println!("CODEOWNERS parsing completed successfully");
     Ok(())

core/src/common.rs

@@ -1,7 +1,7 @@
 use std::path::{Path, PathBuf};
 use utils::error::{Error, Result};
 
-use crate::types::{CodeownersEntry, Owner, OwnerType};
+use crate::types::{CodeownersEntry, Owner, OwnerType, Tag};
 
 /// Find CODEOWNERS files recursively in the given directory and its subdirectories
 pub fn find_codeowners_files<P: AsRef<Path>>(base_path: P) -> Result<Vec<PathBuf>> {
@@ -27,46 +27,59 @@ pub fn find_codeowners_files<P: AsRef<Path>>(base_path: P) -> Result<Vec<PathBuf
 }
 
 /// Parse CODEOWNERS
-pub fn parse_codeowners(content: &str, source_path: &Path) -> Result<Vec<CodeownersEntry>> {
+pub fn parse_codeowners(source_path: &Path) -> Result<Vec<CodeownersEntry>> {
+    let content = std::fs::read_to_string(source_path)?;
+
     content
         .lines()
         .enumerate()
-        .filter_map(|(line_num, line)| parse_line(line, line_num + 1, source_path).transpose())
+        .filter_map(|(line_num, line)| parse_line(line, line_num, source_path).transpose())
         .collect()
 }
 
 /// Parse a line of CODEOWNERS
 fn parse_line(line: &str, line_num: usize, source_path: &Path) -> Result<Option<CodeownersEntry>> {
-    let clean_line = line.split('#').next().unwrap_or("").trim();
-    if clean_line.is_empty() {
+    // Trim the line and check for empty or comment lines
+    let trimmed = line.trim();
+    if trimmed.is_empty() || trimmed.starts_with('#') {
+        return Ok(None);
+    }
+
+    // Split the line by whitespace into a series of tokens
+    let tokens: Vec<&str> = trimmed.split_whitespace().collect();
+    if tokens.is_empty() {
         return Ok(None);
     }
 
-    let mut parts = clean_line.split_whitespace();
-    let pattern = parts
-        .next()
-        .ok_or_else(|| Error::new("Missing pattern"))?
-        .to_string();
-
-    let (owners, tags) = parts.fold((Vec::new(), Vec::new()), |(mut owners, mut tags), part| {
-        if part.starts_with('@') {
-            let owner = parse_owner(part).unwrap();
-            owners.push(owner);
-        } else if part.starts_with('[') && part.ends_with(']') {
-            tags.extend(
-                part[1..part.len() - 1]
-                    .split(',')
-                    .map(|t| t.trim().to_string()),
-            );
+    // The first token is the pattern
+    let pattern = tokens[0].to_string();
+
+    let mut owners: Vec<Owner> = Vec::new();
+    let mut tags: Vec<Tag> = Vec::new();
+
+    let mut i = 1; // Start after the pattern
+
+    // Collect owners until a token starts with '#'
+    while i < tokens.len() && !tokens[i].starts_with('#') {
+        owners.push(parse_owner(tokens[i])?);
+        i += 1;
+    }
+
+    // Collect tags
+    while i < tokens.len() {
+        let token = tokens[i];
+        if token.starts_with('#') {
+            if token == "#" {
+                // Comment starts, break
+                break;
+            } else {
+                tags.push(Tag(token[1..].to_string()));
+            }
         } else {
-            let owner = parse_owner(part).unwrap();
-            owners.push(owner);
+            // Non-tag, part of comment
+            break;
         }
-        (owners, tags)
-    });
-
-    if owners.is_empty() {
-        return Ok(None);
+        i += 1;
     }
 
     Ok(Some(CodeownersEntry {
@@ -80,17 +93,20 @@ fn parse_line(line: &str, line_num: usize, source_path: &Path) -> Result<Option<
 
 /// Parse an owner string into an Owner struct
 fn parse_owner(owner_str: &str) -> Result<Owner> {
-    let (identifier, owner_type) = if owner_str.contains('@') {
-        (owner_str.to_string(), OwnerType::Email)
+    let identifier = owner_str.to_string();
+    let owner_type = if identifier.eq_ignore_ascii_case("NOOWNER") {
+        OwnerType::Unowned
     } else if owner_str.starts_with('@') {
         let parts: Vec<&str> = owner_str[1..].split('/').collect();
         if parts.len() == 2 {
-            (owner_str.to_string(), OwnerType::Team)
+            OwnerType::Team
         } else {
-            (owner_str.to_string(), OwnerType::User)
+            OwnerType::User
         }
+    } else if owner_str.contains('@') {
+        OwnerType::Email
     } else {
-        (owner_str.to_string(), OwnerType::Unknown)
+        OwnerType::Unknown
     };
 
     Ok(Owner {
@@ -158,4 +174,151 @@ mod tests {
         assert!(found_files.is_empty());
         Ok(())
     }
+
+    #[test]
+    fn test_parse_owner_user() -> Result<()> {
+        let owner = parse_owner("@username")?;
+        assert_eq!(owner.identifier, "@username");
+        assert!(matches!(owner.owner_type, OwnerType::User));
+
+        // With hyphens and underscores
+        let owner = parse_owner("@user-name_123")?;
+        assert_eq!(owner.identifier, "@user-name_123");
+        assert!(matches!(owner.owner_type, OwnerType::User));
+
+        // Single character username
+        let owner = parse_owner("@a")?;
+        assert_eq!(owner.identifier, "@a");
+        assert!(matches!(owner.owner_type, OwnerType::User));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_parse_owner_team() -> Result<()> {
+        // Standard team
+        let owner = parse_owner("@org/team-name")?;
+        assert_eq!(owner.identifier, "@org/team-name");
+        assert!(matches!(owner.owner_type, OwnerType::Team));
+
+        // With numbers and special characters
+        let owner = parse_owner("@company123/frontend-team_01")?;
+        assert_eq!(owner.identifier, "@company123/frontend-team_01");
+        assert!(matches!(owner.owner_type, OwnerType::Team));
+
+        // Short names
+        let owner = parse_owner("@o/t")?;
+        assert_eq!(owner.identifier, "@o/t");
+        assert!(matches!(owner.owner_type, OwnerType::Team));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_parse_owner_email() -> Result<()> {
+        // Standard email
+        let owner = parse_owner("user@example.com")?;
+        assert_eq!(owner.identifier, "user@example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // With plus addressing
+        let owner = parse_owner("user+tag@example.com")?;
+        assert_eq!(owner.identifier, "user+tag@example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // With dots and numbers
+        let owner = parse_owner("user.name123@sub.example.com")?;
+        assert_eq!(owner.identifier, "user.name123@sub.example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // Multiple @ symbols - should still be detected as Email
+        let owner = parse_owner("user@example@domain.com")?;
+        assert_eq!(owner.identifier, "user@example@domain.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // IP address domain
+        let owner = parse_owner("user@[192.168.1.1]")?;
+        assert_eq!(owner.identifier, "user@[192.168.1.1]");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_parse_owner_unowned() -> Result<()> {
+        let owner = parse_owner("NOOWNER")?;
+        assert_eq!(owner.identifier, "NOOWNER");
+        assert!(matches!(owner.owner_type, OwnerType::Unowned));
+
+        // Case insensitive
+        let owner = parse_owner("noowner")?;
+        assert_eq!(owner.identifier, "noowner");
+        assert!(matches!(owner.owner_type, OwnerType::Unowned));
+
+        let owner = parse_owner("NoOwNeR")?;
+        assert_eq!(owner.identifier, "NoOwNeR");
+        assert!(matches!(owner.owner_type, OwnerType::Unowned));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_parse_owner_unknown() -> Result<()> {
+        // Random text
+        let owner = parse_owner("plaintext")?;
+        assert_eq!(owner.identifier, "plaintext");
+        assert!(matches!(owner.owner_type, OwnerType::Unknown));
+
+        // Text with special characters (but not @ or email format)
+        let owner = parse_owner("special-text_123")?;
+        assert_eq!(owner.identifier, "special-text_123");
+        assert!(matches!(owner.owner_type, OwnerType::Unknown));
+
+        // URL-like but not an owner
+        let owner = parse_owner("https://example.com")?;
+        assert_eq!(owner.identifier, "https://example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Unknown));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_parse_owner_email_edge_cases() -> Result<()> {
+        // Technically valid by RFC 5322 but unusual emails
+        let owner = parse_owner("\"quoted\"@example.com")?;
+        assert_eq!(owner.identifier, "\"quoted\"@example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // Very short email
+        let owner = parse_owner("a@b.c")?;
+        assert_eq!(owner.identifier, "a@b.c");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // Email with many subdomains
+        let owner = parse_owner("user@a.b.c.d.example.com")?;
+        assert_eq!(owner.identifier, "user@a.b.c.d.example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_parse_owner_ambiguous_cases() -> Result<()> {
+        // Contains @ but also has prefix
+        let owner = parse_owner("prefix-user@example.com")?;
+        assert_eq!(owner.identifier, "prefix-user@example.com");
+        assert!(matches!(owner.owner_type, OwnerType::Email));
+
+        // Has team-like structure but without @ prefix
+        let owner = parse_owner("org/team-name")?;
+        assert_eq!(owner.identifier, "org/team-name");
+        assert!(matches!(owner.owner_type, OwnerType::Unknown));
+
+        // Contains "NOOWNER" as substring but isn't exactly NOOWNER
+        let owner = parse_owner("NOOWNER-plus")?;
+        assert_eq!(owner.identifier, "NOOWNER-plus");
+        assert!(matches!(owner.owner_type, OwnerType::Unknown));
+
+        Ok(())
+    }
 }

core/src/types.rs

@@ -7,7 +7,7 @@ pub struct CodeownersEntry {
     pub line_number: usize,
     pub pattern: String,
     pub owners: Vec<Owner>,
-    pub tags: Vec<String>,
+    pub tags: Vec<Tag>,
 }
 
 /// Detailed owner representation
@@ -23,9 +23,14 @@ pub enum OwnerType {
     User,
     Team,
     Email,
+    Unowned,
     Unknown,
 }
 
+/// Tag representation
+#[derive(Debug)]
+pub struct Tag(pub String);
+
 #[derive(Clone, Debug, PartialEq)]
 pub enum OutputFormat {
     Text,