Skip to content

refactor(analyzer): 优化上下文注册分析器中的方法解析逻辑#173

Merged
GeWuYou merged 2 commits into
mainfrom
refactor/analyzer-method-resolution
Apr 5, 2026
Merged

refactor(analyzer): 优化上下文注册分析器中的方法解析逻辑#173
GeWuYou merged 2 commits into
mainfrom
refactor/analyzer-method-resolution

Conversation

@GeWuYou

@GeWuYou GeWuYou commented Apr 4, 2026

Copy link
Copy Markdown
Owner
  • 移除对已具备源码方法的特殊处理逻辑
  • 统一使用 ResolveHierarchyMethodImplementation 进行方法解析
  • 优先解析到当前具体架构类型上的 override 方法
  • 为模块安装路径添加一致的 override 解析逻辑
  • 添加完整的单元测试验证分析器行为

Summary by Sourcery

在上下文注册分析器中优化方法解析逻辑,使其在任何情况下都一致地优先选择具体的重写实现,并使模块辅助方法的解析与架构行为保持一致。

Bug 修复:

  • 确保当架构初始化或模块安装流程经过在派生类型中被重写的虚拟辅助方法时,分析器不会错误地报告问题。

增强功能:

  • 统一方法解析逻辑,对架构和模块辅助方法始终使用基于继承层级的实现查找方式。

测试:

  • 添加分析器测试,用于覆盖调用已重写虚拟辅助方法的继承初始化和模块安装路径。
Original summary in English

Summary by Sourcery

Refine method resolution in the context registration analyzer to consistently prefer concrete override implementations and align module helper resolution with architecture behavior.

Bug Fixes:

  • Ensure analyzer does not falsely report issues when architecture initialization or module installation flows through virtual helper methods overridden in derived types.

Enhancements:

  • Unify method resolution logic by always using hierarchy-based implementation lookup for architecture and module helper methods.

Tests:

  • Add analyzer tests covering inherited initialization and module installation paths that invoke overridden virtual helper methods.

- 移除对已具备源码方法的特殊处理逻辑
- 统一使用 ResolveHierarchyMethodImplementation 进行方法解析
- 优先解析到当前具体架构类型上的 override 方法
- 为模块安装路径添加一致的 override 解析逻辑
- 添加完整的单元测试验证分析器行为
@sourcery-ai

sourcery-ai Bot commented Apr 4, 2026

Copy link
Copy Markdown

审阅者指南

重构 ContextRegistrationAnalyzer 的方法解析逻辑,使其在架构和模块辅助方法上都一致地使用基于继承层级的实现查找机制,优先选择具体类型上的重写实现;并新增测试,用于验证继承的初始化和模块安装路径在跟随被重写的虚拟辅助方法时能够正确工作且不会报告诊断信息。

ContextRegistrationAnalyzer 中架构辅助方法解析的时序图

sequenceDiagram
    participant Analyzer as ContextRegistrationAnalyzer
    participant OnInit as Architecture_OnInitialize
    participant SymbolHelpers as SymbolHelpers
    participant ArchitectureBase as ArchitectureBase
    participant GameArchitecture as GameArchitecture
    participant TargetMethod as RegisterComponents_base
    participant OverrideMethod as RegisterComponents_override

    OnInit->>Analyzer: Analyze invocation of RegisterComponents
    Analyzer->>Analyzer: TryResolveArchitectureHelperMethod(targetMethod, architectureType)
    Analyzer->>SymbolHelpers: ResolveHierarchyMethodImplementation(targetMethod, architectureType)
    SymbolHelpers->>ArchitectureBase: Search implementation in hierarchy
    ArchitectureBase-->>SymbolHelpers: No concrete override on ArchitectureBase
    SymbolHelpers->>GameArchitecture: Check for override of RegisterComponents
    GameArchitecture-->>SymbolHelpers: Return OverrideMethod
    SymbolHelpers-->>Analyzer: OverrideMethod
    Analyzer->>Analyzer: helperMethod = OverrideMethod
    Analyzer->>OverrideMethod: Check DeclaringSyntaxReferences
    OverrideMethod-->>Analyzer: Has syntax references
    Analyzer-->>OnInit: Use OverrideMethod as helper, no diagnostic reported
Loading

ContextRegistrationAnalyzer 中模块辅助方法解析的时序图

