Fix disabling FILE_NAMES_LOWER_SNAKE_CASE #474
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…te visitor methods to accept proto parameter Close #451.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors visitor finalization to receive the full *parser.Proto context, adds support for disabling the FILE_NAMES_LOWER_SNAKE_CASE rule via a comment directive, and covers this behavior with new tests.
- Updated
Finallysignatures on theHasExtendedVisitorinterface and all visitor implementations to accept*parser.Proto. - Introduced directive-based disabling for the file name snake case rule.
- Added tests to verify disabling and non-disabling scenarios for file renaming.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| linter/visitor/hasExtendedVisitor.go | Changed Finally() to Finally(*parser.Proto) and removed OnStart. |
| linter/visitor/extendedDisableRuleVisitor.go | Updated Finally to check disable directive but passing no comments. |
| internal/addon/rules/fileNamesLowerSnakeCaseRule.go | Swapped OnStart for Finally to perform rename logic. |
| internal/addon/rules/fileNamesLowerSnakeCaseRule_test.go | Added test cases for disabling and non-disabling scenarios. |
Comments suppressed due to low confidence (1)
internal/addon/rules/fileNamesLowerSnakeCaseRule_test.go:202
- This test case is named as if it includes a disable directive, but the
inputProtodoesn't include a correspondingparser.Comment. Add a comment node with the disable directive so the rule actually skips renaming.
name: "no fix for a proto with disable directive",
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several changes to enhance the functionality of proto file linting, primarily by refactoring the
Finallymethods across various visitors to accept a*parser.Protoparameter. Additionally, it improves handling of file name rules and introduces new test cases to validate these updates.Refactoring of Visitor Methods:
Updated the
Finallymethod inBaseVisitor,BaseFixableVisitor, and other visitor implementations (indentVisitor,orderVisitor, etc.) to accept a*parser.Protoparameter for better context during finalization. [1] [2] [3] [4] [5]Modified the
extendedAutoDisableVisitorandextendedDisableRuleVisitorto pass the*parser.Protoobject to their inner visitors during theFinallymethod call. [1] [2]File Name Rule Enhancements:
Introduced support for disabling the
FILE_NAMES_LOWER_SNAKE_CASErule using a directive (// protolint:disable FILE_NAMES_LOWER_SNAKE_CASE) in proto files.Adjusted the
fileNamesLowerSnakeCaseVisitorto rename files only when the rule is not disabled and updated itsFinallymethod for clarity.Test Case Additions:
Added test cases to verify that the
FILE_NAMES_LOWER_SNAKE_CASErule is correctly disabled when the directive is present and that no renaming occurs in such cases. [1] [2]Enhanced the test suite for
Finallymethod behavior, ensuring proper handling of the*parser.Protoparameter across various scenarios.Interface and Usage Adjustments:
HasExtendedVisitorinterface to remove theOnStartmethod and modify theFinallymethod to accept a*parser.Protoparameter. Adjusted theRunVisitorAutoDisablefunction accordingly. [1] [2]