-
Notifications
You must be signed in to change notification settings - Fork 503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Indent rule cannot be suppressed via ktlint-disable directives in the middle of a file. #631
Comments
Maybe this is desired behavior since it can only run on the entire file in order to properly calculate indent. |
Imho, this rule should be possible to disable for some parts of the file. Though not sure how often it will be used, so I would propose to wait for other users requests to implement this. |
I agree, I think it would be useful to be able to turn this feature off for a specific line. Below is something replicating our scenario. Whether the example below should be additionally logged as a separate issue I'm not sure: let me know if you'd like me to log it.
The code above is how IntelliJ formats it for us. The error from ktlint is: When we ask ktlint to fix the code, it changes it to this:
I don't think the fix above looks right (and it disagrees with what IntelliJ does), which is why we wanted to disable the rule just for that line. |
Is already fixed by #796 |
@paul-dingemans how has #796 fixed it? |
I meant that the example below has already been fixed by another issue:
Based on the existing unit test below, I just copied the issue number from the comment without checking it thoroughly:
The commit message in which this change was made refers to #833 which in turn refers again to #796. |
Up until ktlint 0.46 the Rule class provided only one life cycle hook. This "visit" hook was called in a depth-first-approach on all nodes in the file. A rule like the IndentationRule used the RunOnRootOnly visitor modifier to call this lifecycle hook for the root node only in combination with an alternative way of traversing the ASTNodes. Downside of this approach was that suppression of the rule on blocks inside a file was not possible (#631). More generically, this applied to all rules, applying alternative traversals of the AST. The Rule class now offers new life cycle hooks: beforeFirstNode: This method is called once before the first node is visited. It can be used to initialize the state of the rule before processing of nodes starts. The ".editorconfig" properties (including overrides) are provided as parameter. beforeVisitChildNodes: This method is called on a node in AST before visiting its child nodes. This is repeated recursively for the child nodes resulting in a depth first traversal of the AST. This method is the equivalent of the "visit" life cycle hooks. However, note that in KtLint 0.48, the UserData of the rootnode no longer provides access to the ".editorconfig" properties. This method can be used to emit Lint Violations and to autocorrect if applicable. afterVisitChildNodes: This method is called on a node in AST after all its child nodes have been visited. This method can be used to emit Lint Violations and to autocorrect if applicable. afterLastNode: This method is called once after the last node in the AST is visited. It can be used for teardown of the state of the rule. The "visit" life cycle hook will be removed in Ktlint 0.48. In KtLint 0.47 the "visit" life cycle hook will be called only when hook "beforeVisitChildNodes" is not overridden. It is recommended to migrate to the new lifecycle hooks in KtLint 0.47. Please create an issue, in case you need additional assistence to implement the new life cycle hooks in your rules. Closes #631 Highlight of changes included: * Extend Rule hooks with beforeFirstNode, beforeVisitChildNodes, afterVisitChildNodes and afterLastNode and deprecating visit. * Simplification of logic - Remove node parameter from VisitorProvider result to simplify code - Simplify condition to rebuild supressed region locator - Checking for a suppressed region on the root node is not necessary. By default, the root node (e.g. the node with ElementType FILE) can not be suppressed. A '@file:Suppress' annotation on top of the file is already contained in a non-root node. - Remove indentation logic of complex arguments in ArgumentListWrappingRule and ParameterListWrappingRule as this is the responsibility of the IndentationRule * Remove concurrent visitor provider: Despite the name, the concurrent visitor is not faster than the sequential visitor as each file is processed on a single thread. There seem to be no other benefits for having two different visitor providers. Inlined the sequentialVisitor. * Fix early return when no rules have to be run * Extract PreparedCode and move PsiFileFactory to this vlass * Move logic to check whether to run on root node only or on all nodes from VisitorProvider to Ktlint. The visitor provider is only responsible for running the relevant rules in the correct order. On which node is to be executed is not relevant. * Deprecate the RunOnRootNodeOnly visitor modifier: This visitor modifier was typically used to run a custom node visit algorithm using the root node and the generic "package.visit" methods. The "package.visit" method however did not support rule suppression. With the new lifecycle hooks on the Rule class the visit modifier has become obsolete. * Refactor rules to comply with new life cycle hooks. Some rules needed a bigger refactoring to achieve this. * Provide EditorConfigProperties directly instead of via ASTNode UserData: Prepare to remove ".editorconfig" properties from the UserData in ktlint 0.48. Those properties are only required on the root node but the "getUserData" method can be called on any node. The new Rule lifecycle hooks "beforeFirstNode" provides the ".editorconfig" properties directly to the rule. This contract is cleaner and more explicit.
Since it operates on the root node, it can only be disabled via a
// ktlint-disable
directive at the very top of the file.The final newline rule exhibits the same behavior but that's less confusing.
The text was updated successfully, but these errors were encountered: