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

Support error codes and ignoring only specific errors #7239

Closed
JukkaL opened this issue Jul 18, 2019 · 10 comments
Closed

Support error codes and ignoring only specific errors #7239

JukkaL opened this issue Jul 18, 2019 · 10 comments

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jul 18, 2019

It would be handy to support error codes to allow ignoring only specific error messages on each line. For example, here we only ignore an error about an incompatible override in class B:

class A:
    def f(self) -> None: 
        ...

class B(A):
    def f(self, x: int) -> None:  # type: ignore[override]
        ...

If there are any other errors on that line (such as "invalid type"), they will still be reported. Using the existing blanket # type: ignore comment risks ignoring too many errors, resulting in false negatives.

For this we need these changes:

  1. Make it possible to display error codes with errors (either optionally or by default).
  2. Support ignoring specific errors using # type: ignore[code1, ..., codeN].
  3. Implement error codes for all common errors (at least).

This is potentially a pretty disruptive change, so we need to be careful about how to do this. Here are some ideas. There is an existing PR that implements some of this (#6472). My proposal differs in various details, but the end goal is the same.

Format of error codes. There are various options such as numeric codes, strings derived from messages (as in #6472), and short error identifiers that are chosen manually. I'm leaning towards the final option. I'd like error codes to be pretty short but still understandable, and perhaps even memorable. I'm proposing that most error codes are of the form foo, foo-bar or foo-bar-lulz (one to three words separated by dashes).

Granularity of error codes. I think that not every error message needs a separate code, since this could mean hundreds of different codes. The number of codes should small enough that the list of all codes is easy enough to browse. I'm thinking of maybe having 20-50 distinct error codes in the long term. We don't need a unique error code for every rare message. We can either use a category code (such as indexing for various errors related to x[y] expressions) or a fallback misc/other category for all unclassified errors. The misc/other category also makes adoption easy -- we can initially have lots of errors in this category, and gradually migrate additional errors to more specific codes.

Displaying error codes. We could display error codes like this in mypy output:

foo.py:124: error: Some bad stuff going on here [bad-stuff]

The rationale is that the error code is usually not important, so we shouldn't make it too prominent. This is also one of the reasons why the error codes should be pretty short. This way we could probably enable error codes by default, as they are perhaps not too distracting.

Implementing error codes. My idea would be to have error codes totally separate from error messages. Each error reporting call would optionally include the error code (it would default to a generic other/misc category code). Error codes would defined with some metadata in a new error code registry module. The metadata could be used for grouping and help output. Here's a rough idea (this will need some iteration):

# Arguments are 1) error code 2) description in help output 3) category
BAD_STUFF = ErrorCode('bad-stuff', 'Bad stuff going on', 'General')

The motivation is that no dynamic magic is required, and everything is very explicit.

Errors from plugins. I haven't thought about this much, but plugins should be able to provide a list of error code objects. These could be shown in the error code help output. Maybe plugins should generally define relatively few error codes per plugin. It would be okay to just have a single error code for all errors from a plugin, and plugins can also reuse predefined error codes.

Sample error codes. Here are few actual error codes we might want to define (these are not final and may need iteration):

"x" has no attribute "y" [attr-defined]
Unexpected keyword argument "x" for "y" [call-arg]
Signature of "x" incompatible with supertype "x" [override]
Function is missing a type annotation [no-untyped-def]

The idea is that the name of the error code typically refers to what is being checked (whether an attribute is defined; whether an argument is valid in a call; validity of a method override). Short names are preferred, as long as they are sufficiently descriptive. For errors from optional strictness options we name the error code no-x, where x refers to what is being disallowed.

Postponed topics. We could also migrate some existing strictness flags to error codes, and to allow enabling and disabling particular errors globally or per file. These feel less important than being able to ignore specific errors on a line right now, but they would be nice features to have. We'll also want a way to display all available error codes through a command line option, but again this isn't required initially.

Next steps. I'm planning to implement this soon, since we have a use case which requires this feature at Dropbox. I can reuse some code and ideas from #6472 if @chadrik isn't opposed to it.

cc @chadrik

@chadrik
Copy link
Contributor

chadrik commented Jul 18, 2019

Super excited about this. I'm all for "short error identifiers that are chosen manually". this will be a lot easier to do now that new-semanal is done.

That's what I'm doing in this mypy-runner that I wrote: https://github.com/chadrik/mypy-runner/blob/master/mypyrun.py

I've avoided broadcasting that project too publicly because I know all the error codes will change once this feature is implemented officially in mypy.

IIRC, one of the trickier bits is designing a sane system to allow the error messages to receive format arguments.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jul 19, 2019

one of the trickier bits is designing a sane system to allow the error messages to receive format arguments.

The way I'm thinking about this is that error codes and message formatting are orthogonal, so hopefully this won't be an issue.

@ilevkivskyi
Copy link
Member

I think this is an important feature that will help many users. I agree with the plan, here are couple comments:

We could display error codes like this in mypy output:

I would rather prefer to show them on the left like this:

foo.py:124: error[bad-stuff]: Some bad stuff going on here

I looked at few other tools (Pylint, flake8, Pyre) there is no preferred pattern among other tools, so probably we just need to experiment with this.

We could also migrate some existing strictness flags to error codes, and to allow enabling and disabling particular errors globally or per file.

Since we already support ignoring all errors in a file using # type: ignore at the top of file, it may be that adding support for # type: ignore[code1, ..., codeN] will naturally allow ignoring given error codes in the whole file.

msullivan added a commit to msullivan/pyflakes that referenced this issue Jul 19, 2019
This is to support allowing typecheckers to implement ignores for
specific errors, using syntax like `# type: ignore=E1000` or
`# type: ignore[type-mismatch` or some such.
mypy is about to add support for ignoring specific errors following
this design: python/mypy#7239

Support for extra text in type comments was implemented
in CPython as https://bugs.python.org/issue36878
and in typed_ast as python/typed_ast#116.
msullivan added a commit to msullivan/pyflakes that referenced this issue Jul 19, 2019
This is to support allowing typecheckers to implement ignores for
specific errors, using syntax like `# type: ignore=E1000` or
`# type: ignore[type-mismatch` or some such.
mypy is about to add support for ignoring specific errors following
this design: python/mypy#7239

Support for extra text in type comments was implemented
in CPython as https://bugs.python.org/issue36878
and in typed_ast as python/typed_ast#116.
msullivan added a commit to msullivan/pyflakes that referenced this issue Jul 19, 2019
This is to support allowing typecheckers to implement ignores for
specific errors, using syntax like `# type: ignore=E1000` or
`# type: ignore[type-mismatch` or some such.
mypy is about to add support for ignoring specific errors following
this design: python/mypy#7239

Support for extra text in type comments was implemented
in CPython as https://bugs.python.org/issue36878
and in typed_ast as python/typed_ast#116.
@JukkaL JukkaL self-assigned this Jul 26, 2019
JukkaL added a commit that referenced this issue Aug 7, 2019
This PR adds a foundation for error codes and implements a few error
codes. It also adds support for `# type: ignore[code1, ...]` which 
ignores only specific error codes on a line.

Only a few errors include interesting error codes at this point. I'll add 
support for more error codes in additional PRs. Most errors will implicitly 
fall back to a `misc` error code.

Error codes are only shown if `--show-error-codes` is used.

The error codes look like this in mypy output:

```
t.py:3: error: "str" has no attribute "trim"  [attr-defined]
```

Error codes are intended to be short but human-readable. The name
of an error code refers to the check that produces this error. In the above
example we generate a "no attribute" error when we check whether an 
attribute is defined.

Work towards #7239.
sigmavirus24 pushed a commit to PyCQA/pyflakes that referenced this issue Aug 12, 2019
* In PEP 484 type comments, allow text after "# type: ignore"

This is to support allowing typecheckers to implement ignores for
specific errors, using syntax like `# type: ignore=E1000` or
`# type: ignore[type-mismatch` or some such.
mypy is about to add support for ignoring specific errors following
this design: python/mypy#7239

Support for extra text in type comments was implemented
in CPython as https://bugs.python.org/issue36878
and in typed_ast as python/typed_ast#116.

* add test back
@JukkaL
Copy link
Collaborator Author

JukkaL commented Aug 14, 2019

#7334 added a bunch of error codes (enabled by --show-error-codes). This is now implemented, though documentation and some polish is still needed.

@kaste
Copy link

kaste commented Aug 14, 2019

On how to format the error line. Please take into consideration that tools for IDE's like ale, SublimeLinter etc. have to parse such a line and break it into message, rule_name and so on. Message can have arbitrary [] pairs so for these tools e.g. error[rule_name]: message is way easier especially if --show-error-codes is actually a toggle and we want a regex that works with and without errors with rule names.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Aug 15, 2019

@kaste Currently the error code is always at the end of the message, and two spaces separate it from the main message. I think that this should be enough to find it reliably with a regular expression, since the rest of a message should not contain double spaces followed by [.

@Michael0x2a
Copy link
Collaborator

I wonder if maybe a better approach would be to add a flag to mypy that makes it report all the errors in JSON format? That would let linters skip having to manually parse things.

@kaste
Copy link

kaste commented Aug 15, 2019

All linter frameworks do regex based parsing, usually declarative in that the named capturing groups just map to the expected api of that framework. That's really easy.

@anentropic
Copy link

Shouldn't each error have its own code (as per every other linting tool)?

maybe broad categories of errors have their place too, but eg if I want to ignore the error for recursive types --show-error-codes gives:

(possible cyclic definition)  [misc]

I only want to ignore that specific error, misc does not sound very specific 😞

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 3, 2020

Mypy probably generates hundreds of different errors, and implementing and documenting an error code for all of them is a lot of work. The misc error code is a fallback for unclassified errors. Pretty much all the common errors have specific error codes, so it's not very likely that a line would generate multiple errors with the misc error code.

However, we accept PRs that add new error codes.

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

No branches or pull requests

6 participants