Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 142 additions & 27 deletions rust/lithe-core/src/languages/spring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,7 @@ macro_rules! spring_annotations {
#[cfg(test)]
pub(crate) const ALL: &'static [Self] = &[$(Self::$variant),+];

#[cfg(test)]
pub(crate) fn name(self) -> &'static str {
match self {
$(Self::$variant => $name),+
Expand Down Expand Up @@ -1278,9 +1279,10 @@ spring_mapping_annotations! {
}

impl SpringAnnotation {
/// Annotations that declare a Spring component on a type declaration. The
/// order also drives the alternation in [`component_name`], so changing it
/// changes which annotation wins on a type carrying several of them.
/// Annotations that declare a Spring component on a type declaration.
/// Detection and naming share this closed set so they cannot drift; when
/// several are present, the leftmost source match wins rather than this
/// array's declaration order.
const COMPONENTS: [Self; 6] = [
Self::Component,
Self::Service,
Expand All @@ -1304,24 +1306,31 @@ fn has_component_annotation(context: &str) -> bool {
.any(|annotation| annotation.is_present(context))
}

/// Returns the explicit name of the earliest component annotation in source
/// order. The value is read only from that annotation's own argument list so a
/// later neighbor such as `@Component("c")` cannot pollute `@Service("s")`.
/// An empty string such as `@Component("")` is not a name; callers then use
/// the default type name.
fn component_name(context: &str) -> Option<String> {
// Built from COMPONENTS so the recognized set cannot diverge from the one
// has_component_annotation uses.
static ANNOTATION: LazyLock<Regex> = LazyLock::new(|| {
let alternation = SpringAnnotation::COMPONENTS
.iter()
.map(|annotation| regex::escape(annotation.name()))
.collect::<Vec<_>>()
.join("|");
Regex::new(&format!(
r#"@({alternation})\s*\([^\)]*[\"']([^\"']+)[\"']"#
))
.expect("escaped annotation names produce a valid pattern")
});
ANNOTATION
.captures(context)
.and_then(|capture| capture.get(2))
.map(|value| value.as_str().to_string())
let start = earliest_component_annotation_start(context)?;
quoted_values(isolate_annotation_at(context, start))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里改用 quoted_values(...).next() 后,会接受空字符串,因为 quoted_values 的捕获组使用了 *。因此 @Component("") 返回 Some(""),bean_index 中的 unwrap_or(default_name) 不会执行,最终生成空 Bean 名和异常 ID。旧实现的捕获组使用 +,该场景会返回 None 并回退到类名。请过滤空值(或使用组件专用的非空提取逻辑),并补充 @Component("") 的单元与 spring.index 集成测试。

.into_iter()
.find(|value| !value.is_empty())
}

/// Locates every exact component annotation with the cached boundary patterns
/// and keeps the leftmost source start. Array order in [`SpringAnnotation::COMPONENTS`]
/// is not a priority.
fn earliest_component_annotation_start(context: &str) -> Option<usize> {
SpringAnnotation::COMPONENTS
.iter()
.filter_map(|annotation| {
annotation
.pattern()
.find(context)
.map(|found| found.start())
})
.min()
}

fn qualifier_names(context: &str) -> Vec<String> {
Expand All @@ -1335,13 +1344,16 @@ fn qualifier_names(context: &str) -> Vec<String> {
.collect()
}

/// Returns declared `@Bean` aliases from the exact `@Bean` annotation only.
/// A prefix decoy such as `@BeanFactory("decoy")` cannot supply the name.
fn bean_names(context: &str) -> Vec<String> {
let Some(start) = context.find("@Bean") else {
let Some(found) = SpringAnnotation::Bean.pattern().find(context) else {
return Vec::new();
};
let remaining = &context[start..];
let end = remaining.find(')').unwrap_or(remaining.len());
quoted_values(&remaining[..end])
quoted_values(isolate_annotation_at(context, found.start()))
.into_iter()
.filter(|value| !value.is_empty())
.collect()
}

fn quoted_values(value: &str) -> Vec<String> {
Expand Down Expand Up @@ -1573,11 +1585,12 @@ fn find_mapping_annotation(text: &str) -> Option<(SpringMappingAnnotation, &str)
}

/// Slices one annotation starting at `start` (`@Name` followed by an optional
/// argument list) so route extraction cannot read string literals from a
/// neighboring decoy.
/// argument list) so name or route extraction cannot read string literals from
/// a neighboring decoy.
///
/// A later annotation's `(` is not this annotation's argument list. After the
/// name, only whitespace may appear before `(`.
/// name, only whitespace may appear before `(`. Parentheses inside quoted
/// strings, including escaped quotes, do not change the argument-list depth.
fn isolate_annotation_at(text: &str, start: usize) -> &str {
let rest = &text[start..];
let name_end = rest
Expand All @@ -1600,8 +1613,25 @@ fn isolate_annotation_at(text: &str, start: usize) -> &str {
}
let open_index = name_end + whitespace_len;
let mut depth = 0isize;
let mut in_string = None;
let mut escaped = false;
for (index, character) in rest[open_index..].char_indices() {
if let Some(quote) = in_string {
if escaped {
escaped = false;
continue;
}
if character == '\\' {
escaped = true;
continue;
}
if character == quote {
in_string = None;
}
continue;
}
match character {
'"' | '\'' => in_string = Some(character),
'(' => depth += 1,
')' => {
depth -= 1;
Expand Down Expand Up @@ -1786,6 +1816,91 @@ mod tests {
assert_eq!(component_name("@Qualifier(\"custom\")"), None);
}

/// `@BeanFactory` shares a prefix with `@Bean`, so a naive `@Bean` search
/// would take the decoy name. Naming must reuse the cached boundary match.
#[test]
fn bean_names_uses_the_exact_bean_annotation_not_a_prefix() {
assert_eq!(
bean_names(r#"@BeanFactory("decoy") @Bean("real")"#),
vec!["real".to_string()]
);
assert_eq!(bean_names(r#"@BeanFactory("decoy")"#), Vec::<String>::new());
assert_eq!(bean_names(r#"@Bean("real")"#), vec!["real".to_string()]);
assert_eq!(bean_names("@Bean"), Vec::<String>::new());
assert_eq!(
bean_names(r#"@BeanFactory("decoy") @Bean"#),
Vec::<String>::new()
);
}

/// A `(` inside a string is not the annotation argument list. Counting it
/// would swallow the real closer and pull a later neighbor into the aliases.
#[test]
fn isolate_annotation_at_ignores_parentheses_inside_strings() {
assert_eq!(
isolate_annotation_at(r#"@Bean("foo(") @Bean("bar")"#, 0),
r#"@Bean("foo(")"#
);
assert_eq!(
isolate_annotation_at(r#"@Bean('foo(') @Service("s")"#, 0),
r#"@Bean('foo(')"#
);
assert_eq!(
isolate_annotation_at(r#"@Bean("foo\")") @Bean("bar")"#, 0),
r#"@Bean("foo\")")"#
);
assert_eq!(
bean_names(r#"@Bean("foo(") @Bean("bar")"#),
vec!["foo(".to_string()]
);
assert_eq!(
component_name(r#"@Service("s(") @Component("c")"#).as_deref(),
Some("s(")
);
let mapping_with_paren = mapping(r#"@GetMapping("/foo(") @PostMapping("/bar")"#)
.expect("GetMapping with a parenthesis in its route should still isolate");
assert_eq!(mapping_with_paren.0, SpringMappingAnnotation::GetMapping);
assert_eq!(mapping_with_paren.2, vec!["/foo(".to_string()]);
}

/// A wide capture can start at the first closing quote and swallow
/// `) @Component(`. Isolation plus source-position selection must keep
/// the first annotation's own value, independent of COMPONENTS order.
#[test]
fn component_name_selects_the_earliest_annotation_and_isolates_its_value() {
assert_eq!(
component_name(r#"@Service("s") @Component("c")"#).as_deref(),
Some("s")
);
assert_eq!(
component_name(r#"@Component("c") @Service("s")"#).as_deref(),
Some("c")
);
assert_ne!(
Comment thread
1lck marked this conversation as resolved.
component_name(r#"@Service("s") @Component("c")"#).as_deref(),
Some(") @Component(")
);
assert_eq!(component_name("@Service @Component"), None);
assert_eq!(component_name(r#"@Service @Component("c")"#), None);
assert_eq!(component_name("@Service"), None);
assert_eq!(component_name("@ServiceLocator(\"x\")"), None);
}

/// `quoted_values` accepts empty captures. An empty annotation value is not
/// an explicit bean name, so naming must return None and let bean_index use
/// the default type or method name.
#[test]
fn empty_annotation_values_are_not_explicit_bean_names() {
assert_eq!(component_name(r#"@Component("")"#), None);
assert_eq!(component_name(r#"@Service('')"#), None);
assert_eq!(
component_name(r#"@Component("") @Service("s")"#),
None,
"an empty leftmost name must not take a later neighbor"
);
assert_eq!(bean_names(r#"@Bean("")"#), Vec::<String>::new());
}

/// Record components and constructor parameters share one pattern, so a
/// declaration parsed by one path must be parsed identically by the other.
#[test]
Expand Down
134 changes: 134 additions & 0 deletions rust/lithe-core/src/tests/spring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,140 @@ public class RealConfig {
fs::remove_dir_all(root).expect("Spring fixture should be removable");
}

/// Bean and component names must come from the exact annotation's own
/// argument list. A prefix decoy or a later neighbor cannot supply the name.
#[test]
fn spring_index_isolates_bean_and_component_names_from_neighbor_annotations() {
Comment thread
1lck marked this conversation as resolved.
let root = temporary_root("spring-bean-name-isolation");
let java = root.join("src/main/java/demo");
fs::create_dir_all(&java).expect("Java fixture directory should be creatable");
fs::write(
java.join("ClockConfig.java"),
r#"package demo;
@Configuration
public class ClockConfig {
@BeanFactory("decoy") @Bean("real")
public Clock clock() { return null; }
@Bean("foo(") @Bean("bar")
public Clock parenClock() { return null; }
@BeanFactory("decoyOnly")
public Clock decoyClock() { return null; }
@Bean
public Clock unnamedClock() { return null; }
}
"#,
)
.expect("bean name fixture should be writable");
fs::write(
java.join("Demo.java"),
"package demo;\n@Service(\"s\") @Component(\"c\")\npublic class Demo {}\n",
)
.expect("component name fixture should be writable");
fs::write(
java.join("Ordered.java"),
"package demo;\n@Component(\"c\") @Service(\"s\")\npublic class Ordered {}\n",
)
.expect("reversed component fixture should be writable");

let paths = [
"src/main/java/demo/ClockConfig.java",
"src/main/java/demo/Demo.java",
"src/main/java/demo/Ordered.java",
];
let response = execute_spring(&root, &paths, serde_json::json!({}));
assert_eq!(response["ok"], true, "{response}");

let beans = response["data"]["beans"].as_array().unwrap();
let names = beans
.iter()
.map(|value| value["name"].as_str().unwrap())
.collect::<Vec<_>>();
assert!(names.contains(&"real"), "{response}");
assert!(names.contains(&"foo("), "{response}");
assert!(names.contains(&"unnamedClock"), "{response}");
assert!(names.contains(&"clockConfig"), "{response}");
assert!(names.contains(&"s"), "{response}");
assert!(names.contains(&"c"), "{response}");
assert!(!names.contains(&"decoy"), "{response}");
assert!(!names.contains(&"decoyOnly"), "{response}");
assert!(!names.contains(&"decoyClock"), "{response}");
assert!(!names.contains(&"bar"), "{response}");
assert!(!names.contains(&") @Component("), "{response}");

let demo = beans
.iter()
.find(|value| value["typeName"] == "Demo")
.unwrap_or_else(|| panic!("missing Demo bean: {response}"));
assert_eq!(demo["name"], "s");
let ordered = beans
.iter()
.find(|value| value["typeName"] == "Ordered")
.unwrap_or_else(|| panic!("missing Ordered bean: {response}"));
assert_eq!(ordered["name"], "c");
let clock = beans
.iter()
.find(|value| value["name"] == "real" && value["kind"] == "beanMethod")
.unwrap_or_else(|| panic!("missing named Clock bean: {response}"));
assert_eq!(clock["typeName"], "Clock");
let paren = beans
.iter()
.find(|value| value["name"] == "foo(" && value["kind"] == "beanMethod")
.unwrap_or_else(|| panic!("missing parenthesis Clock bean: {response}"));
assert_eq!(paren["typeName"], "Clock");

fs::remove_dir_all(root).expect("Spring fixture should be removable");
}

/// An empty `@Component("")` value is not a bean name. Indexing must fall back
/// to the default type name instead of recording an empty id.
#[test]
fn spring_index_falls_back_when_a_component_name_is_an_empty_string() {
let root = temporary_root("spring-empty-component-name");
let java = root.join("src/main/java/demo");
fs::create_dir_all(&java).expect("Java fixture directory should be creatable");
fs::write(
java.join("EmptyName.java"),
"package demo;\n@Component(\"\")\npublic class EmptyName {}\n",
)
.expect("empty component fixture should be writable");
fs::write(
java.join("Neighbor.java"),
"package demo;\n@Component(\"\") @Service(\"s\")\npublic class Neighbor {}\n",
)
.expect("empty-then-neighbor fixture should be writable");

let response = execute_spring(
&root,
&[
"src/main/java/demo/EmptyName.java",
"src/main/java/demo/Neighbor.java",
],
serde_json::json!({}),
);
assert_eq!(response["ok"], true, "{response}");

let beans = response["data"]["beans"].as_array().unwrap();
let empty_name = beans
.iter()
.find(|value| value["typeName"] == "EmptyName")
.unwrap_or_else(|| panic!("missing EmptyName bean: {response}"));
assert_eq!(empty_name["name"], "emptyName");
assert!(
empty_name["id"].as_str().unwrap().ends_with(":emptyName"),
"{response}"
);
let neighbor = beans
.iter()
.find(|value| value["typeName"] == "Neighbor")
.unwrap_or_else(|| panic!("missing Neighbor bean: {response}"));
assert_eq!(neighbor["name"], "neighbor");
assert_ne!(empty_name["name"], "");
assert_ne!(neighbor["name"], "");
assert_ne!(neighbor["name"], "s");

fs::remove_dir_all(root).expect("Spring fixture should be removable");
}

/// Custom annotations that only share a Mapping prefix must not become
/// endpoints or class-level base routes; exact Spring Mapping names still do.
#[test]
Expand Down
Loading