Skip to content
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

New rule to flag special characters in identifiers #1958

Merged
merged 37 commits into from Nov 24, 2021

Conversation

jpers36
Copy link
Contributor

@jpers36 jpers36 commented Nov 22, 2021

Fixes #1746

Brief summary of the change made

A new linting rule: flag special characters in identifiers.

If an identifier includes a character other than alphanumeric or underscore, this rule will flag it.
This rule includes configuration settings to control which types of identifiers fall under the rule.
This rule also includes a configuration setting to allow or disallow spaces as part of an identifier.

+L057 python script
+L057 test YAML
+L057 configuration defaults
+allow_space_in_identifier description in config_info.py

@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #1958 (465f286) into main (67023b8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #1958   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          147       148    +1     
  Lines        10284     10300   +16     
=========================================
+ Hits         10284     10300   +16     
Impacted Files Coverage Δ
src/sqlfluff/core/rules/config_info.py 100.00% <ø> (ø)
src/sqlfluff/rules/L057.py 100.00% <100.00%> (ø)

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 67023b8...465f286. Read the comment docs.

@jpers36 jpers36 changed the title Rule special characters in names New rule to flag special characters in identifiers Nov 23, 2021
@jpers36 jpers36 marked this pull request as ready for review November 23, 2021 16:45
Copy link
Contributor

@jpy-git jpy-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few quick tweaks whilst i look through the rule logic

src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jpy-git jpy-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpers36 a couple suggestions formatting wise, plus note on typing, and extra unit tests suggestions.
For the most part its looking great! 💪

(always find it a bit mad that sql allows these non alnum characters)

src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Outdated Show resolved Hide resolved
src/sqlfluff/rules/L057.py Show resolved Hide resolved
test/fixtures/rules/std_rule_cases/L057.yml Show resolved Hide resolved
test/fixtures/rules/std_rule_cases/L057.yml Show resolved Hide resolved
Copy link
Contributor

@jpy-git jpy-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🥇

@jpy-git
Copy link
Contributor

jpy-git commented Nov 24, 2021

@barrywhart I don't have write access so can't approve.

I've looked over the code and all feedback has been addressed so can you take a quick look over and if there's nothing else to add then approve and merge 😄

@barrywhart barrywhart merged commit 65cfeb5 into sqlfluff:main Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New rule suggestion: identify and flag special characters in object names
3 participants