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

Add support for unknown to array-type rule. #4219

Merged
merged 5 commits into from
Jan 3, 2019
Merged

Add support for unknown to array-type rule. #4219

merged 5 commits into from
Jan 3, 2019

Conversation

Retsam
Copy link
Contributor

@Retsam Retsam commented Oct 12, 2018

PR checklist

Overview of change:

Currently, when used with TS 3.0, the array-simple rule will error on unknown[], instead enforcing that it should be written as Array<unknown>. A small change was needed for unknown to be considered a simple type with TS 3.0.

Backwards and forwards compatibility

This change works with both Typescript 2.x and 3.x.

The tests are backwards compatible with TS 2.x, because in unknown is already treated as a simple type, in that case. (It's treated as a non-generic type reference)

The implementation is backwards compatible with TS 2.x, because ts.SyntaxKind.UnknownKeyword: will evaluate to undefined: and so that case of the switch statement will just never match with nodeType.kind, which should never be undefined.

CHANGELOG.md entry:

[enhancement] array-type rule handles Typescript 3.0's unknown type properly

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @Retsam! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@JoshuaKGoldberg
Copy link
Contributor

👋 @Retsam - I just merged a similar PR that didn't have all the Prettier noise. You should still get the "contributor" badge for you work on this one so I'll clean up the conflicts here and try to merge it in. Thanks for the PR!

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An extra testling line per test file post-merge - merging in anyway because both contributions were valid. 😊

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

Successfully merging this pull request may close these issues.

3 participants