vscode-sqlfluff
vscode-sqlfluff copied to clipboard
Add quick fix for dbt rule L016 "line is too long" - add `-- noqa: L016` at EOL
If formatter is DBT, then, sometimes code creates too long lines that cannot be split. But it violates rule L016.
This is the use case for -- noqa: L016
=> suggest it as a quick fix in cases when this rule is violated.
Maybe: adding this quick fix prompt to all DBT rule violations Cons:
- end users may start to abuse this new freedom and noqa everything
- not all DBT rules are fixable by noqa, e.g. L044 "Somewhere along the “path” to the source data, specify columns explicitly"
I think adding that quick fix option for all violations would be a good idea. Leave it up to the user if they want to noqa everything. I don't think it would be too difficult to add either.
@Gregory108 Do you know if there is a list of rules that are not able to be disabled with noqa? If not I will probably add the prompt to all rules, which might be the way to go anyways.
I am not aware of such a list, nor or the flag associated with the rule that marks local (un)fix-ability. *Hypothetically, it should be possible to derive it from the way parser works with the rule, but that seems to be an overkill.
- Multiline violations may be quick fixed with this suggested syntaxis.
- If there are, say, two violations on the same line, both noqa'ed, then it would be nice to have not repeated
-- noqa: L000 -- noqa: L111
, but more concise spelling as in an example. Some rules I checked are definitely same-line noqa "ingnorable", e.g. 009 and 050.
The following list of suspects - they, IMO, might either need multiline noqa or evaluate not a line, but piece of of code in entirety. *May have missed some; Some included may be resolvable in one-line. Please, specify, if you need this information or need this be double-checked
I walked through the rules for sqlfluff version 1.4.1. Here are the suspects to be non-fixable with the same-line -- noqa:
:
- 015: "DISTINCT used with parentheses." - might be that parenthesis are on different lines and one noqa not enough/confuses the parser
- 017: "Function name not immediately followed by parenthesis." - same suspicion, parenthesis on different lines
- 019: "Leading/Trailing comma enforcement." - as in antipattern example, comma on the next line is "double" violation - of missing trailing comma on the line above and redundant leading comma on the line
- 030: "Inconsistent capitalisation of function names." - not sure how violations are "assigned": inconsistent capitalisation are evaluated in the entirety of spelling variants
-
032: "Prefer specifying join keys instead of using USING." -
USING (key1, key2, key3)
may span several lines -
034: "Select wildcards then simple targets before calculations and aggregates." -
SELECT
has to be evaluated in its entirety -
035: "Do not specify
else null
in a case when statement (redundant)." -else null
might be multiline - 037: "Ambiguous ordering directions for columns in order by clause." - might be multiline
- 038: "Trailing commas within select clause." - one strange, but possible problem might be that the comma is on a separate line from the last variable
- 040: "Inconsistent capitalisation of boolean/null literal." - all uses throughout the code are taken into account by the rule
-
041: "
SELECT
modifiers (e.g.DISTINCT
) must be on the same line asSELECT
." - the violation is multiline by nature - 042: "Join/From clauses should not contain subqueries. Use CTEs instead." - usually, multiline violation
- 043: "Join/From clauses should not contain subqueries. Use CTEs instead." - usually, multiline violation
- 044: "Query produces an unknown number of result columns." / "Somewhere along the “path” to the source data, specify columns explicitly" - evaluates the code in its entirety
- 054: "Inconsistent column references in GROUP BY/ORDER BY clauses." - multiline by nature
- 058: "Nested CASE statement in ELSE clause could be flattened." - multiline by nature
-
063: "Inconsistent capitalisation of datatypes." - especially with parameter
extended_capitalisation_policy="consistent"
evaluates the code in its entirety -
064: "Consistent usage of preferred quotes for quoted literals." - especially with parameter
preferred_quoted_literal_style="consistent"
@Gregory108 Thanks for putting together that list. I have release v0.1.8 that adds the noqa
and noqa: <rule>
options.
Currently, I have those options for all the rules, but my plan is to add a configuration option where you can disable rules so it won't show up for some of them. Then I will add those rules as the default to that list.
I could also add the noqa: disable=<rule>
option for all rules that way you still have a way to disable those rules within the code.
Let me know your thoughts on how it works and if I should continue with the plan I outlined or if you think there is a better way to handle this.
Completed partial testing. Single and compound one-line noqas work fine so far. Regarding UX:
It seems better to sort suggested options by frequency of use, which, I expect, would be:
- Ignore rule X for this line
- Ignore all rules for this line
- Exclude rule from Workspace settings
- Exclude rule from Global settings (btw, for new users it may be confusing what they are configuring by this action) .. after all, if one excluded the rule from Global settings, they would never need to ignore the rule for any line. One- vs multi-time-use options
Thank you for adding this shortcut! I did not expect it would take so much code (as I see in the commits of the Release)
Yeah, some of the code was just refactoring. Also, I already knew what needed to be added so it was not too difficult.
In release v0.1.9 I have added some additional configuration settings and re-ordered the rules as you suggested.
"sqlfluff.codeActions.excludeRules.global": true,
"sqlfluff.codeActions.excludeRules.workspace": true,
// "sqlfluff.codeActions.noqa": false (to disable the code actions altogether)
"sqlfluff.codeActions.noqa": [
"L015","L017","L019","L030","L032","L034","L035","L037", "L038",
"L040","L041","L042","L043","L044","L054","L058","L063","L064"
],
The default for sqlfluff.codeActions.noqa
is the rules you listed so those rules will not see the noqa
code actions. Probably needs a better setting name, but the description makes that more clear. Let me know if this is helpful.
I did a lot of refactoring in the v0.1.9 : Re-order code actions. Refactor code commit so if you are wanting to look at the changes the v0.1.9 : Implement sqlfluff.codeActions options to disable code actions has the addition of those settings and doesn't have refactoring.