feat: enhance using statement quick fix with 150+ common .NET classes

[gaoconggit]

Summary

Enhanced the # using statement quick fix feature by expanding support from ~43 to 150+ commonly used .NET classes. The new classes are organized into 11 categories for better maintainability.

Changes

• Added comprehensive namespace mappings for commonly used .NET classes
• Organized into 11 categories: System types, Collections, Text, I/O, Networking, Threading, XML, Data, Diagnostics, Security, Reflection
• Expanded LINQ methods support from 30 to 49 methods
• Total support increased from ~43 to 150+ classes

Resolves #50

---

Generated with Claude Code

[gaoconggit]

@codex 你也来评审一下

[claude]

Code Review: PR #51 - Enhanced Using Statement Quick Fix

Summary

This PR significantly expands the namespace mapping dictionary from ~43 to 150+ .NET classes, organized into 11 logical categories. Overall, this is a valuable enhancement that will improve developer experience.

✅ Strengths

Code Quality

• Well-organized structure: The 11 categories (System types, Collections, Text, I/O, Networking, Threading, XML, Data, Diagnostics, Security, Reflection) provide excellent maintainability
• Comprehensive coverage: Excellent expansion of commonly-used .NET classes, especially for collections, I/O, and threading
• Consistent patterns: Maintains existing code style and conventions

Developer Experience

• Significant UX improvement - adding 107 new class mappings will reduce friction
• LINQ methods expanded from 30 to 49, covering modern operators like Chunk, DistinctBy, TakeLast, SkipLast

⚠️ Issues & Concerns

1. CRITICAL: Deprecated Namespace Reference

Location: Lines 248-251

["SqlConnection"] = new[] { "System.Data.SqlClient" },
["SqlCommand"] = new[] { "System.Data.SqlClient" },
["SqlDataReader"] = new[] { "System.Data.SqlClient" },
["SqlDataAdapter"] = new[] { "System.Data.SqlClient" },

Issue: System.Data.SqlClient is deprecated in modern .NET. These classes have moved to:

• Microsoft.Data.SqlClient (recommended NuGet package)
• Or use the legacy compatibility package

Recommendation: Either remove these mappings or update to use Microsoft.Data.SqlClient. Since this is a NuGet package (not in the BCL), consider whether it's appropriate to include here.

2. Performance Consideration: Dictionary Re-creation

Location: Lines 116-326

Issue: The large commonNamespaces dictionary (150+ entries) is recreated on every invocation of CreateAddUsingActions. This method is called frequently during code editing.

Impact:

• Memory allocations on every diagnostic
• Unnecessary GC pressure
• Potential performance impact with multiple simultaneous code actions

Recommendation: Make this dictionary a static readonly field at the class level:

private static readonly Dictionary<string, string[]> CommonNamespaces = new Dictionary<string, string[]>
{
    // ... mappings
};

This would be a one-time allocation shared across all invocations.

3. Missing Error Handling Context

Location: Lines 338-341

catch (System.Exception ex)
{
    System.Diagnostics.Debug.WriteLine($"Error creating add using actions: {ex.Message}");
}

Issue: Only logs ex.Message, losing stack trace and inner exceptions.

Recommendation: Log full exception or at minimum include ex.ToString():

System.Diagnostics.Debug.WriteLine($"Error creating add using actions: {ex}");

4. Ambiguous Type Mappings

Location: Various

Issue: Some types exist in multiple namespaces:

• Timer: Both System.Threading.Timer and System.Timers.Timer exist
• Match: Could be regex or other contexts

Current behavior: Only maps to one namespace, which may not be what the user expects.

Recommendation: Consider supporting multiple namespace options for ambiguous types (the code already supports string[] for this purpose, just needs the mappings).

5. Test Coverage

Issue: No test project found in the repository.

Recommendation: Add unit tests to verify:

• Dictionary contains expected entries
• Quick fixes work for common scenarios
• No duplicate entries
• Performance benchmarks for dictionary lookup

🔍 Security Considerations

• ✅ No security concerns identified
• ✅ No user input directly used in namespace construction
• ✅ Proper validation exists for namespace existence check (lines 351-357)

📊 Maintainability Suggestions

Consider: External Configuration

With 150+ mappings, consider moving to:

• JSON configuration file
• Separate static class
• Code generation from BCL metadata

This would make future updates easier without modifying core logic.

Consider: Documentation

Add XML documentation comments explaining:

• Why certain namespaces are included
• Criteria for adding new mappings
• How to extend the list

🎯 Recommendations Priority

Must Fix Before Merge:

1. ❗ Remove or update deprecated System.Data.SqlClient references

Strongly Recommended:
2. ⚡ Move dictionary to static readonly field for performance
3. 📝 Improve exception logging to include full context

Nice to Have:
4. 🧪 Add unit tests
5. 📚 Add XML documentation
6. 🔄 Consider ambiguous type mappings

✅ Approval Recommendation

Conditional Approval - pending fix for the deprecated namespace issue. The performance optimization should also be strongly considered given the frequency of invocation.

Overall, this is a valuable enhancement that will significantly improve the developer experience in SharpPad. Great work on the organization and comprehensive coverage! 🚀

---

Review generated by Claude Code

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

• @codex fix this CI failure
• @codex address that feedback

[claude]

Code Review - PR #51: Enhanced Using Statement Quick Fix

✅ Strengths

1. Comprehensive Coverage

• Excellent expansion from ~43 to 150+ commonly used .NET classes
• Well-organized into 11 logical categories (System types, Collections, Text, I/O, Networking, Threading, XML, Data, Diagnostics, Security, Reflection)
• Includes modern .NET types (DateOnly, TimeOnly, Half, Span, ReadOnlySpan, Memory)
• Great coverage of ASP.NET Core, Entity Framework, and popular third-party libraries (Dapper, AutoMapper, Serilog, MediatR, etc.)

2. Code Organization

• Chinese comments clearly delineate each category
• Logical grouping makes the dictionary easy to maintain and extend
• Consistent formatting throughout

3. Functional Completeness

• LINQ methods expanded from 30 to 49, covering most common operations
• Includes both shortened attribute names and full names (e.g., Obsolete and ObsoleteAttribute)
• Handles types with multiple possible namespaces (e.g., Guid can be from both System.Runtime.InteropServices and System)

⚠️ Issues and Concerns

1. Performance Concern - Dictionary Size
The dictionary now contains ~500 entries (lines 116-905). This dictionary is created every time CreateAddUsingActions is called for CS0103/CS0246 errors.

Recommendation: Make the dictionary static and readonly:

private static readonly Dictionary<string, string[]> CommonNamespaces = new Dictionary<string, string[]>
{
    // ... entries
};

private async Task<List<CodeActionResult>> CreateAddUsingActions(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
{
    // ... 
    if (CommonNamespaces.ContainsKey(identifier))
    {
        foreach (var namespaceName in CommonNamespaces[identifier])
        // ...

2. Duplicate/Conflicting Entries

• Line 423: ["Guid"] maps to both System.Runtime.InteropServices and System, but Guid is primarily from System namespace
• Line 608: ["IServiceProvider"] maps to both System and Microsoft.Extensions.DependencyInjection - these are different interfaces
• Line 743: ["Timer"] maps to both System.Windows.Forms and System.Threading - these are completely different classes
• Line 776: ["JsonSerializer"] maps to both Newtonsoft.Json and System.Text.Json - different implementations

Recommendation: When a type exists in multiple namespaces with different meanings, either:

• Choose the most commonly used one, or
• Add logic to determine context (e.g., check if other types from that namespace are used)

3. Missing Validation
Some entries may reference types that don't exist in standard .NET or have incorrect namespaces:

• Line 195: ["ContextStatic"] - Should be checked; this is rare and might be ContextStaticAttribute
• Several middleware extension methods (lines 589-606) are listed, but these are extension methods, not types, so they won't trigger CS0103/CS0246 errors

4. Code Quality Issues

Line 307: Deprecated namespace - System.Data.SqlClient is deprecated in favor of Microsoft.Data.SqlClient in modern .NET:

["SqlConnection"] = new[] { "Microsoft.Data.SqlClient" },  // Updated namespace

5. Test Coverage
No tests visible in this PR. Consider adding tests for:

• Common scenarios (Console, List, Task, etc.)
• Edge cases (types with multiple namespaces)
• Performance benchmarks (dictionary lookup time)

6. Potential False Positives
Some entries might cause confusion:

• Generic extension methods like AddControllers, AddScoped (lines 620-654) won't work with this approach since they're extension methods on IServiceCollection, not standalone types
• Method names like Where, Select (LINQ) are captured, but might conflict with user-defined methods

🔒 Security Concerns

✅ No security issues identified. This is purely IntelliSense/code assistance functionality.

📊 Performance Considerations

Current Implementation:

• Dictionary created on every diagnostic: O(500) allocation cost
• Dictionary lookup: O(1) - acceptable
• Memory: ~20-30KB per allocation

Expected Impact:

• High: If a file has multiple CS0103 errors, this dictionary is created multiple times
• Recommendation: Make it static (see Issue feat # 触发运行时自动保存到 localStorage #1)

🧪 Test Coverage

Status: ❌ No tests included in this PR

Recommended Tests:

1. Unit tests for common types (Console, List, Task)
2. Tests for types with multiple namespaces
3. Integration tests with actual code snippets
4. Performance tests for dictionary lookup

📋 Suggested Improvements

1. Extract to separate file or use source generator

   • Consider moving this large dictionary to a separate JSON/XML file
   • Or use a source generator to build it at compile-time

2. Add documentation

   • Document which .NET version each type requires
   • Add XML comments explaining the design decisions

3. Prioritize namespaces

   • When multiple namespaces exist, return them in order of likelihood
   • Consider usage statistics

4. Add configuration

   • Allow users to customize which libraries to include (e.g., disable WinForms if not used)

✨ Overall Assessment

Rating: 7.5/10

This is a valuable enhancement that will significantly improve the developer experience. The implementation is straightforward and the coverage is impressive. However, the performance issue (creating a 500-entry dictionary repeatedly) and the duplicate/conflicting entries need to be addressed before merging.

Recommendation:

• ✅ Approve with changes - Fix the static dictionary issue and resolve duplicate entries
• Consider splitting this into multiple PRs if cleanup is extensive

---

📝 Generated with Claude Code - Code Review Agent