Add namespace check to BlazorUI source generator (#9682) #9697
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request focuses on refactoring the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSyntaxContextReceiver.cs (2)
6-7: Remove unused namespace import.The
System.Diagnosticsnamespace appears to be unused in the current implementation.using System; -using System.Diagnostics;
27-33: Consider improving namespace validation robustness.While the namespace validation is a good addition, consider these improvements:
- Extract the namespace string to a constant for maintainability.
- Use pattern matching for safer type conversion.
+private const string EXPECTED_NAMESPACE_PREFIX = "Bit.BlazorUI"; var namespaceParent = classDeclarationSyntax.Parent; if (namespaceParent is null) return; -var namespaceDeclarationSyntax = (BaseNamespaceDeclarationSyntax)namespaceParent; +if (namespaceParent is not BaseNamespaceDeclarationSyntax namespaceDeclarationSyntax) return; -if (namespaceDeclarationSyntax.Name.ToString().StartsWith("Bit.BlazorUI") is false) return; +if (namespaceDeclarationSyntax.Name.ToString().StartsWith(EXPECTED_NAMESPACE_PREFIX) is false) return;src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSourceGenerator.cs (3)
65-67: Consider improving null safety in parameters dictionary handling.The current implementation makes assumptions about null safety that could be made more robust:
- var parametersDictionary = parameters.ToDictionary() as Dictionary<string, object>; - foreach (var parameter in parametersDictionary!) + if (parameters.ToDictionary() is not Dictionary<string, object> parametersDictionary) + { + return base.SetParametersAsync(parameters); + } + foreach (var parameter in parametersDictionary)
53-54: Consider adding XML documentation for generated members.The generated fields and properties for two-way binding would benefit from XML documentation to explain their purpose and usage:
+ /// <summary> + /// Tracks whether the parameter has been set during parameter initialization. + /// </summary> private bool {sym.Name}HasBeenSet; + /// <summary> + /// Event callback for two-way binding of {sym.Name}. + /// </summary> [Parameter] public EventCallback<{sym.Type.ToDisplayString()}> {sym.Name}Changed {{ get; set; }}
57-59: Consider enabling testing of generated code.While the
[ExcludeFromCodeCoverage]attribute is appropriate for generated code, consider generating corresponding test helper methods to facilitate testing of components that use this generated code. This would help ensure that the generated parameter handling behaves correctly in various scenarios.Would you like me to propose a pattern for generating test helper methods?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSourceGenerator.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSyntaxContextReceiver.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (4)
src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSyntaxContextReceiver.cs (2)
23-23: LGTM! Improved casting approach.The direct cast is a good optimization since the null check is already performed earlier.
25-26: LGTM! Improved readability.The simplified modifier check and added line break enhance code readability while maintaining the same functionality.
src/BlazorUI/Bit.BlazorUI.SourceGenerators/Component/ComponentSourceGenerator.cs (2)
45-47: LGTM! Modern C# namespace declaration.The use of file-scoped namespace declaration is a good practice that reduces nesting and improves readability.
129-152: LGTM! Well-structured assignment method.The generated
Assignmethod implementation:
- Properly handles two-way binding
- Includes appropriate debug attributes
- Manages class/style builder resets
- Executes custom callbacks when needed
- Returns meaningful status about the assignment
closes #9682
Summary by CodeRabbit
Style
ComponentSourceGeneratorclassComponentSyntaxContextReceiverclassChores
Note: These changes are primarily internal improvements and do not directly impact end-user functionality.