Skip to content

Avoid raising bare Exception #1168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libcst/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -242,6 +242,7 @@
"CSTVisitorT",
"FlattenSentinel",
"MaybeSentinel",
"CSTLogicError",
"MetadataException",
"ParserSyntaxError",
"PartialParserConfig",
Expand Down
47 changes: 5 additions & 42 deletions libcst/_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions libcst/_nodes/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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__}"
)
Expand Down Expand Up @@ -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(
Expand All @@ -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

Expand All @@ -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:
Expand Down
8 changes: 5 additions & 3 deletions libcst/_nodes/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.")

Expand Down Expand Up @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions libcst/_nodes/statement.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion libcst/_nodes/tests/test_funcdef.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
4 changes: 3 additions & 1 deletion libcst/_nodes/tests/test_namedexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion libcst/_nodes/tests/test_removal_behavior.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
53 changes: 53 additions & 0 deletions libcst/_parser/_parsing_check.py
Original file line number Diff line number Diff line change
@@ -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}."
10 changes: 3 additions & 7 deletions libcst/_parser/base_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Loading