feat: implement method-signature-style rule

[delino]

Summary

This PR implements the TypeScript-ESLint method-signature-style rule for rslint, which enforces consistent use of either method signature or property signature style in interfaces and type literals.

Background

TypeScript interfaces and type literals support two different syntaxes for function members:

• Method signature: foo(arg: string): number;
• Property signature: foo: (arg: string) => number;

These behave differently under TypeScript's strict mode: method signatures are bivariant in their arguments, while property signatures are contravariant. This rule helps enforce consistency and can improve type safety when using strict mode.

Implementation Details

Features Implemented

✅ Supports both property (default) and method style preferences
✅ Auto-fix capability to convert between styles
✅ Handles overloaded method signatures by converting to intersection types
✅ Preserves trailing delimiters (semicolons, commas)
✅ Handles edge cases:

• Implicit return types (defaults to any)
• Global modules (no auto-fix to prevent issues)
• Computed property names
• Optional methods
• Generic type parameters with constraints

Files Changed

• New Rule Implementation: internal/plugins/typescript/rules/method_signature_style/method_signature_style.go
• Rule Registration: internal/config/config.go
• Test Suite: packages/rslint-test-tools/tests/typescript-eslint/rules/method-signature-style.test.ts (all test cases ported from TypeScript-ESLint)
• Test Configuration: packages/rslint-test-tools/rstest.config.mts (enabled the test)

Test Coverage

All 52 test cases from the original TypeScript-ESLint implementation have been ported:

• 28 valid test cases (covering both style options and edge cases)
• 24 invalid test cases with auto-fix (testing conversions in both directions)

References

• Original Rule: https://typescript-eslint.io/rules/method-signature-style/
• Original Tests: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/tests/rules/method-signature-style.test.ts

Testing

The implementation follows the existing patterns in the codebase and has been verified to:

• Compile without errors
• Follow Go formatting standards
• Match the test structure of other TypeScript-ESLint rules in the project

Related Issues

This addresses the requirement to implement TypeScript-ESLint rules for rslint.

---

🤖 Generated with Claude Code

[kdy1]

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

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

Implement the TypeScript-ESLint method-signature-style rule for rslint.

Objective: Create a complete implementation that enforces using either method signature or property signature style in interfaces and type literals.

Documentation & Resources:

• Rule documentation: https://typescript-eslint.io/rules/method-signature-style/
• TypeScript-ESLint GitHub: https://github.com/typescript-eslint/typescript-eslint
• Original test cases: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/tests/rules/method-signature-style.test.ts

Implementation Requirements:

1. Copy all test cases from the original TypeScript-ESLint repository test file
2. Implement the rule from scratch to differentiate between method and property signatures
3. Support both style preferences (method vs property)
4. Implement auto-fix capability to convert between styles

Testing Requirements:

1. Port all valid and invalid test cases from the original repository
2. Test both configuration options (method/property)
3. Verify auto-fix works correctly
4. Enable the test in packages/rslint-test-tools/rstest.config.mts by uncommenting the line:
   './tests/typescript-eslint/rules/method-signature-style.test.ts'

Success Criteria:

• Rule correctly identifies both signature styles
• Auto-fix converts between styles correctly
• All original test cases pass
• Test is enabled in rstest.config.mts

---

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

[claude]

Code Review: method-signature-style Rule Implementation

Thank you for implementing the method-signature-style rule! This is a comprehensive implementation that follows the existing patterns in the codebase well. Here's my detailed review:

---

✅ Strengths

1. Excellent Code Structure

• Well-organized with clear separation of concerns
• Helper functions are appropriately named and scoped
• Follows the existing patterns from other TypeScript-ESLint rules (consistent with array_type, consistent_type_definitions, etc.)
• Proper use of Go conventions and idiomatic code

2. Comprehensive Edge Case Handling

• ✅ Handles overloaded method signatures with intersection types
• ✅ Preserves trailing delimiters (semicolons, commas)
• ✅ Handles computed property names, optional methods, and generic type parameters
• ✅ Correctly skips getters/setters (lines 450-453)
• ✅ Handles global module detection to prevent unsafe auto-fixes

3. Good Test Coverage

• All 52 test cases from TypeScript-ESLint ported
• Tests cover both valid and invalid cases
• Tests verify auto-fix behavior
• Edge cases like multiline signatures, overloads, and delimiters are tested

4. Auto-fix Implementation

