Skip to content

Conversation

@Soulter
Copy link
Member

@Soulter Soulter commented Oct 1, 2025


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 / 验证步骤

    @filter.command("t00")
    async def test00(self, event: AstrMessageEvent, p1=None):
        yield event.plain_result(f"test {p1}")

    @filter.command("t01")
    async def test01(self, event: AstrMessageEvent, p1="1"):
        yield event.plain_result(f"test {p1}")

    @filter.command("t02")
    async def yest11(self, event: AstrMessageEvent, p1=1):
        yield event.plain_result(f"test {p1}")

    @filter.command("t03")
    async def t03(self, event: AstrMessageEvent, p1=True):
        yield event.plain_result(f"test {p1}")

    @filter.command("t0")
    async def test0(self, event: AstrMessageEvent, p1: str = None):
        yield event.plain_result(f"test {p1}")

    @filter.command("t")
    async def test1(self, event: AstrMessageEvent, p1: str):
        yield event.plain_result(f"test {p1}")

    @filter.command("t2") # 有问题
    async def test2(self, event: AstrMessageEvent, p2: str | None):
        yield event.plain_result(f"test {p2}")

    @filter.command("t3")
    async def test3(self, event: AstrMessageEvent, p3: str = "1"):
        yield event.plain_result(f"test {p3}")

    @filter.command("test")
    async def test4(self, event: AstrMessageEvent, p4: str | None = "1"):
        yield event.plain_result(f"test {p4}")

    @filter.command("test5")
    async def test5(self, event: AstrMessageEvent, p5: str | None = None):
        yield event.plain_result(f"test {p5}")

    @filter.command("test51")
    async def test51(self, event: AstrMessageEvent, p6: Union[str, None]):
        yield event.plain_result(f"test {p6}")

    @filter.command("test6")
    async def test6(self, event: AstrMessageEvent, p6: Union[str, None] = "1"):
        yield event.plain_result(f"test {p6}")

    @filter.command("test7")
    async def test7(self, event: AstrMessageEvent, p7: Union[str, None] = None):
        yield event.plain_result(f"test {p7}")

    @filter.command("test8") # 有问题
    async def test71(self, event: AstrMessageEvent, p8: Optional[str]):
        yield event.plain_result(f"test {p8}")

    @filter.command("test8")
    async def test8(self, event: AstrMessageEvent, p8: Optional[str] = "1"):
        yield event.plain_result(f"test {p8}")

    @filter.command("test9")
    async def test9(self, event: AstrMessageEvent, p9: Optional[str] = None):
        yield event.plain_result(f"test {p9}")

    @filter.command("test10")
    async def test10(self, event: AstrMessageEvent, p10: int):
        yield event.plain_result(f"test {p10}")

    @filter.command("test10")
    async def test10(self, event: AstrMessageEvent, p10: int):
        yield event.plain_result(f"test {p10}")

    @filter.command("test11")
    async def test11(self, event: AstrMessageEvent, p11: int | None = None):
        yield event.plain_result(f"test {p11}")

    @filter.command("test13")
    async def test13(self, event: AstrMessageEvent, p11: int | None):
        yield event.plain_result(f"test {p11}")

    @filter.command("test12")
    async def test12(self, event: AstrMessageEvent, p12: int = None):
        yield event.plain_result(f"test {p12}")

    @filter.command("test121")
    async def test121(self, event: AstrMessageEvent, p12: int = "abc"):
        yield event.plain_result(f"test {p12}")

Screenshots or Test Results / 运行截图或测试结果

Compatibility & Breaking Changes / 兼容性与破坏性变更

  • 这是一个破坏性变更 (Breaking Change)。/ This is a breaking change.
  • 这不是一个破坏性变更。/ This is NOT a breaking change.

Checklist / 检查清单

  • 😊 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。/ If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  • 👀 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。/ My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  • 🤓 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到了 requirements.txtpyproject.toml 文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
  • 😮 我的更改没有引入恶意代码。/ My changes do not introduce malicious code.

Sourcery 总结

修复命令过滤器,使其能正确处理参数的 UnionOptional 类型注解,防止联合类型导致的调用错误。

错误修复:

  • 修复参数转换逻辑,以避免当注解使用 UnionOptional 时出现的“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:

  • Fix parameter conversion logic to avoid 'types.UnionType object is not callable' errors when annotations use Union or Optional

Enhancements:

  • Add unwrap_optional helper to extract underlying types from Optional/Union annotations
  • Extend print_types and validation logic to support typing.Union and PEP 604 union types in command handlers

@auto-assign auto-assign bot requested review from Larch-C and Raven95676 October 1, 2025 06:19
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a 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>

Sourcery 对开源免费 - 如果你喜欢我们的评论,请考虑分享它们 ✨
帮助我变得更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进你的评论。
Original comment in English

Hey there - I've reviewed your changes - here's some feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Soulter Soulter requested a review from Copilot October 1, 2025 12:02
Copy link
Contributor

Copilot AI left a 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.Union and PEP 604 union types

Soulter and others added 3 commits October 1, 2025 21:33
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Soulter Soulter merged commit f8a4b54 into master Oct 1, 2025
5 checks passed
@Soulter Soulter deleted the fix/command-union-type branch October 18, 2025 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants