Tslint: Read optional properties without check

[NN---]

Is it possible to have rule that warns for this code:

interface A { x?: number; }

function f(a :A): number {
 return a.x; // TSLint warning may return undefined.
}

And doesn't warn here:

interface A { x?: number; }

function f(a :A): number {
 if (!a.x) 
// or if(a.x !== undefined)
// or if(typeof a.x !== "undefined")
 return a.x; // TSLint warning may return undefined.
else
 return -1;
}

[adidahiya]

+1

[dallonf]

It would be even more helpful if this rule could detect situations like this:

interface Person {
  address?: {
    line1: string
  }
}

let person = getPerson();
console.log(person.address.line1); // `person.address` might be null or undefined here

but would allow:

if (person.address) {
  console.log(person.address.line1);
}
// or
console.log(person.address && person.address.line1);

and, of course, would allow the access of members of required fieds, no questions asked.

I find that these sorts of TypeErrors are still very common even in TypeScript, and it would be great to statically detect them!

[myitcv]

interface A { x?: number; }

function f(a :A): number {
    return a.x; // TSLint warning may return undefined.
}

This is, I think, impossible to achieve sensibly because Undefined is a sub type of all types.

let a: A = undefined; // this is valid
let n: number = undefined; // as is this

Hence you would also have to check that the argument a is not undefined... as well as then checking that a.x is not undefined....

I think you're after the Go equivalent of struct and non-nullable types

[NN---]

@myitcv Correct, 'a' has to be checked for undefined too.

if (a !== undefined && a.x !== undefined) {
 return a.x; // a.x is not undefined
}

[myitcv]

@NN--- so under your proposal you would have to guard the usage over every single variable, correct?

[adidahiya]

looks like @myitcv is right since we don't have non-nullable types in TS -- so I think a rule more along the lines of @dallonf's suggestion is more feasible where we lint for nested access on properties that are explicitly optional.

[dallonf]

Unfortunately, it seems that "required" vs "optional" - across the board - doesn't mean what we would think and has nothing to do with whether the value is null or not - the only thing that "required" means is that it must be specified. To extend my previous example:

interface Person {
  address: { // Required field
    line1: string
  }
}

let person : Person = {
  address: undefined
};

is perfectly valid according to the compiler. That said, a rule that checks for unprotected access could still be useful - I seem to remember Android Studio running a lint rule like this by default in Java, which doesn't have non-nullable types either. Maybe "allow unprotected access of required fields' members" could be an option to this rule (with a big warning in the documentation that it might not mean what you think it means)?

[jkillian]

@dallonf Do you think it's common for people to write TS code where every method call or field access is guarded by a check for null/undefined? In theory I agree with you, but I'm wondering in practice if this is ever what really happens and if a rule that enforces this would ever really be used.

It seems that there are often implicit contracts that aren't enforced explicitly by code:

function sum(x:number, y:number) {
  // can assume that x and y are defined
  return x + y;
}

would we really want a rule that forced you to do everywhere:

function sum(x:number, y:number) {
  if(x != null && y != null) {
    return x + y;
  } else {
    theWorldIsEnding();
  }
}

Edit: After writing this, I can see the potential usefulness. While it would definitely makes the code more verbose, the rule would help w/ explicit correctness. Still curious though how many TS codebases would want this strong of a lint rule.

[dallonf]

@JKillian IMO it only makes sense to do this for member access (accessing x is fine, but x.y should be protected), but you're right in that even then it would raise warnings all over the place. Without language support for non-nullable types, it's hard to make this work sanely.

[NN---]

Good comments.
I agree that function which accepts 'number' by required and not by optional parameter doesn't expect it to be undefined.
Definitely non-nullable (or non-undefined) types is the better solution for this problem.

I see the only possibility for now is to analyze the caller.
In most cases passing 'undefined' or a variable initialized explicitly with 'undefined' to a function receiving a required argument is an error and not an intended behavior.
Perhaps this case is already handled by TSLint ?

Thanks.

[andy-hanson]

We have --strictNullChecks in TypeScript now, so I don't think there's any need for this.