Skip to content

Improves error message for parsing slice in annotations - #11241

Merged
hauntsaninja merged 5 commits into
python:masterfrom
sobolevn:issue-10266
Oct 3, 2021
Merged

hauntsaninja merged 5 commits into
python:masterfrom
sobolevn:issue-10266

Conversation

@sobolevn

@sobolevn sobolevn commented Oct 1, 2021

Copy link
Copy Markdown
Member

Closes #10266

Comment thread test-data/unit/check-fastparse.test Outdated
@Zac-HD

Zac-HD commented Oct 2, 2021

Copy link
Copy Markdown
Contributor

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

sobolevn commented Oct 2, 2021 •

Copy link
Copy Markdown
Member Author

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

sobolevn commented Oct 2, 2021

Copy link
Copy Markdown
Member Author

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

@sobolevn

sobolevn commented Oct 2, 2021 •

Copy link
Copy Markdown
Member Author

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

Zac-HD commented Oct 2, 2021

Copy link
Copy Markdown
Contributor

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