Skip to content

Commit

Permalink
Fix warnings from ruff and ruff-format
Browse files Browse the repository at this point in the history
  • Loading branch information
Takishima committed Dec 15, 2023
1 parent 3db5313 commit 5b38184
Showing 19 changed files with 216 additions and 151 deletions.
71 changes: 54 additions & 17 deletions cmake_pc_hooks/_argparse.py
Original file line number Diff line number Diff line change
@@ -28,6 +28,44 @@
# ==============================================================================


class InvalidExecutablePathError(argparse.ArgumentTypeError):
"""Exception raised when a path to an executable is invalid."""

def __init__(self, path: Path) -> None:
"""Initialize an InvalidExecutablePathError."""
super().__init__(f'{path} is not a valid file and/or does not appear executable')


class TOMLFileNotFoundError(FileNotFoundError):
"""Exception raised when a TOML file cannot be found."""

def __init__(self, path: Path) -> None:
"""Initialize a TOMLFileNotFoundError object."""
super().__init__(f'Unable to locate TOML file {path}')


class TOMLSectionKeyError(KeyError):
"""Exception raised when a TOML key cannot be found."""

def __init__(self, section: str, path: Path) -> None:
"""Initialize a TOMLSectionKeyError object."""
super().__init__(f'Unable to locate section {section} in TOML file {path}')


class TOMLTypeError(argparse.ArgumentTypeError):
"""Exception raised when a TOML key does not match the type of a default value."""

def __init__(self, value_type: type, default_value_type: type, toml_key: str) -> None:
"""Initialize a TOMLSectionKeyError object."""
super().__init__(
f'TOML value type {value_type} not compatible with parser argument '
f'{default_value_type} for key "{toml_key}"'
)


# ==============================================================================


