Avoid raising bare Exception #6
Conversation
… on exception messages)
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR improves exception handling throughout the LibCST codebase by replacing bare Exception raises with more specific exception types.
- Introduced
CSTLogicErrorinlibcst/_excep.pyfor logical errors within CST processing - Replaced generic
Exceptionraises withValueError,CSTValidationError,ParserSyntaxError, andCSTLogicErroracross multiple files - Updated
libcst/__init__.pyto includeCSTLogicErrorin the__all__list for easier importing - Modified error messages in various files to provide more precise information about the cause of exceptions
- Improved consistency in exception handling across different modules and components of LibCST
30 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
| raise ValueError( | ||
| "Cannot have multiple star ('*') markers in a single argument " | ||
| + "list." | ||
| ) |
There was a problem hiding this comment.
style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors
| raise ValueError( | ||
| "Cannot have multiple slash ('/') markers in a single argument " | ||
| + "list." | ||
| ) |
There was a problem hiding this comment.
style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors
| # 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.") |
There was a problem hiding this comment.
style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors
| raise ValueError( | ||
| "Expected a keyword argument but found a starred positional " | ||
| + "argument expansion." | ||
| ) |
There was a problem hiding this comment.
style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors
| raise ValueError( | ||
| "Multiple starred keyword argument expansions are not allowed in a " | ||
| + "single argument list" | ||
| ) |
There was a problem hiding this comment.
style: Consider using ParserSyntaxError instead of ValueError for consistency with other parsing errors
Summary
Before tackling issue Instagram#457, there are some bare Exceptions thrown from within the code.
Here is the logic for changes in Exceptions:
ValueErrorwhen issue caused by user inputCSTValidationError&TypeErrorwhen issue occurs during the CST validation processParserSyntaxErrorwhen issue occurs during the parsing processCSTLogicErrorto replace all Exception which contained "Logic error" in the cause messageI'm all for changes if some Exceptions changes are not OK,
In particular, there are some
ParserSyntaxErrorwhich may not be meaningful - with raw_line=0, raw_column=0 paramsTest Plan
Keep the current test plan.