TCE-1578 Categorize better all the rules - #5
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
While the PR successfully introduces the mapping utility and sets the language field to PHP, there is a significant implementation gap regarding rule categorization. Codacy analysis indicates the project is 'Up to Standards', but the current logic fails to meet the internal documentation requirements specified in the code comments.
Specifically, the prefix-based categorization for phpdoc_* rules mentioned in RuleCategories.scala is not implemented, causing these rules to default to CodeStyle instead of Documentation. Additionally, the PR lacks any tests to verify the mapping logic or fallback behavior, which is critical for ensuring categorization accuracy.
About this PR
- This PR lacks unit or integration tests to verify the new mapping logic, the 'CodeStyle' fallback, or that the 'languages' field is correctly set to PHP. Adding tests is recommended to ensure the generator produces the expected metadata.
- The PR description is empty. Providing context for the categorization choices would help in reviewing the accuracy of the mappings in
RuleCategories.scala.
Test suggestions
- Missing recommended test scenario: Verify that a mapped rule (e.g., 'random_api_migration') is correctly assigned its specific category (Security).
- Missing recommended test scenario: Verify that an unmapped rule defaults to the 'CodeStyle' category.
- Missing recommended test scenario: Verify that all generated patterns have the language set to PHP.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing recommended test scenario: Verify that a mapped rule (e.g., 'random_api_migration') is correctly assigned its specific category (Security).
2. Missing recommended test scenario: Verify that an unmapped rule defaults to the 'CodeStyle' category.
3. Missing recommended test scenario: Verify that all generated patterns have the language set to PHP.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| // Documentation - phpdoc_* handled via prefix check below, plus a few stragglers | ||
| "header_comment" -> Pattern.Category.Documentation, |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The implementation is missing the prefix check for phpdoc_* rules mentioned in the comments. These rules will currently default to CodeStyle instead of being categorized as Documentation.
To fix this, consider adding a helper method in RuleCategories.scala:
def categoryFor(patternId: String): Pattern.Category = {
if (patternId.startsWith("phpdoc_")) Pattern.Category.Documentation
else byPatternId.getOrElse(patternId, Pattern.Category.CodeStyle)
}Then, update Generator.scala to use this method instead of a direct map lookup.
No description provided.