fix(mybatis): skip method-level annotations when parsing method declarations (#1759) (#1793)

Signed-off-by: Inf1mum <[email protected]>
Co-authored-by: arookieofc <[email protected]>

plugins/official/mybatis/dist/plugin.wasm.sources

@@ -1 +1 @@
-b3c715c1d3489db1c9f6067f66aa31623ffd23ac6ad9d9fb9cc3b5d548088473
+a6d1331afb12c8b215d8dc4450b8c4a424a6951b359dadb925da5ef0c52ecedc

plugins/official/mybatis/src/lib.rs

@@ -70,51 +70,156 @@ fn ident(c: char) -> bool {
     c.is_alphanumeric() || c == '_' || c == '$'
 }
 
-/// Index of the `(` opening a method's parameter list inside a declaration
-/// fragment. `(`s that belong to annotation arguments — `@Anno(…)` — are
-/// skipped so `@DataPermission(column = "id") int count()` finds the paren
-/// after `count`, not the one after `DataPermission`.
-fn method_paren(stmt: &str) -> Option<usize> {
-    let mut rest = stmt;
-    let mut base = 0;
-    while let Some(i) = rest.find('(') {
-        // Is the token right before this `(` an annotation name (`@Anno`, `@a.b.Anno`)?
-        let before = rest[..i].trim_end();
-        let annotation = before.rfind('@').is_some_and(|a| before[a + 1..].chars().all(|c| ident(c) || c == '.'));
-        if !annotation {
-            return Some(base + i);
+/// The index just past the string or char literal opening at `bytes[i]`.
+fn skip_string(bytes: &[u8], mut i: usize) -> usize {
+    let quote = bytes[i];
+    i += 1;
+    while i < bytes.len() {
+        match bytes[i] {
+            b'\\' => i += 2,
+            b if b == quote => return i + 1,
+            _ => i += 1,
         }
-        // Skip the annotation's argument list (it may nest and hold literals).
-        let mut depth = 1usize;
-        let mut end = None;
-        let mut chars = rest[i + 1..].char_indices().peekable();
-        while let Some((j, c)) = chars.next() {
-            match c {
-                '"' | '\'' => {
-                    for (_, d) in chars.by_ref() {
-                        if d == c {
-                            break;
-                        }
-                    }
+    }
+    i
+}
+
+/// The index just past the `//…` or `/*…*/` comment opening at `bytes[i]`.
+fn skip_comment(bytes: &[u8], mut i: usize) -> usize {
+    if bytes.get(i + 1) == Some(&b'/') {
+        bytes[i + 2..].iter().position(|&b| b == b'\n').map_or(bytes.len(), |n| i + 2 + n)
+    } else {
+        i += 2;
+        while i + 1 < bytes.len() && (bytes[i] != b'*' || bytes[i + 1] != b'/') {
+            i += 1;
+        }
+        (i + 2).min(bytes.len())
+    }
+}
+
+/// A method declaration in a fragment of an interface's body: the name's byte
+/// range and its parameter list's `(` and `)` (offsets in the fragment).
+#[derive(Debug, PartialEq, Eq)]
+struct Declaration {
+    name: (usize, usize),
+    parens: (usize, usize),
+}
+
+/// The method declaration in `stmt`, skipping comments, strings and annotations
+/// on the way: an annotation's arguments nest parentheses and may hold strings
+/// and comments, so the name is the identifier before the first `(` left over —
+/// `@Select("…") User find(…)` names `find`, not `Select`. Anything past that
+/// `(` (where `@Param(…)` lives) only matters for its matching `)`, so it never
+/// confuses the scan.
+fn scan_declaration(stmt: &str) -> Option<Declaration> {
+    let bytes = stmt.as_bytes();
+    let mut i = 0;
+    let mut nested = 0usize; // paren depth inside an annotation's arguments
+    while i < bytes.len() {
+        match bytes[i] {
+            b' ' | b'\t' | b'\r' | b'\n' => i += 1,
+            b'/' if matches!(bytes.get(i + 1), Some(b'/' | b'*')) => i = skip_comment(bytes, i),
+            b'"' | b'\'' => i = skip_string(bytes, i),
+            b'@' if nested == 0 => {
+                // The annotation's qualified name, then (optionally) its arguments.
+                i += 1;
+                while i < bytes.len() && (ident(bytes[i] as char) || bytes[i] == b'.') {
+                    i += 1;
                 }
-                '(' => depth += 1,
-                ')' => {
-                    depth -= 1;
-                    if depth == 0 {
-                        end = Some(i + 1 + j + 1);
-                        break;
-                    }
+                let mut j = i;
+                while j < bytes.len() && bytes[j].is_ascii_whitespace() {
+                    j += 1;
+                }
+                if bytes.get(j) == Some(&b'(') {
+                    nested = 1;
+                    i = j + 1;
                 }
-                _ => {}
             }
+            b'(' if nested > 0 => {
+                nested += 1;
+                i += 1;
+            }
+            b')' if nested > 0 => {
+                nested -= 1;
+                i += 1;
+            }
+            b'(' => {
+                let before = stmt[..i].trim_end();
+                let start = before.rfind(|c: char| !ident(c)).map_or(0, |p| p + 1);
+                if start == before.len() {
+                    return None;
+                }
+                // The matching `)`, minding strings and nested parens.
+                let (mut close, mut depth) = (i + 1, 1usize);
+                while close < bytes.len() {
+                    match bytes[close] {
+                        b'"' | b'\'' => close = skip_string(bytes, close),
+                        b'/' if matches!(bytes.get(close + 1), Some(b'/' | b'*')) => close = skip_comment(bytes, close),
+                        b'(' => {
+                            depth += 1;
+                            close += 1;
+                        }
+                        b')' => {
+                            depth -= 1;
+                            if depth == 0 {
+                                return Some(Declaration { name: (start, before.len()), parens: (i, close) });
+                            }
+                            close += 1;
+                        }
+                        _ => close += 1,
+                    }
+                }
+                return None; // an unterminated parameter list is no declaration
+            }
+            _ => i += 1,
         }
-        let skip = end?;
-        base += skip;
-        rest = &rest[skip..];
     }
     None
 }
 
+/// The parameter names in a declaration's parameter list: `@Param("…")` wins,
+/// otherwise the parameter's last identifier. Split at commas outside nested
+/// parens, braces, brackets and strings, so `@Size(min = 1, max = 5) String n`
+/// stays one parameter and a `@Param("a,b")` keeps its whole name.
+fn params_of(list: &str) -> Vec<String> {
+    let bytes = list.as_bytes();
+    let mut parts = Vec::new();
+    let (mut depth, mut start) = (0usize, 0usize);
+    let mut i = 0;
+    while i < bytes.len() {
+        match bytes[i] {
+            b'"' | b'\'' => i = skip_string(bytes, i),
+            b'/' if matches!(bytes.get(i + 1), Some(b'/' | b'*')) => i = skip_comment(bytes, i),
+            b'(' | b'{' | b'[' => {
+                depth += 1;
+                i += 1;
+            }
+            b')' | b'}' | b']' => {
+                depth = depth.saturating_sub(1);
+                i += 1;
+            }
+            b',' if depth == 0 => {
+                parts.push(&list[start..i]);
+                start = i + 1;
+                i += 1;
+            }
+            _ => i += 1,
+        }
+    }
+    parts.push(&list[start..]);
+    parts
+        .into_iter()
+        .filter_map(|p| {
+            // `@Param("id") long id` → the @Param name wins.
+            if let Some(q) = p.find("@Param(\"") {
+                return p[q + 8..].split('"').next().map(str::to_owned);
+            }
+            p.split_whitespace().last().map(|n| n.trim_matches(|c: char| !ident(c)).to_owned())
+        })
+        .filter(|n| !n.is_empty())
+        .collect()
+}
+
 /// The interface's name and methods (with their parameter names).
 pub fn parse_java(text: &str) -> MapperJava {
     let package =
@@ -140,28 +245,14 @@ pub fn parse_java(text: &str) -> MapperJava {
             }
             ';' if depth == 0 => {
                 let stmt = &text[stmt_start..k];
-                if let Some(paren) = method_paren(stmt) {
-                    let before = stmt[..paren].trim_end();
-                    let name_start = before.rfind(|c: char| !ident(c)).map_or(0, |p| p + 1);
-                    let method = &before[name_start..];
-                    if !method.is_empty() && !before.contains(" default ") {
-                        let s = stmt_start + name_start;
-                        let params = stmt[paren + 1..stmt.rfind(')').unwrap_or(stmt.len())]
-                            .split(',')
-                            .filter_map(|p| {
-                                // `@Param("id") long id` → the @Param name wins.
-                                if let Some(q) = p.find("@Param(\"") {
-                                    return p[q + 8..].split('"').next().map(str::to_owned);
-                                }
-                                p.split_whitespace().last().map(|n| n.trim_matches(|c: char| !ident(c)).to_owned())
-                            })
-                            .filter(|n| !n.is_empty())
-                            .collect();
-                        methods.push((
-                            Named { name: method.to_owned(), range: range_of(text, s, s + method.len()) },
-                            params,
-                        ));
-                    }
+                if let Some(decl) = scan_declaration(stmt)
+                    && !stmt[..decl.parens.0].contains(" default ")
+                {
+                    let (s, e) = (stmt_start + decl.name.0, stmt_start + decl.name.1);
+                    methods.push((
+                        Named { name: stmt[decl.name.0..decl.name.1].to_owned(), range: range_of(text, s, e) },
+                        params_of(&stmt[decl.parens.0 + 1..decl.parens.1]),
+                    ));
                 }
                 stmt_start = k + 1;
             }
@@ -372,6 +463,22 @@ mod tests {
         assert_eq!(xml.statements[0].range, Range::new(Position::new(2, 14), Position::new(2, 22)));
     }
 
+    const ANNOTATED: &str = concat!(
+        "package com.acme;\n\n",
+        "import org.apache.ibatis.annotations.Param;\n\n",
+        "public interface UserMapper {\n",
+        "    @Select(\"SELECT (a, b) FROM users WHERE c = ')' OR d = 'x, y'\")\n",
+        "    List<User> selectAnnotated(Long id);\n\n",
+        "    @Cacheable(key = @Key(value = \"x(y)\", of = {@Part(\"a)b\"), @Part(\"c,d\")}))\n",
+        "    @Override\n",
+        "    // a comment (with parens) before the name\n",
+        "    User findAnnotated(@Param(\"id\") Long id, @Size(min = 1, max = 5) String name);\n\n",
+        "    List<User> selectPlain(Long id);\n",
+        "}\n"
+    );
+
+    const ANNOTATED_XML: &str = "<?xml version=\"1.0\"?>\n<mapper namespace=\"com.acme.UserMapper\">\n  <select id=\"selectAnnotated\" resultType=\"User\">SELECT (a, b) FROM users</select>\n  <select id=\"findAnnotated\">SELECT 1</select>\n</mapper>\n";
+
     fn setup() -> MockHost {
         let host = MockHost::install();
         host.add_file("src/main/java/com/acme/UserMapper.java", JAVA);
@@ -380,6 +487,68 @@ mod tests {
         host
     }
 
+    fn annotated_setup() -> MockHost {
+        let host = MockHost::install();
+        host.add_file("src/main/java/com/acme/UserMapper.java", ANNOTATED);
+        host.add_file("src/main/resources/mapper/UserMapper.xml", ANNOTATED_XML);
+        host.add_file("pom.xml", "<project/>");
+        host
+    }
+
+    /// Methods behind method-level annotations keep their names, parameters and
+    /// positions: the scanner skips comments, strings and annotation arguments
+    /// (nested annotations, `(` `)` `{` `}` `,` in strings) before the name.
+    #[test]
+    fn parses_methods_with_annotations() {
+        let java = parse_java(ANNOTATED);
+        let names: Vec<_> = java.methods.iter().map(|(m, p)| (m.name.as_str(), p.clone())).collect();
+        assert_eq!(
+            names,
+            [
+                ("selectAnnotated", vec!["id".to_owned()]),
+                ("findAnnotated", vec!["id".to_owned(), "name".to_owned()],),
+                ("selectPlain", vec!["id".to_owned()])
+            ],
+            "annotation names must not become method names, @Param wins, argument commas must not split a parameter"
+        );
+        // `    List<User> ` before the name on its line (UTF-16 columns).
+        assert_eq!(java.methods[0].0.range, Range::new(Position::new(6, 15), Position::new(6, 30)));
+        assert_eq!(java.methods[1].0.range, Range::new(Position::new(11, 9), Position::new(11, 22)));
+    }
+
+    /// A multibyte annotation string on the name's line keeps UTF-16 columns:
+    /// `中文` is one unit each and `😀` two, while bytes would run ahead.
+    #[test]
+    fn keeps_utf16_columns_around_multibyte_annotations() {
+        let text = "package com.acme;\n\npublic interface UserMapper {\n    @Select(\"中文 😀 注释\") List<User> find(Long id);\n}\n";
+        let java = parse_java(text);
+        assert_eq!(java.methods.len(), 1);
+        assert_eq!(java.methods[0].0.name, "find");
+        // `"中文 😀 注释"` costs 10 UTF-16 units (😀 two); `find` starts at 4 + 8 + 1 + 10 + 1 + 11 = 35.
+        assert_eq!(java.methods[0].0.range, Range::new(Position::new(3, 35), Position::new(3, 39)));
+        assert_eq!(java.methods[0].1, ["id".to_owned()]);
+    }
+
+    /// Annotated methods still get both directions of navigation.
+    #[test]
+    fn navigates_annotated_methods() {
+        let _host = annotated_setup();
+        let mut n = Navigator::default();
+        let java = doc("/project/src/main/java/com/acme/UserMapper.java", "java", ANNOTATED);
+        let markers = n.line_markers(&java);
+        assert_eq!(markers.iter().map(|m| m.line).collect::<Vec<_>>(), [6, 11], "selectPlain has no statement");
+        assert_eq!(markers[1].tooltip, "Go to UserMapper.xml: findAnnotated");
+
+        let xml = doc("/project/src/main/resources/mapper/UserMapper.xml", "xml", ANNOTATED_XML);
+        let to_java = n.definition(&xml, Position::new(2, 20));
+        assert_eq!(to_java.len(), 1);
+        assert_eq!(
+            (to_java[0].path.ends_with("UserMapper.java"), to_java[0].range.start),
+            (true, Position::new(6, 15)),
+            "the XML statement jumps back to the annotated method"
+        );
+    }
+
     fn doc(path: &str, language: &str, text: &str) -> Document {
         Document { path: path.into(), language: language.into(), version: 1, text: text.into() }
     }