Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Should not report an error on "null" when using in type position #1265

Closed
yuit opened this issue May 23, 2016 · 9 comments
Closed

Should not report an error on "null" when using in type position #1265

yuit opened this issue May 23, 2016 · 9 comments

Comments

@yuit
Copy link

yuit commented May 23, 2016

We are trying to add linting to our library source files and run into issue below.

Bug Report (delete this for feature requests)

  • TSLint version: @next
  • TypeScript version: latest source-code built in master branch
  • Running TSLint via: Node.js API

TypeScript code being linted

match(regexp: string): RegExpMatchArray | null;

with tslint.json:

     ....
     "no-null-keyword": true,

Actual behavior

Get an error Use 'undefined' instead of 'null'

Expected behavior

There shouldn't be an error when using null in type position

@jkillian
Copy link
Contributor

Good catch, null should be allowed as a type

@adidahiya
Copy link
Contributor

specifically, this has to do with the no-null-keyword rule

@jkillian
Copy link
Contributor

Two quick questions @yuit:

  • Do you know an easy way when dealing with the AST to tell if a node is being used as a type?
  • Does TS produce the same AST no matter if the --strictNullChecks flag is on or not?

@jkillian
Copy link
Contributor

Okay, as far as I understand from this, no matter what the compiler options, null is still allowed in a type position. So, TSLint should be able to disregard whether --strictNullChecks is being used by the end user which is very good.

So question two from above taken care of I think 😃

@jkillian
Copy link
Contributor

As for my first question, although a little different than the top-down approach TSLint normally uses, seemed easiest to just traverse through the ancestors of each found null keyword and check if any ancestor has a type kind. Theoretically this could be re-traversing a lot of nodes, but in this context the issue shouldn't arise.

@yuit
Copy link
Author

yuit commented May 27, 2016

@jkillian I agree to check that null is used in type position. you will have to walk up the parents to check if any has a type kind. However, as you mentions it shouldn't be too much performance hit since the level of traversing shouldn't be too deep

jkillian added a commit that referenced this issue May 27, 2016
@jkillian
Copy link
Contributor

Great, fix is merged! This will be in a release shortly

@yuit
Copy link
Author

yuit commented May 27, 2016

Thanks ! 👍

@jkillian
Copy link
Contributor

Released in v3.10.0-dev.3 which is what tslint@next currently points to, cheers!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants