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

check-script-has-no-table-name doesn't ignore text within Jinja comment blocks #9

Closed
neddonaldson opened this issue Feb 23, 2021 · 1 comment · Fixed by #11
Closed
Assignees
Labels
bug Something isn't working

Comments

@neddonaldson
Copy link
Contributor

Describe the bug
If we use the word "from" or "join" in a Jinja comment block the parser thinks that the next word is the name of a table or CTE and passes an error.

sql:

{# This is a test of the check-script-has-no-table-name hook, from pre-commit-dbt

We would expect the hook to ignore this text because it is in a jinja comment block
and not actually a join to any other table.

#}

output from pre-commit run

Check the script has not table name......................................Failed
- hook id: check-script-has-no-table-name
- exit code: 1

test.sql: does not use source() or ref() macros for tables:
- pre-commit-dbt
- to

To Reproduce
Steps to reproduce the behavior:

  1. create a new sql file
  2. add into the file
{# This is a test of the check-script-has-no-table-name hook, from pre-commit-dbt

We would expect the hook to ignore this text because it is in a jinja comment block
and not actually a join to any other table.

#}
  1. Run pre-commit with the check-script-has-no-table-name hook enabled

Expected behavior
I'd expect the parser to ignore comment blocks since it's not important if they don't use ref() or source()

Version:
v0.1.1

@tomsej
Copy link
Contributor

tomsej commented Feb 24, 2021

Good catch @neddonaldson!! Thanks a lot. I fixed that in #11. If you do not wait for a new version of pre-commit-dbt you can run pre-commit autoupdate --bleeding-edge to update to HEAD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants