refactor: Manual fixes for ruff warnings (43 warnings fixed)

docs/conf.py:208:9: PERF203 `try`-`except` within a loop incurs performance overhead src/libvcs/_internal/shortcuts.py:108:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/_internal/shortcuts.py:108:19: F821 Undefined name `LibVCSException` src/libvcs/_internal/shortcuts.py:110:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/_internal/shortcuts.py:110:19: F821 Undefined name `LibVCSException` src/libvcs/_internal/shortcuts.py:120:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/_internal/subprocess.py:437:15: TRY002 Create your own exception src/libvcs/_internal/subprocess.py:437:15: TRY003 Avoid specifying long messages outside the exception class src/libvcs/cmd/svn.py:822:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/sync/git.py:143:19: TRY002 Create your own exception src/libvcs/sync/git.py:143:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/sync/git.py:275:19: TRY002 Create your own exception src/libvcs/sync/git.py:275:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/sync/git.py:385:13: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:410:23: TRY003 Avoid specifying long messages outside the exception class src/libvcs/sync/git.py:439:13: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:447:17: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:459:21: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:465:17: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:473:21: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:480:21: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:498:25: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:498:25: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:509:17: TRY400 Use `logging.exception` instead of `logging.error` src/libvcs/sync/git.py:589:19: TRY002 Create your own exception src/libvcs/sync/git.py:589:19: TRY003 Avoid specifying long messages outside the exception class src/libvcs/sync/git.py:676:23: TRY002 Create your own exception src/libvcs/sync/git.py:676:23: TRY003 Avoid specifying long messages outside the exception class src/libvcs/sync/svn.py:107:12: PTH110 `os.path.exists()` should be replaced by `Path.exists()` src/libvcs/sync/svn.py:107:45: PTH112 `os.path.isdir()` should be replaced by `Path.is_dir()` src/libvcs/sync/svn.py:118:26: PTH118 `os.path.join()` should be replaced by `Path` with `/` operator src/libvcs/sync/svn.py:119:20: PTH110 `os.path.exists()` should be replaced by `Path.exists()` src/libvcs/sync/svn.py:159:24: PTH118 `os.path.join()` should be replaced by `Path` with `/` operator src/libvcs/sync/svn.py:160:12: PTH110 `os.path.exists()` should be replaced by `Path.exists()` src/libvcs/sync/svn.py:161:18: PTH123 `open()` should be replaced by `Path.open()` src/libvcs/sync/svn.py:175:23: TRY003 Avoid specifying long messages outside the exception class src/libvcs/url/git.py:291:20: B007 Loop control variable `v` not used within loop body src/libvcs/url/hg.py:220:20: B007 Loop control variable `v` not used within loop body src/libvcs/url/svn.py:204:20: B007 Loop control variable `v` not used within loop body tests/sync/test_git.py:110:12: PTH110 `os.path.exists()` should be replaced by `Path.exists()` tests/sync/test_hg.py:55:10: B017 `pytest.raises(Exception)` should be considered evil tests/sync/test_svn.py:29:12: PTH110 `os.path.exists()` should be replaced by `Path.exists()` tests/test_exc.py:12:15: TRY003 Avoid specifying long messages outside the exception class tests/test_exc.py:38:15: TRY003 Avoid specifying long messages outside the exception class
vcs-python · Aug 20, 2023 · 1018173 · 1018173
1 parent 06f4b43
commit 1018173
Show file tree

Hide file tree

Showing 13 changed files with 45 additions and 39 deletions.
diff --git a/docs/conf.py b/docs/conf.py
@@ -205,7 +205,7 @@ def linkcode_resolve(domain: str, info: dict[str, str]) -> t.Union[None, str]:
     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

