yet another strict null checks branch #1605
Conversation
src/lsp/main.ts
Outdated
| if (client) { | ||
| resolve(client); | ||
| } else if (error) { | ||
| reject(new Error('clojure-lsp: ' + error)); |
There was a problem hiding this comment.
this section is just a refactoring to prevent multiple fetches and not repeat ourselves as much. plus it makes the condition branching here a lot clearer
It also makes things a bit clearer
Also ensure values are boolean
corasaurus-hex
force-pushed
the
cora/yasncb-yet-another-strict-null-checks-branch
branch
from
a78ead4 to
3ab4ccb
Compare
| const newID = uuid.uuid(); | ||
| void this.store.update(KEY, newID); | ||
| return newID; | ||
| } else { | ||
| return this.store.get(KEY); | ||
| return value; |
There was a problem hiding this comment.
We have established that value is not undefined at this point and the other conditional branch also returns a string and so we have removed the possibility of undefined.
| 'format-as-you-type': workspaceConfig.get<boolean>('formatAsYouType'), | ||
| 'keep-comment-forms-trail-paren-on-own-line?': workspaceConfig.get<boolean>( | ||
| 'format-as-you-type': !!workspaceConfig.get<boolean>('formatAsYouType'), | ||
| 'keep-comment-forms-trail-paren-on-own-line?': !!workspaceConfig.get<boolean>( |
There was a problem hiding this comment.
Using !! to ensure we return booleans instead of undefined.
There was a problem hiding this comment.
I thought I had removed the format-as-you-type setting, relying on VS Code's setting for this... But that was probably in the old failed Parinfer branches... Note to self to remove this setting at some point soon.
| return false; | ||
| } | ||
|
|
||
| wsEdit.set(document.uri, edits); |
There was a problem hiding this comment.
As discussed, if edits is undefined we log an error and return false instead of trying to apply undefined edits.
| if (isUndefined(r.edits)) { | ||
| console.error('Edits were undefined!', cloneDeep({ editBuilder, r, editor })); | ||
| return; | ||
| } |
There was a problem hiding this comment.
As discussed, log an error and return if there are no edits because there is nothing to do if that is the case.
| 'edit.text was undefined!', | ||
| cloneDeep({ edit, editBuilder, r, editor }) | ||
| ); | ||
| return; |
There was a problem hiding this comment.
Skip applying undefined edit text since that makes no sense.
src/calva-fmt/src/infer.ts
Outdated
| if (isUndefined(r.character)) { | ||
| console.error('Expected result character to be defined!', { r }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
I don't know if, in practice, these will ever actually be undefined or not. It would be nice to have the types reflect that if that's the case.
There was a problem hiding this comment.
They will never be undefined. This is the code we use to calculate them:
(defn raplacement-edits-for-diffing-lines
"Returns a list of replacement edits to apply to `old-text` to get `new-text`.
Edits will be in the form `[:replace [range] text]`,
where `range` is in the form `[[start-line start-char] [end-line end-char]]`.
NB: The two versions need to have the same amount of lines."
[old-text new-text]
(let [old-lines (util/split-into-lines old-text)
new-lines (util/split-into-lines new-text)]
(->> (map vector (range) old-lines new-lines)
(remove (fn [[line o n]] (= o n)))
(mapv (fn [[line o n]]
{:edit "replace"
:start {:line line
:character 0}
:end {:line line
:character (count o)}
:text n})))))There was a problem hiding this comment.
cool, I'll modify these to use non-null-assertions (!) then
| } else { | ||
| throw new Error('Expected a uri to be passed in or a config to exist at .calva/config.edn'); | ||
| } | ||
| const data = await vscode.workspace.fs.readFile(resolvedUri); |
There was a problem hiding this comment.
Fixing the issue where state.resolvePath can return undefined and make sure we have a better error message when that's the case.
| (configOptions.inspect('customREPLHoverSnippets') | ||
| .workspaceValue as customREPLCommandSnippet[]) ?? [] | ||
| configOptions.inspect<customREPLCommandSnippet[]>('customREPLHoverSnippets')?.workspaceValue ?? | ||
| [] |
There was a problem hiding this comment.
since inspect may return undefined here we need the optional ?
| @@ -680,15 +680,18 @@ export async function getClojuredocs(symName: string, symNs: string): Promise<an | |||
| } | |||
|
|
|||
| // TODO: This feels a bit brute, what are other ways to wait for the client to initialize? | |||
| function getClient(timeout: number): Promise<LanguageClient> | undefined { | |||
| function getClient(timeout: number): Promise<LanguageClient> { | |||
| const start = Date.now(); | |||
| return new Promise(waitForClientStarted); | |||
There was a problem hiding this comment.
This always returns a promise and so we can remove the undefined return.
| if (client) { | ||
| resolve(client); | ||
| } else if (error) { | ||
| reject(new Error('clojure-lsp: ' + error)); |
There was a problem hiding this comment.
Avoid multiple fetches and make the code a little more clear.
| export async function isDocumentWritable( | ||
| document: vscode.TextDocument | ||
| ): Promise<boolean | undefined> { | ||
| export async function isDocumentWritable(document: vscode.TextDocument): Promise<boolean> { |
There was a problem hiding this comment.
turns out we're no longer possibly returning undefined here!
corasaurus-hex
changed the title
|
Down to 224 errors now, btw, down from 245 on the dev branch. we're asymptotically approaching zero type errors |
What has Changed?
More strict null checks changes. Nothing drastic, so it should be an easy merge.
My Calva PR Checklist
I have:
devbranch. (Or have specific reasons to target some other branch.)published. (Sorry for the nagging.)[Unreleased]entry inCHANGELOG.md, linking the issue(s) that the PR is addressing.ci/circleci: buildtest.npm run prettier-format)npm run eslintbefore creating your PR, or runnpm run eslint-watchto eslint as you go).