sequenceDiagram
    participant Analyzer as ContextRegistrationAnalyzer
    participant Install as Module_Install
    participant SymbolHelpers as SymbolHelpers
    participant ModuleBase as ModuleBase
    participant DerivedModule as DerivedInventoryModule
    participant TargetMethod as RegisterComponents_base
    participant OverrideMethod as RegisterComponents_override

    Install->>Analyzer: Analyze invocation of RegisterComponents(architecture)
    Analyzer->>Analyzer: TryResolveModuleHelperMethod(targetMethod, moduleType)
    Analyzer->>SymbolHelpers: ResolveHierarchyMethodImplementation(targetMethod, moduleType)
    SymbolHelpers->>ModuleBase: Search implementation in hierarchy
    ModuleBase-->>SymbolHelpers: No concrete override on ModuleBase
    SymbolHelpers->>DerivedModule: Check for override of RegisterComponents
    DerivedModule-->>SymbolHelpers: Return OverrideMethod
    SymbolHelpers-->>Analyzer: OverrideMethod
    Analyzer->>Analyzer: helperMethod = OverrideMethod
    Analyzer->>OverrideMethod: Check DeclaringSyntaxReferences
    OverrideMethod-->>Analyzer: Has syntax references
    Analyzer-->>Install: Use OverrideMethod as helper, no diagnostic reported
Loading

ContextRegistrationAnalyzer 中更新后方法解析逻辑的类图

classDiagram
    class ContextRegistrationAnalyzer {
        +bool TryResolveArchitectureHelperMethod(IMethodSymbol targetMethod, INamedTypeSymbol architectureType, out IMethodSymbol helperMethod)
        +bool TryResolveModuleHelperMethod(IMethodSymbol targetMethod, INamedTypeSymbol moduleType, out IMethodSymbol helperMethod)
    }

    class SymbolHelpers {
        +bool IsWithinTypeHierarchy(INamedTypeSymbol candidateType, INamedTypeSymbol rootType)
        +IMethodSymbol ResolveHierarchyMethodImplementation(IMethodSymbol targetMethod, INamedTypeSymbol concreteType)
    }

    class Architecture {
        +void OnInitialize()
        +void RegisterModel(IModel model)
        +void RegisterSystem(ISystem system)
    }

    class ArchitectureBase {
        +void OnInitialize()
        +void RegisterComponents()
    }

    class GameArchitecture {
        +void OnInitialize()
        +void RegisterComponents()
    }

    class IArchitectureModule {
        +void Install(IArchitecture architecture)
    }

    class ModuleBase {
        +void Install(IArchitecture architecture)
        +void RegisterComponents(IArchitecture architecture)
    }

    class DerivedInventoryModule {
        +void RegisterComponents(IArchitecture architecture)
    }

    class IArchitecture {
        +void RegisterModel(IModel model)
        +void RegisterSystem(ISystem system)
        +void InstallModule(IArchitectureModule module)
    }

    class IModel
    class ISystem

    ContextRegistrationAnalyzer ..> SymbolHelpers : uses
    ContextRegistrationAnalyzer ..> Architecture : analyzes
    ContextRegistrationAnalyzer ..> IArchitectureModule : analyzes

    ArchitectureBase --|> Architecture
    GameArchitecture --|> ArchitectureBase

    ModuleBase ..|> IArchitectureModule
    DerivedInventoryModule --|> ModuleBase

    Architecture ..|> IArchitecture
Loading

文件级变更

变更 详情 文件
将架构路径中的辅助方法解析统一为始终优先选择具体架构类型上的重写实现。
  • 移除之前对具有源码声明的方法进行特殊处理、直接保留其作为目标方法而不是解析其重写实现的逻辑。
  • 更新辅助方法解析逻辑,使其始终使用具体架构类型调用 ResolveHierarchyMethodImplementation,并仅在解析失败时回退到原始方法。
  • 保留“解析得到的辅助方法必须具有源码声明”这一前置要求。
GFramework.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.cs
使模块辅助方法解析与架构路径保持一致,优先选择具体模块类型上的重写实现。
  • 移除当目标方法有源码且为非抽象时跳过层级解析的条件逻辑。
  • 对模块类型调用 ResolveHierarchyMethodImplementation,并在失败时回退到原始目标方法。
  • 保留“解析得到的辅助方法必须具有源码声明”的检查逻辑。
