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.

Sign up for GitHub

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

Jump to bottom

Simple API config path #2080

Merged
merged 7 commits into from Dec 9, 2021
Merged

Simple API config path #2080

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bfa8419
Add ability to use default and extra config files in simple API
jpy-git Dec 9, 2021
2fbb019
Refined code and added unit test
jpy-git Dec 9, 2021
e385d9e
Merge branch 'main' into simple_api_config_paths
jpy-git Dec 9, 2021
4281ac3
Fix docstrings
jpy-git Dec 9, 2021
2d9a1f3
Merge branch 'simple_api_config_paths' of https://github.com/jpy-git/…
jpy-git Dec 9, 2021
166d2b3
Update src/sqlfluff/api/simple.py
jpy-git Dec 9, 2021
e258668
Improve coverage
jpy-git Dec 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
124 changes: 94 additions & 30 deletions src/sqlfluff/api/simple.py
View file
Open in desktop
@@ -1,11 +1,49 @@
"""The simple public API methods."""

from typing import Any, Dict, List, Optional
from sqlfluff.core import Linter
from sqlfluff.core import SQLBaseError
from sqlfluff.core import (
dialect_selector,
FluffConfig,
Linter,
SQLBaseError,
SQLFluffUserError,
)
from sqlfluff.core.linter import ParsedString


def get_simple_config(
dialect: str = "ansi",
rules: Optional[List[str]] = None,
exclude_rules: Optional[List[str]] = None,
config_path: Optional[str] = None,
) -> FluffConfig:
"""Get a config object from simple API arguments."""
# Check the requested dialect exists and is valid.
try:
dialect_selector(dialect)
except SQLFluffUserError as err:
raise SQLFluffUserError(f"Error loading dialect '{dialect}': {str(err)}")
except KeyError:
raise SQLFluffUserError(f"Error: Unknown dialect '{dialect}'")
jpy-git marked this conversation as resolved.
Show resolved Hide resolved

# Create overrides for simple API arguments.
overrides = {
"dialect": dialect,
"rules": ",".join(rules) if rules is not None else None,
"exclude_rules": ",".join(exclude_rules) if exclude_rules is not None else None,
}

# Instantiate a config object.
try:
return FluffConfig.from_root(
extra_config_path=config_path,
ignore_local_config=True,
overrides=overrides,
)
except SQLFluffUserError as err: # pragma: no cover
raise SQLFluffUserError(f"Error loading config: {str(err)}")


class APIParsingError(ValueError):
"""An exception which holds a set of violations."""

Expand All @@ -22,23 +60,31 @@ def lint(
dialect: str = "ansi",
rules: Optional[List[str]] = None,
exclude_rules: Optional[List[str]] = None,
config_path: Optional[str] = None,
) -> List[Dict[str, Any]]:
"""Lint a sql string or file.
"""Lint a SQL string.

Args:
sql (:obj:`str`): The sql to be linted
either as a string or a subclass of :obj:`TextIOBase`.
dialect (:obj:`str`, optional): A reference to the dialect of the sql
sql (:obj:`str`): The SQL to be linted.
dialect (:obj:`str`, optional): A reference to the dialect of the SQL
to be linted. Defaults to `ansi`.
rules (:obj:list of :obj:`str`, optional): A subset of rule
references to lint for.
exclude_rules (:obj:list of :obj:`str`, optional): A subset of rule
references to avoid linting for.
rules (:obj:`Optional[List[str]`, optional): A list of rule
references to lint for. Defaults to None.
exclude_rules (:obj:`Optional[List[str]`, optional): A list of rule
references to avoid linting for. Defaults to None.
config_path (:obj:`Optional[str]`, optional): A path to a .sqlfluff config.
Defaults to None.

