feat: add diagnostics for abstract test classes with data sources

[thomhurst]

Fixes#2768

[claude]

Claude finished @thomhurst's task —— View job

---

🔍 PR Review: Add diagnostics for abstract test classes with data sources

I've completed a thorough review of this PR that adds analyzer rule TUnit0059. Here's my comprehensive feedback:

✅ Overall Assessment: Excellent Implementation

This is a well-designed analyzer that addresses a real usability issue. The implementation is clean, comprehensive, and follows TUnit's established patterns.

---

📋 Code Quality & Best Practices

✅ Excellent:

• Clean architecture: Properly inherits from ConcurrentDiagnosticAnalyzer for thread safety
• Modern C# patterns: Uses pattern matching (is not) and collection expressions appropriately
• Consistent naming: Follows TUnit's PascalCase conventions for public members
• Proper separation: Analyzer logic cleanly separated from test code
• Resource management: Well-integrated with existing Resources.resx infrastructure

✅ Good error messaging:

• Clear diagnostic title: "Abstract test class with data sources requires [InheritsTests]"
• Helpful message format includes the class name for easy identification
• Descriptive explanation guides users to the solution

---

🧪 Test Coverage: Comprehensive

✅ Excellent test coverage with 6 test scenarios:

1. ✅ Concrete classes with data sources (should not trigger)
2. ✅ Abstract classes without data sources (should not trigger)
3. ✅ Abstract classes without tests (should not trigger)
4. ✅ Abstract classes with MethodDataSource (should trigger)
5. ✅ Abstract classes with InstanceMethodDataSource (should trigger)
6. ✅ Abstract classes with Arguments (should trigger)
7. ✅ Abstract classes with ClassDataSource (should trigger)

Test quality highlights:

• Uses Roslyn analyzer testing framework correctly
• Clear test names that describe the scenario
• Proper use of diagnostic location markers ({|#0:AbstractTestBase|})
• Covers all major data source attribute types

---

🔧 Integration & Infrastructure

✅ Proper integration:

• ✅ Added to Rules.cs with consistent ID (TUnit0059) and Warning severity
• ✅ Registered in AnalyzerReleases.Shipped.md
• ✅ Resources added to Resources.resx with proper localization structure
• ✅ Minor cleanup in TestDataAnalyzer.cs (formatting improvement)

---

⚡ Performance Considerations

✅ Performance optimized:

• Early returns: Efficiently filters non-abstract classes first
• Lazy evaluation: Uses Any() to short-circuit on first match
• Minimal allocations: Uses LINQ efficiently with ToList() only when needed
• Symbol analysis: Operates at the symbol level, avoiding syntax tree traversal

Potential micro-optimization (very minor):

// Current: Creates list then checks if emptyvartestMethods=namedTypeSymbol.GetMembers().OfType<IMethodSymbol>().Where(m =>m.IsTestMethod(context.Compilation)).ToList();// Alternative: Short-circuit without allocationif(!namedTypeSymbol.GetMembers().OfType<IMethodSymbol>().Any(m =>m.IsTestMethod(context.Compilation))){return;}

---

🔒 Security Assessment

✅ No security concerns:

• Static analysis tool with no runtime execution
• No file system or network access
• No user input processing
• Standard Roslyn analyzer patterns used correctly

---

🐛 Potential Issues Found

⚠️ Minor: Data source detection could be more robust

The current detection logic at lines 69-79 uses string-based matching:

if(typeName.Contains("DataSource")||typeName=="ArgumentsAttribute"){returntrue;}

Recommendation: Consider using the same robust detection pattern as TestDataAnalyzer.cs which checks for IDataSourceAttribute interface implementation. This would be more maintainable and catch custom data source attributes.

✅ Good news: The current approach works correctly for all TUnit built-in attributes and the tests verify this.

---

📈 Code Consistency

✅ Excellent consistency:

• Matches patterns used in TestDataAnalyzer.cs for similar checks
• Uses same extension methods (IsTestClass, IsTestMethod)
• Follows established analyzer conventions in the codebase
• Diagnostic severity (Warning) is appropriate for this issue

---

🎯 Recommendations

1. Consider enhanced data source detection (optional improvement):

   // Use interface-based detection like TestDataAnalyzerreturnattributeClass.AllInterfaces.Any(i =>i.GloballyQualified()==WellKnown.AttributeFullyQualifiedClasses.IDataSourceAttribute.WithGlobalPrefix);

2. Documentation: Consider adding XML documentation to the analyzer class for future maintainers

---

✅ Final Verdict

This is a high-quality implementation that:

• ✅ Solves a real user problem with clear guidance
• ✅ Has excellent test coverage
• ✅ Follows all TUnit conventions and patterns
• ✅ Is properly integrated into the analyzer infrastructure
• ✅ Has good performance characteristics
• ✅ Introduces no security concerns

Recommendation: ✅ Approve - This PR is ready to merge. The suggested improvements are minor optimizations that don't affect functionality.

---