GFramework.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.cs
添加测试,确保当虚拟辅助方法在派生架构/模块类型中被重写并通过继承入口点被调用时,分析器不会报告诊断信息。
  • 新增一个测试:抽象的 ArchitectureBase.OnInitialize 调用虚拟的 RegisterComponents 方法,该方法在派生的 GameArchitecture 中被重写并注册 model 和 system;断言没有诊断。
  • 新增一个测试:ModuleBase.Install 调用虚拟的 RegisterComponents,该方法在 DerivedInventoryModule 中被重写并注册一个 model,而 GameArchitecture 安装此模块并注册一个 system;断言没有诊断。
  • 复用现有的 Wrap 辅助方法,将测试用例嵌入带有标准分析器前言的代码中。
GFramework.SourceGenerators.Tests/Analyzers/ContextRegistrationAnalyzerTests.cs

技巧与命令

与 Sourcery 交互

  • 触发新的审阅: 在 pull request 中评论 @sourcery-ai review
  • 继续讨论: 直接回复 Sourcery 的审阅评论。
  • 从审阅评论生成 GitHub Issue: 在审阅评论下回复,要求 Sourcery 从该评论创建一个 issue。你也可以直接回复 @sourcery-ai issue 来从该评论创建 issue。
  • 生成 pull request 标题: 在 pull request 标题的任意位置写上 @sourcery-ai,即可随时生成标题。也可以在 pull request 中评论 @sourcery-ai title 以(重新)生成标题。
  • 生成 pull request 摘要: 在 pull request 正文任意位置写上 @sourcery-ai summary,即可在该位置生成 PR 摘要。也可以在 pull request 中评论 @sourcery-ai summary 以在任意时间(重新)生成摘要。
  • 生成审阅者指南: 在 pull request 中评论 @sourcery-ai guide,即可随时(重新)生成审阅者指南。
  • 一次性解决所有 Sourcery 评论: 在 pull request 中评论 @sourcery-ai resolve,即可解决所有 Sourcery 评论。如果你已经处理完所有评论且不想再看到它们,这会很有用。
  • 关闭所有 Sourcery 审阅: 在 pull request 中评论 @sourcery-ai dismiss,即可关闭所有已有的 Sourcery 审阅。尤其适用于你希望从一次全新的审阅开始的情况——别忘了再评论一次 @sourcery-ai review 以触发新的审阅!

自定义你的体验

打开你的 控制面板 以:

  • 启用或禁用审阅功能,例如 Sourcery 生成的 pull request 摘要、审阅者指南等。
  • 更改审阅语言。
  • 添加、移除或编辑自定义审阅指令。
  • 调整其他审阅设置。

获取帮助

Original review guide in English

Reviewer's Guide

Refactors the ContextRegistrationAnalyzer’s method resolution logic to consistently use hierarchy-based implementation lookup for both architecture and module helper methods, preferring overrides on the concrete type, and adds tests to validate that inherited initialization and module installation paths correctly follow overridden virtual helper methods without reporting diagnostics.

Sequence diagram for architecture helper method resolution in ContextRegistrationAnalyzer

sequenceDiagram
    participant Analyzer as ContextRegistrationAnalyzer
    participant OnInit as Architecture_OnInitialize
    participant SymbolHelpers as SymbolHelpers
    participant ArchitectureBase as ArchitectureBase
    participant GameArchitecture as GameArchitecture
    participant TargetMethod as RegisterComponents_base
    participant OverrideMethod as RegisterComponents_override

    OnInit->>Analyzer: Analyze invocation of RegisterComponents
    Analyzer->>Analyzer: TryResolveArchitectureHelperMethod(targetMethod, architectureType)
    Analyzer->>SymbolHelpers: ResolveHierarchyMethodImplementation(targetMethod, architectureType)
    SymbolHelpers->>ArchitectureBase: Search implementation in hierarchy
    ArchitectureBase-->>SymbolHelpers: No concrete override on ArchitectureBase
    SymbolHelpers->>GameArchitecture: Check for override of RegisterComponents
    GameArchitecture-->>SymbolHelpers: Return OverrideMethod
    SymbolHelpers-->>Analyzer: OverrideMethod
    Analyzer->>Analyzer: helperMethod = OverrideMethod
    Analyzer->>OverrideMethod: Check DeclaringSyntaxReferences
    OverrideMethod-->>Analyzer: Has syntax references
    Analyzer-->>OnInit: Use OverrideMethod as helper, no diagnostic reported
Loading

Sequence diagram for module helper method resolution in ContextRegistrationAnalyzer

