Add eslint-plugin-newline-destructuring#4975
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the eslint-plugin-newline-destructuring plugin, including updates to the package dependencies, plugin registration, and documentation. It also introduces new test cases for the newline-destructuring/newline pattern. A regression was identified in docs/patterns.json where a relative path was inadvertently changed to an absolute local path, which needs to be reverted to ensure environment portability.
| { | ||
| "name": "langDir", | ||
| "default": "node_modules/eslint-plugin-spellcheck/rules/utils/dicts" | ||
| "default": "/Users/lorenzo/codacy/github/codacy-eslint/node_modules/eslint-plugin-spellcheck/rules/utils/dicts" |
There was a problem hiding this comment.
The default value for langDir has been updated to an absolute path from a local machine (/Users/lorenzo/...). This is a regression that will cause the tool to fail in other environments (CI, Docker, etc.) where this path does not exist. It should be reverted to the relative path.
| "default": "/Users/lorenzo/codacy/github/codacy-eslint/node_modules/eslint-plugin-spellcheck/rules/utils/dicts" | |
| "default": "node_modules/eslint-plugin-spellcheck/rules/utils/dicts" |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| 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.
Register the plugin and add a multiple-test that verifies the newline-destructuring/newline rule via a project .eslintrc.json.
There was a problem hiding this comment.
Pull Request Overview
The integration of eslint-plugin-newline-destructuring is mostly complete, but there are critical omissions in the documentation metadata and test configuration. Although the rule is registered and utilized in tests, the metadata in docs/patterns.json and docs/description/description.json lacks the definition of the 'items' parameter. This will prevent users from configuring the rule via the platform UI. Additionally, the integration test patterns.xml fails to enable the rule module, which likely prevents the test from actually validating the logic. These issues should be resolved before merging to ensure maintainability and correct functionality.
About this PR
- The rule
newline-destructuring/newlinesupports an 'items' configuration parameter (as used in your integration tests). However, this parameter is not defined in the documentation metadata, which will prevent it from being configurable via the platform's user interface.
Test suggestions
- Verify that the plugin is correctly registered in the engine's plugin registry.
- Verify that the newline-destructuring/newline rule is present in the generated documentation files.
- Integration test: the newline-destructuring/newline rule flags a single-line destructuring assignment containing 3 items when 'items' threshold is set to 2.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| <module name="root"> | ||
| <module name="BeforeExecutionExclusionFileFilter"> | ||
| <property name="fileNamePattern" value=".*\.json" /> | ||
| </module> | ||
| </module> |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The 'patterns.xml' file defines an exclusion filter but fails to enable the actual pattern being tested. Patterns must be explicitly listed as modules to be active during execution.
| <module name="root"> | |
| <module name="BeforeExecutionExclusionFileFilter"> | |
| <property name="fileNamePattern" value=".*\.json" /> | |
| </module> | |
| </module> | |
| <module name="root"> | |
| <module name="BeforeExecutionExclusionFileFilter"> | |
| <property name="fileNamePattern" value=".*\.json" /> | |
| </module> | |
| <module name="newline-destructuring_newline" /> | |
| </module> |
| { | ||
| "parameters": [], |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The pattern metadata is incomplete. It lacks a 'description' field and does not define the 'items' parameter, which is supported by the rule and used in the test suite. Defining these is necessary for proper documentation and UI-based configuration.
Suggested update: Update the metadata to include the 'items' parameter (type integer) and a descriptive text (e.g., 'Enforce newline usage in destructuring assignment').
| "patternId": "newline-destructuring_newline", | ||
| "level": "Info", | ||
| "category": "CodeStyle", | ||
| "parameters": [], |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: The 'items' parameter supported by this rule should be added to the parameters array in the documentation metadata.
cc65b41 to
d08e15b
Compare
The rule meta type is layout, which Codacy maps to Info level.
The rule meta type is layout, which Codacy maps to Info level.
Summary
eslint-plugin-newline-destructuringas a dependency and register it insrc/eslintPlugins.tsnewline-destructuring_newlineappears inpatterns.jsonandall-patternsdocs/multiple-tests/newline-destructuring/) that enables the rule via.eslintrc.jsonand asserts the expected checkstyle outputTest plan
plugins_testpasses (multiple-tests includingnewline-destructuring)Made with Cursor