Ruff rules: Stricter code quality rules (#417)

Author: tony

CHANGES

@@ -21,6 +21,14 @@ $ pipx install --suffix=@next 'vcspull' --pip-args '\--pre' --force
 
 <!-- Maintainers, insert changes / features for the next release here -->
 
+### Development
+
+- Code quality improved via [ruff] rules (#147)
+
+  This includes fixes made by hand, and with ruff's automated fixes. Despite
+  selecting additional rules, which include import sorting, ruff runs nearly
+  instantaneously when checking the whole codebase.
+
 ## vcspull v1.21.1 (2023-05-28)
 
 _Maintenance only, no bug fixes, or new features_

conftest.py

@@ -8,7 +8,7 @@
 @pytest.fixture(autouse=True)
 def add_doctest_fixtures(
     request: pytest.FixtureRequest,
-    doctest_namespace: t.Dict[str, t.Any],
+    doctest_namespace: dict[str, t.Any],
 ) -> None:
     from _pytest.doctest import DoctestItem
 

docs/conf.py

@@ -1,9 +1,9 @@
 # flake8: noqa: E501
 import inspect
+import pathlib
 import sys
 import typing as t
 from os.path import relpath
-import pathlib
 
 import vcspull
 
@@ -178,9 +178,7 @@
 }
 
 
-def linkcode_resolve(
-    domain: str, info: dict[str, str]
-) -> t.Union[None, str]:  # NOQA: C901
+def linkcode_resolve(domain: str, info: dict[str, str]) -> t.Union[None, str]:
     """
     Determine the URL corresponding to Python object
 
@@ -203,7 +201,7 @@ def linkcode_resolve(
     for part in fullname.split("."):
         try:
             obj = getattr(obj, part)
-        except Exception:
+        except Exception:  # noqa: PERF203
             return None
 
     # strip decorators, which would resolve to the source of the decorator
@@ -228,10 +226,7 @@ def linkcode_resolve(
     except Exception:
         lineno = None
 
-    if lineno:
-        linespec = "#L%d-L%d" % (lineno, lineno + len(source) - 1)
-    else:
-        linespec = ""
+    linespec = "#L%d-L%d" % (lineno, lineno + len(source) - 1) if lineno else ""
 
     fn = relpath(fn, start=pathlib.Path(vcspull.__file__).parent)
 

pyproject.toml

@@ -72,7 +72,7 @@ gp-libs = "*"
 sphinx-autobuild = "*"
 sphinx-autodoc-typehints = "*"
 sphinx-inline-tabs = "*"
-sphinxext-opengraph = "<0.8"  # https://github.com/wpilibsuite/sphinxext-opengraph/issues/100
+sphinxext-opengraph = "<0.8" # https://github.com/wpilibsuite/sphinxext-opengraph/issues/100
 sphinx-copybutton = "*"
 sphinxext-rediraffe = "*"
 sphinx-argparse = "*"
@@ -128,7 +128,7 @@ python_version = 3.9
 warn_unused_configs = true
 files = [
   "src",
-  "tests"
+  "tests",
 ]
 strict = true
 
@@ -161,6 +161,32 @@ exclude_lines = [
   "@overload( |$)",
 ]
 
+[tool.ruff]
+target-version = "py39"
+select = [
+  "E", # pycodestyle
+  "F", # pyflakes
+  "I", # isort
+  "UP", # pyupgrade
+  "B", # flake8-bugbear
+  "C4", # flake8-comprehensions
+  "Q", # flake8-quotes
+  "PTH", # flake8-use-pathlib
+  "SIM", # flake8-simplify
+  "TRY", # Trycertatops
+  "PERF", # Perflint
+  "RUF", # Ruff-specific rules
+]
+
+[tool.ruff.isort]
+known-first-party = [
+  "vcspull",
+]
+combine-as-imports = true
+
+[tool.ruff.per-file-ignores]
+"*/__init__.py" = ["F401"]
+
 [build-system]
 requires = ["poetry_core>=1.0.0", "setuptools>50"]
 build-backend = "poetry.core.masonry.api"

scripts/generate_gitlab.py

@@ -2,13 +2,14 @@
 
 import argparse
 import os
+import pathlib
 import sys
 
 import requests
 import yaml
-
 from libvcs.sync.git import GitRemote
-from vcspull.cli.sync import guess_vcs
+
+from vcspull.cli.sync import CouldNotGuessVCSFromURL, guess_vcs
 from vcspull.types import RawConfig
 
 try:
@@ -39,10 +40,10 @@
 args = vars(parser.parse_args())
 gitlab_host = args["gitlab_host"]
 gitlab_namespace = args["gitlab_namespace"]
-config_filename = args["config_file_name"]
+config_filename = pathlib.Path(args["config_file_name"])
 
 try:
-    if os.path.isfile(config_filename):
+    if config_filename.is_file():
         result = input(
             "The target config file (%s) already exists, \
             do you want to overwrite it? [y/N] "
@@ -57,24 +58,25 @@
             )
             sys.exit(0)
 
-    config_file = open(config_filename, "w")
-except IOError:
-    print("File %s not accesible" % (config_filename))
+    config_file = config_filename.open(mode="w")
+except OSError:
+    print(f"File {config_filename} not accesible")
     sys.exit(1)
 
 response = requests.get(
-    "%s/api/v4/groups/%s/projects" % (gitlab_host, gitlab_namespace),
+    f"{gitlab_host}/api/v4/groups/{gitlab_namespace}/projects",
     params={"include_subgroups": "true", "per_page": "100"},
     headers={"Authorization": "Bearer %s" % (gitlab_token)},
 )
 
-if 200 != response.status_code:
+if response.status_code != 200:
     print("Error: ", response)
     sys.exit(1)
 
-path_prefix = os.getcwd()
+path_prefix = pathlib.Path().cwd()
 config: RawConfig = {}
 
+
 for group in response.json():
     url_to_repo = group["ssh_url_to_repo"].replace(":", "/")
     namespace_path = group["namespace"]["full_path"]
@@ -90,7 +92,7 @@
 
     vcs = guess_vcs(url_to_repo)
     if vcs is None:
-        raise Exception(f"Could not guess VCS for URL: {url_to_repo}")
+        raise CouldNotGuessVCSFromURL(url_to_repo)
 
     config[path][reponame] = {
         "name": reponame,

src/vcspull/__init__.py

@@ -12,6 +12,6 @@
 import logging
 from logging import NullHandler
 
-from . import cli  # NOQA
+from . import cli
 
 logging.getLogger(__name__).addHandler(NullHandler())

src/vcspull/_internal/config_reader.py

@@ -9,7 +9,7 @@
 
     FormatLiteral = Literal["json", "yaml"]
 
-    RawConfigData: TypeAlias = t.Dict[t.Any, t.Any]
+    RawConfigData: TypeAlias = dict[t.Any, t.Any]
 
 
 class ConfigReader:
@@ -26,7 +26,7 @@ def __init__(self, content: "RawConfigData") -> None:
         self.content = content
 
     @staticmethod
-    def _load(format: "FormatLiteral", content: str) -> t.Dict[str, t.Any]:
+    def _load(format: "FormatLiteral", content: str) -> dict[str, t.Any]:
         """Load raw config data and directly return it.
 
         >>> ConfigReader._load("json", '{ "session_name": "my session" }')
@@ -37,14 +37,14 @@ def _load(format: "FormatLiteral", content: str) -> t.Dict[str, t.Any]:
         """
         if format == "yaml":
             return t.cast(
-                t.Dict[str, t.Any],
+                dict[str, t.Any],
                 yaml.load(
                     content,
                     Loader=yaml.SafeLoader,
                 ),
             )
         elif format == "json":
-            return t.cast(t.Dict[str, t.Any], json.loads(content))
+            return t.cast(dict[str, t.Any], json.loads(content))
         else:
             raise NotImplementedError(f"{format} not supported in configuration")
 
@@ -72,7 +72,7 @@ def load(cls, format: "FormatLiteral", content: str) -> "ConfigReader":
         )
 
     @classmethod
-    def _from_file(cls, path: pathlib.Path) -> t.Dict[str, t.Any]:
+    def _from_file(cls, path: pathlib.Path) -> dict[str, t.Any]:
         r"""Load data from file path directly to dictionary.
 
         **YAML file**
@@ -102,7 +102,7 @@ def _from_file(cls, path: pathlib.Path) -> t.Dict[str, t.Any]:
         {'session_name': 'my session'}
         """
         assert isinstance(path, pathlib.Path)
-        content = open(path).read()
+        content = path.open().read()
 
         if path.suffix in [".yaml", ".yml"]:
             format: "FormatLiteral" = "yaml"

src/vcspull/cli/__init__.py

@@ -35,7 +35,7 @@
 @overload
 def create_parser(
     return_subparsers: t.Literal[True],
-) -> t.Tuple[argparse.ArgumentParser, t.Any]:
+) -> tuple[argparse.ArgumentParser, t.Any]:
     ...
 
 
@@ -46,7 +46,7 @@ def create_parser(return_subparsers: t.Literal[False]) -> argparse.ArgumentParse
 
 def create_parser(
     return_subparsers: bool = False,
-) -> t.Union[argparse.ArgumentParser, t.Tuple[argparse.ArgumentParser, t.Any]]:
+) -> t.Union[argparse.ArgumentParser, tuple[argparse.ArgumentParser, t.Any]]:
     parser = argparse.ArgumentParser(
         prog="vcspull",
         formatter_class=argparse.RawDescriptionHelpFormatter,
@@ -80,7 +80,7 @@ def create_parser(
     return parser
 
 
-def cli(_args: t.Optional[t.List[str]] = None) -> None:
+def cli(_args: t.Optional[list[str]] = None) -> None:
     parser, sync_parser = create_parser(return_subparsers=True)
     args = parser.parse_args(_args)
 

src/vcspull/cli/sync.py

@@ -11,6 +11,7 @@
 from libvcs.sync.git import GitSync
 from libvcs.url import registry as url_tools
 
+from .. import exc
 from ..config import filter_repos, find_config_files, load_configs
 
 log = logging.getLogger(__name__)
@@ -55,7 +56,7 @@ def create_sync_subparser(parser: argparse.ArgumentParser) -> argparse.ArgumentP
 
 
 def sync(
-    repo_patterns: t.List[str],
+    repo_patterns: list[str],
     config: pathlib.Path,
     exit_on_error: bool,
     parser: t.Optional[
@@ -91,8 +92,8 @@ def sync(
     for repo in found_repos:
         try:
             update_repo(repo)
-        except Exception:
-            print(
+        except Exception as e:  # noqa: PERF203
+            log.info(
                 f'Failed syncing {repo.get("name")}',
             )
             if log.isEnabledFor(logging.DEBUG):
@@ -102,8 +103,7 @@ def sync(
             if exit_on_error:
                 if parser is not None:
                     parser.exit(status=1, message=EXIT_ON_ERROR_MSG)
-                else:
-                    raise SystemExit(EXIT_ON_ERROR_MSG)
+                raise SystemExit(EXIT_ON_ERROR_MSG) from e
 
 
 def progress_cb(output: str, timestamp: datetime) -> None:
@@ -124,6 +124,11 @@ def guess_vcs(url: str) -> t.Optional[VCSLiteral]:
     return t.cast(VCSLiteral, vcs_matches[0].vcs)
 
 
+class CouldNotGuessVCSFromURL(exc.VCSPullException):
+    def __init__(self, repo_url: str, *args: object, **kwargs: object) -> None:
+        return super().__init__(f"Could not automatically determine VCS for {repo_url}")
+
+
 def update_repo(
     repo_dict: t.Any,
     # repo_dict: Dict[str, Union[str, Dict[str, GitRemote], pathlib.Path]]
@@ -138,9 +143,7 @@ def update_repo(
     if repo_dict.get("vcs") is None:
         vcs = guess_vcs(url=repo_dict["url"])
         if vcs is None:
-            raise Exception(
-                f'Could not automatically determine VCS for {repo_dict["url"]}'
-            )
+            raise CouldNotGuessVCSFromURL(repo_url=repo_dict["url"])
 
         repo_dict["vcs"] = vcs