From 69fadc2de931d822b47e5a7db0fb15f11018c6a0 Mon Sep 17 00:00:00 2001 From: Manuel Barkhau Date: Thu, 28 Mar 2019 23:37:58 +0100 Subject: [PATCH] Fix catchall exceptions. Using ValueError for everything caused underlying issues to be hidden. Now there are explicit classes for each error condition. --- src/pycalver/cli.py | 4 ++-- src/pycalver/rewrite.py | 22 +++++++++++++++++----- src/pycalver/version.py | 14 +++++++++----- test/test_rewrite.py | 7 +++---- test/test_version.py | 6 +++--- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/pycalver/cli.py b/src/pycalver/cli.py index b10d7d7..fcf217e 100755 --- a/src/pycalver/cli.py +++ b/src/pycalver/cli.py @@ -224,7 +224,7 @@ def _bump(cfg: config.Config, new_version: str, allow_dirty: bool = False) -> No try: rewrite.rewrite(new_version, cfg.file_patterns) - except ValueError as ex: + except Exception as ex: log.error(str(ex)) sys.exit(1) @@ -337,7 +337,7 @@ def bump( if dry or verbose >= 2: try: _print_diff(cfg, new_version) - except ValueError as ex: + except Exception as ex: log.error(str(ex)) sys.exit(1) diff --git a/src/pycalver/rewrite.py b/src/pycalver/rewrite.py index 413ebf1..5e0d309 100644 --- a/src/pycalver/rewrite.py +++ b/src/pycalver/rewrite.py @@ -41,6 +41,18 @@ def detect_line_sep(content: str) -> str: return "\n" +class NoPatternMatch(Exception): + """Pattern not found in content. + + log.error is used to show error info about the patterns so + that users can debug what is wrong with them. The class + itself doesn't capture that info. This approach is used so + that all patter issues can be shown, rather than bubbling + all the way up the stack on the very first pattern with no + matches. + """ + + def rewrite_lines( pattern_strs: typ.List[str], new_version: str, old_lines: typ.List[str] ) -> typ.List[str]: @@ -72,7 +84,7 @@ def rewrite_lines( log.error(f"No match for pattern '{non_matched_pattern}'") compiled_pattern = patterns._compile_pattern(non_matched_pattern) log.error(f"Pattern compiles to regex '{compiled_pattern}'") - raise ValueError("Invalid pattern(s)") + raise NoPatternMatch("Invalid pattern(s)") else: return new_lines @@ -110,7 +122,7 @@ def _iter_file_paths( file_paths = glob.glob(globstr) if not any(file_paths): errmsg = f"No files found for path/glob '{globstr}'" - raise ValueError(errmsg) + raise FileNotFoundError(errmsg) for file_path_str in file_paths: file_path = pl.Path(file_path_str) yield (file_path, pattern_strs) @@ -188,15 +200,15 @@ def diff(new_version: str, file_patterns: config.PatternsByGlob) -> str: try: rfd = rfd_from_content(pattern_strs, new_version, content) - except ValueError: + except NoPatternMatch: errmsg = f"No patterns matched for '{file_path}'" - raise ValueError(errmsg) + raise NoPatternMatch(errmsg) rfd = rfd._replace(path=str(file_path)) lines = diff_lines(rfd) if len(lines) == 0: errmsg = f"No patterns matched for '{file_path}'" - raise ValueError(errmsg) + raise NoPatternMatch(errmsg) full_diff += "\n".join(lines) + "\n" diff --git a/src/pycalver/version.py b/src/pycalver/version.py index af9c3a7..52cd993 100644 --- a/src/pycalver/version.py +++ b/src/pycalver/version.py @@ -183,6 +183,10 @@ PEP440_TAGS: typ.Dict[str, str] = { VersionInfoKW = typ.Dict[str, typ.Union[str, int, None]] +class PatternError(Exception): + pass + + def _parse_pattern_groups(pattern_groups: typ.Dict[str, str]) -> typ.Dict[str, str]: for part_name in pattern_groups.keys(): is_valid_part_name = ( @@ -190,7 +194,7 @@ def _parse_pattern_groups(pattern_groups: typ.Dict[str, str]) -> typ.Dict[str, s ) if not is_valid_part_name: err_msg = f"Invalid part '{part_name}'" - raise ValueError(err_msg) + raise PatternError(err_msg) items = [ (field_name, pattern_groups[part_name]) @@ -203,7 +207,7 @@ def _parse_pattern_groups(pattern_groups: typ.Dict[str, str]) -> typ.Dict[str, s if any(duplicate_fields): err_msg = f"Multiple parts for same field {duplicate_fields}." - raise ValueError(err_msg) + raise PatternError(err_msg) return dict(items) @@ -302,7 +306,7 @@ def parse_version_info(version_str: str, pattern: str = "{pycalver}") -> Version err_msg = ( f"Invalid version string '{version_str}' for pattern '{pattern}'/'{regex.pattern}'" ) - raise ValueError(err_msg) + raise PatternError(err_msg) return _parse_version_info(match.groupdict()) @@ -322,7 +326,7 @@ def is_valid(version_str: str, pattern: str = "{pycalver}") -> bool: try: parse_version_info(version_str, pattern) return True - except ValueError: + except PatternError: return False @@ -439,7 +443,7 @@ def incr( """ try: old_ver_nfo = parse_version_info(old_version, pattern) - except ValueError as ex: + except PatternError as ex: log.error(str(ex)) return None diff --git a/test/test_rewrite.py b/test/test_rewrite.py index af277a4..9eb8e94 100644 --- a/test/test_rewrite.py +++ b/test/test_rewrite.py @@ -77,8 +77,8 @@ def test_error_bad_path(): (project.dir / "setup.py").unlink() try: list(rewrite._iter_file_paths(cfg.file_patterns)) - assert False, "expected ValueError" - except ValueError as ex: + assert False, "expected FileNotFoundError" + except FileNotFoundError as ex: assert "setup.py" in str(ex) @@ -93,6 +93,5 @@ def test_error_bad_pattern(): try: list(rewrite.diff("v201809.1234", patterns)) - assert False, "expected ValueError" - except ValueError as ex: + except rewrite.NoPatternMatch as ex: assert "setup.py" in str(ex) diff --git a/test/test_version.py b/test/test_version.py index 26126a8..3da513d 100644 --- a/test/test_version.py +++ b/test/test_version.py @@ -107,7 +107,7 @@ def test_parse_error_empty(): try: version.parse_version_info("") assert False - except ValueError as err: + except version.PatternError as err: pass @@ -115,7 +115,7 @@ def test_parse_error_noprefix(): try: version.parse_version_info("201809.0002") assert False - except ValueError as err: + except version.PatternError as err: pass @@ -123,7 +123,7 @@ def test_parse_error_nopadding(): try: version.parse_version_info("v201809.2b0") assert False - except ValueError as err: + except version.PatternError as err: pass