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

Improve error messages for typed_kwarg type mismatches in containers #9599

Merged

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Nov 19, 2021

Currently if an argument is T.List[str], but you pass T.List[int], then you get a message like should ahve been List[str], but was list. That's not helpful.

This series makes a few small tweaks to make this all much more useful. First, it replaces that wit "but was" message with a more useful but was list[int]. Then it replaces "list" with "array", since meson has array's, not lists. Finally, it moves to the more standard | as a representation for union types, ie array[int | str], which both python and typescript use, so it should be clearer.

@eli-schwartz
Copy link
Member

And you even have a free fix for run_single_test being useless for tests that invert the error code and test that certain things should produce errors! That one's been bugging me for a while...

@codecov
Copy link

codecov bot commented Nov 19, 2021

Codecov Report

Merging #9599 (d412f0a) into master (958b1a7) will increase coverage by 0.00%.
The diff coverage is 93.93%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9599   +/-   ##
=======================================
  Coverage   67.41%   67.41%           
=======================================
  Files         402      402           
  Lines       85478    85498   +20     
  Branches    17620    17632   +12     
=======================================
+ Hits        57625    57640   +15     
- Misses      23246    23249    +3     
- Partials     4607     4609    +2     
Impacted Files Coverage Δ
mesonbuild/interpreterbase/decorators.py 91.36% <93.93%> (-0.28%) ⬇️
mesonbuild/scripts/vcstagger.py 87.50% <0.00%> (-4.17%) ⬇️
interpreterbase/decorators.py 90.88% <0.00%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 150b110...d412f0a. Read the comment docs.

The inner closure of the typed_kwargs function is already complicated
enough without defining closures in the middle of a loop. Let's just
pass the types_tuple as an argument to both avoid redefining the
function over and over, and also make the whole thing easier to read.
@dcbaker dcbaker force-pushed the submit/typed_kwargs-message-improvements branch from bba07e2 to 28c422e Compare November 22, 2021 19:07
Currently, if you pass a `[]string`, but the argument expects
`[]number`, then you get a message like `expected list[str] but got
list`. That isn't helpful. With this patch arrays and dictionaries will
both print messages with the types provided.
Python uses this syntax now, as does typescript and other languages
We don't want to get something like "expected array[str], but got
array[int | int]", we really want `arrayp[int]`.
@dcbaker dcbaker force-pushed the submit/typed_kwargs-message-improvements branch from 28c422e to d412f0a Compare November 22, 2021 20:27
@jpakkane jpakkane merged commit 11e26a0 into mesonbuild:master Nov 27, 2021
@dcbaker dcbaker deleted the submit/typed_kwargs-message-improvements branch November 28, 2021 03:33
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.

3 participants