Code review feedback.

Author: UnboundVariable

Cargo.lock

@@ -16,6 +16,7 @@ ruff_db = { workspace = true }
 ruff_python_ast = { workspace = true }
 ruff_python_parser = { workspace = true }
 ruff_python_trivia = { workspace = true }
+ruff_source_file = { workspace = true }
 ruff_text_size = { workspace = true }
 ty_python_semantic = { workspace = true }
 

crates/ty_ide/Cargo.toml

@@ -8,6 +8,7 @@
 
 use regex::Regex;
 use ruff_python_trivia::leading_indentation;
+use ruff_source_file::UniversalNewlines;
 use std::collections::HashMap;
 use std::sync::LazyLock;
 
@@ -57,67 +58,6 @@ pub fn get_parameter_documentation(docstring: &str) -> HashMap<String, String> {
     param_docs
 }
 
-/// Iterator that splits text on universal newlines (`\r\n`, `\r`, `\n`) similar to Python's `str.splitlines()`.
-/// This ensures consistent behavior across different platforms and line ending styles.
-struct UniversalLinesIterator<'a> {
-    text: &'a str,
-    position: usize,
-}
-
-impl<'a> UniversalLinesIterator<'a> {
-    fn new(text: &'a str) -> Self {
-        Self { text, position: 0 }
-    }
-}
-
-impl<'a> Iterator for UniversalLinesIterator<'a> {
-    type Item = &'a str;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if self.position >= self.text.len() {
-            return None;
-        }
-
-        let start = self.position;
-        let remaining = &self.text[start..];
-
-        // Find the next line ending
-        if let Some(pos) = remaining.find('\n') {
-            // Check if it's \r\n
-            let end = if pos > 0 && remaining.as_bytes()[pos - 1] == b'\r' {
-                start + pos - 1 // Don't include the \r
-            } else {
-                start + pos // Don't include the \n
-            };
-            self.position = start + pos + 1; // Move past the \n
-            Some(&self.text[start..end])
-        } else if let Some(pos) = remaining.find('\r') {
-            // Just \r (old Mac style)
-            let end = start + pos;
-            self.position = start + pos + 1; // Move past the \r
-            Some(&self.text[start..end])
-        } else {
-            // No more line endings, return the rest
-            self.position = self.text.len();
-            if start < self.text.len() {
-                Some(&self.text[start..])
-            } else {
-                None
-            }
-        }
-    }
-}
-
-trait UniversalLines {
-    fn universal_lines(&self) -> UniversalLinesIterator<'_>;
-}
-
-impl UniversalLines for str {
-    fn universal_lines(&self) -> UniversalLinesIterator<'_> {
-        UniversalLinesIterator::new(self)
-    }
-}
-
 /// Extract parameter documentation from Google-style docstrings.
 fn extract_google_style_params(docstring: &str) -> Option<HashMap<String, String>> {
     let mut param_docs = HashMap::new();
@@ -126,7 +66,8 @@ fn extract_google_style_params(docstring: &str) -> Option<HashMap<String, String
     let mut current_param: Option<String> = None;
     let mut current_doc = String::new();
 
-    for line in docstring.universal_lines() {
+    for line_obj in docstring.universal_newlines() {
+        let line = line_obj.as_str();
         if GOOGLE_SECTION_REGEX.is_match(line) {
             in_args_section = true;
             continue;
@@ -213,7 +154,10 @@ fn get_indentation_level(line: &str) -> usize {
 fn extract_numpy_style_params(docstring: &str) -> Option<HashMap<String, String>> {
     let mut param_docs = HashMap::new();
 
-    let mut lines = docstring.universal_lines().peekable();
+    let mut lines = docstring
+        .universal_newlines()
+        .map(|line| line.as_str())
+        .peekable();
     let mut in_params_section = false;
     let mut found_underline = false;
     let mut current_param: Option<String> = None;
@@ -384,7 +328,8 @@ fn extract_rest_style_params(docstring: &str) -> Option<HashMap<String, String>>
     let mut current_param: Option<String> = None;
     let mut current_doc = String::new();
 
-    for line in docstring.universal_lines() {
+    for line_obj in docstring.universal_newlines() {
+        let line = line_obj.as_str();
         if let Some(captures) = REST_PARAM_REGEX.captures(line) {
             // Save previous parameter if exists
             if let Some(param_name) = current_param.take() {

crates/ty_ide/src/docstring.rs

@@ -2270,8 +2270,8 @@ impl<'db> Binding<'db> {
 
     /// Returns a vector where each index corresponds to an argument position,
     /// and the value is the parameter index that argument maps to (if any).
-    pub(crate) fn argument_to_parameter_mapping(&self) -> Vec<Option<usize>> {
-        self.argument_parameters.to_vec()
+    pub(crate) fn argument_to_parameter_mapping(&self) -> Box<[Option<usize>]> {
+        self.argument_parameters.clone()
     }
 }
 

crates/ty_python_semantic/src/types/call/bind.rs

@@ -394,7 +394,7 @@ pub struct CallSignatureDetails<'db> {
 
     /// Mapping from argument indices to parameter indices. This helps
     /// determine which parameter corresponds to which argument position.
-    pub argument_to_parameter_mapping: Vec<Option<usize>>,
+    pub argument_to_parameter_mapping: Box<[Option<usize>]>,
 }
 
 /// Extract signature details from a callable type.
@@ -476,7 +476,7 @@ fn create_argument_mapping(
     callable: crate::types::CallableType<'_>,
     signature: &Signature<'_>,
     arguments: &ast::Arguments,
-) -> Vec<Option<usize>> {
+) -> Box<[Option<usize>]> {
     let call_arguments = CallArguments::from_arguments(arguments);
 
     let mut argument_forms = vec![None; call_arguments.len()];

crates/ty_python_semantic/src/types/ide_support.rs

@@ -15,16 +15,6 @@ use ruff_db::source::{line_index, source_text};
 use ty_ide::signature_help;
 use ty_project::ProjectDatabase;
 
-/// Client capabilities for signature help feature.
-#[derive(Debug, Clone, Default)]
-struct SignatureHelpClientCapabilities {
-    /// Does the client support signature label offsets?
-    signature_label_offset_support: bool,
-
-    /// Does the client support per-signature active parameter?
-    active_parameter_support: bool,
-}
-
 pub(crate) struct SignatureHelpRequestHandler;
 
 impl RequestHandler for SignatureHelpRequestHandler {
@@ -61,10 +51,6 @@ impl BackgroundDocumentRequestHandler for SignatureHelpRequestHandler {
 
         // Extract signature help capabilities from the client
         let resolved_capabilities = snapshot.resolved_client_capabilities();
-        let client_capabilities = SignatureHelpClientCapabilities {
-            signature_label_offset_support: resolved_capabilities.signature_label_offset_support,
-            active_parameter_support: resolved_capabilities.signature_active_parameter_support,
-        };
 
         let Some(signature_help_info) = signature_help(db, file, offset) else {
             return Ok(None);
@@ -86,7 +72,7 @@ impl BackgroundDocumentRequestHandler for SignatureHelpRequestHandler {
                     .parameters
                     .into_iter()
                     .map(|param| {
-                        let label = if client_capabilities.signature_label_offset_support {
+                        let label = if resolved_capabilities.signature_label_offset_support {
                             // Find the parameter's offset in the signature label
                             if let Some(start) = sig.label.find(&param.label) {
                                 let start_u32 = u32::try_from(start).unwrap_or(u32::MAX);
@@ -107,7 +93,7 @@ impl BackgroundDocumentRequestHandler for SignatureHelpRequestHandler {
                     })
                     .collect();
 
-                let active_parameter = if client_capabilities.active_parameter_support {
+                let active_parameter = if resolved_capabilities.signature_active_parameter_support {
                     sig.active_parameter.and_then(|p| u32::try_from(p).ok())
                 } else {
                     None