Feature nullability #5400
Merged
Feature nullability #5400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And also fix bug where fix all might be applied to the wrong editor
JoeRobich
approved these changes
Oct 10, 2022
| @@ -1188,17 +1188,20 @@ | |||
| { | |||
| "command": "o.fixAll.solution", | |||
| "title": "Fix all occurrences of a code issue within solution", | |||
| "category": "OmniSharp" | |||
| "category": "OmniSharp", | |||
| "enablement": "editorFocus" | |||
There was a problem hiding this comment.
Thanks, this is a nice change.
| @@ -119,18 +119,20 @@ export default class OmnisharpCompletionProvider extends AbstractProvider implem | |||
| return; | |||
| } | |||
|
|
|||
| const responseLine = response.Line; | |||
| const responseColumn = response.Column; | |||
| const editor = window.visibleTextEditors.find(editor => editor.document === document); | |||
There was a problem hiding this comment.
This seems like a bug fix. Did you run into issues with this during development?
There was a problem hiding this comment.
I actually caught it via TS warning that window.activeTextEditor could be undefined! Though I can definitely see the bug occurring during normal development.
| export async function reAnalyze(server: OmniSharpServer, request: any) { | ||
| return server.makeRequest<any>(protocol.Requests.ReAnalyze, request); | ||
| export async function reAnalyze(server: OmniSharpServer, request: protocol.ReAnalyzeRequest) { | ||
| return server.makeRequest<protocol.ReAnalyzeReponse>(protocol.Requests.ReAnalyze, request); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: I'm aware of #5276 and that the feature/ folder may well become obsolete in the near future.
Nevertheless, given there's no set timeline yet, I figure the sooner strict mode can be turned on globally, the better, and I also got to learn some new TypeScript tricks along the way (like type guards).
Note that none of these changes have been validated on an actual codebase.
A good chunk of the fixes fall into one of two buckets: a) simply explicitly returning undefined (or an empty array) when we have nothing to return to VS Code or b) reducing a
hascheck on a Map followed by agetto justgetand then a check against undefined.Otherwise:
Impact:
Brings strict errors down to 234 from 377 when compared to master @ 9336244.
(If we ignore tests, the number of violations goes down further to just 40, 27 of which are in folders I haven't looked at yet.)