pinterest / ktlint

An anti-bikeshedding Kotlin linter with built-in formatter
https://pinterest.github.io/ktlint/
MIT License
6.23k stars 512 forks source link

Format (-F) command is not honouring the baseline file #1072

Closed girish3 closed 5 months ago

girish3 commented 3 years ago

I created a baseline file from the below command, ktlint --baseline=ktlint-baseline.xml

and I could see no errors are reported if I run it again since there are no changes against the baseline file. But when I add the format flag, the baseline file is getting ignored and the terminal spit out the fixed errors,

ktlint --baseline=ktlint-baseline.xml -F does not work as expected!

Can anyone help?

Tapchicoma commented 3 years ago

Current baseline implementation does not prevent linting, but rather it just suppresses found errors.

So in case of -F flag - format is still happening and, if non-formattable error present in the baseline still occurs, it will be printed. Another issue with format and baseline - it relies on error position in text file, so when format changes Kotlin file, errors positions may be updated.

androiddevcoding commented 3 years ago

I also ran into this inconvenience. I am looking for a workaround.

Kardelio commented 1 year ago

same! ^^^ would love if we could filter formatting with the baseline file

paul-dingemans commented 1 year ago

same! ^^^ would love if we could filter formatting with the baseline file

Please feel free to contribute a PR ;-)

Goooler commented 1 year ago

@paul-dingemans Could you validate this issue? I believe it should be fixed in 0.49.0-SNAPSHOT.

paul-dingemans commented 1 year ago

@paul-dingemans Could you validate this issue? I believe it should be fixed in 0.49.0-SNAPSHOT.

Unfortunately it is not solved. Also, I have some major doubts whether we will be able to solve it in a good way.

The baseline functionality is functionality that lives only in Ktlint CLI. So the KtlintRuleEngine and the Rules don't know anything about the baseline.

An interesting idea would be to introduce a new kind of callback which "asks" permission to autocorrect a LintError before actually applying a fix. API Consumers could use this functionality for a kind of step-through linting/formatting where the user can determine per case whether the fix should be applied, or the API consumer could try to automatically add @Suppres("ktlint:rule-id") annotations. This would be a breaking change and all rules need to be accomodated for that.

But this will not solve the issue with the Ktlint CLI Baseline. The baseline contains information about a line number, an offset on the line and a rule-id. This location does not necessarily matches with the current location of the node that caused the issue when the baseline was created. Reasons for this:

  1. The code above the error location was altered by adding or removing lines. Upon invoking Ktlint again, the error location already does not match anymore. When using the baseline functionality with pure linting, this also results in the behavior that the base does not seem to be respected.
  2. Some node in the AST which was processed before the error node altered the code by adding or removing lines. Upon start of Ktlint the error location migth have been pointing at the expected node but due to formatting prior nodes, it is no longer found at the expected position.
paul-dingemans commented 1 year ago

This issue should not be marked as bug. It can be seen as an enhancement. A possbile way to solve would be as follows:

Introduction of the annotation above also have other implecations that need to be mitigated:

This change will require a considerable refactoring. I will not implement it myself as I am not convinced that it is worth to maintain the baseline functionality in the long term. I am a strong believer that a project that implements ktlint also should take the effort to resolve existing problems.

paul-dingemans commented 5 months ago

Fixed by #2671 for rules that implement the new RuleAutocorrectApprovalHandler interface which lets the API Consumer (in this case Ktlint CLI) decide which LintError is to be autocorrected, or not. When using in combination with external rulesets, that have not yet implemented the new RuleAutocorrectApprovalHandler interface, the behavior remains unchanged.