• Proper text extraction using utils.TrimNodeTextRange
• Preserves formatting and comments in type parameters
• Handles both conversion directions (method ↔ property)

---

🐛 Potential Issues

1. Potential Index Out of Bounds (High Priority)

Location: Lines 117-129, 205-227, 237-253, 326-339, 350-363

In multiple places, the code searches backwards/forwards for delimiters (<, >, (, )) without proper bounds checking:

// Line 118-119
for openPos > 0 && ctx.SourceFile.Text()[openPos] \!= '<' {
    openPos--
}

Issue: If the delimiter is not found and openPos reaches 0, the code continues but may produce incorrect results. While the condition openPos > 0 prevents negative indexing, if < is never found, openPos will be 0 and ctx.SourceFile.Text()[0:closePos] may include unintended text.

Recommendation: Add validation after the loop:

for openPos > 0 && ctx.SourceFile.Text()[openPos] \!= '<' {
    openPos--
}
if ctx.SourceFile.Text()[openPos] \!= '<' {
    // Handle error - delimiter not found
    return ""
}

2. Duplicate Code (Medium Priority)

Location: Lines 108-132 (repeated 4 times with slight variations)

The code for extracting type parameters and parameters text is duplicated across:

• convertMethodToProperty (lines 108-160)
• convertPropertyToMethod (lines 204-256)
• convertOverloadedMethodsToProperty (lines 318-366)

Recommendation: Extract this into reusable helper functions:

extractTypeParameters := func(typeParams *ast.NodeList) string {
    // ... shared implementation
}

extractParameters := func(params *ast.NodeList) string {
    // ... shared implementation
}

This would improve maintainability and reduce the risk of bugs from inconsistent implementations.

3. Missing Edge Case: Getter/Setter Check

Location: Lines 450-453

// Skip getters and setters
if methodSig.Kind == ast.KindGetAccessor || methodSig.Kind == ast.KindSetAccessor {
    continue
}

Issue: This check is only in the single method handling (lines 442-465) but not in the overload handling (lines 408-441). While getter/setter overloads are rare/invalid TypeScript, it's good to be defensive.

Recommendation: Consider adding this check in the overload handling as well for consistency.

4. Redundant Iteration (Low Priority)

Location: Lines 468-496

This loop checks for method signatures that weren't already handled by overloadGroups, but based on the logic in groupOverloadedMethods (lines 271-293), all method signatures should already be in the groups.

Observation: The comment "shouldn't happen, but just in case" suggests this is defensive programming, which is fine. However, consider if this is truly necessary or if it could mask bugs in groupOverloadedMethods.

---

⚡ Performance Considerations

1. String Concatenation in Loops

Location: Line 377

functionTypes = append(functionTypes, fmt.Sprintf("(%s%s => %s)", typeParamsText, paramsText, returnTypeText))

Observation: Using append with fmt.Sprintf is fine for small numbers of overloads (typical case). For very large numbers of overloads (unlikely in practice), consider using strings.Builder, but this is likely premature optimization.

2. Multiple Text Scans

Location: Lines 66-88 (getTrailingDelimiter)

The function scans character-by-character to find delimiters. This is reasonable for small distances but could be optimized if needed.

Recommendation: Current implementation is fine for typical use cases. Only optimize if profiling shows this is a bottleneck.

---

🔒 Security Considerations

✅ No security concerns identified