diff --git a/src/libvcs/_internal/shortcuts.py b/src/libvcs/_internal/shortcuts.py
@@ -105,9 +105,9 @@ def create_project(
         vcs_matches = url_tools.registry.match(url=url, is_explicit=True)
 
         if len(vcs_matches) == 0:
-            raise LibVCSException(f"No vcs found for {url}")
+            raise VCSNoMatchFoundForUrl(url=url)
         if len(vcs_matches) > 1:
-            raise LibVCSException(f"No exact matches for {url}")
+            raise VCSMultipleMatchFoundForUrl(url=url)
 
         assert vcs_matches[0].vcs is not None
 
@@ -117,7 +117,7 @@ def is_vcs(val: t.Any) -> "TypeGuard[VCSLiteral]":
         if is_vcs(vcs_matches[0].vcs):
             vcs = vcs_matches[0].vcs
         else:
-            raise InvalidVCS(f"{url} does not have supported vcs: {vcs}")
+            raise VCSNotSupported(url=url, vcs=vcs_matches[0].vcs)
 
     if vcs == "git":
         return GitSync(url=url, dir=dir, progress_callback=progress_callback, **kwargs)

diff --git a/src/libvcs/_internal/subprocess.py b/src/libvcs/_internal/subprocess.py
@@ -434,7 +434,8 @@ def check_output(
         output = subprocess.check_output(input=input, **params)
         if isinstance(output, (bytes, str)):
             return output
-        raise Exception(f"output is not str or bytes: {output}")
+
+        raise SubprocessCheckOutputError(output=output)
 
     @overload
     def run(

diff --git a/src/libvcs/cmd/svn.py b/src/libvcs/cmd/svn.py
@@ -819,7 +819,7 @@ def propset(
         elif isinstance(value_path, pathlib.Path):
             local_flags.extend(["--file", str(pathlib.Path(value_path).absolute())])
         else:
-            raise ValueError("Must enter a value or value_path")
+            raise SvnPropsetValueOrValuePathRequired()
 
         if path is not None:
             if isinstance(path, (str, pathlib.Path)):

diff --git a/src/libvcs/sync/git.py b/src/libvcs/sync/git.py
@@ -140,7 +140,7 @@ def from_stdout(cls, value: str) -> "GitStatus":
         matches = pattern.search(value)
 
         if matches is None:
-            raise Exception("Could not find match")
+            raise GitStatusParsingException(git_status_output=value)
         return cls(**matches.groupdict())
 
 
@@ -272,7 +272,7 @@ def __init__(
             else next(iter(self._remotes.items()))[1]
         )
         if origin is None:
-            raise Exception("Missing origin")
+            raise GitRemoteOriginMissing(remotes=list(self._remotes.keys()))
         self.url = self.chomp_protocol(origin.fetch_url)
 
     @classmethod
@@ -382,7 +382,7 @@ def update_repo(self, set_remotes: bool = False, *args: Any, **kwargs: Any) -> N
                 commit="HEAD", max_count=1, check_returncode=True
             )
         except exc.CommandError:
-            self.log.error("Failed to get the hash for HEAD")
+            self.log.exception("Failed to get the hash for HEAD")
             return
 
         self.log.debug("head_sha: %s" % head_sha)
@@ -398,7 +398,7 @@ def update_repo(self, set_remotes: bool = False, *args: Any, **kwargs: Any) -> N
         # we must strip the remote from the tag.
         git_remote_name = self.get_current_remote_name()
 
-        if "refs/remotes/%s" % git_tag in show_ref_output:
+        if f"refs/remotes/{git_tag}" in show_ref_output:
             m = re.match(
                 r"^[0-9a-f]{40} refs/remotes/"
                 r"(?P<git_remote_name>[^/]+)/"
@@ -407,7 +407,7 @@ def update_repo(self, set_remotes: bool = False, *args: Any, **kwargs: Any) -> N
                 re.MULTILINE,
             )
             if m is None:
-                raise exc.CommandError("Could not fetch remote names")
+                raise GitRemoteRefNotFound(git_tag=git_tag, ref_output=show_ref_output)
             git_remote_name = m.group("git_remote_name")
             git_tag = m.group("git_tag")
         self.log.debug("git_remote_name: %s" % git_remote_name)
@@ -436,15 +436,15 @@ def update_repo(self, set_remotes: bool = False, *args: Any, **kwargs: Any) -> N
         try:
             process = self.cmd.fetch(log_in_real_time=True, check_returncode=True)
         except exc.CommandError:
-            self.log.error("Failed to fetch repository '%s'" % url)
+            self.log.exception("Failed to fetch repository '%s'" % url)
             return
 
         if is_remote_ref:
             # Check if stash is needed
             try:
                 process = self.cmd.status(porcelain=True, untracked_files="no")
             except exc.CommandError:
-                self.log.error("Failed to get the status")
+                self.log.exception("Failed to get the status")
                 return
             need_stash = len(process) > 0
 
@@ -456,28 +456,28 @@ def update_repo(self, set_remotes: bool = False, *args: Any, **kwargs: Any) -> N
                 try:
                     process = self.cmd.stash.save(message=git_stash_save_options)
                 except exc.CommandError:
-                    self.log.error("Failed to stash changes")
+                    self.log.exception("Failed to stash changes")
 
             # Checkout the remote branch
             try:
                 process = self.cmd.checkout(branch=git_tag)
             except exc.CommandError:
-                self.log.error("Failed to checkout tag: '%s'" % git_tag)
+                self.log.exception("Failed to checkout tag: '%s'" % git_tag)
                 return
 
             # Rebase changes from the remote branch
             try:
                 process = self.cmd.rebase(upstream=git_remote_name + "/" + git_tag)
             except exc.CommandError as e:
                 if any(msg in str(e) for msg in ["invalid_upstream", "Aborting"]):
-                    self.log.error(e)
+                    self.log.exception("Invalid upstream remote. Rebase aborted.")
                 else:
                     # Rebase failed: Restore previous state.
                     self.cmd.rebase(abort=True)
                     if need_stash:
                         self.cmd.stash.pop(index=True, quiet=True)
 
-                    self.log.error(
+                    self.log.exception(
                         "\nFailed to rebase in: '%s'.\n"
                         "You will have to resolve the conflicts manually" % self.dir
                     )
@@ -495,18 +495,18 @@ def update_repo(self, set_remotes: bool = False, *args: Any, **kwargs: Any) -> N
                         # Stash pop failed: Restore previous state.
                         self.cmd.reset(pathspec=head_sha, hard=True, quiet=True)
                         self.cmd.stash.pop(index=True, quiet=True)
-                        self.log.error(
-                            "\nFailed to rebase in: '%s'.\n"
-                            "You will have to resolve the "
-                            "conflicts manually" % self.dir
+                        self.log.exception(
+                            f"\nFailed to rebase in: '{self.dir}'.\n"
+                            + "You will have to resolve the "
+                            + "conflicts manually"
                         )
                         return
 
         else:
             try:
                 process = self.cmd.checkout(branch=git_tag)
             except exc.CommandError:
-                self.log.error("Failed to checkout tag: '%s'" % git_tag)
+                self.log.exception(f"Failed to checkout tag: '{git_tag}'")
                 return
 
         self.cmd.submodule.update(recursive=True, init=True, log_in_real_time=True)
@@ -586,7 +586,7 @@ def set_remote(
 
         remote = self.remote(name=name)
         if remote is None:
-            raise Exception("Remote {name} not found after setting")
+            raise GitRemoteSetError(remote_name=name)
         return remote
 
     @staticmethod
@@ -673,7 +673,7 @@ def get_current_remote_name(self) -> str:
 
         if match.branch_upstream is None:  # no upstream set
             if match.branch_head is None:
-                raise Exception("No branch found for git repository")
+                raise GitNoBranchFound()
             return match.branch_head
         if match.branch_head is None:
             return match.branch_upstream

diff --git a/src/libvcs/sync/svn.py b/src/libvcs/sync/svn.py
@@ -26,6 +26,11 @@
 logger = logging.getLogger(__name__)
 
 
+class SvnUrlRevFormattingError(ValueError):
+    def __init__(self, data: str, *args: object):
+        return super().__init__(f"Badly formatted data: {data!r}")
+
+
 class SvnSync(BaseSync):
     bin_name = "svn"
     schemes = ("svn", "svn+ssh", "svn+http", "svn+https", "svn+svn")
@@ -104,7 +109,7 @@ def get_revision(self, location: Optional[str] = None) -> int:
         if not location:
             location = self.url
 
-        if os.path.exists(location) and not os.path.isdir(location):
+        if pathlib.Path(location).exists() and not pathlib.Path(location).is_dir():
             return self.get_revision_file(location)
 
         # Note: taken from setuptools.command.egg_info
@@ -115,8 +120,8 @@ def get_revision(self, location: Optional[str] = None) -> int:
                 dirs[:] = []
                 continue  # no sense walking uncontrolled subdirs
             dirs.remove(".svn")
-            entries_fn = os.path.join(base, ".svn", "entries")
-            if not os.path.exists(entries_fn):
+            entries_fn = pathlib.Path(base) / ".svn" / "entries"
+            if not entries_fn.exists():
                 # FIXME: should we warn?
                 continue
 
@@ -156,9 +161,9 @@ def _get_svn_url_rev(cls, location: str) -> tuple[Optional[str], int]:
         _svn_info_xml_rev_re = re.compile(r'\s*revision="(\d+)"')
         _svn_info_xml_url_re = re.compile(r"<url>(.*)</url>")
 
-        entries_path = os.path.join(location, ".svn", "entries")
-        if os.path.exists(entries_path):
-            with open(entries_path) as f:
+        entries_path = pathlib.Path(location) / ".svn" / "entries"
+        if entries_path.exists():
+            with entries_path.open() as f:
                 data = f.read()
         else:  # subversion >= 1.7 does not have the 'entries' file
             data = ""
@@ -172,7 +177,7 @@ def _get_svn_url_rev(cls, location: str) -> tuple[Optional[str], int]:
         elif data.startswith("<?xml"):
             match = _svn_xml_url_re.search(data)
             if not match:
-                raise ValueError(f"Badly formatted data: {data!r}")
+                raise SvnUrlRevFormattingError(data=data)
             url = match.group(1)  # get repository URL
             revs = [int(m.group(1)) for m in _svn_rev_re.finditer(data)] + [0]
         else:

diff --git a/src/libvcs/url/git.py b/src/libvcs/url/git.py
@@ -288,7 +288,7 @@ def __post_init__(self) -> None:
                 if v is not None:
                     setattr(self, k, v)
 
-            for k, v in rule.defaults.items():
+            for k in rule.defaults:
                 if getattr(self, k, None) is None:
                     setattr(self, k, rule.defaults[k])
             break

diff --git a/src/libvcs/url/hg.py b/src/libvcs/url/hg.py
@@ -217,7 +217,7 @@ def __post_init__(self) -> None:
             for k, v in groups.items():
                 setattr(self, k, v)
 
-            for k, v in rule.defaults.items():
+            for k in rule.defaults:
                 if getattr(self, k, None) is None:
                     setattr(self, k, rule.defaults[k])
 

diff --git a/src/libvcs/url/svn.py b/src/libvcs/url/svn.py
@@ -201,7 +201,7 @@ def __post_init__(self) -> None:
             for k, v in groups.items():
                 setattr(self, k, v)
 
-            for k, v in rule.defaults.items():
+            for k in rule.defaults:
                 if getattr(self, k, None) is None:
                     setattr(self, k, rule.defaults[k])
 

diff --git a/tests/sync/test_git.py b/tests/sync/test_git.py
@@ -1,6 +1,5 @@
 """Tests for libvcs git repos."""
 import datetime
-import os
 import pathlib
 import random
 import shutil
@@ -107,7 +106,7 @@ def test_repo_git_obtain_full(
     test_repo_revision = run(["git", "rev-parse", "HEAD"], cwd=git_remote_repo)
 
     assert git_repo.get_revision() == test_repo_revision
-    assert os.path.exists(tmp_path / "myrepo")
+    assert (tmp_path / "myrepo").exists()
 
 
 @pytest.mark.parametrize(

diff --git a/tests/sync/test_hg.py b/tests/sync/test_hg.py
@@ -4,6 +4,7 @@
 
 import pytest
 
+from libvcs import exc
 from libvcs._internal.run import run
 from libvcs._internal.shortcuts import create_project
 
@@ -52,7 +53,7 @@ def test_vulnerability_2022_03_12_command_injection(
     mercurial_repo = create_project(
         url="--config=alias.clone=!touch ./HELLO", vcs="hg", dir="./"
     )
-    with pytest.raises(Exception):
+    with pytest.raises(exc.CommandError):
         mercurial_repo.update_repo()
 
     assert not pathlib.Path(

diff --git a/tests/sync/test_svn.py b/tests/sync/test_svn.py
@@ -1,5 +1,4 @@
 """tests for libvcs svn repos."""
-import os
 import pathlib
 import shutil
 
@@ -26,7 +25,7 @@ def test_repo_svn(tmp_path: pathlib.Path, svn_remote_repo: pathlib.Path) -> None
     assert svn_repo.get_revision() == 0
     assert svn_repo.get_revision_file("./") == 0
 
-    assert os.path.exists(tmp_path / repo_name)
+    assert (tmp_path / repo_name).exists()
 
 
 def test_repo_svn_remote_checkout(

diff --git a/tests/test_exc.py b/tests/test_exc.py
@@ -1,3 +1,4 @@
+# ruff: noqa: TRY003
 """tests for libvcs exceptions."""
 import pytest