-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: 修复插件指令注解为联合类型时处理异常的问题 #2925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
你好 - 我已审阅你的更改 - 以下是一些反馈意见:
- 考虑在构建
handler_params时缓存 UnionType 检测(例如is_union_type标志),以避免在每次参数转换期间重复调用 typing.get_origin。 - 对于包含多个非 None 类型的联合类型,你可能需要实现一个简单的类型匹配策略或显式回退,而不是始终返回原始字符串,以避免静默不匹配。
Prompt for AI Agents
请处理此代码审查中的评论:
## 总体评论
- 考虑在构建 `handler_params` 时缓存 UnionType 检测(例如 `is_union_type` 标志),以避免在每次参数转换期间重复调用 typing.get_origin。
- 对于包含多个非 None 类型的联合类型,你可能需要实现一个简单的类型匹配策略或显式回退,而不是始终返回原始字符串,以避免静默不匹配。
## 单独评论
### 评论 1
<location> `astrbot/core/star/filter/command.py:54-56` </location>
<code_context>
- if isinstance(v, type):
+ if isinstance(v, Type):
result += f"{k}({v.__name__}),"
+ elif isinstance(v, (types.UnionType, typing.Union)):
+ result += f"{k}({v}),"
else:
</code_context>
<issue_to_address>
**建议:** 打印联合类型可能无法产生可读的输出。
考虑格式化联合类型以清晰地显示其组成类型,而不是依赖其默认的字符串表示形式。
```suggestion
import typing
import types
for k, v in self.handler_params.items():
if isinstance(v, Type):
result += f"{k}({v.__name__}),"
elif isinstance(v, (types.UnionType, typing.Union)):
# Format Union types to display their constituent types
if hasattr(v, "__args__"):
type_names = ", ".join(
t.__name__ if hasattr(t, "__name__") else str(t)
for t in v.__args__
)
result += f"{k}(Union[{type_names}]),"
else:
result += f"{k}(Union),"
```
</issue_to_address>
### 评论 2
<location> `astrbot/core/star/filter/command.py:114` </location>
<code_context>
if i >= len(params):
if (
- isinstance(param_type_or_default_val, Type)
+ isinstance(param_type_or_default_val, (Type, types.UnionType))
or param_type_or_default_val is inspect.Parameter.empty
):
</code_context>
<issue_to_address>
**建议:** 使用 types.UnionType 进行类型检查可能无法覆盖所有联合类型情况。
types.UnionType 仅匹配 Python 3.10+ 中使用 | 语法的联合类型。为了完全兼容,也请检查 typing.Union。
建议的实现:
```python
elif isinstance(v, (types.UnionType, typing.Union)):
result += f"{k}({v}),"
```
```python
if (
isinstance(param_type_or_default_val, (Type, types.UnionType, typing.Union))
or param_type_or_default_val is inspect.Parameter.empty
):
```
```python
origin = typing.get_origin(param_type_or_default_val)
if origin in (typing.Union, types.UnionType):
# 注解是联合类型
# NOTE: 目前没有处理联合类型嵌套相关的注解写法
nn_types = unwrap_optional(param_type_or_default_val)
```
如果文件或代码库中还有其他地方执行联合类型检查,请确保它们也包含 `types.UnionType` 和 `typing.Union` 以实现完全兼容。
</issue_to_address>
### 评论 3
<location> `astrbot/core/star/filter/command.py:151-27` </location>
<code_context>
result[param_name] = float(params[i])
else:
- result[param_name] = param_type_or_default_val(params[i])
+ origin = typing.get_origin(param_type_or_default_val)
+ if origin in (typing.Union, types.UnionType):
+ # 注解是联合类型
+ # NOTE: 目前没有处理联合类型嵌套相关的注解写法
+ nn_types = unwrap_optional(param_type_or_default_val)
+ if len(nn_types) == 1:
+ # 只有一个非 NoneType 类型
+ result[param_name] = nn_types[0](params[i])
+ else:
+ # 没有或者有多个非 NoneType 类型,这里我们暂时直接赋值为原始值。
+ # NOTE: 目前还没有做类型校验
</code_context>
<issue_to_address>
**问题:** 联合类型的类型转换可能导致意外行为。
对于具有多个选项的联合类型,应添加显式类型验证,以防止静默类型不匹配并维护类型安全。
</issue_to_address>帮助我变得更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Consider caching the UnionType detection (e.g.
is_union_typeflag) when you buildhandler_paramsto avoid repeated calls to typing.get_origin during each parameter conversion. - For unions with multiple non-None types, you may want to implement a simple type‐matching strategy or explicit fallback rather than always returning the raw string to avoid silent mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider caching the UnionType detection (e.g. `is_union_type` flag) when you build `handler_params` to avoid repeated calls to typing.get_origin during each parameter conversion.
- For unions with multiple non-None types, you may want to implement a simple type‐matching strategy or explicit fallback rather than always returning the raw string to avoid silent mismatches.
## Individual Comments
### Comment 1
<location> `astrbot/core/star/filter/command.py:54-56` </location>
<code_context>
- if isinstance(v, type):
+ if isinstance(v, Type):
result += f"{k}({v.__name__}),"
+ elif isinstance(v, (types.UnionType, typing.Union)):
+ result += f"{k}({v}),"
else:
</code_context>
<issue_to_address>
**suggestion:** Printing Union types may not yield readable output.
Consider formatting Union types to display their constituent types clearly, rather than relying on their default string representation.
```suggestion
import typing
import types
for k, v in self.handler_params.items():
if isinstance(v, Type):
result += f"{k}({v.__name__}),"
elif isinstance(v, (types.UnionType, typing.Union)):
# Format Union types to display their constituent types
if hasattr(v, "__args__"):
type_names = ", ".join(
t.__name__ if hasattr(t, "__name__") else str(t)
for t in v.__args__
)
result += f"{k}(Union[{type_names}]),"
else:
result += f"{k}(Union),"
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/star/filter/command.py:114` </location>
<code_context>
if i >= len(params):
if (
- isinstance(param_type_or_default_val, Type)
+ isinstance(param_type_or_default_val, (Type, types.UnionType))
or param_type_or_default_val is inspect.Parameter.empty
):
</code_context>
<issue_to_address>
**suggestion:** Type checking with types.UnionType may not cover all Union cases.
types.UnionType only matches unions using the | syntax in Python 3.10+. For full compatibility, also check for typing.Union.
Suggested implementation:
```python
elif isinstance(v, (types.UnionType, typing.Union)):
result += f"{k}({v}),"
```
```python
if (
isinstance(param_type_or_default_val, (Type, types.UnionType, typing.Union))
or param_type_or_default_val is inspect.Parameter.empty
):
```
```python
origin = typing.get_origin(param_type_or_default_val)
if origin in (typing.Union, types.UnionType):
# 注解是联合类型
# NOTE: 目前没有处理联合类型嵌套相关的注解写法
nn_types = unwrap_optional(param_type_or_default_val)
```
If there are other places in the file or codebase where Union type checks are performed, ensure they also include both `types.UnionType` and `typing.Union` for full compatibility.
</issue_to_address>
### Comment 3
<location> `astrbot/core/star/filter/command.py:151-27` </location>
<code_context>
result[param_name] = float(params[i])
else:
- result[param_name] = param_type_or_default_val(params[i])
+ origin = typing.get_origin(param_type_or_default_val)
+ if origin in (typing.Union, types.UnionType):
+ # 注解是联合类型
+ # NOTE: 目前没有处理联合类型嵌套相关的注解写法
+ nn_types = unwrap_optional(param_type_or_default_val)
+ if len(nn_types) == 1:
+ # 只有一个非 NoneType 类型
+ result[param_name] = nn_types[0](params[i])
+ else:
+ # 没有或者有多个非 NoneType 类型,这里我们暂时直接赋值为原始值。
+ # NOTE: 目前还没有做类型校验
</code_context>
<issue_to_address>
**issue:** Type conversion for Union types may lead to unexpected behavior.
Explicit type validation should be added for Union types with multiple options to prevent silent type mismatches and maintain type safety.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where plugin command annotations with union types (like str | None or Union[str, None]) without default values would cause runtime errors when invoking the plugin, specifically "'types.UnionType' object is not callable" exceptions.
- Adds support for handling union type annotations in command parameter processing
- Introduces a helper function to unwrap Optional/Union types to extract underlying types
- Extends validation logic to properly handle both
typing.Unionand PEP 604 union types
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Motivation / 动机
当前插件如果使用联合类型来作为指令的参数的注解并且未提供默认值,在调用该插件时,可能会造成报错。表现为:
'types.UnionType' object is not callable等异常。Modifications / 改动点
增加对联合类型的处理支持。
需要注意的是,由于目前 handler_params 的值可能是 Type 也可能是 types.UnionType 也可能是 typing.Union 也可能是具体基本类型的值,因此在动态处理时,可能会多次调用
origin = typing.get_origin(param_type_or_default_val),这个可以通过重构 handler_params 来避免,在初次加载的时候附上is_union_type的 flag。Verification Steps / 验证步骤
Screenshots or Test Results / 运行截图或测试结果
Compatibility & Breaking Changes / 兼容性与破坏性变更
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Sourcery 总结
修复命令过滤器,使其能正确处理参数的
Union和Optional类型注解,防止联合类型导致的调用错误。错误修复:
Union或Optional时出现的“types.UnionType object is not callable”错误功能增强:
unwrap_optional辅助函数,用于从Optional/Union注解中提取底层类型print_types和验证逻辑,以在命令处理器中支持typing.Union和 PEP 604 联合类型Original summary in English
Summary by Sourcery
Fix command filter to correctly handle Union and Optional type annotations for parameters, preventing callable errors with union types
Bug Fixes:
Enhancements: