| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Code looks good, but it looks like this fix shows for any incorrect addition, like number + HTMLElement, and then proceeds to do nothing when invoked. Did you think about making specific errors for + et al when one of the sides is a Promise? That would make the codefix suggestion quite a bit more accurate. What do you think?
Sorry, something went wrong.
| const errorCodes = [ | ||
| Diagnostics.An_arithmetic_operand_must_be_of_type_any_number_bigint_or_an_enum_type.code, | ||
| Diagnostics.The_left_hand_side_of_an_arithmetic_operation_must_be_of_type_any_number_bigint_or_an_enum_type.code, | ||
| Diagnostics.The_right_hand_side_of_an_arithmetic_operation_must_be_of_type_any_number_bigint_or_an_enum_type.code, |
There was a problem hiding this comment.
This will show for any old type combination, like number + Event, right? What happens in that case? Does it just fail to do anything when you try to "add missing await"?
Sorry, something went wrong.
There was a problem hiding this comment.
It's supposed to be filtered out further in getCodeActions (it doesn’t show anything if you return undefined / empty array in there). This is back to the tradeoffs we were discussing about using related info as a “bonus error message” vs. copying and pasting all these into new, unique messages. I wrote this down as a larger LS design/API change to consider at some later point.
Sorry, something went wrong.
Hum, yes, I did think about it, but it should be handled by isMissingAwaitError. Something is amiss. (Related, I had a discussion with Wes in Teams about how VS Code doesn’t request code fixes for related info error codes, which would make this much cleaner. I’m thinking about PRing them to suggest a change there.) |
Sorry, something went wrong.
Co-Authored-By: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Ah, I thought you were saying you observed this happening, but I think you were just looking at the error codes? I believe it’s working correctly, but I just pushed a test for that negative case specifically. It’s a good thing to assert in the tests. I gave some more context about what’s happening and why I took this approach in the inline comments, but TL;DR, the error codes are necessary but not sufficient to surface a quick fix. Once getCodeActions is invoked with a possible candidate, it correlates the error with the related message Did you forget to use 'await'? that the checker added via semantic knowledge that there’s a strong chance await is appropriate (#32239). If it doesn’t find that, it bails, and a code fix doesn’t show. |
Sorry, something went wrong.
|
Nope, I didn't run it, just read the code. I think I was thinking of the refactor API instead of the codefix API? IIRC the refactor API is the one with two calls, and the first one has to be fast because it runs a lot. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #30646
Most of this PR is not terribly interesting, so let me point out the interesting parts: