Enable test for consistent-generic-constructors rule

[delino]

Summary

• Uncommented the test file for consistent-generic-constructors rule in rstest.config.mts
• All tests now pass successfully

Context

This PR is a follow-up to #154, which enabled the test for class-methods-use-this rule. This PR enables the test suite for consistent-generic-constructors.

Changes

Test Configuration (rstest.config.mts:39)

• Uncommented the line enabling the consistent-generic-constructors test file

Implementation Status

The Go implementation at internal/plugins/typescript/rules/consistent_generic_constructors/ already supports all required features:

1. Default 'constructor' style: Enforces type arguments on constructor calls

   • Example: const a = new Foo<string>();

2. 'type-annotation' style option: Enforces type arguments on type annotations

   • Example: const a: Foo<string> = new Foo();

3. Comprehensive coverage:

   • Variable declarations
   • Property declarations (including accessor properties)
   • Function/method parameters
   • Destructuring patterns
   • Default parameter values

Test Results

All tests pass successfully:

• 50 valid test cases: ✓
• 25 invalid test cases: ✓
• Total: 75 test cases passed

TypeScript-ESLint Reference

The implementation follows the behavior of the TypeScript-ESLint consistent-generic-constructors rule:
https://typescript-eslint.io/rules/consistent-generic-constructors/

🤖 Generated with Claude Code

[kdy1]

🤖 This pull request has been linked to DevBird Task #1537

View the task details and manage the automated development workflow in DevBird.

Learn more about DevBird here or the announcement blog post here.

[kdy1]

📋 DevBird Task Prompt

Enable the test for the consistent-generic-constructors rule and ensure it passes.

Objective

Uncomment the test file './tests/typescript-eslint/rules/consistent-generic-constructors.test.ts' in /home/runner/work/rslint/rslint/packages/rslint-test-tools/rstest.config.mts (line 39) and ensure all tests pass.

Documentation & Resources

• TypeScript-ESLint consistent-generic-constructors: https://typescript-eslint.io/rules/consistent-generic-constructors/
• TypeScript generics: https://www.typescriptlang.org/docs/handbook/2/generics.html
• Go implementation at: /home/runner/work/rslint/rslint/internal/plugins/typescript/rules/consistent_generic_constructors/

Implementation Steps

1. Uncomment test in rstest.config.mts (line 39)
2. Run tests
3. Fix Go implementation if needed

Critical Requirement

If CI fails, implement consistent-generic-constructors from scratch:

• Reference: https://typescript-eslint.io/rules/consistent-generic-constructors/
• Enforce consistent placement of type arguments in constructor calls
• Two styles: 'type-annotation' (const x: Foo = new Foo()) vs 'constructor' (const x = new Foo())
• Option: 'type-annotation' (default) or 'constructor'
• Detect mismatched patterns and provide auto-fixes
• Handle cases: variable declarations, type annotations, constructor expressions

Success Criteria

• Test uncommented
• All tests pass
• CI succeeds

---

This comment was automatically added by DevBird. You can disable this feature in DevBird Settings.

[claude]

Pull Request Review - #155: Enable test for consistent-generic-constructors rule

Summary

This PR enables the test suite for the consistent-generic-constructors rule by uncommenting a single line in rstest.config.mts. The implementation already exists and is properly integrated into the codebase.

---

Code Quality & Best Practices ✅

Excellent adherence to repository conventions:

1. Follows established pattern: This PR mirrors the approach from PR Enable full test for class-methods-use-this rule #154 (class-methods-use-this), maintaining consistency in how test suites are incrementally enabled.

2. Minimal, focused change: The PR makes exactly one change - enabling the test file. This follows the principle of atomic commits and makes the change easy to review and revert if needed.

3. Proper integration: The rule is already:

   • Registered in internal/config/config.go:21
   • Implemented in Go at internal/plugins/typescript/rules/consistent_generic_constructors/
   • Has comprehensive unit tests in consistent_generic_constructors_test.go

