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

Feature proposal: Allow specifying out as list #96

Open
antonagestam opened this issue Jan 22, 2022 · 7 comments
Open

Feature proposal: Allow specifying out as list #96

antonagestam opened this issue Jan 22, 2022 · 7 comments

Comments

@antonagestam
Copy link
Contributor

I think the current format for output could be iterated on, and be made a little friendlier to work with. Instead of today's format using multiline strings:

out: |
  main:7: error: Argument 1 has incompatible type "int"; expected "str"  [arg-type]
  main:9: note: Revealed type is "builtins.bool*"

We could allow specifying out as a list of objects with properties:

out:
  - error: 'Argument 1 has incompatible type "int"; expected "str"  [arg-type]'
    line: 7
  - note: 'Revealed type is "builtins.bool"'
    line: 9

This would unlock a few different things we could then implement at a higher level of abstraction. There are probably more that will make sense, these are just the ones I can think of for now.

Special support for error-codes

out:
  - error: 'Argument 1 has incompatible type "int"; expected "str"'
    code: "arg-type"
    line: 7

Special support for revealed type

out:
  - revealed_type: "builtins.bool"
    line: 9

Allow mixing regex with non-regex

out:
  - error: 'Argument 1 has incompatible type "int\*?"; expected "str\*?"'
    line: 7
    regex: true

Omitting line numbers

This probably requires more knowledge of the internals than I currently have, but I think it might make sense to allow omitting line numbers. I'm proposing that this example would match if there's revealed type on line nine, followed by an error message on any line after that (strictly greater than line 9).

out:
  - note: 'Revealed type is "builtins.bool"'
    line: 9
  - error: 'Argument 1 has incompatible type "int"; expected "str"'
    code: "arg-type"

What do you think? 🙂

@sobolevn
Copy link
Member

I like it!

But, I think I need to explain why current format exists (and will be supported in the future).

It main feature is that you can copy-paste "Actual" output from mypy there. I sometime do this:

  1. I write some test
  2. I leave out: empty
  3. I run test
  4. I copy-paste "Actual" output and check it to be correct
  5. Done!

With this format, it will be easier to read, but harder to write.

@antonagestam
Copy link
Contributor Author

@sobolevn Right, I think that makes sense too, there wouldn't be any need to replace the current format.

Just piling on with some more thoughts here:

Another possible "win" with introducing a higher level of abstraction is that the library could "take the hit" for unstable output from mypy. It would sure take work to implement, but it could possibly be a way forward for addressing issues like #45.

@zero323
Copy link
Contributor

zero323 commented Jan 22, 2022

This is a nice idea, but I have a few concerns.

This probably requires more knowledge of the internals than I currently have, but I think it might make sense to allow omitting line numbers. I'm proposing that this example would match if there's revealed type on line nine, followed by an error message on any line after that (strictly greater than line 9).

This shouldn't be hard, but at the cost of further cementing match-in-order logic, which is already pretty blunt tool (#77). Also, but it is probably a lesser concern, it could result in false negatives, when

  • Expected error doesn't happen.
  • Unexpected, but matching error, happens at another line.

Another possible "win" with introducing a higher level of abstraction is that the library could "take the hit" for unstable output from mypy.

At the moment, dealing with changing mypy output is primarily a responsibility of the end users. It is a little bit of pain, but manageable, since you can control mypy version.

In contrast, making more sense out of mypy output within library, might require version specific parser ‒ something that can be hard to maintain, when checker output is not really part of the contract.

Not a big concern in case of the core idea, but can quickly get out of hand if you keep adding features.

@abelcheung
Copy link

Another possible "win" with introducing a higher level of abstraction is that the library could "take the hit" for unstable output from mypy. It would sure take work to implement, but it could possibly be a way forward for addressing issues like #45.

@antonagestam I'm also a victim of mypy changing output string over and over. Well, it's kind of expected when people have to deal with plain text output, but still. Pinning mypy version as suggested by @zero323 is a way to deal with such problem.

If you are interested in seeing more fine-grained mypy output, you can take a look at python/mypy#10816 and python/mypy#11396. However don't hold your breath for it — given the people involved in the discussion over time, I am fairly confident that mypy core dev has completely no interest and no intention to make output more accessible.

@sobolevn
Copy link
Member

Well, since I am mypy core dev myself, I am pretty interested in this :)

But, this is a complex project that requires quite a lot of details to get it right.
If you have any proposals, please feel free to share them.

@antonagestam
Copy link
Contributor Author

@abelcheung Oh cool, structured output from mypy directly seems like a much more viable approach.

@tushar-deepsource
Copy link

tushar-deepsource commented Apr 19, 2023

@sobolevn I have a proposal in python/mypy#11396, but there has been zero core dev interaction on that PR. If you can take a look at it, not only will we get structured output but it'll also unlock future endeavours like being able to define a custom output format with a plugin, or to add new fields to the output like exact issue codes.

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

No branches or pull requests

5 participants