def _append_in_namespace(namespace, key, values):
current = getattr(namespace, key, [])
if current is None:
@@ -65,14 +103,14 @@ def executable_path(path: str) -> Path:
"""
if Path(path).is_file() and os.access(path, os.X_OK):
return path
raise argparse.ArgumentTypeError(f'{path} is not a valid file and/or does not appear executable')
raise InvalidExecutablePathError(path)


# ==============================================================================


def _load_data_from_toml(
path: Path, section: str, path_must_exist: bool = True, section_must_exist: bool = True
path: Path, section: str, *, path_must_exist: bool = True, section_must_exist: bool = True
) -> dict:
"""
Load a TOML file and return the corresponding config dictionary.
@@ -98,15 +136,16 @@ def _load_data_from_toml(
else:
config = {key: value for key, value in config.items() if not isinstance(value, dict)}
logging.debug('Loading data from root table of %s', path)
return config
except FileNotFoundError as err:
if path_must_exist:
raise FileNotFoundError(f'Unable to locate TOML file {path}') from err
raise TOMLFileNotFoundError(path) from err
logging.debug('TOML file %s does not exist (not an error)', str(path))
except KeyError as err:
if section_must_exist:
raise KeyError(f'Unable to locate section {section} in TOML file {path}') from err
raise TOMLSectionKeyError(section, path) from err
logging.debug('TOML file %s does not have a %s section (not an error)', str(path), section)
else:
return config
return {}


@@ -226,13 +265,11 @@ def parse_known_args(
if namespace.dump_toml:
exclude_keys = {'positionals', 'dump_toml'}
print(
toml.dumps(
{
key: value
for key, value in vars(namespace).items()
if value != self._default_args[key] and key not in exclude_keys
}
)
toml.dumps({
key: value
for key, value in vars(namespace).items()
if value != self._default_args[key] and key not in exclude_keys
})
)
sys.exit(0)

@@ -243,6 +280,7 @@ def _load_from_toml( # noqa: PLR0913
namespace: argparse.Namespace,
path: Path,
section: str = '',
*,
path_must_exist: bool = True,
section_must_exist: bool = True,
overridable_keys: set | None = None,
@@ -258,7 +296,9 @@ def _load_from_toml( # noqa: PLR0913
section_must_exist: Whether a missing section in the TOML file is considered an error or not
overridable_keys: List of keys that can be overridden by values in the TOML file
"""
config = _load_data_from_toml(path, section, path_must_exist, section_must_exist)
config = _load_data_from_toml(
path, section, path_must_exist=path_must_exist, section_must_exist=section_must_exist
)

for key, value in config.items():
if key not in self._default_args:
@@ -267,10 +307,7 @@ def _load_from_toml( # noqa: PLR0913
# NB: we can only do proper type check if the default value is given...
default_value = self._default_args[key]
if default_value is not None and not isinstance(value, type(default_value)):
raise argparse.ArgumentTypeError(
f'TOML value type {type(value)} not compatible with parser argument '
f'{type(default_value)} for key "{key}"'
)
raise TOMLTypeError(type(value), type(default_value), key)
if overridable_keys is not None and key not in overridable_keys:
logging.debug(' skipping non-overridable key: "%s"', key)
continue
107 changes: 59 additions & 48 deletions cmake_pc_hooks/_cmake.py
Original file line number Diff line number Diff line change
@@ -14,6 +14,8 @@

"""CMake related function and classes."""

from __future__ import annotations

import contextlib
import json
import logging
@@ -33,6 +35,25 @@
# ==============================================================================


def _try_calling_cmake(cmake_cmd: list[str | Path]) -> bool:
"""
Try to call CMake using the provided command.
Args:
cmake_cmd: CMake command line
Return:
True if command line is valid, False otherwise
"""
with Path(os.devnull).open(mode='w', encoding='utf-8') as devnull:
try:
sp.check_call([*cmake_cmd, '--version'], stdout=devnull, stderr=devnull)
except (OSError, sp.CalledProcessError):
return False
else:
return True


def get_cmake_command(cmake_names=None): # pragma: nocover
"""
Get the path to a CMake executable on the PATH or in the virtual environment.
@@ -45,42 +66,33 @@ def get_cmake_command(cmake_names=None): # pragma: nocover
cmake_names = ['cmake', 'cmake3']

for cmake in cmake_names:
with Path(os.devnull).open(mode='w', encoding='utf-8') as devnull:
try:
sp.check_call([cmake, '--version'], stdout=devnull, stderr=devnull)
return [shutil.which(cmake)]
except (OSError, sp.CalledProcessError):
pass

# CMake not in PATH, should have installed Python CMake module
# -> try to find out where it is
python_executable = Path(sys.executable)
try:
root_path = Path(os.environ['VIRTUAL_ENV'])
python = python_executable.name
except KeyError:
root_path = python_executable.parent
python = python_executable.name

search_paths = [root_path, root_path / 'bin', root_path / 'Scripts']

# First try executing CMake directly
for base_path in search_paths:
try:
cmake_cmd = base_path / cmake
sp.check_call([cmake_cmd, '--version'], stdout=devnull, stderr=devnull)
return [cmake_cmd]
except (OSError, sp.CalledProcessError):
pass

# That did not work: try calling it through Python
for base_path in search_paths:
try:
cmake_cmd = [python, base_path / 'cmake']
sp.check_call([*cmake_cmd, '--version'], stdout=devnull, stderr=devnull)
return cmake_cmd
except (OSError, sp.CalledProcessError):
pass
cmake_cmd = [shutil.which(cmake)]
if cmake_cmd is not None and _try_calling_cmake(cmake_cmd):
return cmake_cmd

# CMake not in PATH, should have installed Python CMake module
# -> try to find out where it is
python_executable = Path(sys.executable)
try:
root_path = Path(os.environ['VIRTUAL_ENV'])
python = python_executable.name
except KeyError:
root_path = python_executable.parent
python = python_executable.name

search_paths = [root_path, root_path / 'bin', root_path / 'Scripts']

# First try executing CMake directly
for base_path in search_paths:
cmake_cmd = [base_path / cmake]
if _try_calling_cmake(cmake_cmd):
return cmake_cmd

# That did not work: try calling it through Python
for base_path in search_paths:
cmake_cmd = [python, base_path / 'cmake']
if _try_calling_cmake(cmake_cmd):
return cmake_cmd

# Nothing worked -> give up!
return None
@@ -107,7 +119,8 @@ def __init__(self, cmake_names=None):
self.cmake_configured_files = []
self.no_cmake_configure = False

def add_cmake_arguments_to_parser(self, parser):
@staticmethod
def add_cmake_arguments_to_parser(parser):
"""Add CMake options to an argparse.ArgumentParser."""
# Create option group here to control the order
cmake_options = parser.add_argument_group(
@@ -203,7 +216,7 @@ def add_cmake_arguments_to_parser(self, parser):
'-Wno-dev', dest='no_dev_warnings', action='store_true', help='Suppress developer warnings.'
)

def resolve_build_directory(self, build_dir_list=None, automatic_discovery=True):
def resolve_build_directory(self, build_dir_list=None, *, automatic_discovery=True):
"""Locate a valid build directory based on internal list and automatic discovery if enabled."""
# First try to locate a valid build directory based on internal list
build_dir_list = [] if build_dir_list is None else [Path(path) for path in build_dir_list]
@@ -232,7 +245,7 @@ def resolve_build_directory(self, build_dir_list=None, automatic_discovery=True)
self.build_dir = Path(build_dir_list[0]).resolve()
logging.info('Unable to locate a valid build directory. Will be creating one at %s', str(self.build_dir))

def setup_cmake_args(self, cmake_args):
def setup_cmake_args(self, cmake_args): # noqa: C901
"""
Setup CMake arguments.
@@ -305,7 +318,7 @@ def setup_cmake_args(self, cmake_args):
for arg in platform_args:
self.cmake_args.append(arg.strip('"\''))

def configure(self, command, clean_build=False):
def configure(self, command, *, clean_build=False):
"""
Run a CMake configure step (multi-process safe).
@@ -357,14 +370,12 @@ def _call_cmake(self, extra_args=None):
result = _call_process.call_process(
[*command, str(self.source_dir), *self.cmake_args, *extra_args], cwd=str(self.build_dir)
)
result.stdout = '\n'.join(
[
f'Running CMake with: {[*command, str(self.source_dir), *self.cmake_args]}',
f' from within {self.build_dir}',
result.stdout,
'',
]
)
result.stdout = '\n'.join([
f'Running CMake with: {[*command, str(self.source_dir), *self.cmake_args]}',
f' from within {self.build_dir}',
result.stdout,
'',
])

return result

14 changes: 11 additions & 3 deletions cmake_pc_hooks/_utils.py
Original file line number Diff line number Diff line change
@@ -32,6 +32,13 @@
logging.getLogger('filelock').setLevel(logging.WARNING)


class CMakePresetError(Exception):
"""Exception raised if a command line incompatibility with --preset is detected."""

def __init__(self) -> None:
super().__init__('You *must* specify -B|--build-dir if you pass --preset as a CMake argument!')


def _read_compile_commands_json(compile_db: Path) -> list[str]:
"""Read a JSON compile database and return the list of files contained within."""
if compile_db.exists():
@@ -98,7 +105,7 @@ def parse_args(self, args):
self.build_dir_list.extend(known_args.build_dir if known_args.build_dir else [])

if not known_args.build_dir and known_args.preset:
raise RuntimeError('You *must* specify -B|--build-dir if you pass --preset as a CMake argument!')
raise CMakePresetError

if self.setup_cmake:
self.cmake.setup_cmake_args(known_args)
@@ -152,10 +159,11 @@ def _clinters_compat(self):
self.stderr = self.history[-1].stderr.encode()
self.returncode = self.history[-1].returncode

def _parse_output(self, result): # noqa: ARG002
def _parse_output(self, result): # noqa: ARG002, PLR6301
return NotImplemented

def _resolve_compilation_database(self, cmake_build_dir, build_dir_list):
@staticmethod
def _resolve_compilation_database(cmake_build_dir: Path, build_dir_list: list[Path]) -> Path | None:
"""Locate a compilation database based on internal list of directories."""
if cmake_build_dir and cmake_build_dir / 'compile_commands.json':
return cmake_build_dir / 'compile_commands.json'
2 changes: 1 addition & 1 deletion cmake_pc_hooks/clang_tidy.py
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@ def __init__(self, args):
if not self.cmake.no_cmake_configure or compile_db:
self.add_if_missing([f'-p={compile_db}'])

def _parse_output(self, result):
def _parse_output(self, result): # noqa: PLR6301
"""
Parse output and check whether some errors occurred.
6 changes: 3 additions & 3 deletions cmake_pc_hooks/cppcheck.py
Original file line number Diff line number Diff line change
@@ -62,9 +62,9 @@ def _parse_output(self, result):
# Useless error see https://stackoverflow.com/questions/6986033
logging.debug('parsing output from %s', result.stderr)
useless_error_part = 'Cppcheck cannot find all the include files'
result.stderr = ''.join(
[line for line in result.stderr.splitlines(keepends=True) if useless_error_part not in line]
)
result.stderr = ''.join([
line for line in result.stderr.splitlines(keepends=True) if useless_error_part not in line
])
self._clinters_compat()
return result.returncode != 0

8 changes: 5 additions & 3 deletions cmake_pc_hooks/include_what_you_use.py
Original file line number Diff line number Diff line change
@@ -76,9 +76,11 @@ def __init__(self, args):
IWYUToolCmd.command_for_version = get_iwyu_command()

if IWYUToolCmd.command is None:
raise FileNotFoundError('Unable to locate path to iwyu-tool')
msg = 'Unable to locate path to iwyu-tool'
raise FileNotFoundError(msg)
if IWYUToolCmd.command_for_version is None:
raise FileNotFoundError('Unable to locate path to include-what-you-use executable!')
msg = 'Unable to locate path to include-what-you-use executable!'
raise FileNotFoundError(msg)

super().__init__(self.command, self.lookbehind, args)
self.check_installed()
@@ -90,7 +92,7 @@ def __init__(self, args):
if compile_db:
self.add_if_missing([f'-p={compile_db}'])

def _parse_output(self, result):
def _parse_output(self, result): # noqa: PLR6301
"""
Parse output and check whether some errors occurred.
14 changes: 0 additions & 14 deletions setup.cfg
Original file line number Diff line number Diff line change
@@ -55,18 +55,4 @@ test =
[bdist_wheel]
universal = True


# ==============================================================================

[flake8]

max-line-length = 120
exclude =
.git,
__pycache__,
build,
dist,
__init__.py
docstring-quotes = """
# ==============================================================================
Loading

0 comments on commit 5b38184

Please sign in to comment.