fix: macro arguments resolution

crates/hir/src/semantics.rs

@@ -430,8 +430,11 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
                     InFile::new(file_id, body_node),
                     Some(offset),
                 );
-                let arg = analyzer.resolve_named_arg(self.db, &node, &parent)?;
-                return Some(DefResolution::Local(arg));
+
+                if let Some(arg) = analyzer.resolve_named_arg(self.db, &node, &parent) {
+                    // Only return if we find an argument. If we don't we were trying to resolve the value.
+                    return Some(DefResolution::Local(arg));
+                }
             }
             _ => {}
         }

crates/ide/src/goto_definition.rs

@@ -1,8 +1,9 @@
 use base_db::FilePosition;
+use fxhash::FxHashMap;
 use hir::{DefResolution, HasSource, Semantics};
 
+use preprocessor::Offset;
 use syntax::{
-    range_contains_pos,
     utils::{lsp_position_to_ts_point, ts_range_to_lsp_range},
     TSKind,
 };
@@ -23,73 +24,49 @@ pub(crate) fn goto_definition(
     let sema = &Semantics::new(db);
     let preprocessing_results = sema.preprocess_file(pos.file_id);
     let offsets = preprocessing_results.offsets();
-    if let Some(offset) = offsets
-        .get(&pos.position.line)
-        .and_then(|offsets| offsets.iter().find(|offset| offset.contains(pos.position)))
-    {
-        let def = sema
-            .find_macro_def(offset.file_id, offset.idx)
-            .map(DefResolution::from)?;
-        let file_id = def.file_id(db);
-        let u_range = offset.range;
-        let source_tree = sema.parse(file_id);
-        let def_node = def.source(db, &source_tree)?.value;
-        let mut name_range = def_node.range();
-        if let Some(name_node) = def_node.child_by_field_name("name") {
-            name_range = name_node.range();
-        }
-        let navs = vec![NavigationTarget {
-            file_id,
-            full_range: ts_range_to_lsp_range(&def_node.range()),
-            focus_range: ts_range_to_lsp_range(&name_range).into(),
-        }];
+    let tree = sema.parse(pos.file_id);
+    let root_node = tree.root_node();
 
-        return RangeInfo::new(u_range, navs).into();
+    if let Some(res) = find_macro_def(offsets, &pos.position, sema) {
+        return res.into();
     }
 
-    let diff = if let Some(diff) = preprocessing_results
-        .args_map()
-        .get(&pos.position.line)
-        .and_then(|mapped_ranges| {
-            mapped_ranges
-                .iter()
-                .find(|(range, _)| range_contains_pos(range, &pos.position))
-        })
-        .map(|(range, mapped_range)| {
-            mapped_range.start.character as i32 - range.start.character as i32
-        }) {
-        pos.position.character = pos.position.character.saturating_add_signed(diff);
-        diff.into()
-    } else {
-        None
-    };
+    let source_u_range =
+        u_pos_to_s_pos(preprocessing_results.args_map(), offsets, &mut pos.position);
 
-    let tree = sema.parse(pos.file_id);
-    let root_node = tree.root_node();
-    let s_pos = u_pos_to_s_pos(offsets, pos.position);
     let node = root_node.descendant_for_point_range(
-        lsp_position_to_ts_point(&s_pos),
-        lsp_position_to_ts_point(&s_pos),
+        lsp_position_to_ts_point(&pos.position),
+        lsp_position_to_ts_point(&pos.position),
     )?;
-
     let def = sema.find_def(pos.file_id, &node)?;
-
-    let mut u_range = ts_range_to_lsp_range(&node.range());
-    if let Some(diff) = diff {
-        u_range.start.character = u_range.start.character.saturating_add_signed(-diff);
-        u_range.end.character = u_range.end.character.saturating_add_signed(-diff);
-    } else {
-        u_range = s_range_to_u_range(offsets, u_range);
-    }
+    let u_range = match source_u_range {
+        Some(u_range) => u_range,
+        None => s_range_to_u_range(offsets, ts_range_to_lsp_range(&node.range())),
+    };
 
     let file_id = def.file_id(db);
     let source_tree = sema.parse(file_id);
     let def_node = def.source(db, &source_tree)?.value;
 
-    let mut name_range = def_node.range();
-    let inner_name_range = match TSKind::from(def_node) {
+    let name_range = find_inner_name_range(&def_node);
+
+    let target_preprocessing_results = sema.preprocess_file(file_id);
+    let target_offsets = target_preprocessing_results.offsets();
+    let navs = vec![NavigationTarget {
+        file_id,
+        full_range: s_range_to_u_range(target_offsets, ts_range_to_lsp_range(&def_node.range())),
+        focus_range: s_range_to_u_range(target_offsets, name_range).into(),
+    }];
+
+    RangeInfo::new(u_range, navs).into()
+}
+
+/// Find the range of the inner name node of a definition node if there is one.
+/// Otherwise, return the range of the definition node.
+fn find_inner_name_range(node: &tree_sitter::Node) -> lsp_types::Range {
+    let name_range = match TSKind::from(node) {
         TSKind::methodmap_property_native | TSKind::methodmap_property_method => {
-            def_node.children(&mut def_node.walk()).find_map(|child| {
+            node.children(&mut node.walk()).find_map(|child| {
                 if matches!(
                     TSKind::from(child),
                     TSKind::methodmap_property_getter | TSKind::methodmap_property_setter
@@ -100,20 +77,36 @@ pub(crate) fn goto_definition(
                 }
             })
         }
-        _ => def_node
+        _ => node
             .child_by_field_name("name")
             .map(|name_node| name_node.range()),
-    };
-    if let Some(inner_name_range) = inner_name_range {
-        name_range = inner_name_range;
     }
+    .unwrap_or_else(|| node.range());
 
-    let target_preprocessing_results = sema.preprocess_file(file_id);
-    let target_offsets = target_preprocessing_results.offsets();
+    ts_range_to_lsp_range(&name_range)
+}
+
+/// Try to find the definition of a macro at the given position.
+fn find_macro_def(
+    offsets: &FxHashMap<u32, Vec<Offset>>,
+    pos: &lsp_types::Position,
+    sema: &Semantics<RootDatabase>,
+) -> Option<RangeInfo<Vec<NavigationTarget>>> {
+    let offset = offsets
+        .get(&pos.line)
+        .and_then(|offsets| offsets.iter().find(|offset| offset.contains(*pos)))?;
+    let def = sema
+        .find_macro_def(offset.file_id, offset.idx)
+        .map(DefResolution::from)?;
+    let file_id = def.file_id(sema.db);
+    let u_range = offset.range;
+    let source_tree = sema.parse(file_id);
+    let def_node = def.source(sema.db, &source_tree)?.value;
+    let name_range = find_inner_name_range(&def_node);
     let navs = vec![NavigationTarget {
         file_id,
-        full_range: s_range_to_u_range(target_offsets, ts_range_to_lsp_range(&def_node.range())),
-        focus_range: s_range_to_u_range(target_offsets, ts_range_to_lsp_range(&name_range)).into(),
+        full_range: ts_range_to_lsp_range(&def_node.range()),
+        focus_range: name_range.into(),
     }];
 
     RangeInfo::new(u_range, navs).into()

crates/ide/src/lib.rs

@@ -17,8 +17,9 @@ use hir_def::{print_item_tree, DefDatabase};
 use hover::HoverResult;
 use ide_db::RootDatabase;
 use itertools::Itertools;
-use preprocessor::{db::PreprocDatabase, Offset};
+use preprocessor::{db::PreprocDatabase, ArgsMap, Offset};
 use salsa::{Cancelled, ParallelDatabase};
+use syntax::range_contains_pos;
 use vfs::FileId;
 
 pub use goto_definition::NavigationTarget;
@@ -219,24 +220,51 @@ impl Analysis {
 }
 
 /// Convert a position seen by the user to a position seen by the server (preprocessed).
+///
+/// Will try to look for a mapped range of a macro argument and return the offsetted position.
+/// If no mapped range is found, will try to apply the offsets to the position.
+///
+/// # Arguments
+///
+/// * `args_map` - The preprocessed arguments map.
+/// * `offsets` - The preprocessed offsets.
+/// * `pos` - The position to convert.
+///
+/// # Returns
+///
+/// The source position range, if a mapped range was found.
 fn u_pos_to_s_pos(
+    args_map: &ArgsMap,
     offsets: &FxHashMap<u32, Vec<Offset>>,
-    u_pos: lsp_types::Position,
-) -> lsp_types::Position {
-    if let Some(diff) = offsets.get(&u_pos.line).map(|offsets| {
-        offsets
-            .iter()
-            .filter(|offset| offset.range.end.character <= u_pos.character)
-            .map(|offset| offset.diff)
-            .sum::<i32>()
+    pos: &mut lsp_types::Position,
+) -> Option<lsp_types::Range> {
+    let mut source_u_range = None;
+
+    match args_map.get(&pos.line).and_then(|args| {
+        args.iter()
+            .find(|(range, _)| range_contains_pos(range, pos))
     }) {
-        lsp_types::Position {
-            line: u_pos.line,
-            character: u_pos.character.saturating_add_signed(diff),
+        Some((u_range, s_range)) => {
+            *pos = s_range.start;
+            source_u_range = Some(*u_range);
+        }
+        None => {
+            if let Some(diff) = offsets.get(&pos.line).map(|offsets| {
+                offsets
+                    .iter()
+                    .filter(|offset| offset.range.end.character <= pos.character)
+                    .map(|offset| offset.diff.saturating_sub_unsigned(offset.args_diff))
+                    .sum::<i32>()
+            }) {
+                *pos = lsp_types::Position {
+                    line: pos.line,
+                    character: pos.character.saturating_add_signed(diff),
+                };
+            }
         }
-    } else {
-        u_pos
     }
+
+    source_u_range
 }
 
 /// Convert a range seen by the server to a range seen by the user.
@@ -247,13 +275,18 @@ fn s_range_to_u_range(
     if let Some(offsets) = offsets.get(&s_range.start.line) {
         for offset in offsets.iter() {
             if offset.range.start.character < s_range.start.character {
-                s_range.start.character =
-                    s_range.start.character.saturating_add_signed(-offset.diff);
+                s_range.start.character = s_range
+                    .start
+                    .character
+                    .saturating_add_signed(-offset.diff.saturating_sub_unsigned(offset.args_diff));
             }
         }
         for offset in offsets.iter() {
             if offset.range.start.character < s_range.end.character {
-                s_range.end.character = s_range.end.character.saturating_add_signed(-offset.diff);
+                s_range.end.character = s_range
+                    .end
+                    .character
+                    .saturating_add_signed(-offset.diff.saturating_sub_unsigned(offset.args_diff));
             }
         }
     }

crates/preprocessor/src/evaluator.rs

@@ -2,25 +2,24 @@ use std::sync::Arc;
 
 use fxhash::{FxHashMap, FxHashSet};
 use lsp_types::{Position, Range};
-use sourcepawn_lexer::{Literal, Operator, Symbol, TokenKind};
+use sourcepawn_lexer::{Literal, Operator, TokenKind};
 use vfs::FileId;
 
 use super::{
     errors::{EvaluationError, ExpansionError, MacroNotFoundError},
     macros::expand_identifier,
     preprocessor_operator::PreOperator,
 };
-use crate::{extend_args_map, ArgsMap, Macro, MacrosMap, Offset};
+use crate::{Macro, MacrosMap, Offset, Token};
 
 #[derive(Debug)]
 pub struct IfCondition<'a> {
-    pub symbols: Vec<Symbol>,
+    pub tokens: Vec<Token>,
     pub(super) macro_not_found_errors: Vec<MacroNotFoundError>,
     macro_store: &'a mut MacrosMap,
-    expansion_stack: Vec<Symbol>,
+    expansion_stack: Vec<Token>,
     line_nb: u32,
     offsets: &'a mut FxHashMap<u32, Vec<Offset>>,
-    args_map: &'a mut ArgsMap,
     disabled_macros: &'a mut FxHashSet<Arc<Macro>>,
 }
 
@@ -29,17 +28,15 @@ impl<'a> IfCondition<'a> {
         macro_store: &'a mut MacrosMap,
         line_nb: u32,
         offsets: &'a mut FxHashMap<u32, Vec<Offset>>,
-        args_map: &'a mut ArgsMap,
         disabled_macros: &'a mut FxHashSet<Arc<Macro>>,
     ) -> Self {
         Self {
-            symbols: vec![],
+            tokens: vec![],
             macro_not_found_errors: vec![],
             macro_store,
             expansion_stack: vec![],
             line_nb,
             offsets,
-            args_map,
             disabled_macros,
         }
     }
@@ -62,12 +59,15 @@ impl<'a> IfCondition<'a> {
             Position::new(self.line_nb, 1000),
         );
         let mut symbol_iter = self
-            .symbols
+            .tokens
             .clone() // TODO: This is horrible.
             .into_iter()
+            .map(|token| token.symbol().to_owned())
             .peekable();
         while let Some(symbol) = if !self.expansion_stack.is_empty() {
-            self.expansion_stack.pop()
+            self.expansion_stack
+                .pop()
+                .map(|token| token.symbol().to_owned())
         } else {
             let symbol = symbol_iter.next();
             if let Some(symbol) = &symbol {
@@ -207,13 +207,12 @@ impl<'a> IfCondition<'a> {
                         match expand_identifier(
                             &mut symbol_iter,
                             self.macro_store,
-                            &symbol,
+                            &symbol.clone().into(),
                             &mut self.expansion_stack,
                             false,
                             self.disabled_macros
                         ) {
-                            Ok((args_map, args_diff)) => {
-                                extend_args_map(self.args_map, args_map);
+                            Ok(args_diff) => {
                                 if let Some((idx, file_id)) = attr {
                                     self.offsets.entry(symbol.range.start.line).or_default().push(Offset {
                                         file_id,