Avoid raising bare Exception (#1168)

* Keep old exception messages (avoid breaking-changes for users relying on exception messages)

* Move ``get_expected_str`` out of _exceptions.py, where it does not belong, to its own file in _parser/_parsing_check.py

Author: zaicruvoir1rominet

libcst/__init__.py

@@ -4,7 +4,7 @@
 # LICENSE file in the root directory of this source tree.
 
 from libcst._batched_visitor import BatchableCSTVisitor, visit_batched
-from libcst._exceptions import MetadataException, ParserSyntaxError
+from libcst._exceptions import CSTLogicError, MetadataException, ParserSyntaxError
 from libcst._flatten_sentinel import FlattenSentinel
 from libcst._maybe_sentinel import MaybeSentinel
 from libcst._metadata_dependent import MetadataDependent
@@ -242,6 +242,7 @@
     "CSTVisitorT",
     "FlattenSentinel",
     "MaybeSentinel",
+    "CSTLogicError",
     "MetadataException",
     "ParserSyntaxError",
     "PartialParserConfig",

libcst/_exceptions.py

@@ -4,59 +4,22 @@
 # LICENSE file in the root directory of this source tree.
 
 from enum import auto, Enum
-from typing import Any, Callable, final, Iterable, Optional, Sequence, Tuple, Union
+from typing import Any, Callable, final, Optional, Sequence, Tuple
 
-from libcst._parser.parso.pgen2.generator import ReservedString
-from libcst._parser.parso.python.token import PythonTokenTypes, TokenType
-from libcst._parser.types.token import Token
 from libcst._tabs import expand_tabs
 
-_EOF_STR: str = "end of file (EOF)"
-_INDENT_STR: str = "an indent"
-_DEDENT_STR: str = "a dedent"
+
 _NEWLINE_CHARS: str = "\r\n"
 
 
 class EOFSentinel(Enum):
     EOF = auto()
 
 
-def get_expected_str(
-    encountered: Union[Token, EOFSentinel],
-    expected: Union[Iterable[Union[TokenType, ReservedString]], EOFSentinel],
-) -> str:
-    if (
-        isinstance(encountered, EOFSentinel)
-        or encountered.type is PythonTokenTypes.ENDMARKER
-    ):
-        encountered_str = _EOF_STR
-    elif encountered.type is PythonTokenTypes.INDENT:
-        encountered_str = _INDENT_STR
-    elif encountered.type is PythonTokenTypes.DEDENT:
-        encountered_str = _DEDENT_STR
-    else:
-        encountered_str = repr(encountered.string)
-
-    if isinstance(expected, EOFSentinel):
-        expected_names = [_EOF_STR]
-    else:
-        expected_names = sorted(
-            [
-                repr(el.name) if isinstance(el, TokenType) else repr(el.value)
-                for el in expected
-            ]
-        )
+class CSTLogicError(Exception):
+    """General purpose internal error within LibCST itself."""
 
-    if len(expected_names) > 10:
-        # There's too many possibilities, so it's probably not useful to list them.
-        # Instead, let's just abbreviate the message.
-        return f"Unexpectedly encountered {encountered_str}."
-    else:
-        if len(expected_names) == 1:
-            expected_str = expected_names[0]
-        else:
-            expected_str = f"{', '.join(expected_names[:-1])}, or {expected_names[-1]}"
-        return f"Encountered {encountered_str}, but expected {expected_str}."
+    pass
 
 
 # pyre-fixme[2]: 'Any' type isn't pyre-strict.

libcst/_nodes/base.py

@@ -8,6 +8,7 @@
 from dataclasses import dataclass, field, fields, replace
 from typing import Any, cast, ClassVar, Dict, List, Mapping, Sequence, TypeVar, Union
 
+from libcst import CSTLogicError
 from libcst._flatten_sentinel import FlattenSentinel
 from libcst._nodes.internal import CodegenState
 from libcst._removal_sentinel import RemovalSentinel
@@ -237,7 +238,7 @@ def visit(
 
         # validate return type of the user-defined `visitor.on_leave` method
         if not isinstance(leave_result, (CSTNode, RemovalSentinel, FlattenSentinel)):
-            raise Exception(
+            raise CSTValidationError(
                 "Expected a node of type CSTNode or a RemovalSentinel, "
                 + f"but got a return value of {type(leave_result).__name__}"
             )
@@ -382,7 +383,7 @@ def deep_replace(
         new_tree = self.visit(_ChildReplacementTransformer(old_node, new_node))
         if isinstance(new_tree, (FlattenSentinel, RemovalSentinel)):
             # The above transform never returns *Sentinel, so this isn't possible
-            raise Exception("Logic error, cannot get a *Sentinel here!")
+            raise CSTLogicError("Logic error, cannot get a *Sentinel here!")
         return new_tree
 
     def deep_remove(
@@ -399,7 +400,7 @@ def deep_remove(
 
         if isinstance(new_tree, FlattenSentinel):
             # The above transform never returns FlattenSentinel, so this isn't possible
-            raise Exception("Logic error, cannot get a FlattenSentinel here!")
+            raise CSTLogicError("Logic error, cannot get a FlattenSentinel here!")
 
         return new_tree
 
@@ -421,7 +422,7 @@ def with_deep_changes(
         new_tree = self.visit(_ChildWithChangesTransformer(old_node, changes))
         if isinstance(new_tree, (FlattenSentinel, RemovalSentinel)):
             # This is impossible with the above transform.
-            raise Exception("Logic error, cannot get a *Sentinel here!")
+            raise CSTLogicError("Logic error, cannot get a *Sentinel here!")
         return new_tree
 
     def __eq__(self: _CSTNodeSelfT, other: object) -> bool:

libcst/_nodes/expression.py

@@ -17,6 +17,8 @@
 )
 from typing import Callable, Generator, Literal, Optional, Sequence, Union
 
+from libcst import CSTLogicError
+
 from libcst._add_slots import add_slots
 from libcst._maybe_sentinel import MaybeSentinel
 from libcst._nodes.base import CSTCodegenError, CSTNode, CSTValidationError
@@ -666,7 +668,7 @@ def quote(self) -> StringQuoteLiteral:
         if len(quote) not in {1, 3}:
             # We shouldn't get here due to construction validation logic,
             # but handle the case anyway.
-            raise Exception(f"Invalid string {self.value}")
+            raise CSTLogicError(f"Invalid string {self.value}")
 
         # pyre-ignore We know via the above validation that we will only
         # ever return one of the four string literals.
@@ -1010,7 +1012,7 @@ def _validate(self) -> None:
         elif isinstance(right, FormattedString):
             rightbytes = "b" in right.prefix
         else:
-            raise Exception("Logic error!")
+            raise CSTLogicError("Logic error!")
         if leftbytes != rightbytes:
             raise CSTValidationError("Cannot concatenate string and bytes.")
 
@@ -1688,7 +1690,7 @@ def _codegen_impl(
             if default_indicator == "->":
                 state.add_token(" ")
         else:
-            raise Exception("Logic error!")
+            raise CSTLogicError("Logic error!")
 
         # Now, output the indicator and the rest of the annotation
         state.add_token(default_indicator)

libcst/_nodes/statement.py

@@ -9,6 +9,8 @@
 from dataclasses import dataclass, field
 from typing import Literal, Optional, Pattern, Sequence, Union
 
+from libcst import CSTLogicError
+
 from libcst._add_slots import add_slots
 from libcst._maybe_sentinel import MaybeSentinel
 from libcst._nodes.base import CSTNode, CSTValidationError
@@ -1165,12 +1167,10 @@ def _validate(self) -> None:
                 )
         try:
             self.evaluated_name
-        except Exception as e:
-            if str(e) == "Logic error!":
-                raise CSTValidationError(
-                    "The imported name must be a valid qualified name."
-                )
-            raise e
+        except CSTLogicError as e:
+            raise CSTValidationError(
+                "The imported name must be a valid qualified name."
+            ) from e
 
     def _visit_and_replace_children(self, visitor: CSTVisitorT) -> "ImportAlias":
         return ImportAlias(
@@ -1199,7 +1199,7 @@ def _name(self, node: CSTNode) -> str:
         elif isinstance(node, Attribute):
             return f"{self._name(node.value)}.{node.attr.value}"
         else:
-            raise Exception("Logic error!")
+            raise CSTLogicError("Logic error!")
 
     @property
     def evaluated_name(self) -> str:

libcst/_nodes/tests/test_funcdef.py

@@ -1052,7 +1052,9 @@ def _parse_statement_force_38(code: str) -> cst.BaseCompoundStatement:
         code, config=cst.PartialParserConfig(python_version="3.8")
     )
     if not isinstance(statement, cst.BaseCompoundStatement):
-        raise Exception("This function is expecting to parse compound statements only!")
+        raise ValueError(
+            "This function is expecting to parse compound statements only!"
+        )
     return statement
 
 

libcst/_nodes/tests/test_namedexpr.py

@@ -22,7 +22,9 @@ def _parse_statement_force_38(code: str) -> cst.BaseCompoundStatement:
         code, config=cst.PartialParserConfig(python_version="3.8")
     )
     if not isinstance(statement, cst.BaseCompoundStatement):
-        raise Exception("This function is expecting to parse compound statements only!")
+        raise ValueError(
+            "This function is expecting to parse compound statements only!"
+        )
     return statement
 
 

libcst/_nodes/tests/test_removal_behavior.py

@@ -95,7 +95,7 @@ def test_removal_pass_behavior(
         self, before: str, after: str, visitor: Type[CSTTransformer]
     ) -> None:
         if before.endswith("\n") or after.endswith("\n"):
-            raise Exception("Test cases should not be newline-terminated!")
+            raise ValueError("Test cases should not be newline-terminated!")
 
         # Test doesn't have newline termination case
         before_module = parse_module(before)

libcst/_parser/_parsing_check.py

@@ -0,0 +1,53 @@
+# Copyright (c) Meta Platforms, Inc. and affiliates.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+from typing import Iterable, Union
+
+from libcst._exceptions import EOFSentinel
+from libcst._parser.parso.pgen2.generator import ReservedString
+from libcst._parser.parso.python.token import PythonTokenTypes, TokenType
+from libcst._parser.types.token import Token
+
+_EOF_STR: str = "end of file (EOF)"
+_INDENT_STR: str = "an indent"
+_DEDENT_STR: str = "a dedent"
+
+
+def get_expected_str(
+    encountered: Union[Token, EOFSentinel],
+    expected: Union[Iterable[Union[TokenType, ReservedString]], EOFSentinel],
+) -> str:
+    if (
+        isinstance(encountered, EOFSentinel)
+        or encountered.type is PythonTokenTypes.ENDMARKER
+    ):
+        encountered_str = _EOF_STR
+    elif encountered.type is PythonTokenTypes.INDENT:
+        encountered_str = _INDENT_STR
+    elif encountered.type is PythonTokenTypes.DEDENT:
+        encountered_str = _DEDENT_STR
+    else:
+        encountered_str = repr(encountered.string)
+
+    if isinstance(expected, EOFSentinel):
+        expected_names = [_EOF_STR]
+    else:
+        expected_names = sorted(
+            [
+                repr(el.name) if isinstance(el, TokenType) else repr(el.value)
+                for el in expected
+            ]
+        )
+
+    if len(expected_names) > 10:
+        # There's too many possibilities, so it's probably not useful to list them.
+        # Instead, let's just abbreviate the message.
+        return f"Unexpectedly encountered {encountered_str}."
+    else:
+        if len(expected_names) == 1:
+            expected_str = expected_names[0]
+        else:
+            expected_str = f"{', '.join(expected_names[:-1])}, or {expected_names[-1]}"
+        return f"Encountered {encountered_str}, but expected {expected_str}."

libcst/_parser/base_parser.py

@@ -26,12 +26,8 @@
 from dataclasses import dataclass, field
 from typing import Generic, Iterable, List, Sequence, TypeVar, Union
 
-from libcst._exceptions import (
-    EOFSentinel,
-    get_expected_str,
-    ParserSyntaxError,
-    PartialParserSyntaxError,
-)
+from libcst._exceptions import EOFSentinel, ParserSyntaxError, PartialParserSyntaxError
+from libcst._parser._parsing_check import get_expected_str
 from libcst._parser.parso.pgen2.generator import DFAState, Grammar, ReservedString
 from libcst._parser.parso.python.token import TokenType
 from libcst._parser.types.token import Token
@@ -103,7 +99,7 @@ def __init__(
     def parse(self) -> _NodeT:
         # Ensure that we don't re-use parsers.
         if self.__was_parse_called:
-            raise Exception("Each parser object may only be used to parse once.")
+            raise ValueError("Each parser object may only be used to parse once.")
         self.__was_parse_called = True
 
         for token in self.tokens: