Move PluginClassInfo & Add code regions #23
Conversation
📝 WalkthroughWalkthroughThis PR reorganizes and refactors the plugin analysis logic. The Changes
Sequence Diagram(s)sequenceDiagram
participant Analyzer
participant Helper
participant PluginClassInfo
Analyzer->>Helper: GetPluginClassInfo(ClassDeclarationSyntax, SemanticModel, ct)
Helper-->>Analyzer: Returns PluginClassInfo or null
alt PluginClassInfo exists
Analyzer->>Analyzer: Validate properties (IsStatic, IsPrivate, IsProtected)
else No plugin found
Analyzer->>Analyzer: Exit analysis early
end
sequenceDiagram
participant SourceGenerator
participant Helper
participant PluginClassInfo
SourceGenerator->>Helper: Inline lambda: GetPluginClassInfo(...)
Helper-->>SourceGenerator: Returns PluginClassInfo or null
SourceGenerator->>SourceGenerator: Proceed with source generation based on plugin info
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 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)
Flow.Launcher.Localization.Shared/Helper.cs (1)
28-60: Check for namespace collisions inIsPluginEntryClass.
IsPluginEntryClassmatches the interface name byi.Name == Constants.PluginInterfaceName. Consider also verifyingi.ContainingNamespaceif a similarly named interface exists. Otherwise, the code thoroughly handles null checks and correct fallback paths.-return namedTypeSymbol?.Interfaces.Any(i => i.Name == Constants.PluginInterfaceName) ?? false; +return namedTypeSymbol?.Interfaces.Any(i => + i.Name == Constants.PluginInterfaceName && + i.ContainingNamespace.ToDisplayString() == "Flow.Launcher.Plugin") ?? false;Flow.Launcher.Localization.Shared/Classes.cs (3)
13-13: Fix typo in property nameThere's a typo in the property name:
CodeFixLocatioinshould beCodeFixLocation.-public Location CodeFixLocatioin { get; } +public Location CodeFixLocation { get; }
26-26: Update property assignment to match corrected property nameSince the property name should be corrected, the assignment in the constructor needs to be updated as well.
- CodeFixLocatioin = codeFixLocatioin; + CodeFixLocation = codeFixLocatioin;
18-18: Update parameter name to match corrected property nameThe parameter name in the constructor should also be updated to match the corrected property name.
- public PluginClassInfo(Location location, string className, string propertyName, bool isStatic, bool isPrivate, bool isProtected, Location codeFixLocatioin) + public PluginClassInfo(Location location, string className, string propertyName, bool isStatic, bool isPrivate, bool isProtected, Location codeFixLocation)Flow.Launcher.Localization.Analyzers/Localize/ContextAvailabilityAnalyzer.cs (1)
60-60: Update property references to match corrected property nameSince the property name in
PluginClassInfoshould be corrected fromCodeFixLocatiointoCodeFixLocation, references to it should be updated accordingly.- pluginClassInfo.CodeFixLocatioin + pluginClassInfo.CodeFixLocationAlso applies to: 69-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Flow.Launcher.Localization.Analyzers/Localize/ContextAvailabilityAnalyzer.cs(3 hunks)Flow.Launcher.Localization.Shared/Classes.cs(1 hunks)Flow.Launcher.Localization.Shared/Flow.Launcher.Localization.Shared.csproj(1 hunks)Flow.Launcher.Localization.Shared/Helper.cs(2 hunks)Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs(1 hunks)Flow.Launcher.Localization.Analyzers/Localize/ContextAvailabilityAnalyzerCodeFixProvider.cs(4 hunks)
🔇 Additional comments (13)
Flow.Launcher.Localization.Shared/Flow.Launcher.Localization.Shared.csproj (1)
11-11:✅ Verification successful
Confirm necessity of switched package reference.
Switching from a generic Roslyn package to
Microsoft.CodeAnalysis.CSharpis logically consistent given the new C#-specific features in the shared classes. Please ensure there's no lingering dependency onMicrosoft.CodeAnalysis.Commonin the rest of the codebase that could cause build or analyzer issues.
🏁 Script executed:
#!/bin/bash # Searches the entire codebase for references to Microsoft.CodeAnalysis.Common rg --json "Microsoft\.CodeAnalysis\.Common"Length of output: 301
Verified: No lingering dependency on Microsoft.CodeAnalysis.Common
I searched the entire repository for any references to
Microsoft.CodeAnalysis.Common(including within all csproj files) and found none. This confirms that switching to a C#-specific package reference is appropriate and won’t cause any build or analyzer issues.
- File in question:
Flow.Launcher.Localization.Shared/Flow.Launcher.Localization.Shared.csproj(line 11)- The package reference
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.13.0" />correctly replaces the generic Roslyn dependency.Flow.Launcher.Localization.Shared/Helper.cs (2)
1-6: Namespaces align well with Roslyn requirements.These imports are consistent with the Roslyn Analyzer APIs used elsewhere in the file. No issues detected.
78-82:❓ Verification inconclusive
Verify that
DeclaringSyntaxReferencescollection is never empty.
GetDeclaredSymbol(property)?.DeclaringSyntaxReferences[0]is assumed valid. If a reflection-based scenario or unusual syntax tree state returns an empty array, this could throw an index error. Ensure thatpropertyis always a valid Roslyn symbol, to avoid potential runtime exceptions.
🏁 Script executed:
#!/bin/bash # Collect declarations of Properties that might lack references ast-grep --pattern $'property_declaration' | rg -A 5 'DeclaringSyntaxReferences'Length of output: 77
Manual Verification Required: Guard Against Empty DeclaringSyntaxReferences
The current implementation assumes that the symbol returned by
semanticModel.GetDeclaredSymbol(property)always has at least one entry in itsDeclaringSyntaxReferencescollection. Since no evidence was found confirming this assumption (and our initial command produced no output), please manually verify that for all property declarations, the following holds true:
semanticModel.GetDeclaredSymbol(property)is non-null.- The
DeclaringSyntaxReferencescollection is never empty before accessing its first element.If there is any risk of the collection being empty (for example, in reflection-based scenarios or when the syntax tree is in an unusual state), consider adding a safeguard such as a length check or a null-check before trying to index the array.
Flow.Launcher.Localization.SourceGenerators/Localize/LocalizeSourceGenerator.cs (1)
59-60: Great reuse of shared method.Replacing localized plugin checks with
Helper.GetPluginClassInfoconsolidates logic and simplifies maintenance. This direct call aligns with the new shared architecture.Flow.Launcher.Localization.Shared/Classes.cs (1)
1-28: Good implementation of thePluginClassInfoclassThe class is well-structured with proper encapsulation, immutable properties, and a clear purpose. Moving this to a shared project is a good decision since it's now usable by both the analyzer and source generator.
Flow.Launcher.Localization.Analyzers/Localize/ContextAvailabilityAnalyzerCodeFixProvider.cs (3)
20-87: Good organization with region directivesAdding region directives to organize the code improves readability and maintainability. The
CodeFixProviderregion appropriately encapsulates the core provider functionality.
89-152: Well-organized fix methods regionThe "Fix Methods" region groups related functionality together, making the code more navigable.
153-197: Appropriate utility methods extractionMoving
GetStaticContextPropertyDeclarationandGetFormattedDocumentto a "Utils" region makes the code more organized and highlights their reusable nature.Flow.Launcher.Localization.Analyzers/Localize/ContextAvailabilityAnalyzer.cs (5)
14-30: Good organization with region directivesAdding region directives for the DiagnosticAnalyzer section improves code readability and organization.
32-34: Well-structured Analyze Methods regionThe "Analyze Methods" region appropriately encapsulates the analysis logic separate from the analyzer setup.
46-51: Good refactoring to use PluginClassInfoUsing
Helper.GetPluginClassInfoinstead of inline detection logic improves code maintainability and reusability. The early return with a descriptive comment is also good practice.
53-76: Well-structured plugin class validationThe code now uses the properties from
pluginClassInfofor validation checks, which makes the code more readable and maintainable.
78-102: Good explanatory commentsAdding comments to explain the different validation checks for context properties and fields improves code readability and helps maintain the codebase.
Jack251970
changed the title
Important
I have thoroughly tested the generator diagnostics and code fixes related to the
PluginInitContextand I believe them are ready to move to production.Move PluginClassInfo to Shared project
Let
ContextAvailabilityAnalyzer.csandLocalizeSourceGenerator.csboth use this class. AddCodeFixLocationproperty for code fix.Add regions
In
ContextAvailabilityAnalyzer.csandContextAvailabilityAnalyzerCodeFixProvider.csfor code quality.Test