Returns:
:obj:`list` of :obj:`dict` for each violation found.
:obj:`List[Dict[str, Any]]` for each violation found.
"""
linter = Linter(dialect=dialect, rules=rules, exclude_rules=exclude_rules)
cfg = get_simple_config(
dialect=dialect,
rules=rules,
exclude_rules=exclude_rules,
config_path=config_path,
)
linter = Linter(config=cfg)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can hopefully remove those dialect/rules/exclude_rules arguments to the linter in the future as they should be part of the config. i.e. decouple linter and simple API 👍

Copy link
Member

Choose a reason for hiding this comment

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

That would mean you'd have to have a config file to pass. Might be overkill for some simple options. For example I presume https://online.sqlfluff.com/ uses the simple API and just needs to specify the dialect.

But it is duplication which is annoying.

BTW what happens if you specify a dialect AND a config file? Which ones wins out? Or should that error and it should be either/or but not both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tunetheweb you can still pass a dialect to the Simple API (and the dialect would overwrite that in the config as you'd expect)?
The Simple API itself is unchanged from a user perspective:

res = sqlfluff.lint(
    dialect="snowflake",
    rules=["L001", "L002"],
    config_path="some/path/to/config/.sqlfluff"
)

I'm specifically talking about Linter in the comment which doesn't need those arguments 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as in we don't need to be able to do Linter(dialect="snowflake") since the simple api was the only thing using that. Everything else does Linter(config=cfg).

Copy link
Member

Choose a reason for hiding this comment

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

Ah Gothca.

Yes In agree explicit dialect or rule should override config, so that's good, but I wonder if we need to expose this in the docs though so others know that's the way it's implemented?


result = linter.lint_string_wrapped(sql)
result_records = result.as_records()
Expand All @@ -51,42 +97,60 @@ def fix(
dialect: str = "ansi",
rules: Optional[List[str]] = None,
exclude_rules: Optional[List[str]] = None,
config_path: Optional[str] = None,
) -> str:
"""Fix a sql string or file.
"""Fix a SQL string.

Args:
sql (:obj:`str`): The sql to be linted
either as a string or a subclass of :obj:`TextIOBase`.
dialect (:obj:`str`, optional): A reference to the dialect of the sql
to be linted. Defaults to `ansi`.
rules (:obj:list of :obj:`str`, optional): A subset of rule
references to lint for.
exclude_rules (:obj:list of :obj:`str`, optional): A subset of rule
references to avoid linting for.
sql (:obj:`str`): The SQL to be fixed.
dialect (:obj:`str`, optional): A reference to the dialect of the SQL
to be fixed. Defaults to `ansi`.
rules (:obj:`Optional[List[str]`, optional): A subset of rule
references to fix for. Defaults to None.
exclude_rules (:obj:`Optional[List[str]`, optional): A subset of rule
references to avoid fixing for. Defaults to None.
config_path (:obj:`Optional[str]`, optional): A path to a .sqlfluff config.
Defaults to None.

Returns:
:obj:`str` for the fixed sql if possible.
:obj:`str` for the fixed SQL if possible.
"""
linter = Linter(dialect=dialect, rules=rules, exclude_rules=exclude_rules)
cfg = get_simple_config(
dialect=dialect,
rules=rules,
exclude_rules=exclude_rules,
config_path=config_path,
)
linter = Linter(config=cfg)

result = linter.lint_string_wrapped(sql, fix=True)
fixed_string = result.paths[0].files[0].fix_string()[0]
return fixed_string


def parse(sql: str, dialect: str = "ansi") -> ParsedString:
"""Parse a sql string or file.
def parse(
sql: str,
dialect: str = "ansi",
config_path: Optional[str] = None,
) -> ParsedString:
"""Parse a SQL string.

Args:
sql (:obj:`str`): The sql to be linted
either as a string or a subclass of :obj:`TextIOBase`.
dialect (:obj:`str`, optional): A reference to the dialect of the sql
to be linted. Defaults to `ansi`.
sql (:obj:`str`): The SQL to be parsed.
dialect (:obj:`str`, optional): A reference to the dialect of the SQL
to be parsed. Defaults to `ansi`.
config_path (:obj:`Optional[str]`, optional): A path to a .sqlfluff config.
Defaults to None.

Returns:
:obj:`ParsedString` containing the parsed structure.
"""
linter = Linter(dialect=dialect)
cfg = get_simple_config(
dialect=dialect,
config_path=config_path,
)
linter = Linter(config=cfg)

parsed = linter.parse_string(sql)
# If we encounter any parsing errors, raise them in a combined issue.
if parsed.violations:
Expand Down
17 changes: 17 additions & 0 deletions test/api/simple_test.py
View file
Open in desktop
Expand Up @@ -211,3 +211,20 @@ def test__api__parse_fail():
Line 1, Position 14: Found unparsable section: ' +++'
Line 1, Position 41: Found unparsable section: 'blah'"""
)


def test__api__config_path():
"""Test that we can load a specified config file in the Simple API."""
# Load test SQL file.
with open("test/fixtures/api/api_config_test.sql", "r") as f:
sql = f.read()

# Pass a config path to the Simple API.
res = sqlfluff.parse(
sql,
config_path="test/fixtures/api/extra_configs/.sqlfluff",
)

# Check there are no errors and the template is rendered correctly.
assert len(res.violations) == 0
assert res.tree.raw == "SELECT foo FROM bar;\n"
1 change: 1 addition & 0 deletions test/fixtures/api/api_config_test.sql
View file
Open in desktop
@@ -0,0 +1 @@
SELECT foo FROM {{ table_name }};
2 changes: 2 additions & 0 deletions test/fixtures/api/extra_configs/.sqlfluff
View file
Open in desktop
@@ -0,0 +1,2 @@
[sqlfluff:templater:jinja:context]
table_name=bar