CodeRabbit Generated Unit Tests: Generate unit tests for PR changes

src/analyzers/complexity.rs

@@ -378,7 +378,7 @@ fn find_function_at_line<'a>(
         // Only descend if line is within this node's range
         if start <= line && line <= end {
             let kind = node.kind();
-            if kind.contains("function") || kind.contains("method") || kind == "impl_item" {
+            if kind.contains("function") || kind.contains("method") {
                 return Some(node);
             }
 
@@ -931,6 +931,39 @@ fn nested(x: i32, y: i32) {
         assert!(result.functions[0].metrics.max_nesting >= 2);
     }
 
+    #[test]
+    fn test_complexity_rust_impl_methods_are_measured_individually() {
+        let code = br#"
+struct Service;
+
+impl Service {
+    fn simple(&self) {
+        println!("simple");
+    }
+
+    fn branchy(&self, x: i32) {
+        if x > 0 {
+            println!("positive");
+        }
+    }
+}
+"#;
+        let result = parse_and_analyze(code, Language::Rust, "test.rs");
+        let simple = result
+            .functions
+            .iter()
+            .find(|f| f.name == "simple")
+            .expect("simple method should be extracted");
+        let branchy = result
+            .functions
+            .iter()
+            .find(|f| f.name == "branchy")
+            .expect("branchy method should be extracted");
+
+        assert_eq!(simple.metrics.cyclomatic, 1);
+        assert!(branchy.metrics.cyclomatic > simple.metrics.cyclomatic);
+    }
+
     #[test]
     fn test_complexity_go_simple_function() {
         let code = b"package main\n\nfunc simple() { x := 1 }";
@@ -1460,4 +1493,76 @@ function classify(x: number): string {
             "TypeScript switch with 2 case clauses should count each case as a decision point"
         );
     }
+
+    // --- Tests for the find_function_at_line change (impl_item removed) ---
+
+    #[test]
+    fn test_complexity_rust_impl_block_declaration_line_not_treated_as_function() {
+        // Lines that are part of an `impl` block declaration itself (e.g. the
+        // `impl Service {` line) should NOT be returned as a function node
+        // now that the `impl_item` match was removed from find_function_at_line.
+        // We verify this indirectly: the only functions extracted from an impl
+        // block should be the actual method nodes, not a synthetic entry for
+        // the impl container.
+        let code = br#"
+struct Widget;
+
+impl Widget {
+    fn render(&self) {}
+}
+"#;
+        let result = parse_and_analyze(code, Language::Rust, "test.rs");
+        // Exactly one method – the impl block itself must not appear as a function.
+        assert_eq!(
+            result.functions.len(),
+            1,
+            "impl container must not be counted as a function; got {:?}",
+            result.functions.iter().map(|f| &f.name).collect::<Vec<_>>()
+        );
+        assert_eq!(result.functions[0].name, "render");
+    }
+
+    #[test]
+    fn test_complexity_rust_multiple_impl_blocks_methods_all_extracted() {
+        // When a type has several impl blocks (e.g. trait impls), every method
+        // must still be individually extracted after removing the impl_item catch.
+        let code = br#"
+struct Counter(i32);
+
+impl Counter {
+    fn increment(&mut self) {
+        self.0 += 1;
+    }
+    fn value(&self) -> i32 {
+        self.0
+    }
+}
+
+impl Default for Counter {
+    fn default() -> Self {
+        Counter(0)
+    }
+}
+"#;
+        let result = parse_and_analyze(code, Language::Rust, "test.rs");
+        let names: Vec<&str> = result.functions.iter().map(|f| f.name.as_str()).collect();
+        assert!(
+            names.contains(&"increment"),
+            "increment not found in {:?}",
+            names
+        );
+        assert!(names.contains(&"value"), "value not found in {:?}", names);
+        assert!(
+            names.contains(&"default"),
+            "default not found in {:?}",
+            names
+        );
+        // No extra entry for the impl blocks themselves
+        assert_eq!(
+            result.functions.len(),
+            3,
+            "expected exactly 3 methods, got {:?}",
+            names
+        );
+    }
 }

src/mcp/mod.rs

@@ -363,7 +363,7 @@ impl McpServer {
         // Try to open a git repository at the path
         let git_root = GitRepo::open(&path).ok().map(|r| r.root().to_path_buf());
 
-        let mut ctx = AnalysisContext::new(&file_set, &self.config, Some(&self.root_path));
+        let mut ctx = AnalysisContext::new(&file_set, &self.config, Some(&path));
         if let Some(ref git_path) = git_root {
             ctx = ctx.with_git_path(git_path);
         }
@@ -818,6 +818,60 @@ mod tests {
         assert!(response.get("content").is_some());
     }
 
+    #[test]
+    fn test_handle_tool_call_uses_requested_path_as_analysis_root() {
+        let (server, _server_root) = create_test_server();
+        let target_dir = TempDir::new().unwrap();
+        std::fs::write(
+            target_dir.path().join("target.rs"),
+            "fn target_function() {}\n",
+        )
+        .unwrap();
+
+        let params = json!({
+            "name": "complexity",
+            "arguments": {"path": target_dir.path().to_str().unwrap()}
+        });
+        let response = server.handle_tool_call(Some(params)).unwrap();
+        let text = response["content"][0]["text"]
+            .as_str()
+            .expect("tool response text should be a string");
+
+        assert!(
+            text.contains("target_function"),
+            "expected MCP analysis to read files from requested path, got {text}"
+        );
+    }
+
+    #[test]
+    fn test_handle_tool_call_without_path_uses_server_root() {
+        // When the caller does not supply an explicit "path" argument, the
+        // server must fall back to its own root_path for file discovery.
+        // This is the pre-existing default-path behaviour; the fix in this PR
+        // must not regress it.
+        let (server, server_root) = create_test_server();
+        std::fs::write(
+            server_root.path().join("root_only.rs"),
+            "fn root_symbol() {}\n",
+        )
+        .unwrap();
+
+        // No "path" key in arguments – should analyse server_root
+        let params = json!({
+            "name": "complexity",
+            "arguments": {}
+        });
+        let response = server.handle_tool_call(Some(params)).unwrap();
+        let text = response["content"][0]["text"]
+            .as_str()
+            .expect("tool response text should be a string");
+
+        assert!(
+            text.contains("root_symbol"),
+            "expected server root to be analysed when no path is given, got {text}"
+        );
+    }
+
     #[test]
     fn test_handle_tool_call_satd() {
         let (server, temp_dir) = create_test_server();

src/semantic/sync.rs

@@ -71,8 +71,7 @@ impl<'a> SyncManager<'a> {
 
         // Find files to remove (deleted or moved)
         for indexed_path in &indexed_files {
-            let full_path = root_path.join(indexed_path);
-            if !current_files.contains(&full_path) {
+            if !current_files.contains(&PathBuf::from(indexed_path)) {
                 self.cache.remove_file(indexed_path)?;
                 stats.removed += 1;
             }
@@ -81,16 +80,19 @@ impl<'a> SyncManager<'a> {
         // Check each current file for changes
         let files_to_index: Vec<_> = current_files
             .iter()
-            .filter(|path| {
-                let rel_path = path
-                    .strip_prefix(root_path)
-                    .unwrap_or(path)
-                    .to_string_lossy()
-                    .to_string();
-
-                self.check_file_changed(path, &rel_path).unwrap_or(true)
+            .filter_map(|path| {
+                let full_path = root_path.join(path);
+                let rel_path = path.to_string_lossy().to_string();
+
+                if self
+                    .check_file_changed(&full_path, &rel_path)
+                    .unwrap_or(true)
+                {
+                    Some(full_path)
+                } else {
+                    None
+                }
             })
-            .cloned()
             .collect();
 
         stats.checked = current_files.len();
@@ -429,4 +431,57 @@ mod tests {
         assert_eq!(stats.symbols, 0);
         assert_eq!(stats.errors, 0);
     }
+
+    #[test]
+    fn test_sync_removes_stale_indexed_files() {
+        // First sync: index a file so it ends up in the cache.
+        let temp = tempfile::tempdir().unwrap();
+        let rust_file = temp.path().join("stale.rs");
+        std::fs::write(&rust_file, "fn stale_symbol() {}\n").unwrap();
+
+        let config = crate::config::Config::default();
+        let file_set_with = FileSet::from_path(temp.path(), &config).unwrap();
+        let cache = EmbeddingCache::in_memory().unwrap();
+        let sync = SyncManager::new(&cache);
+
+        let stats_first = sync.sync(&file_set_with, temp.path()).unwrap();
+        assert_eq!(stats_first.indexed, 1, "first sync should index the file");
+
+        // Now delete the file and sync with an empty FileSet.
+        std::fs::remove_file(&rust_file).unwrap();
+        let file_set_empty = FileSet::from_path(temp.path(), &config).unwrap();
+        assert!(file_set_empty.is_empty(), "file set should be empty after deletion");
+
+        let stats_second = sync.sync(&file_set_empty, temp.path()).unwrap();
+        assert_eq!(
+            stats_second.removed,
+            1,
+            "second sync should remove the stale indexed file"
+        );
+        assert_eq!(stats_second.indexed, 0, "no new files to index");
+    }
+
+    #[test]
+    fn test_sync_indexes_files_relative_to_root_path() {
+        let temp = tempfile::tempdir().unwrap();
+        let nested = temp.path().join("only_in_temp");
+        std::fs::create_dir_all(&nested).unwrap();
+        std::fs::write(
+            nested.join("unique_semantic_sync_fixture.rs"),
+            "fn indexed_symbol() {}\n",
+        )
+        .unwrap();
+
+        let config = crate::config::Config::default();
+        let file_set = FileSet::from_path(temp.path(), &config).unwrap();
+        let cache = EmbeddingCache::in_memory().unwrap();
+        let sync = SyncManager::new(&cache);
+
+        let stats = sync.sync(&file_set, temp.path()).unwrap();
+
+        assert_eq!(stats.checked, 1);
+        assert_eq!(stats.indexed, 1);
+        assert_eq!(stats.errors, 0);
+        assert_eq!(cache.symbol_count().unwrap(), 1);
+    }
 }