sequenceDiagram
    participant Analyzer as ContextRegistrationAnalyzer
    participant Install as Module_Install
    participant SymbolHelpers as SymbolHelpers
    participant ModuleBase as ModuleBase
    participant DerivedModule as DerivedInventoryModule
    participant TargetMethod as RegisterComponents_base
    participant OverrideMethod as RegisterComponents_override

    Install->>Analyzer: Analyze invocation of RegisterComponents(architecture)
    Analyzer->>Analyzer: TryResolveModuleHelperMethod(targetMethod, moduleType)
    Analyzer->>SymbolHelpers: ResolveHierarchyMethodImplementation(targetMethod, moduleType)
    SymbolHelpers->>ModuleBase: Search implementation in hierarchy
    ModuleBase-->>SymbolHelpers: No concrete override on ModuleBase
    SymbolHelpers->>DerivedModule: Check for override of RegisterComponents
    DerivedModule-->>SymbolHelpers: Return OverrideMethod
    SymbolHelpers-->>Analyzer: OverrideMethod
    Analyzer->>Analyzer: helperMethod = OverrideMethod
    Analyzer->>OverrideMethod: Check DeclaringSyntaxReferences
    OverrideMethod-->>Analyzer: Has syntax references
    Analyzer-->>Install: Use OverrideMethod as helper, no diagnostic reported
Loading

Class diagram for updated method resolution logic in ContextRegistrationAnalyzer

classDiagram
    class ContextRegistrationAnalyzer {
        +bool TryResolveArchitectureHelperMethod(IMethodSymbol targetMethod, INamedTypeSymbol architectureType, out IMethodSymbol helperMethod)
        +bool TryResolveModuleHelperMethod(IMethodSymbol targetMethod, INamedTypeSymbol moduleType, out IMethodSymbol helperMethod)
    }

    class SymbolHelpers {
        +bool IsWithinTypeHierarchy(INamedTypeSymbol candidateType, INamedTypeSymbol rootType)
        +IMethodSymbol ResolveHierarchyMethodImplementation(IMethodSymbol targetMethod, INamedTypeSymbol concreteType)
    }

    class Architecture {
        +void OnInitialize()
        +void RegisterModel(IModel model)
        +void RegisterSystem(ISystem system)
    }

    class ArchitectureBase {
        +void OnInitialize()
        +void RegisterComponents()
    }

    class GameArchitecture {
        +void OnInitialize()
        +void RegisterComponents()
    }

    class IArchitectureModule {
        +void Install(IArchitecture architecture)
    }

    class ModuleBase {
        +void Install(IArchitecture architecture)
        +void RegisterComponents(IArchitecture architecture)
    }

    class DerivedInventoryModule {
        +void RegisterComponents(IArchitecture architecture)
    }

    class IArchitecture {
        +void RegisterModel(IModel model)
        +void RegisterSystem(ISystem system)
        +void InstallModule(IArchitectureModule module)
    }

    class IModel
    class ISystem

    ContextRegistrationAnalyzer ..> SymbolHelpers : uses
    ContextRegistrationAnalyzer ..> Architecture : analyzes
    ContextRegistrationAnalyzer ..> IArchitectureModule : analyzes

    ArchitectureBase --|> Architecture
    GameArchitecture --|> ArchitectureBase

    ModuleBase ..|> IArchitectureModule
    DerivedInventoryModule --|> ModuleBase

    Architecture ..|> IArchitecture
Loading

File-Level Changes

Change Details Files
Unify helper method resolution in architecture path to always prefer overrides on the concrete architecture type.
  • Remove special-case handling that kept methods with source declarations as the target rather than resolving overrides.
  • Update helper method resolution to always call ResolveHierarchyMethodImplementation with the concrete architecture type and fall back to the original method only when resolution fails.
  • Keep the requirement that the resolved helper method must have source declarations before proceeding.
GFramework.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.cs
Align module helper method resolution with the architecture path, preferring overrides on the concrete module type.
  • Remove conditional logic that bypassed hierarchy resolution when the target method had source and was non-abstract.
  • Use ResolveHierarchyMethodImplementation against the module type and fall back to the original target method on failure.
  • Retain the check that the resolved helper method must have source declarations.
GFramework.SourceGenerators/Analyzers/ContextRegistrationAnalyzer.cs
Add tests to ensure analyzer does not report diagnostics when virtual helper methods are overridden in derived architecture/module types and invoked via inherited entry points.
  • Add a test where an abstract ArchitectureBase.OnInitialize calls a virtual RegisterComponents method that is overridden in a derived GameArchitecture registering model and system; assert no diagnostics.
  • Add a test where a ModuleBase.Install calls a virtual RegisterComponents that is overridden in a DerivedInventoryModule registering a model, and GameArchitecture installs this module and registers a system; assert no diagnostics.
  • Reuse existing Wrap helper to embed test cases with the standard analyzer preamble.
GFramework.SourceGenerators.Tests/Analyzers/ContextRegistrationAnalyzerTests.cs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepsource-io

deepsource-io Bot commented Apr 4, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 9239e51...37049be on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
C# Apr 5, 2026 1:14a.m. Review ↗
Secrets Apr 5, 2026 1:14a.m. Review ↗

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - 我在这里给出了一些总体反馈:

  • 将行为更改为始终优先使用 ResolveHierarchyMethodImplementation 会移除之前对“源代码中定义的非抽象方法”的保护机制;请考虑一下:原本希望通过显式调用 base.Xxx() 保持在基类实现上的调用,现在是否可能会被错误地解析为 architectureType/moduleType 上的重写实现?如果可能,需要考虑是否应该通过在 ResolveHierarchyMethodImplementation 中增加额外检查或选项来保留这一行为。
  • 由于 TryResolveArchitectureHelperMethodTryResolveModuleHelperMethod 现在基本共享相同的解析模式,可以考虑抽取一个共享的辅助方法,通过传入目标方法和具体类型来减少重复,并使 architecture/module 两条路径的行为保持高度一致。
面向 AI Agent 的提示
Please address the comments from this code review:

## Overall Comments
- The change to always prefer `ResolveHierarchyMethodImplementation` removes the previous safeguard for source-defined non-abstract methods; consider whether explicit `base.Xxx()` calls that are meant to remain on the base implementation could now incorrectly resolve to overrides on `architectureType/moduleType`, and if so, whether that should be preserved via an additional check or option in `ResolveHierarchyMethodImplementation`.
- Now that `TryResolveArchitectureHelperMethod` and `TryResolveModuleHelperMethod` share essentially the same resolution pattern, consider extracting a shared helper that takes the target method and concrete type to reduce duplication and keep the architecture/module paths behavior-tightly aligned.

Sourcery 对开源项目是免费的——如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请对每条评论点 👍 或 👎 ,我会根据这些反馈改进后续的评审质量。
Original comment in English

Hey - I've left some high level feedback:

  • The change to always prefer ResolveHierarchyMethodImplementation removes the previous safeguard for source-defined non-abstract methods; consider whether explicit base.Xxx() calls that are meant to remain on the base implementation could now incorrectly resolve to overrides on architectureType/moduleType, and if so, whether that should be preserved via an additional check or option in ResolveHierarchyMethodImplementation.
  • Now that TryResolveArchitectureHelperMethod and TryResolveModuleHelperMethod share essentially the same resolution pattern, consider extracting a shared helper that takes the target method and concrete type to reduce duplication and keep the architecture/module paths behavior-tightly aligned.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The change to always prefer `ResolveHierarchyMethodImplementation` removes the previous safeguard for source-defined non-abstract methods; consider whether explicit `base.Xxx()` calls that are meant to remain on the base implementation could now incorrectly resolve to overrides on `architectureType/moduleType`, and if so, whether that should be preserved via an additional check or option in `ResolveHierarchyMethodImplementation`.
- Now that `TryResolveArchitectureHelperMethod` and `TryResolveModuleHelperMethod` share essentially the same resolution pattern, consider extracting a shared helper that takes the target method and concrete type to reduce duplication and keep the architecture/module paths behavior-tightly aligned.

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.

- 修改 TryResolveArchitectureHelperMethod 和 TryResolveModuleHelperMethod 方法参数
- 添加新的 TryResolveHelperMethod 通用方法处理辅助方法调用解析
- 实现对显式 base 调用的特殊处理以保留基类语义
- 添加 IsExplicitBaseInvocation 方法检测显式基础调用
- 更新测试文件验证分析器核心行为
@GeWuYou GeWuYou merged commit a22e522 into main Apr 5, 2026
9 checks passed
@GeWuYou GeWuYou deleted the refactor/analyzer-method-resolution branch April 5, 2026 02:03
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.

1 participant