Use non-nullable types in vscode.d.ts

[jrieken]

TypeScript will introduce support for strict null check and explicit handling of null and undefined - microsoft/TypeScript#7140. This is a great opportunity for us to make the API more explicit and hopefully a chance to prevent programming errors. Parts of the API can move from jsdoc comments into type annotations. For instance, the active editor is declared like this today:

/**
 * The currently active editor or `undefined`. ...
 */
export let activeTextEditor: TextEditor;

and we can move jsdoc type info back into code like so

/**
 * The currently...
 */
export let activeTextEditor: TextEditor | undefined;

[jrieken]

depends on TypeStrong/typedoc#234

[jrieken]

@egamma @dbaeumer Looking for some guidance here. Challenge is that many of the providers define method return types ala Foo | Thenable<Foo>. The current behaviour (spec'd in jsdoc) is that they are allowed to return undefined or null sync or async from with in the promise. That results in the following, very verbose, strict-null-aware signature:

// option 1a, inline
interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): CodeLens[] | undefined | null | Thenable<CodeLens[] | undefined | null>
}

// option 1b, inline but even more verbose
interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): CodeLens[] | undefined | null | Thenable<CodeLens[]> | Thenable<undefined> | Thenable<null>
}

// option 1c, one type for null or undefined
type Nully = null | undefined;

interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): CodeLens[] | Nully | Thenable<CodeLens[] | Nully>
}

// option 2, generic result-type
type Result<T> = T | undefined | null | Thenable<T | undefined | null>

interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): Result<CodeLens[]>
}

// option 3, generic nullable-type
type Nullable<T> = T | undefined | null;    

interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): Nullable<CodeLens[]> | Thenable<Nullable<CodeLens[]>>
}

Options 2 and 3 have the advantage that they can be applied to all providers and make a shorter method signature. On the downside that add another indirection and might make things harder to understand... Tho, also option 1 isn't exactly easy to read and understand. I'd prefer option 1c

[dbaeumer]

@jrieken hard call: I would actually go for 2.) and name it ProviderResult. Using Nullable makes it bound to nullable. If TS comes up with another feature we might need to change the signature again. With ProviderResult we might be able to simple change that type then.

[bpasero]

+1 for option 2 (generic result-type)

[joaomoreno]

I'd combine 2 and 3, each a different problem in itself, but composable with each other:

type Maybe<T> = T | undefined | null;
type Result<T> = Maybe<T> | Thenable<Maybe<T>>;

interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): Result<T>;
}

[aeschli]

Wanted to suggest Optional<T> but actually like Maybe<T> even better.
+1 for option 3 with Maybe<T>.
I'd rather not combine it with 2, too many types, and, especially, 'Result' is too general.

[joaomoreno]

type Eventually<T> = Maybe<T> | Thenable<Maybe<T>>;

😄

[chrmarti]

Keeping the two concepts separate might be easier to read and would allow for more variants of reuse:

type Maybe<T> = T | undefined | null;
type Eventually<T> = T | Thenable<T>;

interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): Eventually<Maybe<CodeLens[]>>;
}

[jrieken]

Just as a reminder - We aren't just targeting TypeScript-ninjas but also folks that haven't seen a type annotation before.

From my experience, even things like OR-types aren't fully understood/made use of so I am not so sure if a 'double nested generic or-type' is the way to go.

[sandy081]

+1 for option 3

type Nullable<T> = T | undefined | null;

interface CodeLensProvider {
  provideCodeLenses(document: TextDocument, token: CancellationToken): Nullable<CodeLens[]> | Thenable<Nullable<CodeLens[]>>
}

IMO, this makes it API more readable for all kinds of users with out many references. In fact, Nullable is a bit straight so that user do not need to look for its reference.

[chrmarti]

+1 for option 3 then (don't hurt the ninjas!)

[alexdima]

+1 for option 2 with a different name for Result, such as ProviderResult.

Reasons:

• It reduces the burden of reading each and every provider signature, CodeLens[] only appears once
• It makes that I need to understand only one more concept (i.e. no need to follow through to look up two new concepts Eventually and Maybe)

[jrieken]

@Microsoft/vscode I am looking for volunteers that migrate their extensions to use strictNull-checks such that we can verify our API

[weinand]

@jrieken how do I migrate my extension?

[jrieken]

@weinand - to program against the latest API do the following:

• open your extensions package.json file
• change the engine.vscode property to *
• run npm run postinstall (assuming you have the vscode module as dev-dependency)
• undo the engine-version change from step 2

note tho that this no also depends on TypeScript 2.0

[weinand]

@jrieken done. That was easy.
But I have to set "strictNullChecks": truein my tsconfig.json so that the compiler actually checks for this.

[ramya-rao-a]

@jrieken The test item #16515 is linked here, but the test item does not have anything on non-nullable types.

[jrieken]

@ramya-rao-a This was meant for verification-needed but I have special verifier in mind: those that have already updated their extensions to use strict null checks. I have @mjbvz (ts/js) and @joaomoreno (git exploration) in mind