Skip to content

Improves error message for parsing slice in annotations #11241

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

Merged
merged 5 commits into from
Oct 3, 2021
Merged

Improves error message for parsing slice in annotations #11241

merged 5 commits into from
Oct 3, 2021

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 1, 2021

Closes #10266

@Zac-HD
Copy link
Contributor

Zac-HD commented Oct 2, 2021

Just want to check explicitly: is it possible to put a # type: ignore on this, or better yet to ignore all such errors from configuration? The latter would be especially useful for projects using array-related typing extensions.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 2, 2021

is it possible to put a # type: ignore on this, or better yet to ignore all such errors from configuration?

Nope, does not look like it:
Снимок экрана 2021-10-02 в 9 51 24

I think the reason is that it is raised in fastparse.py, which is quite a low level for type: ignore.
I will try to make it work (if no big changes are be required).

@sobolevn
Copy link
Member Author

sobolevn commented Oct 2, 2021

The same with: mypy ex.py --show-error-codes --disable-error-code syntax
It still shows this message.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 2, 2021

Ok, looks like --disable-error-code can be used with this corner case quite easily:

From 006fbd50f6c8a2f99658367afaa15d29ecd5f851 Mon Sep 17 00:00:00 2001
From: sobolevn <[email protected]>
Date: Sat, 2 Oct 2021 10:31:47 +0300
Subject: [PATCH] Adds new error code for slice syntax

---
 mypy/errorcodes.py |  1 +
 mypy/errors.py     | 11 +++++++++++
 mypy/fastparse.py  | 13 +++++++++----
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py
index d9e11a044..3ae079f08 100644
--- a/mypy/errorcodes.py
+++ b/mypy/errorcodes.py
@@ -135,6 +135,7 @@ NAME_MATCH: Final = ErrorCode(
 
 # Syntax errors are often blocking.
 SYNTAX: Final = ErrorCode("syntax", "Report syntax errors", "General")
+SLICE_SYNTAX: Final = ErrorCode("slice-syntax", "Report slice syntax errors", "General")
 
 # This is a catch-all for remaining uncategorized errors.
 MISC: Final = ErrorCode("misc", "Miscellaneous other checks", "General")
diff --git a/mypy/errors.py b/mypy/errors.py
index 3a0e0e14d..a68cfe2ac 100644
--- a/mypy/errors.py
+++ b/mypy/errors.py
@@ -374,6 +374,17 @@ class Errors:
                         return
             if file in self.ignored_files:
                 return
+            if (
+                not self.ignored_lines
+                and info.code
+                and self.is_error_code_enabled(info.code) is False
+            ):
+                # We also might want to ignore some errors during `fastparse`.
+                # At that moment `ignored_lines` might not be ready,
+                # so, we only check for specific error codes.
+                # For example, `Dict[{str: int}]` can be ignored this way.
+                return
+
         if info.only_once:
             if info.message in self.only_once_messages:
                 return
diff --git a/mypy/fastparse.py b/mypy/fastparse.py
index 386bfb948..870278284 100644
--- a/mypy/fastparse.py
+++ b/mypy/fastparse.py
@@ -1359,9 +1359,12 @@ class TypeConverter:
             return None
         return self.node_stack[-2]
 
-    def fail(self, msg: str, line: int, column: int) -> None:
+    def fail(self, msg: str, line: int, column: int,
+             blocker: bool = True, code: Optional[codes.ErrorCode] = None) -> None:
+        if code is None:
+            code = codes.SYNTAX
         if self.errors:
-            self.errors.report(line, column, msg, blocker=True, code=codes.SYNTAX)
+            self.errors.report(line, column, msg, blocker=blocker, code=code)
 
     def note(self, msg: str, line: int, column: int) -> None:
         if self.errors:
@@ -1562,12 +1565,14 @@ class TypeConverter:
             if (isinstance(sliceval, ast3.Slice) or
                 (isinstance(sliceval, ast3.Tuple) and
                  any(isinstance(x, ast3.Slice) for x in sliceval.elts))):
-                self.fail(INVALID_SLICE_ERROR, self.line, getattr(n, 'col_offset', -1))
+                self.fail(INVALID_SLICE_ERROR, self.line, getattr(n, 'col_offset', -1),
+                          blocker=False, code=codes.SLICE_SYNTAX)
                 return AnyType(TypeOfAny.from_error)
         else:
             # Python 3.8 or earlier use a different AST structure for subscripts
             if not isinstance(n.slice, Index):
-                self.fail(INVALID_SLICE_ERROR, self.line, getattr(n, 'col_offset', -1))
+                self.fail(INVALID_SLICE_ERROR, self.line, getattr(n, 'col_offset', -1),
+                          blocker=False, code=codes.SLICE_SYNTAX)
                 return AnyType(TypeOfAny.from_error)
             sliceval = n.slice.value
 
-- 
2.23.0

Снимок экрана 2021-10-02 в 10 36 55

Let's merge this as is: just improving the error message. Later, we can review and merge / decline the patch above.
It might have issues in errors.py part 🤔

@Zac-HD
Copy link
Contributor

Zac-HD commented Oct 2, 2021

Sounds good - thanks for your work on this @sobolevn (and maintainers)!

I'm definitely keen to get the second patch in too - though maybe not classified as a syntax error - so that I can use mypy on projects using the torchtyping package, but this patch is already a clear improvement IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash with slices e.g. Annotated[int, 3:4]
5 participants