diff --git a/client/commands/command.py b/client/commands/command.py index 706a4aaeb29..61f13b9033d 100644 --- a/client/commands/command.py +++ b/client/commands/command.py @@ -50,6 +50,7 @@ class ExitCode(enum.IntEnum): BUCK_ERROR = 3 SERVER_NOT_FOUND = 4 INCONSISTENT_SERVER = 5 + CONFIGURATION_ERROR = 6 # If the process exited due to a signal, this will be the negative signal number. SIGSEGV = -signal.SIGSEGV diff --git a/client/configuration.py b/client/configuration.py index db3299f173b..97999eae65c 100644 --- a/client/configuration.py +++ b/client/configuration.py @@ -161,7 +161,7 @@ def consume( value = self._configuration.pop(key, default) if raise_on_override and current and value: - raise EnvironmentException( + raise InvalidConfiguration( f"Configuration file may not override `{key}` field." ) if current: @@ -316,141 +316,127 @@ def from_arguments( ) def _validate(self) -> None: - try: + def is_list_of_strings(list): + if len(list) == 0: + return True + return not isinstance(list, str) and all( + isinstance(element, str) for element in list + ) - def is_list_of_strings(list): - if len(list) == 0: - return True - return not isinstance(list, str) and all( - isinstance(element, str) for element in list - ) + if not is_list_of_strings(self.source_directories) or not is_list_of_strings( + self.targets + ): + raise InvalidConfiguration( + "`target` and `source_directories` fields must be lists of " "strings." + ) - if not is_list_of_strings( - self.source_directories - ) or not is_list_of_strings(self.targets): - raise InvalidConfiguration( - "`target` and `source_directories` fields must be lists of " - "strings." - ) + if not is_list_of_strings(self.ignore_all_errors): + raise InvalidConfiguration( + "`ignore_all_errors` field must be a list of strings." + ) - if not is_list_of_strings(self.ignore_all_errors): - raise InvalidConfiguration( - "`ignore_all_errors` field must be a list of strings." - ) + if not is_list_of_strings(self.ignore_infer): + raise InvalidConfiguration( + "`ignore_infer` field must be a list of strings." + ) - if not is_list_of_strings(self.ignore_infer): - raise InvalidConfiguration( - "`ignore_infer` field must be a list of strings." - ) + if not is_list_of_strings(self.extensions): + raise InvalidConfiguration("`extensions` field must be a list of strings.") + if not all( + extension.startswith(".") or not extension for extension in self.extensions + ): + raise InvalidConfiguration( + "`extensions` must only contain strings formatted as `.EXT`" + ) - if not is_list_of_strings(self.extensions): - raise InvalidConfiguration( - "`extensions` field must be a list of strings." - ) - if not all( - extension.startswith(".") or not extension - for extension in self.extensions - ): - raise InvalidConfiguration( - "`extensions` must only contain strings formatted as `.EXT`" - ) + if not os.path.exists(self.binary): + raise InvalidConfiguration(f"Binary at `{self.binary}` does not exist.") - if not os.path.exists(self.binary): - raise InvalidConfiguration(f"Binary at `{self.binary}` does not exist.") + if self.number_of_workers < 1: + raise InvalidConfiguration("Number of workers must be greater than 0.") - if self.number_of_workers < 1: - raise InvalidConfiguration("Number of workers must be greater than 0.") + # Validate typeshed path and sub-elements. + assert_readable_directory_in_configuration(self.typeshed, field_name="typeshed") - # Validate typeshed path and sub-elements. - assert_readable_directory_in_configuration( - self.typeshed, field_name="typeshed" + # A courtesy warning since we have changed default behaviour. + if self._typeshed_has_obsolete_value(): + LOG.warning( + f"It appears that `{self.typeshed}` points at a `stdlib` " + "directory. Please note that the `typeshed` configuration must " + "point to the root of the `typeshed` directory." ) - # A courtesy warning since we have changed default behaviour. - if self._typeshed_has_obsolete_value(): - LOG.warning( - f"It appears that `{self.typeshed}` points at a `stdlib` " - "directory. Please note that the `typeshed` configuration must " - "point to the root of the `typeshed` directory." - ) - - expanded_ignore_paths = [] - for path in self.ignore_all_errors: - expanded = glob.glob(path) - if not expanded: - expanded_ignore_paths.append(path) - else: - expanded_ignore_paths += expanded - self.ignore_all_errors = expanded_ignore_paths + expanded_ignore_paths = [] + for path in self.ignore_all_errors: + expanded = glob.glob(path) + if not expanded: + expanded_ignore_paths.append(path) + else: + expanded_ignore_paths += expanded + self.ignore_all_errors = expanded_ignore_paths - non_existent_ignore_paths = [ - path for path in self.ignore_all_errors if not os.path.exists(path) + non_existent_ignore_paths = [ + path for path in self.ignore_all_errors if not os.path.exists(path) + ] + if non_existent_ignore_paths: + LOG.warning( + "Nonexistent paths passed in to `ignore_all_errors`: " + f"`{non_existent_ignore_paths}`" + ) + self.ignore_all_errors = [ + path + for path in self.ignore_all_errors + if path not in non_existent_ignore_paths ] - if non_existent_ignore_paths: - LOG.warning( - "Nonexistent paths passed in to `ignore_all_errors`: " - f"`{non_existent_ignore_paths}`" - ) - self.ignore_all_errors = [ - path - for path in self.ignore_all_errors - if path not in non_existent_ignore_paths - ] - - non_existent_infer_paths = [ - path for path in self.ignore_infer if not os.path.exists(path) + + non_existent_infer_paths = [ + path for path in self.ignore_infer if not os.path.exists(path) + ] + if non_existent_infer_paths: + LOG.warning( + "Nonexistent paths passed in to `ignore_infer`: " + f"`{non_existent_infer_paths}`" + ) + self.ignore_infer = [ + path + for path in self.ignore_infer + if path not in non_existent_infer_paths ] - if non_existent_infer_paths: - LOG.warning( - "Nonexistent paths passed in to `ignore_infer`: " - f"`{non_existent_infer_paths}`" - ) - self.ignore_infer = [ - path - for path in self.ignore_infer - if path not in non_existent_infer_paths - ] - - for typeshed_subdirectory_name in ["stdlib", "third_party"]: - typeshed_subdirectory = os.path.join( - self.typeshed, typeshed_subdirectory_name - ) - assert_readable_directory_in_configuration(typeshed_subdirectory) - for typeshed_version_directory_name in os.listdir( - typeshed_subdirectory - ): - if not typeshed_version_directory_name[0].isdigit(): - raise InvalidConfiguration( - "Directories inside `typeshed` must only contain " - "second-level subdirectories starting with " - "a version number." - ) - typeshed_version_directory = os.path.join( - typeshed_subdirectory, typeshed_version_directory_name - ) - assert_readable_directory_in_configuration( - typeshed_version_directory - ) - # Validate elements of the search path. - for element in self._search_path: - assert_readable_directory_in_configuration( - element.path(), field_name="search_path" + for typeshed_subdirectory_name in ["stdlib", "third_party"]: + typeshed_subdirectory = os.path.join( + self.typeshed, typeshed_subdirectory_name + ) + assert_readable_directory_in_configuration(typeshed_subdirectory) + for typeshed_version_directory_name in os.listdir(typeshed_subdirectory): + if not typeshed_version_directory_name[0].isdigit(): + raise InvalidConfiguration( + "Directories inside `typeshed` must only contain " + "second-level subdirectories starting with " + "a version number." + ) + typeshed_version_directory = os.path.join( + typeshed_subdirectory, typeshed_version_directory_name ) + assert_readable_directory_in_configuration(typeshed_version_directory) - if not is_list_of_strings(self.other_critical_files): - raise InvalidConfiguration( - "`critical_files` field must be a list of strings." - ) + # Validate elements of the search path. + for element in self._search_path: + assert_readable_directory_in_configuration( + element.path(), field_name="search_path" + ) - if not is_list_of_strings(self.do_not_ignore_errors_in): - raise InvalidConfiguration( - "`do_not_ignore_errors_in` field must be a list of strings." - ) - for directory_name in self.do_not_ignore_errors_in: - assert_readable_directory_in_configuration(directory_name) - except InvalidConfiguration as error: - raise EnvironmentException(str(error)) + if not is_list_of_strings(self.other_critical_files): + raise InvalidConfiguration( + "`critical_files` field must be a list of strings." + ) + + if not is_list_of_strings(self.do_not_ignore_errors_in): + raise InvalidConfiguration( + "`do_not_ignore_errors_in` field must be a list of strings." + ) + for directory_name in self.do_not_ignore_errors_in: + assert_readable_directory_in_configuration(directory_name) @property def version_hash(self) -> str: @@ -529,7 +515,7 @@ def _check_nested_configurations(self, local_root: str) -> None: for ignore_element in parent_local_configuration.ignore_all_errors: if Path(ignore_element).resolve() in paths_to_ignore: excluded_from_parent = True - except EnvironmentException as error: + except InvalidConfiguration as error: parent_error = error if not excluded_from_parent: @@ -541,19 +527,19 @@ def _check_nested_configurations(self, local_root: str) -> None: "into a single configuration, or split the parent configuration to " "avoid inconsistent errors." ) - raise EnvironmentException(error_message) + raise InvalidConfiguration(error_message) elif parent_error: - raise EnvironmentException(parent_error) + raise InvalidConfiguration(str(parent_error)) def _check_read_local_configuration(self, path: str) -> None: if not os.path.exists(path): - raise EnvironmentException( + raise InvalidConfiguration( f"Local configuration path `{path}` does not exist." ) local_configuration = os.path.join(path, LOCAL_CONFIGURATION_FILE) if not os.path.exists(local_configuration): - raise EnvironmentException( + raise InvalidConfiguration( f"Local configuration directory `{path}` does not contain " f"a `{LOCAL_CONFIGURATION_FILE}` file." ) @@ -561,7 +547,7 @@ def _check_read_local_configuration(self, path: str) -> None: self._check_nested_configurations(path) self._read(local_configuration) if not self.source_directories and not self.targets: - raise EnvironmentException( + raise InvalidConfiguration( f"Local configuration `{local_configuration}` does not specify " " any sources to type check." ) @@ -719,7 +705,7 @@ def _read(self, path: str) -> None: # We error elsewhere if there weren't enough parameters passed into pyre. self._expand_relative_paths(configuration_directory) except json.JSONDecodeError as error: - raise EnvironmentException( + raise InvalidConfiguration( "Configuration file at `{}` is invalid: {}.".format(path, str(error)) ) diff --git a/client/pyre.py b/client/pyre.py index 6436545d326..35edd8f500b 100755 --- a/client/pyre.py +++ b/client/pyre.py @@ -143,7 +143,11 @@ def run_pyre(arguments: argparse.Namespace) -> ExitCode: raise FailedOutsideLocalConfigurationException( exit_code, command, str(error) ) - except (buck.BuckException, EnvironmentException) as error: + except ( + buck.BuckException, + configuration_module.InvalidConfiguration, + EnvironmentException, + ) as error: if arguments.command == commands.Persistent.from_arguments: try: commands.Persistent.run_null_server(timeout=3600 * 12) @@ -156,11 +160,11 @@ def run_pyre(arguments: argparse.Namespace) -> ExitCode: exit_code = ExitCode.SUCCESS else: client_exception_message = str(error) - exit_code = ( - ExitCode.BUCK_ERROR - if isinstance(error, buck.BuckException) - else ExitCode.FAILURE - ) + exit_code = ExitCode.FAILURE + if isinstance(error, buck.BuckException): + exit_code = ExitCode.BUCK_ERROR + elif isinstance(error, configuration_module.InvalidConfiguration): + exit_code = ExitCode.CONFIGURATION_ERROR except commands.ClientException as error: client_exception_message = str(error) exit_code = ExitCode.FAILURE diff --git a/client/tests/configuration_test.py b/client/tests/configuration_test.py index 01b93d108b5..02340a940f6 100644 --- a/client/tests/configuration_test.py +++ b/client/tests/configuration_test.py @@ -10,12 +10,11 @@ import sys import unittest from pathlib import Path -from typing import Any, NamedTuple, Optional, cast +from typing import NamedTuple, Optional from unittest.mock import MagicMock, PropertyMock, call, mock_open, patch from .. import configuration from ..configuration import Configuration, InvalidConfiguration, SearchPathElement -from ..exceptions import EnvironmentException from ..find_directories import CONFIGURATION_FILE, LOCAL_CONFIGURATION_FILE @@ -87,7 +86,7 @@ def test_init( {"source_directories": ["a"]}, {}, ] - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): configuration = Configuration( "", local_root="local/path", dot_pyre_directory=Path("/.pyre") ) @@ -750,7 +749,7 @@ def test_nested_configurations( }, {}, ] - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="root", local_root="root/local", @@ -776,7 +775,7 @@ def test_nested_configurations( "extensions": [".a", ".b", ""], }, ] - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="root", local_root="root/local", @@ -794,7 +793,7 @@ def test_nonexisting_local_configuration( os_path_exists.return_value = False os_path_isdir.return_value = True os_path_isfile.return_value = False - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="/", local_root="local", dot_pyre_directory=Path("/.pyre") ) @@ -803,14 +802,14 @@ def test_nonexisting_local_configuration( os_path_exists.return_value = False os_path_isdir.return_value = False os_path_isfile.return_value = True - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="/", local_root="local/.some_configuration", dot_pyre_directory=Path("/.pyre"), ) - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="/", local_root="local/.some_configuration", @@ -821,14 +820,14 @@ def test_nonexisting_local_configuration( os_path_exists.side_effect = lambda path: not path.endswith(".local") os_path_isdir.return_value = lambda path: not path.endswith(".local") os_path_isfile.return_value = lambda path: path.endswith(".local") - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="/", local_root="localdir", dot_pyre_directory=Path("/.pyre"), ) - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): Configuration( project_root="/", local_root="localdir", @@ -877,7 +876,7 @@ def test_validate_configuration( except BaseException: self.fail("Configuration should not raise.") - with self.assertRaises(EnvironmentException): + with self.assertRaises(InvalidConfiguration): json_loads.side_effect = [ { "source_directories": ["a"],