fix: macro hover

crates/ide/src/hover.rs

@@ -73,29 +73,53 @@ pub(crate) fn hover(
         let source_text = db.preprocessed_text(file_id);
         let source_text = def_node.utf8_text(source_text.as_bytes()).ok()?;
 
-        let start = offset.range.start.character as usize;
-        let end = offset
-            .range
-            .end
-            .character
-            .saturating_add_signed(-offset.diff) as usize;
+        let mut start = offset.range.start.character;
+        let mut end = offset.range.end.character;
+        offsets[&fpos.position.line]
+            .iter()
+            .filter(|prev_offset| prev_offset.range.start.character < offset.range.start.character)
+            .for_each(|prev_offset| {
+                start = start.saturating_add_signed(
+                    prev_offset
+                        .diff
+                        .saturating_sub_unsigned(prev_offset.args_diff),
+                );
+                end = end.saturating_add_signed(
+                    prev_offset
+                        .diff
+                        .saturating_sub_unsigned(prev_offset.args_diff),
+                );
+            });
+        end = end.saturating_add_signed(offset.diff);
+        let start = start as usize;
+        let end = end as usize;
         let slc = start..end;
         // The preprocessed file might be shorter than the original file
         let hover_text = preprocessed_text
             .lines()
             .nth(fpos.position.line as usize)
             .and_then(|it| it.get(slc))
-            .map(|it| it.to_string())
-            .unwrap_or_default();
-        let res = HoverResult {
-            markup: Markup::from(format!(
-                "{}\nExpands to:\n{}",
-                Markup::fenced_block(source_text),
-                Markup::fenced_block(hover_text)
-            )),
-            actions: vec![],
-        };
-        return Some(RangeInfo::new(offset.range, res));
+            .map(|it| it.to_string());
+        match hover_text {
+            Some(hover_text) => {
+                let res = HoverResult {
+                    markup: Markup::from(format!(
+                        "{}\nExpands to:\n{}",
+                        Markup::fenced_block(source_text),
+                        Markup::fenced_block(hover_text)
+                    )),
+                    actions: vec![],
+                };
+                return Some(RangeInfo::new(offset.range, res));
+            }
+            None => {
+                let res = HoverResult {
+                    markup: Markup::fenced_block(source_text),
+                    actions: vec![],
+                };
+                return Some(RangeInfo::new(offset.range, res));
+            }
+        }
     }
 
     None

crates/preprocessor/src/evaluator.rs

