fix: improve validation for ImportAlias and Try statements (#340)

* fix: improve validation for ImportAlias and Try statements For `Try` statements we ensure that the bare except, if present, is at the last position. For ImportAlias we ensure that the imported name is valid. Fixes #287 * Apply suggestions from code review Add missing periods. * Apply suggestions from code review Add missing periods. * Update libcst/_nodes/tests/test_import.py Co-authored-by: jimmylai <[email protected]>
Instagram · Jul 14, 2020 · f36eacb · f36eacb
1 parent 7208012
commit f36eacb
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 1 deletion.
diff --git a/libcst/_nodes/statement.py b/libcst/_nodes/statement.py
@@ -904,8 +904,11 @@ def _validate(self) -> None:
         if len(self.handlers) == 0 and self.orelse is not None:
             raise CSTValidationError(
                 "A Try statement must have at least one ExceptHandler in order "
-                + "to have an Else"
+                + "to have an Else."
             )
+        # Check bare excepts are always at the last position
+        if any(handler.type is None for handler in self.handlers[:-1]):
+            raise CSTValidationError("The bare except: handler must be the last one.")
 
     def _visit_and_replace_children(self, visitor: CSTVisitorT) -> "Try":
         return Try(
@@ -972,6 +975,14 @@ def _validate(self) -> None:
             raise CSTValidationError(
                 "Must use a Name node for AsName name inside ImportAlias."
             )
+        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
 
     def _visit_and_replace_children(self, visitor: CSTVisitorT) -> "ImportAlias":
         return ImportAlias(

diff --git a/libcst/_nodes/tests/test_import.py b/libcst/_nodes/tests/test_import.py
@@ -195,6 +195,18 @@ def test_valid(self, **kwargs: Any) -> None:
                 ),
                 "expected_re": "at least one space",
             },
+            {
+                "get_node": lambda: cst.Import(
+                    names=[
+                        cst.ImportAlias(
+                            name=cst.Attribute(
+                                value=cst.Float(value="0."), attr=cst.Name(value="A")
+                            )
+                        )
+                    ]
+                ),
+                "expected_re": "imported name must be a valid qualified name.",
+            },
         )
     )
     def test_invalid(self, **kwargs: Any) -> None:

diff --git a/libcst/_nodes/tests/test_try.py b/libcst/_nodes/tests/test_try.py
@@ -374,6 +374,35 @@ def test_valid(self, **kwargs: Any) -> None:
                 ),
                 "expected_re": "at least one ExceptHandler in order to have an Else",
             },
+            {
+                "get_node": lambda: cst.Try(
+                    body=cst.SimpleStatementSuite(body=[cst.Pass()]),
+                    handlers=(
+                        cst.ExceptHandler(
+                            body=cst.SimpleStatementSuite(body=[cst.Pass()]),
+                        ),
+                        cst.ExceptHandler(
+                            body=cst.SimpleStatementSuite(body=[cst.Pass()]),
+                        ),
+                    ),
+                ),
+                "expected_re": "The bare except: handler must be the last one.",
+            },
+            {
+                "get_node": lambda: cst.Try(
+                    body=cst.SimpleStatementSuite(body=[cst.Pass()]),
+                    handlers=(
+                        cst.ExceptHandler(
+                            body=cst.SimpleStatementSuite(body=[cst.Pass()]),
+                        ),
+                        cst.ExceptHandler(
+                            body=cst.SimpleStatementSuite(body=[cst.Pass()]),
+                            type=cst.Name("Exception"),
+                        ),
+                    ),
+                ),
+                "expected_re": "The bare except: handler must be the last one.",
+            },
         )
     )
     def test_invalid(self, **kwargs: Any) -> None: