L031 - Avoid using aliases in join coditions

[piotrgredowski]

This is just a WIP PR for #469
Just wanted to ask if it is good approach to solve presented problem :)
There is a lot to improve - especially docstrings, SQL file name etc. But yeah. It's just a start :)

[alanmcruickshank]

Hi @piotrgredowski , thanks for getting involved.

A few tips on your approach:

• I think given you're overriding _lint_references_and_aliases and _eval, I don't think you need to inherit from Rule_L025 and you can just inherit from BaseCrawler and have a nice clean rule to work with.
• Rather than list(seg.recursive_crawl('identifier')), I wonder if you can leverage the AliasExpressionSegment and look for an element of type alias_expression explicitly to avoid just assuming the identifiers will come in the right order. Although I'm not sure I can think of a scenario where they wouldn't at the moment. 🤔
• Consider how you might implement a fix solution for this rule too. You can probably get some inspiration from the other rules which already exist but feel free to ask for tips here if you need!

[codecov]

Codecov Report

Merging #473 into master will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #473      +/-   ##
==========================================
+ Coverage   92.62%   92.70%   +0.07%     
==========================================
  Files          29       29              
  Lines        4708     4754      +46     
==========================================
+ Hits         4361     4407      +46     
  Misses        347      347              

Flag	Coverage Δ
#py35	92.58% <100.00%> (+0.07%)	⬆️
#py36	92.58% <100.00%> (+0.07%)	⬆️
#py37	92.58% <100.00%> (+0.07%)	⬆️
#py38	92.53% <100.00%> (+0.07%)	⬆️
#py39	92.53% <100.00%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/sqlfluff/rules/std.py	94.21% <100.00%> (+0.28%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa1bb1c...bdba7c5. Read the comment docs.

[piotrgredowski]

This is very dirty version, returning too many LintResults for all edits/deletions - it (and many other things) will be refactored and fixed later (probably today). :)
I've implemented a fix too. But I couldn't find any test case for LintFixes. Am I blind? @alanmcruickshank can you show me some example? :)

edit: nevermind. I've found tests for LintFix :)

[piotrgredowski]

I've added support for self-joins too.

[NiallRees]

Hey @piotrgredowski this is really cool, nice job with the fix implementation too. I'm going to approve but defer to @alanmcruickshank to merge as he had a few questions above (which I'm 99% sure you've addressed).

[NiallRees]

Thank you!

[alanmcruickshank]

Thanks for this @piotrgredowski ! Very neat PR, good work on the fix, excellent test coverage. 👍

I've got a broader question about whether we should also have a rule against initialisms in aliases for tables in a FROM clause. I think that's a vagueness in the fishtown style guide rather than a problem with the work you've done though. I'll raise a separate issue for it.