• No user input is executed
• No file system operations beyond reading source files (which is expected)
• No network operations
• Proper bounds checking for array access (with the exception noted in Issue chore(ci): Add Claude Code action #1)

---

📊 Test Coverage

Strengths:

• ✅ All 52 TypeScript-ESLint test cases ported
• ✅ Tests enabled in rstest.config.mts
• ✅ Tests verify both valid and invalid cases
• ✅ Auto-fix outputs are validated

Observations:

• ⚠️ No Go unit tests (e.g., method_signature_style_test.go)
• Other rules in the codebase have Go unit tests (e.g., array_type_test.go, consistent_type_exports_test.go)

Recommendation: While the TypeScript tests provide good coverage, consider adding a Go test file for testing edge cases specific to the Go implementation, especially:

• Bounds checking in text extraction
• Proper error handling when delimiters aren't found
• Performance testing with large overload sets

---

📝 Code Quality Suggestions

1. Add Error Handling

Several helper functions return empty strings on error (e.g., convertMethodToProperty returns "" if methodSig == nil). Consider:

• Logging warnings when unexpected conditions occur
• Or returning (string, error) tuple for better error propagation

2. Consider Using Constants

Location: Lines 74-84

Magic characters like ';', ',', '\n', '\r' could be defined as constants for clarity:

const (
    semicolon = ';'
    comma     = ','
    newline   = '\n'
    carriageReturn = '\r'
)

3. Documentation Comments

Add package-level documentation and godoc comments for exported types/functions:

// MethodSignatureStyleRule enforces consistent use of method signature or 
// property signature style in TypeScript interfaces and type literals.
// 
// This rule checks for two different syntaxes for function members:
// - Method signature: foo(arg: string): number;
// - Property signature: foo: (arg: string) => number;
var MethodSignatureStyleRule = rule.CreateRule(rule.Rule{
    Name: "method-signature-style",
    Run:  run,
})

---

🎯 Summary

Overall Assessment: ⭐⭐⭐⭐ (4/5 stars)

This is a solid implementation that follows the codebase conventions well. The main concerns are:

Must Fix:

1. ✋ Add proper validation after delimiter search loops to prevent incorrect text extraction

Should Fix:
2. ♻️ Refactor duplicate code into helper functions
3. 🧪 Consider adding Go unit tests

Nice to Have:
4. 📖 Add documentation comments
5. 🎨 Use constants for magic characters

The implementation is production-ready with the fixes for Issue #1. The code quality is high, and the feature is well-tested through the TypeScript test suite.

---

Recommendation: ✅ Approve with requested changes - Address the bounds checking issue in the delimiter search loops, and consider the other suggestions for improved maintainability.

Great work overall! 🎉

[claude]

Pull Request Review: method-signature-style Rule Implementation

Thank you for this comprehensive implementation! Overall, this is a well-structured PR with good attention to detail.

---

✅ Strengths

Code Quality

• Clean Architecture: The implementation follows established patterns well
• Comprehensive Feature Set: Handles edge cases like overloaded methods, global modules, optional methods, and generic type parameters
• Good Documentation: The PR description clearly explains the implementation
• Test Coverage: All 52 test cases from TypeScript-ESLint have been ported

Implementation Details

• Proper use of AST node accessors
• Correct handling of optional tokens with QuestionToken
• Appropriate use of the rule framework fix system

---

🔍 Areas for Improvement

1. Code Duplication in Text Extraction (Medium Priority)

The code has significant duplication when extracting type parameters and function parameters. This pattern appears 4 times throughout method_signature_style.go (lines 108-131, 204-227, 318-340, 230-255).

Recommendation: Extract into helper functions like extractTypeParametersText and extractFunctionParametersText. This would reduce ~100 lines of code and improve maintainability.

2. Potential Index Out of Bounds (Low Priority)

Location: method_signature_style.go:117-129, 143-155

The code searches backward/forward for delimiters without robust bounds checking. If the delimiter isn't found in malformed code, it could lead to incorrect text extraction.

3. Overloaded Method Grouping Logic (Medium Priority)

Location: method_signature_style.go:271-293

The grouping key only uses the name text. Compare with adjacent_overload_signatures.go:159 which uses a composite key including static, callSignature, and nameType. This might incorrectly group methods with the same name but different modifiers (e.g., optional vs required).

4. Double Iteration Over Members (Low Priority)

Location: method_signature_style.go:468-496

After processing overload groups, the code iterates over all members again. This might be unnecessary since groupOverloadedMethods already processes all method signatures.

5. Missing Documentation (Low Priority)

No Godoc comments on exported types and functions. Consider adding documentation for maintainability.

---

🔒 Security Considerations

✅ No security concerns identified. The implementation properly handles text ranges and uses safe string operations.

---

⚡ Performance Considerations

• Minor O(n²) complexity due to multiple iterations
• String concatenation could use strings.Builder for intersection types (line 381)
• Text scanning could be optimized with scanner utilities

Overall Impact: Low - these optimizations likely won't matter for typical codebases.

---

🧪 Test Coverage

✅ Excellent test coverage with all 52 test cases ported. The test improvements adding actual type parameter names are good.

---

🎯 Final Assessment

Score: 8/10

Recommendation: Approve with suggestions

This is solid work ready to merge with minor refinements. I'd recommend:

1. Before merging: Address code duplication by extracting helper functions
2. Before merging: Add bounds checking for delimiter searches
3. Future improvement: Add Godoc comments
4. Future improvement: Review overload grouping logic

Great work! The attention to detail in handling edge cases like global modules and overloaded methods is impressive. 🎉