From 253cfaaf058e364336234cc43f3775224de41dbd Mon Sep 17 00:00:00 2001 From: Joseph Young <80432516+jpy-git@users.noreply.github.com> Date: Sat, 27 Nov 2021 12:51:10 +0000 Subject: [PATCH] Add config CLI argument to lint, fix, and parse (#1986) * Add extra-config CLI argument to lint, fix, and parse * rename --extra-config to --config * Fix missing unit test --- src/sqlfluff/cli/commands.py | 29 ++++++++--- src/sqlfluff/core/config.py | 61 +++++++++++++++++++---- src/sqlfluff/core/linter/linter.py | 4 +- test/cli/commands_test.py | 28 +++++++++++ test/fixtures/cli/extra_config_tsql.sql | 4 ++ test/fixtures/cli/extra_configs/.sqlfluff | 2 + 6 files changed, 111 insertions(+), 17 deletions(-) create mode 100644 test/fixtures/cli/extra_config_tsql.sql create mode 100644 test/fixtures/cli/extra_configs/.sqlfluff diff --git a/src/sqlfluff/cli/commands.py b/src/sqlfluff/cli/commands.py index 5442bc7c24e..91dd416f52d 100644 --- a/src/sqlfluff/cli/commands.py +++ b/src/sqlfluff/cli/commands.py @@ -177,7 +177,6 @@ def core_options(f: Callable) -> Callable: f = click.option( "--rules", default=None, - # short_help='Specify a particular rule, or comma separated rules, to check', help=( "Narrow the search to only specific rules. For example " "specifying `--rules L001` will only search for rule `L001` (Unnecessary " @@ -189,7 +188,6 @@ def core_options(f: Callable) -> Callable: f = click.option( "--exclude-rules", default=None, - # short_help='Specify a particular rule, or comma separated rules to exclude', help=( "Exclude specific rules. For example " "specifying `--exclude-rules L001` will remove rule `L001` (Unnecessary " @@ -200,6 +198,17 @@ def core_options(f: Callable) -> Callable: "`L001` and rule `L002`." ), )(f) + f = click.option( + "--config", + "extra_config_path", + default=None, + help=( + "Include additional config file. By default the config is generated " + "from the standard configuration files described in the documentation. This " + "argument allows you to specify an additional configuration file that overrides " + "the standard configuration files. N.B. cfg format is required." + ), + )(f) f = click.option( "--ignore", default=None, @@ -225,7 +234,7 @@ def core_options(f: Callable) -> Callable: return f -def get_config(**kwargs) -> FluffConfig: +def get_config(extra_config_path: Optional[str] = None, **kwargs) -> FluffConfig: """Get a config object from kwargs.""" if "dialect" in kwargs: try: @@ -249,7 +258,10 @@ def get_config(**kwargs) -> FluffConfig: # Instantiate a config object (filtering out the nulls) overrides = {k: kwargs[k] for k in kwargs if kwargs[k] is not None} try: - return FluffConfig.from_root(overrides=overrides) + return FluffConfig.from_root( + extra_config_path=extra_config_path, + overrides=overrides, + ) except SQLFluffUserError as err: # pragma: no cover click.echo( colorize( @@ -396,6 +408,7 @@ def lint( logger: Optional[logging.Logger] = None, bench: bool = False, disable_progress_bar: Optional[bool] = False, + extra_config_path: Optional[str] = None, **kwargs, ) -> NoReturn: """Lint SQL files via passing a list of files or using stdin. @@ -416,7 +429,7 @@ def lint( echo 'select col from tbl' | sqlfluff lint - """ - config = get_config(**kwargs) + config = get_config(extra_config_path, **kwargs) non_human_output = format != FormatType.human.value lnt, formatter = get_linter_and_formatter(config, silent=non_human_output) @@ -548,6 +561,7 @@ def fix( fixed_suffix: str = "", logger: Optional[logging.Logger] = None, disable_progress_bar: Optional[bool] = False, + extra_config_path: Optional[str] = None, **kwargs, ) -> NoReturn: """Fix SQL files. @@ -560,7 +574,7 @@ def fix( # some quick checks fixing_stdin = ("-",) == paths - config = get_config(**kwargs) + config = get_config(extra_config_path, **kwargs) lnt, formatter = get_linter_and_formatter(config, silent=fixing_stdin) verbose = config.get("verbose") @@ -767,6 +781,7 @@ def parse( bench: bool, nofail: bool, logger: Optional[logging.Logger] = None, + extra_config_path: Optional[str] = None, **kwargs, ) -> NoReturn: """Parse SQL files and just spit out the result. @@ -776,7 +791,7 @@ def parse( character to indicate reading from *stdin* or a dot/blank ('.'/' ') which will be interpreted like passing the current working directory as a path argument. """ - c = get_config(**kwargs) + c = get_config(extra_config_path, **kwargs) # We don't want anything else to be logged if we want json or yaml output non_human_output = format in (FormatType.json.value, FormatType.yaml.value) lnt, formatter = get_linter_and_formatter(c, silent=non_human_output) diff --git a/src/sqlfluff/core/config.py b/src/sqlfluff/core/config.py index 7ede53cd0c0..fafdb57dadb 100644 --- a/src/sqlfluff/core/config.py +++ b/src/sqlfluff/core/config.py @@ -298,6 +298,25 @@ def load_config_at_path(self, path: str) -> dict: self._config_cache[str(path)] = configs return configs + def load_extra_config(self, extra_config_path: str) -> dict: + """Load specified extra config.""" + if not os.path.exists(extra_config_path): + raise SQLFluffUserError( + f"Extra config '{extra_config_path}' does not exist." + ) + + # First check the cache + if str(extra_config_path) in self._config_cache: + return self._config_cache[str(extra_config_path)] + + configs: dict = {} + elems = self._get_config_elems_from_file(extra_config_path) + configs = self._incorporate_vals(configs, elems) + + # Store in the cache + self._config_cache[str(extra_config_path)] = configs + return configs + @staticmethod def _get_user_config_dir_path() -> str: appname = "sqlfluff" @@ -329,13 +348,20 @@ def load_user_config(self) -> dict: user_home_path = os.path.expanduser("~") return self.load_config_at_path(user_home_path) - def load_config_up_to_path(self, path: str) -> dict: + def load_config_up_to_path( + self, path: str, extra_config_path: Optional[str] = None + ) -> dict: """Loads a selection of config files from both the path and its parent paths.""" user_appdir_config = self.load_user_appdir_config() user_config = self.load_user_config() config_paths = self.iter_config_locations_up_to_path(path) config_stack = [self.load_config_at_path(p) for p in config_paths] - return nested_combine(user_appdir_config, user_config, *config_stack) + extra_config = ( + self.load_extra_config(extra_config_path) if extra_config_path else {} + ) + return nested_combine( + user_appdir_config, user_config, *config_stack, extra_config + ) @classmethod def find_ignore_config_files( @@ -395,9 +421,13 @@ class FluffConfig: def __init__( self, configs: Optional[dict] = None, + extra_config_path: Optional[str] = None, overrides: Optional[dict] = None, plugin_manager: Optional[pluggy.PluginManager] = None, ): + self._extra_config_path = ( + extra_config_path # We only store this for child configs + ) self._overrides = overrides # We only store this for child configs # Fetch a fresh plugin manager if we weren't provided with one @@ -472,23 +502,33 @@ def __setstate__(self, state): # in the child processes. @classmethod - def from_root(cls, overrides: Optional[dict] = None) -> "FluffConfig": + def from_root( + cls, extra_config_path: Optional[str] = None, overrides: Optional[dict] = None + ) -> "FluffConfig": """Loads a config object just based on the root directory.""" loader = ConfigLoader.get_global() - c = loader.load_config_up_to_path(path=".") - return cls(configs=c, overrides=overrides) + c = loader.load_config_up_to_path(path=".", extra_config_path=extra_config_path) + return cls(configs=c, extra_config_path=extra_config_path, overrides=overrides) @classmethod def from_path( cls, path: str, + extra_config_path: Optional[str] = None, overrides: Optional[dict] = None, plugin_manager: Optional[pluggy.PluginManager] = None, ) -> "FluffConfig": """Loads a config object given a particular path.""" loader = ConfigLoader.get_global() - c = loader.load_config_up_to_path(path=path) - return cls(configs=c, overrides=overrides, plugin_manager=plugin_manager) + c = loader.load_config_up_to_path( + path=path, extra_config_path=extra_config_path + ) + return cls( + configs=c, + extra_config_path=extra_config_path, + overrides=overrides, + plugin_manager=plugin_manager, + ) @classmethod def from_kwargs( @@ -548,9 +588,12 @@ def get_templater(self, templater_name="jinja", **kwargs): ) def make_child_from_path(self, path: str) -> "FluffConfig": - """Make a new child config at a path but pass on overrides.""" + """Make a new child config at a path but pass on overrides and extra_config_path.""" return self.from_path( - path, overrides=self._overrides, plugin_manager=self._plugin_manager + path, + extra_config_path=self._extra_config_path, + overrides=self._overrides, + plugin_manager=self._plugin_manager, ) def diff_to(self, other: "FluffConfig") -> dict: diff --git a/src/sqlfluff/core/linter/linter.py b/src/sqlfluff/core/linter/linter.py index eec2002e9c3..2ca7dc670bc 100644 --- a/src/sqlfluff/core/linter/linter.py +++ b/src/sqlfluff/core/linter/linter.py @@ -102,7 +102,9 @@ def rule_tuples(self) -> List[RuleTuple]: # These are the building blocks of the linting process. @staticmethod - def _load_raw_file_and_config(fname, root_config): + def _load_raw_file_and_config( + fname: str, root_config: FluffConfig + ) -> Tuple[str, FluffConfig, str]: """Load a raw file and the associated config.""" file_config = root_config.make_child_from_path(fname) encoding = get_encoding(fname=fname, config=file_config) diff --git a/test/cli/commands_test.py b/test/cli/commands_test.py index fed91e396f1..bcf1a467a6c 100644 --- a/test/cli/commands_test.py +++ b/test/cli/commands_test.py @@ -112,6 +112,25 @@ def test__cli__command_dialect_legacy(): assert "Please use the 'exasol' dialect instead." in result.stdout +def test__cli__command_extra_config_fail(): + """Check the script raises the right exception on a non-existant extra config path.""" + result = invoke_assert_code( + ret_code=66, + args=[ + lint, + [ + "--config", + "test/fixtures/cli/extra_configs/.sqlfluffsdfdfdfsfd", + "test/fixtures/cli/extra_config_tsql.sql", + ], + ], + ) + assert ( + "Extra config 'test/fixtures/cli/extra_configs/.sqlfluffsdfdfdfsfd' does not exist." + in result.stdout + ) + + @pytest.mark.parametrize( "command", [ @@ -263,6 +282,15 @@ def test__cli__command_lint_stdin(command): ), # Check nofail works (lint, ["--nofail", "test/fixtures/linter/parse_lex_error.sql"]), + # Check config works (sets dialect to tsql) + ( + lint, + [ + "--config", + "test/fixtures/cli/extra_configs/.sqlfluff", + "test/fixtures/cli/extra_config_tsql.sql", + ], + ), ], ) def test__cli__command_lint_parse(command): diff --git a/test/fixtures/cli/extra_config_tsql.sql b/test/fixtures/cli/extra_config_tsql.sql new file mode 100644 index 00000000000..4728505e3a0 --- /dev/null +++ b/test/fixtures/cli/extra_config_tsql.sql @@ -0,0 +1,4 @@ +-- Some tsql specifc sql to test config cli argument. +BEGIN + SELECT 'Weekend'; +END diff --git a/test/fixtures/cli/extra_configs/.sqlfluff b/test/fixtures/cli/extra_configs/.sqlfluff new file mode 100644 index 00000000000..be4c41cc0c7 --- /dev/null +++ b/test/fixtures/cli/extra_configs/.sqlfluff @@ -0,0 +1,2 @@ +[sqlfluff] +dialect = tsql