@@ -185,7 +185,8 @@ impl<'a> IfCondition<'a> {
                                         file_id: macro_.file_id,
                                         range: symbol.range,
                                         diff: 0, // FIXME: This is the default value, we should calculate it.
-                                        idx: macro_.idx
+                                        idx: macro_.idx,
+                                        args_diff: 0
                                     });
                             output_queue.push(1);
                         } else {
@@ -211,14 +212,15 @@ impl<'a> IfCondition<'a> {
                             false,
                             self.disabled_macros
                         ) {
-                            Ok(args_map) => {
+                            Ok((args_map, args_diff)) => {
                                 extend_args_map(self.args_map, args_map);
                                 if let Some((idx, file_id)) = attr {
                                     self.offsets.entry(symbol.range.start.line).or_default().push(Offset {
                                         file_id,
                                         range: symbol.range,
                                         diff: 0, // FIXME: This is the default value, we should calculate it.
-                                        idx
+                                        idx,
+                                        args_diff
                                     })
                                     ;
                                 }

crates/preprocessor/src/lib.rs

@@ -269,6 +269,7 @@ where
         let mut intrinsics_parse_status = None;
         let mut col_offset: Option<i32> = None;
         let mut expanded_symbol: Option<(Symbol, u32, FileId)> = None;
+        let mut args_diff = 0u32;
         while let Some(symbol) = if !self.expansion_stack.is_empty() {
             let symbol = self.expansion_stack.pop().unwrap();
             col_offset = Some(
@@ -292,7 +293,9 @@ where
                                 - (expanded_symbol.range.end.character
                                     - expanded_symbol.range.start.character)
                                     as i32),
+                            args_diff,
                         });
+                    args_diff = 0;
                 }
             }
 
@@ -353,9 +356,10 @@ where
                                 true,
                                 &mut self.disabled_macros,
                             ) {
-                                Ok(args_map) => {
+                                Ok((args_map, args_diff_)) => {
                                     extend_args_map(&mut self.args_maps, args_map);
                                     expanded_symbol = Some((symbol.clone(), idx, file_id));
+                                    args_diff = args_diff_;
                                     continue;
                                 }
                                 Err(ExpansionError::MacroNotFound(err)) => {

crates/preprocessor/src/macros.rs

@@ -75,23 +75,24 @@ impl ArgumentsCollector {
         symbol: &Symbol,
         context: &mut MacroContext,
         nb_params: usize,
-    ) -> Option<MacroArguments>
+    ) -> Option<(MacroArguments, u32)>
     where
         T: Iterator<Item = Symbol>,
     {
         let mut temp_expanded_stack = vec![];
         let mut paren_depth = 0;
         let mut arg_idx: usize = 0;
         let mut args: MacroArguments = Default::default();
-        let mut found_first_paren = false;
+        let mut first_paren_col = None;
+        let mut last_paren_col = None;
         while let Some(sub_symbol) = if !context.is_empty() {
             Some(context.pop_front().unwrap().symbol)
         } else if !self.popped_symbols_stack.is_empty() {
             self.popped_symbols_stack.pop()
         } else {
             lexer.next()
         } {
-            if !found_first_paren {
+            if first_paren_col.is_none() {
                 if !matches!(
                     &sub_symbol.token_kind,
                     TokenKind::LParen | TokenKind::Comment(sourcepawn_lexer::Comment::BlockComment)
@@ -108,7 +109,7 @@ impl ArgumentsCollector {
                             .extend(temp_expanded_stack.into_iter().rev());
                         return None;
                     }
-                    found_first_paren = true;
+                    first_paren_col = Some(sub_symbol.range.start.character);
                 } else {
                     temp_expanded_stack.push(sub_symbol);
                     continue;
@@ -127,6 +128,7 @@ impl ArgumentsCollector {
                     }
                     paren_depth -= 1;
                     if paren_depth == 0 {
+                        last_paren_col = Some(sub_symbol.range.end.character);
                         break;
                     }
                 }
@@ -152,8 +154,12 @@ impl ArgumentsCollector {
                 }
             }
         }
+        let diff = match (first_paren_col, last_paren_col) {
+            (Some(first), Some(last)) => last.saturating_sub(first),
+            _ => 0,
+        };
 
-        Some(args)
+        Some((args, diff))
     }
 }
 
@@ -186,11 +192,12 @@ pub(super) fn expand_identifier<T>(
     expansion_stack: &mut Vec<Symbol>,
     allow_undefined_macros: bool,
     disabled_macros: &mut FxHashSet<Arc<Macro>>,
-) -> Result<Option<ExpansionOffsets>, ExpansionError>
+) -> Result<(Option<ExpansionOffsets>, u32), ExpansionError>
 where
     T: Iterator<Item = Symbol>,
 {
     let mut args_mapping: Option<ExpansionOffsets> = None;
+    let mut args_diff = 0;
     let mut reversed_expansion_stack = Vec::new();
     let mut args_collector = ArgumentsCollector::default();
     let mut context_stack = vec![VecDeque::from([QueuedSymbol::new(
@@ -228,7 +235,7 @@ where
                         queued_symbol.symbol.range,
                     )
                 } else {
-                    let Some(args) = &args_collector.collect_arguments(
+                    let Some((args, diff)) = &args_collector.collect_arguments(
                         lexer,
                         &queued_symbol.symbol,
                         &mut current_context,
@@ -241,6 +248,9 @@ where
                         context_stack.push(current_context);
                         continue;
                     };
+                    if context_stack.is_empty() {
+                        args_diff += *diff;
+                    }
                     let (ctx, args_map) = expand_macro(args, macro_, &queued_symbol.symbol)?;
                     if context_stack.is_empty() {
                         args_mapping = args
@@ -295,7 +305,7 @@ where
     // produces them in the correct order, therefore we have to reverse them.
     expansion_stack.extend(reversed_expansion_stack.into_iter().rev());
 
-    Ok(args_mapping)
+    Ok((args_mapping, args_diff))
 }
 
 /// Expand a non macro define by returning a new [context](MacroContext) of all the [symbols](Symbol)

crates/preprocessor/src/offset.rs

@@ -9,6 +9,17 @@ pub struct Offset {
     /// The difference in characters between the original symbol and the expanded symbol.
     pub diff: i32,
 
+    /// The difference introduced by the macro arguments.
+    ///
+    /// # Example
+    /// ```c
+    /// #define FOO(x) x
+    /// int foo = FOO(1);
+    /// ```
+    ///
+    /// In this example, the `args_diff` would be `3` because the `(1)` has a length of `3`.
+    pub args_diff: u32,
+
     /// The index of the macro that was expanded.
     pub idx: u32,
 

crates/sourcepawn_lsp/tests/text_document/hover/macros.rs

@@ -40,3 +40,65 @@ int bar = FOO(foo);
 "#,
     ));
 }
+
+#[test]
+fn macro_4() {
+    assert_json_snapshot!(hover(
+        r#"
+%! main.sp
+#define FOO 1 + 1
+int foo = 1;
+int bar = FOO + FOO;
+                 |
+                 ^
+"#,
+    ));
+}
+
+#[test]
+fn macro_5() {
+    assert_json_snapshot!(hover(
+        r#"
+%! main.sp
+#define FOO(%1) %1 + %1
+int foo = 1;
+int bar = FOO(foo) + FOO(foo);
+                      |
+                      ^
+"#,
+    ));
+}
+
+#[test]
+fn macro_6() {
+    assert_json_snapshot!(hover(
+        r#"
+%! main.sp
+#define FOO(%0) 	view_as<int>( %0 )
+#define BAR(%0,%1) foo[FOO( %0 )][%1]
+#define BAZ      (1 << 0)
+int foo[10][10];
+int bar = BAR( 1, 2 ) + BAZ;
+                         |
+                         ^
+"#,
+    ));
+}
+
+#[test]
+fn macro_7() {
+    assert_json_snapshot!(hover(
+        r#"
+%! main.sp
+#define FOO(%0) 	view_as<int>( %0 )
+#define BAR(%0,%1) foo[FOO( %0 )][%1]
+#define BAZ      (1 << 0)
+int foo[10][10];
+int bar = BAR( FOO(1), 2 ) + BAZ;
+                              |
+                              ^
+"#,
+    ));
+}
+
+// 38 - 46

crates/sourcepawn_lsp/tests/text_document/hover/snapshots/main__text_document__hover__macros__macro_1.snap

@@ -6,7 +6,7 @@ expression: "hover(r#\"\n%! main.sp\n#define FOO\nFOO\n |\n ^\n\"#)"
 {
   "contents": {
     "kind": "markdown",
-    "value": "```sourcepawn\n#define FOO\n```\nExpands to:\n```sourcepawn\n\n```"
+    "value": "```sourcepawn\n#define FOO\n```"
   },
   "range": {
     "start": {

crates/sourcepawn_lsp/tests/text_document/hover/snapshots/main__text_document__hover__macros__macro_2.snap

@@ -6,7 +6,7 @@ expression: "hover(r#\"\n%! main.sp\n#define FOO 1\nint foo = FOO;\n           |
 {
   "contents": {
     "kind": "markdown",
-    "value": "```sourcepawn\n#define FOO 1\n```\nExpands to:\n```sourcepawn\n\n```"
+    "value": "```sourcepawn\n#define FOO 1\n```\nExpands to:\n```sourcepawn\n1\n```"
   },
   "range": {
     "start": {

crates/sourcepawn_lsp/tests/text_document/hover/snapshots/main__text_document__hover__macros__macro_3.snap

@@ -6,7 +6,7 @@ expression: "hover(r#\"\n%! main.sp\n#define FOO(%1) %1 + %1\nint foo = 1;\nint
 {
   "contents": {
     "kind": "markdown",
-    "value": "```sourcepawn\n#define FOO(%1) %1 + %1\n```\nExpands to:\n```sourcepawn\n\n```"
+    "value": "```sourcepawn\n#define FOO(%1) %1 + %1\n```\nExpands to:\n```sourcepawn\nfoo + foo\n```"
   },
   "range": {
     "start": {

crates/sourcepawn_lsp/tests/text_document/hover/snapshots/main__text_document__hover__macros__macro_4.snap

@@ -0,0 +1,21 @@
+---
+source: crates/sourcepawn_lsp/tests/text_document/hover/macros.rs
+assertion_line: 46
+expression: "hover(r#\"\n%! main.sp\n#define FOO 1 + 1\nint foo = 1;\nint bar = FOO + FOO;\n                 |\n                 ^\n\"#)"
+---
+{
+  "contents": {
+    "kind": "markdown",
+    "value": "```sourcepawn\n#define FOO 1 + 1\n```\nExpands to:\n```sourcepawn\n1 + 1\n```"
+  },
+  "range": {
+    "start": {
+      "line": 2,
+      "character": 16
+    },
+    "end": {
+      "line": 2,
+      "character": 19
+    }
+  }
+}

crates/sourcepawn_lsp/tests/text_document/hover/snapshots/main__text_document__hover__macros__macro_5.snap

@@ -0,0 +1,21 @@
+---
+source: crates/sourcepawn_lsp/tests/text_document/hover/macros.rs
+assertion_line: 60
+expression: "hover(r#\"\n%! main.sp\n#define FOO(%1) %1 + %1\nint foo = 1;\nint bar = FOO(foo) + FOO(foo);\n                      |\n                      ^\n\"#)"
+---
+{
+  "contents": {
+    "kind": "markdown",
+    "value": "```sourcepawn\n#define FOO(%1) %1 + %1\n```\nExpands to:\n```sourcepawn\nfoo + foo\n```"
+  },
+  "range": {
+    "start": {
+      "line": 2,
+      "character": 21
+    },
+    "end": {
+      "line": 2,
+      "character": 24
+    }
+  }
+}