Skip to content
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

Avoid raising bare Exception #6

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions libcst/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# LICENSE file in the root directory of this source tree.

from libcst._batched_visitor import BatchableCSTVisitor, visit_batched
from libcst._excep import CSTLogicError
from libcst._exceptions import MetadataException, ParserSyntaxError
from libcst._flatten_sentinel import FlattenSentinel
from libcst._maybe_sentinel import MaybeSentinel
Expand Down Expand Up @@ -238,6 +239,7 @@
"CSTNodeT",
"CSTTransformer",
"CSTValidationError",
"CSTLogicError",
"CSTVisitor",
"CSTVisitorT",
"FlattenSentinel",
Expand Down
8 changes: 8 additions & 0 deletions libcst/_excep.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# 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.


class CSTLogicError(Exception):
pass
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._excep 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 @@ -383,7 +384,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 @@ -400,7 +401,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 @@ -422,7 +423,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
7 changes: 4 additions & 3 deletions libcst/_nodes/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from typing import Callable, Generator, Literal, Optional, Sequence, Union

from libcst._add_slots import add_slots
from libcst._excep import CSTLogicError
from libcst._maybe_sentinel import MaybeSentinel
from libcst._nodes.base import CSTCodegenError, CSTNode, CSTValidationError
from libcst._nodes.internal import (
Expand Down Expand Up @@ -666,7 +667,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 +1011,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 +1689,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
13 changes: 6 additions & 7 deletions libcst/_nodes/statement.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from typing import Literal, Optional, Pattern, Sequence, Union

from libcst._add_slots import add_slots
from libcst._excep import CSTLogicError
from libcst._maybe_sentinel import MaybeSentinel
from libcst._nodes.base import CSTNode, CSTValidationError
from libcst._nodes.expression import (
Expand Down Expand Up @@ -1161,12 +1162,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 @@ -1195,7 +1194,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
2 changes: 1 addition & 1 deletion libcst/_parser/base_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,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
56 changes: 46 additions & 10 deletions libcst/_parser/conversions/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
Intnumber as INTNUMBER_RE,
)

from libcst._exceptions import PartialParserSyntaxError
from libcst._excep import CSTLogicError
from libcst._exceptions import ParserSyntaxError, PartialParserSyntaxError
from libcst._maybe_sentinel import MaybeSentinel
from libcst._nodes.expression import (
Arg,
Expand Down Expand Up @@ -327,7 +328,12 @@ def convert_boolop(
# Convert all of the operations that have no precedence in a loop
for op, rightexpr in grouper(rightexprs, 2):
if op.string not in BOOLOP_TOKEN_LUT:
raise Exception(f"Unexpected token '{op.string}'!")
raise ParserSyntaxError(
f"Unexpected token '{op.string}'!",
lines=config.lines,
raw_line=0,
raw_column=0,
)
leftexpr = BooleanOperation(
left=leftexpr,
# pyre-ignore Pyre thinks that the type of the LUT is CSTNode.
Expand Down Expand Up @@ -420,7 +426,12 @@ def convert_comp_op(
)
else:
# this should be unreachable
raise Exception(f"Unexpected token '{op.string}'!")
raise ParserSyntaxError(
f"Unexpected token '{op.string}'!",
lines=config.lines,
raw_line=0,
raw_column=0,
)
else:
# A two-token comparison
leftcomp, rightcomp = children
Expand Down Expand Up @@ -451,7 +462,12 @@ def convert_comp_op(
)
else:
# this should be unreachable
raise Exception(f"Unexpected token '{leftcomp.string} {rightcomp.string}'!")
raise ParserSyntaxError(
f"Unexpected token '{leftcomp.string} {rightcomp.string}'!",
lines=config.lines,
raw_line=0,
raw_column=0,
)


@with_production("star_expr", "'*' expr")
Expand Down Expand Up @@ -493,7 +509,12 @@ def convert_binop(
# Convert all of the operations that have no precedence in a loop
for op, rightexpr in grouper(rightexprs, 2):
if op.string not in BINOP_TOKEN_LUT:
raise Exception(f"Unexpected token '{op.string}'!")
raise ParserSyntaxError(
f"Unexpected token '{op.string}'!",
lines=config.lines,
raw_line=0,
raw_column=0,
)
leftexpr = BinaryOperation(
left=leftexpr,
# pyre-ignore Pyre thinks that the type of the LUT is CSTNode.
Expand Down Expand Up @@ -540,7 +561,12 @@ def convert_factor(
)
)
else:
raise Exception(f"Unexpected token '{op.string}'!")
raise ParserSyntaxError(
f"Unexpected token '{op.string}'!",
lines=config.lines,
raw_line=0,
raw_column=0,
)

return WithLeadingWhitespace(
UnaryOperation(operator=opnode, expression=factor.value), op.whitespace_before
Expand Down Expand Up @@ -651,7 +677,7 @@ def convert_atom_expr_trailer(
)
else:
# This is an invalid trailer, so lets give up
raise Exception("Logic error!")
raise CSTLogicError()
return WithLeadingWhitespace(atom, whitespace_before)


Expand Down Expand Up @@ -870,9 +896,19 @@ def convert_atom_basic(
Imaginary(child.string), child.whitespace_before
)
else:
raise Exception(f"Unparseable number {child.string}")
raise ParserSyntaxError(
f"Unparseable number {child.string}",
lines=config.lines,
raw_line=0,
raw_column=0,
)
else:
raise Exception(f"Logic error, unexpected token {child.type.name}")
raise ParserSyntaxError(
f"Logic error, unexpected token {child.type.name}",
lines=config.lines,
raw_line=0,
raw_column=0,
)


@with_production("atom_squarebrackets", "'[' [testlist_comp_list] ']'")
Expand Down Expand Up @@ -1447,7 +1483,7 @@ def convert_arg_assign_comp_for(
if equal.string == ":=":
val = convert_namedexpr_test(config, children)
if not isinstance(val, WithLeadingWhitespace):
raise Exception(
raise TypeError(
f"convert_namedexpr_test returned {val!r}, not WithLeadingWhitespace"
)
return Arg(value=val.value)
Expand Down
13 changes: 7 additions & 6 deletions libcst/_parser/conversions/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from typing import Any, List, Optional, Sequence, Union

from libcst._excep import CSTLogicError
from libcst._exceptions import PartialParserSyntaxError
from libcst._maybe_sentinel import MaybeSentinel
from libcst._nodes.expression import (
Expand Down Expand Up @@ -121,7 +122,7 @@ def add_param(
# Example code:
# def fn(*abc, *): ...
# This should be unreachable, the grammar already disallows it.
raise Exception(
raise ValueError(
"Cannot have multiple star ('*') markers in a single argument "
+ "list."
)
Comment on lines +125 to 128
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors

Expand All @@ -136,7 +137,7 @@ def add_param(
# Example code:
# def fn(foo, /, *, /, bar): ...
# This should be unreachable, the grammar already disallows it.
raise Exception(
raise ValueError(
"Cannot have multiple slash ('/') markers in a single argument "
+ "list."
)
Comment on lines +140 to 143
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors

Expand Down Expand Up @@ -168,7 +169,7 @@ def add_param(
# Example code:
# def fn(**kwargs, trailing=None)
# This should be unreachable, the grammar already disallows it.
raise Exception("Cannot have any arguments after a kwargs expansion.")
raise ValueError("Cannot have any arguments after a kwargs expansion.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors

elif (
isinstance(param.star, str) and param.star == "*" and param.default is None
):
Expand All @@ -181,7 +182,7 @@ def add_param(
# Example code:
# def fn(*first, *second): ...
# This should be unreachable, the grammar already disallows it.
raise Exception(
raise ValueError(
"Expected a keyword argument but found a starred positional "
+ "argument expansion."
)
Comment on lines +185 to 188
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors

Expand All @@ -197,13 +198,13 @@ def add_param(
# Example code:
# def fn(**first, **second)
# This should be unreachable, the grammar already disallows it.
raise Exception(
raise ValueError(
"Multiple starred keyword argument expansions are not allowed in a "
+ "single argument list"
)
Comment on lines +201 to 204
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors

else:
# The state machine should never end up here.
raise Exception("Logic error!")
raise CSTLogicError("Logic error!")

return current_param

Expand Down
Loading