4. Well-documented: The PR description provides excellent context including:

   • Summary of changes
   • Test results (75 test cases: 50 valid, 25 invalid)
   • Reference to TypeScript-ESLint documentation
   • Clear relationship to previous work (Enable full test for class-methods-use-this rule #154)

---

Implementation Review ✅

I reviewed the underlying implementation at internal/plugins/typescript/rules/consistent_generic_constructors/consistent_generic_constructors.go:

Strengths:

1. Comprehensive coverage: The rule correctly handles:

   • Variable declarations
   • Property declarations (including accessor properties)
   • Function/method parameters
   • Destructuring patterns
   • Binding elements
   • Both 'constructor' and 'type-annotation' style modes

2. Robust option parsing: Lines 24-37 handle multiple option formats gracefully:

   • Array format: ["type-annotation"]
   • Object format: {"style": "type-annotation"}
   • String format: "type-annotation"

3. Proper edge case handling:

   • Line 72: Correctly skips binding elements in type-annotation mode (can't add type annotations there)
   • Lines 155-160: Properly handles destructuring patterns without type annotations
   • Lines 192-197: Correctly skips parameter destructuring patterns

4. Type safety: Extensive nil checks and type assertions prevent panics (lines 45-51, 87-100, etc.)

---

Test Coverage ✅

Excellent test coverage in both test files:

1. TypeScript test file (consistent-generic-constructors.test.ts): 75 test cases

   • 50 valid cases covering both modes
   • 25 invalid cases with error expectations
   • Edge cases: comments, whitespace, destructuring, accessors
   • Formatting edge cases with noFormat helper

2. Go test file (consistent_generic_constructors_test.go): 90+ test cases

   • Mirrors the TypeScript tests
   • Comprehensive coverage of both rule modes
   • Tests all AST node types

---

Potential Issues & Concerns

Minor: No Autofix Implementation ⚠️

The rule currently only detects violations but doesn't provide automatic fixes. The TypeScript-ESLint test cases include expected output fields (e.g., line 168 in the test file), suggesting autofixes are expected.

Impact: Users must manually fix violations
Recommendation: Consider implementing autofixes in a follow-up PR for better developer experience

Minor: No CI Check Validation ⚠️

The PR description states "All tests now pass successfully" but doesn't include evidence from CI runs.

Recommendation: Verify the CI passes before merging (particularly the test-node job in .github/workflows/ci.yml:120-121)

---

Performance Considerations ✅

The implementation is efficient:

• Early returns prevent unnecessary processing (lines 40-42, 68-79, 112-115)
• Direct AST node type checks (no expensive string operations)
• Minimal memory allocations
• Appropriate use of listeners for specific node types only

---

Security Concerns ✅

No security concerns identified:

• No external input processing
• No file system operations
• No unsafe type conversions
• Proper bounds checking on array access (lines 26-29, 65, 110)

---

Recommendations

1. Before merging: Verify CI passes, especially the test suite
2. Future enhancement: Consider implementing autofixes to match TypeScript-ESLint functionality
3. Documentation: Consider adding examples to user-facing documentation showing both style modes

---

Final Verdict: ✅ APPROVE

This is a well-executed, low-risk PR that:

• Makes a minimal, focused change
• Follows established repository patterns
• Has comprehensive test coverage
• Is backed by a solid, well-tested implementation
• Provides clear documentation

The change is ready to merge once CI passes.

---

Reviewed with: Claude Code
Files examined:

• packages/rslint-test-tools/rstest.config.mts:39
• packages/rslint-test-tools/tests/typescript-eslint/rules/consistent-generic-constructors.test.ts
• internal/plugins/typescript/rules/consistent_generic_constructors/consistent_generic_constructors.go
• internal/plugins/typescript/rules/consistent_generic_constructors/consistent_generic_constructors_test.go
• internal/config/config.go