megalinter icon indicating copy to clipboard operation
megalinter copied to clipboard

Support Auto-Fix with Sqlfluff

Open BrunoJuchli opened this issue 1 year ago • 11 comments

Problem

Currently megalinter Sqlfluff integration does not support fixing of files.

I have tried different combinations, like:

passing fix as argument

SQL_SQLFLUFF_ARGUMENTS: "fix"
SQL_SQLFLUFF_CLI_LINT_MODE: "file"

=> Error says fix is not a valid file path

reconfiguring executable

SQL_SQLFLUFF_CLI_EXECUTABLE: ['sqlfluff', 'fix']
SQL_SQLFLUFF_CLI_LINT_MODE: "file"

=> Error says 'lint' is not a valid file path

User Error: Specified path does not exist. Check it/they exist(s): lint.

Proposed Solution

Use the standard way of applying auto-fixes, by either:

-adding 'SQL_SQLFLUFF ' to 'APPLY_FIXES' -setting 'APPLY_FIXES' to 'all'

When auto-fixing is enabled, the following should happen:

  • instead of passing 'lint', passes 'fix' to sqlfluff executable.
  • switches 'SQL_SQLFLUFF_CLI_LINT_MODE' to file (sqlfluff fix only supports a single file or folder path as argument)

Alternative Configurability

Add a new configurable variable

SQL_SQLFLUFF_FIX with a default value of 'false'.

BrunoJuchli avatar Jul 11 '23 09:07 BrunoJuchli

@BrunoJuchli thanks for the analysis and propositions :)

When sqlfluff is called with sqlfluff fix, does it also lint ?

There are ways to replace lint by fix just in descripto

image

but replacing the cli_lint_mode could be done only in a Python class associates to the linter descriptor, something like in overridden method before_lint_files

if self.apply_fixes is True:
   self.cli_lint_mode = file

image

Would you like to make a PR ? :)

nvuillam avatar Jul 11 '23 20:07 nvuillam

@nvuillam Thanks for the input. Sorry it took me so long to respond. Initially I was under the impression that I could also somehow remove the lint argument without actually modifying the mega-linter code. So I waited until I had some time on my hands to try it out... but of course now I know better ^^

re whether sqlfluff fix also lints: I interpret your question to mean "does it list errors it cannot fix". I had to do a search on this, as it isn't obvious to me either :D So finally I found the answer on their slack channel. For fix + lint we should call it as follows:

sqlfluff fix --show-lint-violations

However, not sure if it actually works. At least for a specific scenario it seems to fail to report linting errors: https://github.com/sqlfluff/sqlfluff/issues/4871

Re providing a PR: last time I worked with python was more than 10 years ago... so I fear it might take me quite long => I don't want my boss to get out his Kloppe ;-)

image

BrunoJuchli avatar Jul 17 '23 13:07 BrunoJuchli

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

github-actions[bot] avatar Aug 17 '23 00:08 github-actions[bot]

@BrunoJuchli I learnt python to build MegaLinter, the code is simple :p

And i'll help if necessary :)

nvuillam avatar Aug 21 '23 19:08 nvuillam

@nvuillam I am unsure if it is related to the reported problem, but sqlfluff fix is not working for mega-linter-runner either. I tested the following: mega-linter-runner -r v7 --fix mega-linter-runner -r v7 --flavor cupcake --fix and mega-linter-runner -r v7 -e 'APPLY_FIXES=all' Errors are reported correctly however using --fix parameters is not fixing any of SQL errors. If I run locally sqlfluff fix some are fixed. Have you noticed similar issues?

P107RP avatar Aug 25 '23 16:08 P107RP

@P107RP today sqlfluff fix is not called, it would require a PR ^^

nvuillam avatar Aug 25 '23 23:08 nvuillam

@nvuillam I think it is a very handy feature ;) Do you have any plans to implement this, with a next version maybe?

P107RP avatar Aug 28 '23 11:08 P107RP

I have a crazy backlog, not only on MegaLinter, so i'll probably do it someday but that might be not soon ^^

If someone submits a PR I'll be glad to accept it :)

nvuillam avatar Aug 28 '23 11:08 nvuillam

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

github-actions[bot] avatar Sep 28 '23 00:09 github-actions[bot]

The issue went stale but any chance this will get implemented soon?

YElyousfi avatar Jan 29 '24 21:01 YElyousfi

It depends on if a generous contributor wants to spent some time on it to submit a PR :) On my side... I'm drowning sorry ^^

nvuillam avatar Jan 29 '24 21:01 nvuillam