Fix/analyzer warning reduction batch#283
Merged
Merged
Conversation
- 修复 LocalizationMap 对可变 Dictionary 的直接公共暴露，降低 MA0016 集合暴露风险 - 新增复制输入映射的构造函数，并保留默认映射初始化行为以维持现有消费者兼容性 - 更新 XML 注释，明确只读访问语义和内部状态隔离原因
- 更新 analyzer warning reduction 的 active tracking 到 RP-046 并记录 solution 级 891 条 warning 基线 - 补充前台构建与日志采集形态不一致的环境风险和后续恢复建议
- 修复 UnifiedSettingsFile 与 LocalizationMap 的集合暴露形状，减少可变集合泄漏风险 - 优化 CqrsHandlerRegistryGeneratorTests 的大型 fixture 组织方式，降低 MA0051 噪音 - 更新 analyzer warning reduction 的 active todo 与 trace，回写 0 warning solution 基线
- 更新 analyzer warning reduction 的 active todo 为 RP-048 当前真值 - 补充 plain dotnet build 成功与最新 origin/main baseline - 记录当前批处理已到自然停点并收敛下一步建议
- 更新 AGENTS.md，明确使用 plain dotnet build 作为默认构建告警检查入口 - 归档 analyzer warning reduction 在 RP-042 至 RP-048 的晚期 active 文档细节 - 压缩 active todo 与 trace，只保留当前分支目标所需的恢复真值
- 移除测试项目的警告级别设置 - 将包许可证从 MIT 更改为 Apache-2.0 - 为 GFramework 项目启用 .NET 代码分析器 - 保持目标框架 net8.0、net9.0 和 net10.0 的支持
- 重构 Godot source generator 的长方法与字符串比较逻辑，清理 GFramework.Godot.SourceGenerators 的 MA0051 和 MA0006 告警 - 更新 AutoRegisterExportedCollectionsGenerator 的注册解析阶段拆分，消除剩余的长方法告警 - 更新 AGENTS 与 analyzer-warning-reduction 跟踪文档，明确 warning 检查必须先 clean 再 build
- 重构 GFramework.Godot.SourceGenerators.Tests 的测试模板与诊断辅助，清除项目内全部 analyzer warning - 更新 GeneratorTest 异步等待与 analyzer-warning-reduction 跟踪文档，记录批次验证结果与恢复点
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough本PR通过重构测试基础设施、源生成器和配置更新，减少分析器警告并改进代码组织。主要变更包括：提取测试源代码常量和辅助方法、重构生成器逻辑以增强可维护性、将依赖属性改为接口类型、禁用警告级别0设置、以及更新许可证和分析器配置。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Test Results
Details
Insights
Fail Rate
build-and-test: Run #957
🎉 All tests passed!Slowest Tests
± Comparison with run #954 at 8d1ff2e | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 26 runs. Github Test Reporter by CTRF 💚 |
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| dotnet-format | yes | 1 | no | 4.93s | ||
| ✅ REPOSITORY | gitleaks | yes | no | no | 7.6s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 6.87s |
Detailed Issues
⚠️ CSHARP / dotnet-format - 1 error
Welcome to .NET 9.0!
---------------------
SDK Version: 9.0.114
----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate, run 'dotnet dev-certs https --trust'
Learn about HTTPS: https://aka.ms/dotnet-https
----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what's new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use 'dotnet --help' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
Unhandled exception: System.Exception: Restore operation failed.
at Microsoft.CodeAnalysis.Tools.CodeFormatter.OpenMSBuildWorkspaceAsync(String solutionOrProjectPath, WorkspaceType workspaceType, Boolean noRestore, Boolean requiresSemantics, String binaryLogPath, Boolean logWorkspaceWarnings, ILogger logger, CancellationToken cancellationToken)
at Microsoft.CodeAnalysis.Tools.CodeFormatter.FormatWorkspaceAsync(FormatOptions formatOptions, ILogger logger, CancellationToken cancellationToken, String binaryLogPath)
at Microsoft.CodeAnalysis.Tools.FormatCommandCommon.FormatAsync(FormatOptions formatOptions, ILogger`1 logger, CancellationToken cancellationToken)
at Microsoft.CodeAnalysis.Tools.Commands.RootFormatCommand.FormatCommandDefaultHandler.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
at System.CommandLine.Invocation.InvocationPipeline.InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
See detailed reports in MegaLinter artifacts
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff
Show us your support by starring ⭐ the repository
|
| Filename | Overview |
|---|---|
| GFramework.Godot.SourceGenerators/BindNodeSignalGenerator.cs | Large refactor extracting helper methods (BuildMethodAttributeMap, CollectMethodCandidates, TryResolveBindingTargetNames, etc.) for readability; behaviour is unchanged. Introduces a private IsParameterlessInstanceMethod that is also duplicated in GetNodeGenerator.cs. |
| GFramework.Godot.SourceGenerators/GetNodeGenerator.cs | Extracted IsParameterlessInstanceMethod helper and replaced == with string.Equals(..., StringComparison.Ordinal) for the lookup-mode type check; logic is equivalent. |
| GFramework.Godot.SourceGenerators/GodotProjectMetadataGenerator.cs | Large parsing method decomposed into CollectMappingCandidates, ResolveTypedMappings, TryUpdateSection, TryCollectAutoLoadEntry, TryCollectInputAction, and three Append* helpers; semantics preserved. |
| GFramework.Godot.SourceGenerators/Registration/AutoRegisterExportedCollectionsGenerator.cs | Validation pipeline split into TryResolveCollectionType, TryResolveRegistryTarget, TryResolveElementType, HasCompatibleRegisterMethod, and GetMemberType; logic and diagnostic messages are preserved. |
| GFramework.Godot/Setting/Data/LocalizationMap.cs | Refactored to use private readonly backing fields with defensive copies in constructors; public properties changed from mutable Dictionary setters to read-only IReadOnlyDictionary — no existing callers set these properties, so no breaking impact. |
| GFramework.Game/Data/UnifiedSettingsFile.cs | Property type widened from Dictionary<string,string> to IDictionary<string,string> (internal sealed class — no public-API breakage); default initializer now uses explicit StringComparer.Ordinal. |
| GFramework.Game/Data/UnifiedSettingsDataRepository.cs | CloneFile now handles the generic IDictionary case by falling back to StringComparer.Ordinal when the underlying comparer is unrecoverable, addressing the previously flagged concern. |
| GFramework.Godot.SourceGenerators.Tests/Core/GeneratorTest.cs | Added ConfigureAwait(false) to the async RunAsync call — correct fix for CA2007 in a library/test context. |
| GFramework.csproj | License updated from MIT to Apache-2.0; EnableNETAnalyzers enabled — intentional changes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TryCreateBinding] --> B[TryResolveBindingTargetNames]
A --> C[TryFindCompatibleField]
A --> D[TryFindCompatibleEvent]
A --> E[IsMethodCompatibleWithEvent]
B -->|fail| F[ReportInvalidConstructorArgumentDiagnostic]
C -->|fail| G[ReportNodeFieldNotFoundDiagnostic\nReportNodeFieldMustBeInstanceDiagnostic\nReportFieldTypeMustDeriveFromNodeDiagnostic]
D -->|fail| H[ReportSignalNotFoundDiagnostic]
E -->|fail| I[ReportIncompatibleSignatureDiagnostic]
A -->|success| J[SignalBindingInfo]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: GFramework.Godot.SourceGenerators/GetNodeGenerator.cs
Line: 305-311
Comment:
**Duplicated helper copied verbatim from `BindNodeSignalGenerator`**
`IsParameterlessInstanceMethod` is identical in both `GetNodeGenerator.cs` (line 305) and `BindNodeSignalGenerator.cs` (line 575). Extracting it to a shared internal static utility class (e.g. `SymbolHelpers`) would remove the duplication and prevent the two copies drifting apart if the check ever needs to change.
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "fix(pr-review): 修复设置快照比较器契约" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.cs (1)
138-138:⚠️ Potential issue | 🟡 Minor缺少
.ConfigureAwait(false)导致分析器警告不一致。本文件中所有 5 处
await test.RunAsync();调用（第 138、305、503、558、613 行）均未附加.ConfigureAwait(false)，而项目中其他测试文件（如AutoSceneGeneratorTests.cs、GetNodeGeneratorTests.cs）已全部更新。由于Meziantou.Analyzer在根级Directory.Build.props中启用，MA0004 警告应在此处触发。需补齐所有缺失的.ConfigureAwait(false)以保持一致性。建议修正
- await test.RunAsync(); + await test.RunAsync().ConfigureAwait(false);适用于第 138、305、503、558、613 行。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.cs` at line 138, Update every await test.RunAsync(); call in AutoRegisterExportedCollectionsGeneratorTests to append .ConfigureAwait(false) (i.e., change await test.RunAsync(); to await test.RunAsync().ConfigureAwait(false);) so the analyzer MA0004 warning is eliminated and behavior matches other test files like AutoSceneGeneratorTests and GetNodeGeneratorTests; ensure you update all occurrences of test.RunAsync() in this test class.
🧹 Nitpick comments (14)
GFramework.Godot/Setting/Data/LocalizationMap.cs (1)
46-56: 可选优化：复制时强制使用StringComparer.Ordinal会改变调用方的比较语义当调用方传入的
IReadOnlyDictionary使用了非 Ordinal 的比较器（例如OrdinalIgnoreCase），此处复制后统一变成区分大小写的 Ordinal 查找，可能导致ResolveGodotLocale/ResolveFrameworkLanguage在大小写不一致的键上出现静默回退到默认值。当前仓库内的调用点使用默认构造函数，因此暂无实际影响；但作为公共 API，建议在 XML 注释中显式说明"内部一律按 Ordinal 比较"这一契约，或在可行时保留原比较器语义（例如对Dictionary<TKey,TValue>输入读取其Comparer）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot/Setting/Data/LocalizationMap.cs` around lines 46 - 56, The constructor for LocalizationMap currently forces StringComparer.Ordinal when copying into _languageMap and _frameworkLanguageMap which can change caller semantics; update the LocalizationMap constructor to preserve the source comparer when available (e.g., if the incoming IReadOnlyDictionary is an IDictionary<string,string> or Dictionary<string,string>, use its Comparer) and fall back to StringComparer.Ordinal only when the original comparer cannot be determined, and adjust XML docs for LocalizationMap (and mention ResolveGodotLocale / ResolveFrameworkLanguage behavior) to state that keys are looked up using the preserved comparer or Ordinal fallback.GFramework.Game/Data/UnifiedSettingsDataRepository.cs (1)
286-301: 回退分支的注释与实际语义存在轻微出入。注释声称"退回到按当前内容复制,避免因为 API 抽象化而改变持久化前后的键比较语义",但
new Dictionary<string, string>(source.Sections)的IEnumerable<KeyValuePair>构造函数会使用EqualityComparer<string>.Default(序号比较),若source.Sections的运行时类型恰好是某个使用非默认比较器(例如忽略大小写)的IDictionary实现,这条回退路径反而会改变键比较语义,与注释描述相反。考虑到当前默认值是
new Dictionary<string, string>(),Newtonsoft.Json 反序列化也会产生Dictionary<string, string>,该回退分支在当前调用场景下实际不可达,因此影响有限。建议二选一:
- 将注释改为"回退到默认序号比较,因为此处不持有原比较器信息",更贴近实际行为;
- 或者在签名中保留具体字典类型(仅内部使用的 snapshot 类不暴露为公共 API),避免额外的运行时类型判断和注释负担。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Game/Data/UnifiedSettingsDataRepository.cs` around lines 286 - 301, The comment in CloneFile inaccurately claims the fallback preserves key-comparison semantics, but the fallback path new Dictionary<string, string>(source.Sections) will use EqualityComparer<string>.Default and thus can change comparison behaviour; fix by either (A) updating the comment on CloneFile/Sections to state that the fallback uses the default comparer because the original comparer cannot be recovered from an arbitrary IDictionary, or (B) change the API so the snapshot receives a concrete Dictionary<string,string> (or internal-only type) that preserves the original comparer so you can copy it via new Dictionary<string,string>(dictionary, dictionary.Comparer); pick one approach and apply consistently to the CloneFile method, the UnifiedSettingsFile.Sections handling, and any callers that construct the snapshot.GFramework.SourceGenerators.Tests/Cqrs/CqrsHandlerRegistryGeneratorTests.cs (2)
184-215: 提醒：*Expected常量的缩进风格在文件内不一致。
HiddenImplementationDirectInterfaceRegistrationExpected（第 184-215 行）与MixedReflectedImplementationAndPreciseRegistrationsExpected（第 461-503 行）采用"内容靠左 + 8 空格基准"的写法；而其他同类常量（如HiddenNestedHandlerSelfRegistrationExpected、HiddenArrayResponseFallbackExpected等）则采用与"""开头对齐的深缩进写法。两种写法在 C# 11 原始字符串字面量语义下都能得到相同的最终文本，但在同一文件内混用会增加阅读负担，也容易在后续维护中被误修改缩进导致期望值变化。建议统一为一种风格（推荐继续沿用多数常量使用的、与结束"""对齐的深缩进写法）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.SourceGenerators.Tests/Cqrs/CqrsHandlerRegistryGeneratorTests.cs` around lines 184 - 215, 本文件中原始字符串常量的缩进风格不一致：请把 HiddenImplementationDirectInterfaceRegistrationExpected 常量的缩进改为与多数常量一致的“与结束三引号（"""）对齐的深缩进写法，以统一风格（与 MixedReflectedImplementationAndPreciseRegistrationsExpected 相比）。定位符号：HiddenImplementationDirectInterfaceRegistrationExpected、MixedReflectedImplementationAndPreciseRegistrationsExpected、HiddenNestedHandlerSelfRegistrationExpected、HiddenArrayResponseFallbackExpected；变更时仅调整该常量的缩进/空格对齐，不修改字符串内容或换行，确保最终原始字符串文本不变并通过现有断言。
687-1364: 建议：将抽取出的大体量 fixture 常量拆分到独立的伙伴文件中。本次重构已经把重复的 inline fixture 提升为类级
const，方向是对的。不过提升后本文件已增长到约 1820 行，显著超过仓库编码规范中「单文件大致控制在 800-1000 行」的上限；同时每个*Source常量都重复嵌入了几乎相同的Microsoft.Extensions.DependencyInjection、GFramework.Core.Abstractions.Logging、GFramework.Cqrs.Abstractions.Cqrs、GFramework.Cqrs合同声明，后续新增回归用例会持续放大这份重复。建议考虑把这些 fixture 常量拆到同命名空间下的 partial 类伙伴文件（例如
CqrsHandlerRegistryGeneratorTests.Fixtures.cs），让CqrsHandlerRegistryGeneratorTests.cs只保留测试方法与少量共享辅助。可选地，可以再抽一个常量保存公共合同头，在各*Source中通过字符串拼接组合，以进一步降低重复。此项属于可选优化，不阻塞合入。As per coding guidelines: "Keep a single source file under roughly 800-1000 lines. If a file grows beyond that range, check whether responsibilities should be split before continuing"。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.SourceGenerators.Tests/Cqrs/CqrsHandlerRegistryGeneratorTests.cs` around lines 687 - 1364, The file CqrsHandlerRegistryGeneratorTests.cs has grown too large due to many large fixture constants (e.g., HiddenPointerResponseCompilationErrorSource, MixedDirectAndPreciseRegistrationsSource, MixedReflectedImplementationAndPreciseRegistrationsSource, etc.); move these big string constants into a separate partial companion file (for example CqrsHandlerRegistryGeneratorTests.Fixtures.cs) under the same partial class CqrsHandlerRegistryGeneratorTests, keeping only test methods and small helpers in the original file, and optionally extract the repeated contract header into a shared constant used to compose each *Source to eliminate duplication.GFramework.Godot.SourceGenerators/GodotProjectMetadataGenerator.cs (1)
635-656:Try前缀方法建议返回bool。按 C# 惯例，
Try*前缀通常意味着方法返回bool指示操作是否成功。这里TryCollectInputAction返回void，与TryCollectAutoLoadEntry的签名风格也不一致（一个返回bool，另一个返回void；一个接收AdditionalText，另一个接收filePath字符串）。建议统一：要么返回bool（并在调用处忽略结果），要么改名为CollectInputAction以避免误导。♻️ 建议统一为返回
bool并对齐参数类型- if (TryCollectAutoLoadEntry(file, currentSection, key, value, seenAutoLoads, autoLoads, diagnostics)) + if (TryCollectAutoLoadEntry(file, currentSection, key, value, seenAutoLoads, autoLoads, diagnostics)) continue; - TryCollectInputAction(currentSection, key, seenInputActions, inputActions, diagnostics, file.Path); + TryCollectInputAction(file, currentSection, key, seenInputActions, inputActions, diagnostics);- private static void TryCollectInputAction( - string currentSection, - string key, - ISet<string> seenInputActions, - ICollection<string> inputActions, - ICollection<Diagnostic> diagnostics, - string filePath) - { - if (!string.Equals(currentSection, "input", StringComparison.OrdinalIgnoreCase)) - return; - - if (!seenInputActions.Add(key)) - { - diagnostics.Add(Diagnostic.Create( - GodotProjectDiagnostics.DuplicateInputActionEntry, - CreateFileLocation(filePath), - key)); - return; - } - - inputActions.Add(key); - } + private static bool TryCollectInputAction( + AdditionalText file, + string currentSection, + string key, + ISet<string> seenInputActions, + ICollection<string> inputActions, + ICollection<Diagnostic> diagnostics) + { + if (!string.Equals(currentSection, "input", StringComparison.OrdinalIgnoreCase)) + return false; + + if (!seenInputActions.Add(key)) + { + diagnostics.Add(Diagnostic.Create( + GodotProjectDiagnostics.DuplicateInputActionEntry, + CreateFileLocation(file.Path), + key)); + return true; + } + + inputActions.Add(key); + return true; + }As per coding guidelines: "Follow standard C# naming" 与 "Prefer explicit, readable code over clever shorthand in framework internals"。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators/GodotProjectMetadataGenerator.cs` around lines 635 - 656, Change TryCollectInputAction to return bool and align its parameter type with TryCollectAutoLoadEntry: update the signature of TryCollectInputAction to return a bool and accept an AdditionalText file (instead of string filePath), then return false when the section is not "input" or when seenInputActions.Add(key) is false (after emitting Diagnostic.Create with GodotProjectDiagnostics.DuplicateInputActionEntry and CreateFileLocation(file)), and return true after adding the key to inputActions; callers can ignore the boolean result if they wish.GFramework.Godot.SourceGenerators/GetNodeGenerator.cs (1)
305-311: 可考虑把IsParameterlessInstanceMethod下沉到共享工具类以消除重复。
BindNodeSignalGenerator与GetNodeGenerator现在各自持有完全相同的IsParameterlessInstanceMethod实现（见GFramework.Godot.SourceGenerators/BindNodeSignalGenerator.csLine 575-581）。若后续再有第三个生成器需要复用该生命周期方法判定，建议抽到GFramework.SourceGenerators.Common.Extensions（或类似命名的共享命名空间）作为IMethodSymbol扩展方法，避免两处同时演化导致语义漂移。当前仅两处重复也可接受，属于可选重构。As per coding guidelines: "Code smells such as violations of SOLID, DRY, KISS, or YAGNI principles".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators/GetNodeGenerator.cs` around lines 305 - 311, Two generators (GetNodeGenerator and BindNodeSignalGenerator) duplicate the same IsParameterlessInstanceMethod logic; extract this into a shared IMethodSymbol extension to avoid duplication. Create an extension method (e.g., IMethodSymbolExtensions.IsParameterlessInstanceMethod) in a shared namespace such as GFramework.SourceGenerators.Common.Extensions, move the current implementation from GetNodeGenerator and BindNodeSignalGenerator into that method, update both generators to call the extension (replace direct calls to IsParameterlessInstanceMethod), and remove the original duplicate private methods.GFramework.Godot.SourceGenerators/BindNodeSignalGenerator.cs (1)
349-368:resolvedEvent中间变量可省略。该 helper 只在
resolvedEvent is null时报诊断并返回 false，其余路径直接把resolvedEvent写回eventSymbol。可省去中间变量直接if ((eventSymbol = FindEvent(...)) is null)或直接赋值给 out 参数，但当前写法可读性也没有问题，是否采纳看偏好。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators/BindNodeSignalGenerator.cs` around lines 349 - 368, The local variable resolvedEvent can be eliminated in TryFindCompatibleEvent: assign the result of FindEvent directly to the out parameter (eventSymbol) and check for null in the same expression (e.g. if ((eventSymbol = FindEvent(fieldSymbol.Type, signalName)) is null) { ReportSignalNotFoundDiagnostic(context, candidate, attribute, fieldSymbol, signalName); return false; }) then return true; this removes the unnecessary resolvedEvent and keeps the same behavior; references: TryFindCompatibleEvent, FindEvent, eventSymbol, resolvedEvent, ReportSignalNotFoundDiagnostic.GFramework.Godot.SourceGenerators.Tests/Behavior/AutoSceneGeneratorTests.cs (1)
277-308: helper 抽取合理；nullableDirective的换行符可与其余分隔符风格统一。
CreateAutoSceneSource通过includeBehaviorInfrastructure与nullableEnabled两个布尔参数灵活覆盖了多个测试场景，避免了大段 inline 源代码重复。Line 283 使用字面量
"\n"而其他分隔符统一用Environment.NewLine，对 Roslyn 解析输入源无实际影响（LF 也是合法行结束符），但与本文件其余部分风格不一致。可选地换为：♻️ 风格统一
- string nullableDirective = nullableEnabled ? "#nullable enable\n" : string.Empty; + string nullableDirective = nullableEnabled ? $"#nullable enable{Environment.NewLine}" : string.Empty;两个保留 inline 源的诊断用例（
SceneKeyStr属性冲突、缓存字段冲突）确实需要不同 namespace 布局/字段声明，继续 inline 是合理的。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators.Tests/Behavior/AutoSceneGeneratorTests.cs` around lines 277 - 308, The nullableDirective uses a hardcoded "\n" which is inconsistent with the rest of the file's use of Environment.NewLine; inside CreateAutoSceneSource replace the string literal "\n" in nullableDirective with Environment.NewLine so the directive's line break style matches other separators and keeps platform-consistent newlines.GFramework.Godot.SourceGenerators.Tests/BindNodeSignal/BindNodeSignalGeneratorTests.cs (1)
382-400:VerifyDiagnosticsAsync建议一并消费.ConfigureAwait(false)以保持文件内风格统一。该文件中其他
await ... .ConfigureAwait(false)模式的目的是在此测试项目中一致地压制 CA2007。VerifyDiagnosticsAsync返回Task让调用方去await ... .ConfigureAwait(false)，从效果上等价，但阅读上会让人误以为 helper 缺失ConfigureAwait。可选地把return test.RunAsync();改为await test.RunAsync().ConfigureAwait(false);+ 方法签名改为async Task，或者保持返回Task但在文档注释中注明"由调用方负责 ConfigureAwait"。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators.Tests/BindNodeSignal/BindNodeSignalGeneratorTests.cs` around lines 382 - 400, Change VerifyDiagnosticsAsync to consume ConfigureAwait by making it async: update the method signature to async Task and replace the direct return of test.RunAsync() with awaiting test.RunAsync().ConfigureAwait(false); this ensures the helper follows the file's existing await ...ConfigureAwait(false) pattern (satisfying CA2007) and keeps callers unchanged. Reference: VerifyDiagnosticsAsync and test.RunAsync().GFramework.Godot.SourceGenerators.Tests/GetNode/GetNodeGeneratorTests.cs (1)
263-294: helper 抽取合理，数组拼接可用集合表达式进一步简化。将常用的属性声明、Godot 类型 stub 抽成常量并通过
CreateGetNodeSource组装输入源是清晰的重构方向。只是 Line 268-270 的new string[godotTypes.Length + 1] + Array.Copy组合略显笨重，可用 C# 12 集合表达式收敛为一行（LangVersion=preview已开启）：♻️ 可选的简化
- string[] allGodotTypes = new string[godotTypes.Length + 1]; - allGodotTypes[0] = NodeWithReadyAndLookupMethods; - Array.Copy(godotTypes, 0, allGodotTypes, 1, godotTypes.Length); - - string godotSource = string.Join($"{Environment.NewLine}{Environment.NewLine}", allGodotTypes); + string godotSource = string.Join( + $"{Environment.NewLine}{Environment.NewLine}", + [NodeWithReadyAndLookupMethods, .. godotTypes]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators.Tests/GetNode/GetNodeGeneratorTests.cs` around lines 263 - 294, Replace the verbose allocation + Array.Copy in CreateGetNodeSource with a single C# collection expression: instead of creating new string[godotTypes.Length + 1] and calling Array.Copy, build allGodotTypes in one line using the preview collection expression (e.g. var allGodotTypes = [NodeWithReadyAndLookupMethods, ...godotTypes]; or new[] { NodeWithReadyAndLookupMethods }.Concat(godotTypes).ToArray()); update any using if you choose Concat/ToArray so the code compiles.GFramework.csproj (1)
19-19: 删除冗余的EnableNETAnalyzers配置。该项目目标框架为
net8.0;net9.0;net10.0，EnableNETAnalyzers在 .NET 5+ SDK 中默认启用，无需显式声明。已确认上层Directory.Build.props和Directory.Build.targets中没有对此属性的显式覆盖，因此该行完全冗余。删除以保持配置的简洁性。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.csproj` at line 19, 删除冗余的 EnableNETAnalyzers MSBuild 属性：在 GFramework.csproj 中移除 <EnableNETAnalyzers> 元素（属性名 EnableNETAnalyzers），因为在 .NET 5+ SDK 中该分析器默认启用且上层 Directory.Build.props/targets 未覆盖，直接删除该元素以保持项目文件简洁即可。ai-plan/public/analyzer-warning-reduction/traces/analyzer-warning-reduction-trace.md (1)
59-61: 可选：标注两个同名归档链接的来源差异。第 60 行和 61 行的链接显示文本都是
analyzer-warning-reduction-history-rp042-rp048.md，但分别指向archive/todos/和archive/traces/两份不同文件；阅读时容易混淆。可以考虑在显示文本中加入todos/traces限定，或按第 63–67 行的“历史跟踪归档 / 历史 trace 归档”分组方式分开两行，提升可读性。♻️ 建议
- 当前轮次归档： - - [analyzer-warning-reduction-history-rp042-rp048.md](../archive/todos/analyzer-warning-reduction-history-rp042-rp048.md) - - [analyzer-warning-reduction-history-rp042-rp048.md](../archive/traces/analyzer-warning-reduction-history-rp042-rp048.md) + - 跟踪：[analyzer-warning-reduction-history-rp042-rp048.md](../archive/todos/analyzer-warning-reduction-history-rp042-rp048.md) + - trace：[analyzer-warning-reduction-history-rp042-rp048.md](../archive/traces/analyzer-warning-reduction-history-rp042-rp048.md)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai-plan/public/analyzer-warning-reduction/traces/analyzer-warning-reduction-trace.md` around lines 59 - 61, The two identical link texts "analyzer-warning-reduction-history-rp042-rp048.md" point to different locations (archive/todos vs archive/traces) and may confuse readers; update the links on the lines referencing those files so the visible text disambiguates the source (e.g., "analyzer-warning-reduction-history-rp042-rp048.md (todos)" and "analyzer-warning-reduction-history-rp042-rp048.md (traces)") or split them into clearly labeled groups like the "历史跟踪归档 / 历史 trace 归档" pattern used later (follow the lines containing the two link entries to apply the change).GFramework.Godot.SourceGenerators.Tests/Behavior/AutoUiPageGeneratorTests.cs (1)
192-198: 建议使用Environment.NewLine或统一采用\r\n（可选）。
nullableDirective中硬编码"\n"，与周围 raw string 字面量使用的行结束符（在 CRLF 检出环境下通常为\r\n）不一致。虽然此字符串作为 Roslyn 源码输入不影响语法解析与测试结果，但如果后续该常量被用于与 expected 输出做字符串比较，可能会引入隐蔽的跨平台差异。♻️ 可选统一
- string nullableDirective = nullableEnabled ? "#nullable enable\n" : string.Empty; + string nullableDirective = nullableEnabled ? "#nullable enable" + Environment.NewLine : string.Empty;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators.Tests/Behavior/AutoUiPageGeneratorTests.cs` around lines 192 - 198, Change the hard-coded "\n" in CreateAutoUiPageSource's nullableDirective to use Environment.NewLine (or a consistent "\r\n") so line endings match surrounding raw string literals; update the nullableDirective assignment in the CreateAutoUiPageSource method to build the newline using Environment.NewLine instead of the literal "\n".GFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.cs (1)
87-139: 可选：将剩余诊断测试也迁移到CreateSource/VerifyDiagnosticsAsync。本次只有
Reports_Diagnostic_When_Collection_Member_Is_Not_Instance_Readable迁移到了VerifyDiagnosticsAsync，其它几个Reports_Diagnostic_*测试仍保留了大段内联的 attribute 声明和CSharpSourceGeneratorTest构造样板。建议后续将它们也改造为调用CreateSource+VerifyDiagnosticsAsync，消除重复的 attribute 声明与DisabledDiagnostics = { "GF_Common_Trace_001" }样板，进一步收敛测试基础设施。Also applies to: 249-306, 451-504, 506-559, 561-614
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.cs` around lines 87 - 139, The remaining diagnostic tests in AutoRegisterExportedCollectionsGeneratorTests (e.g., Reports_Diagnostic_When_Collection_Element_Type_Cannot_Be_Inferred and the other Reports_Diagnostic_* tests still using inline attribute/source + CSharpSourceGeneratorTest setup) should be refactored to reuse the existing CreateSource and VerifyDiagnosticsAsync helpers: move the shared attribute/type declarations out of each test into CreateSource, replace manual CSharpSourceGeneratorTest instantiation (including DisabledDiagnostics = { "GF_Common_Trace_001" }) with a call to VerifyDiagnosticsAsync using the generated source, and keep the same expected DiagnosticResult assertions (e.g., GF_AutoExport_005 for "Values"); reference the test class AutoRegisterExportedCollectionsGeneratorTests and the generator AutoRegisterExportedCollectionsGenerator when locating the tests to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@GFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.cs`:
- Line 138: Update every await test.RunAsync(); call in
AutoRegisterExportedCollectionsGeneratorTests to append .ConfigureAwait(false)
(i.e., change await test.RunAsync(); to await
test.RunAsync().ConfigureAwait(false);) so the analyzer MA0004 warning is
eliminated and behavior matches other test files like AutoSceneGeneratorTests
and GetNodeGeneratorTests; ensure you update all occurrences of test.RunAsync()
in this test class.
---
Nitpick comments:
In
`@ai-plan/public/analyzer-warning-reduction/traces/analyzer-warning-reduction-trace.md`:
- Around line 59-61: The two identical link texts
"analyzer-warning-reduction-history-rp042-rp048.md" point to different locations
(archive/todos vs archive/traces) and may confuse readers; update the links on
the lines referencing those files so the visible text disambiguates the source
(e.g., "analyzer-warning-reduction-history-rp042-rp048.md (todos)" and
"analyzer-warning-reduction-history-rp042-rp048.md (traces)") or split them into
clearly labeled groups like the "历史跟踪归档 / 历史 trace 归档" pattern used later
(follow the lines containing the two link entries to apply the change).
In `@GFramework.csproj`:
- Line 19: 删除冗余的 EnableNETAnalyzers MSBuild 属性：在 GFramework.csproj 中移除
<EnableNETAnalyzers> 元素（属性名 EnableNETAnalyzers），因为在 .NET 5+ SDK 中该分析器默认启用且上层
Directory.Build.props/targets 未覆盖，直接删除该元素以保持项目文件简洁即可。
In `@GFramework.Game/Data/UnifiedSettingsDataRepository.cs`:
- Around line 286-301: The comment in CloneFile inaccurately claims the fallback
preserves key-comparison semantics, but the fallback path new Dictionary<string,
string>(source.Sections) will use EqualityComparer<string>.Default and thus can
change comparison behaviour; fix by either (A) updating the comment on
CloneFile/Sections to state that the fallback uses the default comparer because
the original comparer cannot be recovered from an arbitrary IDictionary, or (B)
change the API so the snapshot receives a concrete Dictionary<string,string> (or
internal-only type) that preserves the original comparer so you can copy it via
new Dictionary<string,string>(dictionary, dictionary.Comparer); pick one
approach and apply consistently to the CloneFile method, the
UnifiedSettingsFile.Sections handling, and any callers that construct the
snapshot.
In `@GFramework.Godot.SourceGenerators.Tests/Behavior/AutoSceneGeneratorTests.cs`:
- Around line 277-308: The nullableDirective uses a hardcoded "\n" which is
inconsistent with the rest of the file's use of Environment.NewLine; inside
CreateAutoSceneSource replace the string literal "\n" in nullableDirective with
Environment.NewLine so the directive's line break style matches other separators
and keeps platform-consistent newlines.
In
`@GFramework.Godot.SourceGenerators.Tests/Behavior/AutoUiPageGeneratorTests.cs`:
- Around line 192-198: Change the hard-coded "\n" in CreateAutoUiPageSource's
nullableDirective to use Environment.NewLine (or a consistent "\r\n") so line
endings match surrounding raw string literals; update the nullableDirective
assignment in the CreateAutoUiPageSource method to build the newline using
Environment.NewLine instead of the literal "\n".
In
`@GFramework.Godot.SourceGenerators.Tests/BindNodeSignal/BindNodeSignalGeneratorTests.cs`:
- Around line 382-400: Change VerifyDiagnosticsAsync to consume ConfigureAwait
by making it async: update the method signature to async Task and replace the
direct return of test.RunAsync() with awaiting
test.RunAsync().ConfigureAwait(false); this ensures the helper follows the
file's existing await ...ConfigureAwait(false) pattern (satisfying CA2007) and
keeps callers unchanged. Reference: VerifyDiagnosticsAsync and test.RunAsync().
In `@GFramework.Godot.SourceGenerators.Tests/GetNode/GetNodeGeneratorTests.cs`:
- Around line 263-294: Replace the verbose allocation + Array.Copy in
CreateGetNodeSource with a single C# collection expression: instead of creating
new string[godotTypes.Length + 1] and calling Array.Copy, build allGodotTypes in
one line using the preview collection expression (e.g. var allGodotTypes =
[NodeWithReadyAndLookupMethods, ...godotTypes]; or new[] {
NodeWithReadyAndLookupMethods }.Concat(godotTypes).ToArray()); update any using
if you choose Concat/ToArray so the code compiles.
In
`@GFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.cs`:
- Around line 87-139: The remaining diagnostic tests in
AutoRegisterExportedCollectionsGeneratorTests (e.g.,
Reports_Diagnostic_When_Collection_Element_Type_Cannot_Be_Inferred and the other
Reports_Diagnostic_* tests still using inline attribute/source +
CSharpSourceGeneratorTest setup) should be refactored to reuse the existing
CreateSource and VerifyDiagnosticsAsync helpers: move the shared attribute/type
declarations out of each test into CreateSource, replace manual
CSharpSourceGeneratorTest instantiation (including DisabledDiagnostics = {
"GF_Common_Trace_001" }) with a call to VerifyDiagnosticsAsync using the
generated source, and keep the same expected DiagnosticResult assertions (e.g.,
GF_AutoExport_005 for "Values"); reference the test class
AutoRegisterExportedCollectionsGeneratorTests and the generator
AutoRegisterExportedCollectionsGenerator when locating the tests to update.
In `@GFramework.Godot.SourceGenerators/BindNodeSignalGenerator.cs`:
- Around line 349-368: The local variable resolvedEvent can be eliminated in
TryFindCompatibleEvent: assign the result of FindEvent directly to the out
parameter (eventSymbol) and check for null in the same expression (e.g. if
((eventSymbol = FindEvent(fieldSymbol.Type, signalName)) is null) {
ReportSignalNotFoundDiagnostic(context, candidate, attribute, fieldSymbol,
signalName); return false; }) then return true; this removes the unnecessary
resolvedEvent and keeps the same behavior; references: TryFindCompatibleEvent,
FindEvent, eventSymbol, resolvedEvent, ReportSignalNotFoundDiagnostic.
In `@GFramework.Godot.SourceGenerators/GetNodeGenerator.cs`:
- Around line 305-311: Two generators (GetNodeGenerator and
BindNodeSignalGenerator) duplicate the same IsParameterlessInstanceMethod logic;
extract this into a shared IMethodSymbol extension to avoid duplication. Create
an extension method (e.g.,
IMethodSymbolExtensions.IsParameterlessInstanceMethod) in a shared namespace
such as GFramework.SourceGenerators.Common.Extensions, move the current
implementation from GetNodeGenerator and BindNodeSignalGenerator into that
method, update both generators to call the extension (replace direct calls to
IsParameterlessInstanceMethod), and remove the original duplicate private
methods.
In `@GFramework.Godot.SourceGenerators/GodotProjectMetadataGenerator.cs`:
- Around line 635-656: Change TryCollectInputAction to return bool and align its
parameter type with TryCollectAutoLoadEntry: update the signature of
TryCollectInputAction to return a bool and accept an AdditionalText file
(instead of string filePath), then return false when the section is not "input"
or when seenInputActions.Add(key) is false (after emitting Diagnostic.Create
with GodotProjectDiagnostics.DuplicateInputActionEntry and
CreateFileLocation(file)), and return true after adding the key to inputActions;
callers can ignore the boolean result if they wish.
In `@GFramework.Godot/Setting/Data/LocalizationMap.cs`:
- Around line 46-56: The constructor for LocalizationMap currently forces
StringComparer.Ordinal when copying into _languageMap and _frameworkLanguageMap
which can change caller semantics; update the LocalizationMap constructor to
preserve the source comparer when available (e.g., if the incoming
IReadOnlyDictionary is an IDictionary<string,string> or
Dictionary<string,string>, use its Comparer) and fall back to
StringComparer.Ordinal only when the original comparer cannot be determined, and
adjust XML docs for LocalizationMap (and mention ResolveGodotLocale /
ResolveFrameworkLanguage behavior) to state that keys are looked up using the
preserved comparer or Ordinal fallback.
In `@GFramework.SourceGenerators.Tests/Cqrs/CqrsHandlerRegistryGeneratorTests.cs`:
- Around line 184-215: 本文件中原始字符串常量的缩进风格不一致：请把
HiddenImplementationDirectInterfaceRegistrationExpected
常量的缩进改为与多数常量一致的“与结束三引号（"""）对齐的深缩进写法，以统一风格（与
MixedReflectedImplementationAndPreciseRegistrationsExpected
相比）。定位符号：HiddenImplementationDirectInterfaceRegistrationExpected、MixedReflectedImplementationAndPreciseRegistrationsExpected、HiddenNestedHandlerSelfRegistrationExpected、HiddenArrayResponseFallbackExpected；变更时仅调整该常量的缩进/空格对齐，不修改字符串内容或换行，确保最终原始字符串文本不变并通过现有断言。
- Around line 687-1364: The file CqrsHandlerRegistryGeneratorTests.cs has grown
too large due to many large fixture constants (e.g.,
HiddenPointerResponseCompilationErrorSource,
MixedDirectAndPreciseRegistrationsSource,
MixedReflectedImplementationAndPreciseRegistrationsSource, etc.); move these big
string constants into a separate partial companion file (for example
CqrsHandlerRegistryGeneratorTests.Fixtures.cs) under the same partial class
CqrsHandlerRegistryGeneratorTests, keeping only test methods and small helpers
in the original file, and optionally extract the repeated contract header into a
shared constant used to compose each *Source to eliminate duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49e13b87-fcdd-4f88-9d52-98963181a3bc
📒 Files selected for processing (23)
AGENTS.mdGFramework.Core.Tests/GFramework.Core.Tests.csprojGFramework.Game/Data/UnifiedSettingsDataRepository.csGFramework.Game/Data/UnifiedSettingsFile.csGFramework.Godot.SourceGenerators.Tests/Behavior/AutoSceneGeneratorTests.csGFramework.Godot.SourceGenerators.Tests/Behavior/AutoUiPageGeneratorTests.csGFramework.Godot.SourceGenerators.Tests/BindNodeSignal/BindNodeSignalGeneratorTests.csGFramework.Godot.SourceGenerators.Tests/Core/GeneratorTest.csGFramework.Godot.SourceGenerators.Tests/GetNode/GetNodeGeneratorTests.csGFramework.Godot.SourceGenerators.Tests/Project/GodotProjectMetadataGeneratorTests.csGFramework.Godot.SourceGenerators.Tests/Registration/AutoRegisterExportedCollectionsGeneratorTests.csGFramework.Godot.SourceGenerators/BindNodeSignalGenerator.csGFramework.Godot.SourceGenerators/GetNodeGenerator.csGFramework.Godot.SourceGenerators/GodotProjectMetadataGenerator.csGFramework.Godot.SourceGenerators/Registration/AutoRegisterExportedCollectionsGenerator.csGFramework.Godot.Tests/GFramework.Godot.Tests.csprojGFramework.Godot/Setting/Data/LocalizationMap.csGFramework.SourceGenerators.Tests/Cqrs/CqrsHandlerRegistryGeneratorTests.csGFramework.csprojai-plan/public/analyzer-warning-reduction/archive/todos/analyzer-warning-reduction-history-rp042-rp048.mdai-plan/public/analyzer-warning-reduction/archive/traces/analyzer-warning-reduction-history-rp042-rp048.mdai-plan/public/analyzer-warning-reduction/todos/analyzer-warning-reduction-tracking.mdai-plan/public/analyzer-warning-reduction/traces/analyzer-warning-reduction-trace.md
💤 Files with no reviewable changes (2)
- GFramework.Godot.Tests/GFramework.Godot.Tests.csproj
- GFramework.Core.Tests/GFramework.Core.Tests.csproj
- 修复 UnifiedSettingsFile 与 UnifiedSettingsDataRepository 的 comparer 契约，在无法恢复原比较器时显式回退到 StringComparer.Ordinal - 统一 AutoRegisterExportedCollectionsGeneratorTests 中剩余的 RunAsync 异步等待写法，并补齐 ConfigureAwait(false) - 更新 analyzer-warning-reduction 跟踪文档，记录 PR follow-up 的验证结果与恢复点
Summary
Test Results
Details
Insights
Fail Rate
build-and-test: Run #959
🎉 All tests passed!Slowest Tests
± Comparison with run #957 at 3c1cae4 | 🍂 No flaky tests detected across all runs. | ⏱️ Measured over 27 runs. Github Test Reporter by CTRF 💚 |
This was referenced May 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
发布说明
新增特性
bug 修复
文档
重构
其他