Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: improve token types and add missing type guards #1497

Merged
merged 8 commits into from Jan 24, 2020
Merged

fix: improve token types and add missing type guards #1497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ed9e41c
feat: improve token types and add missing type guards
armano2 Jan 22, 2020
d245d9a
fix: add better type-guards for eslint-utils
armano2 Jan 22, 2020
3c3dee4
fix: simplify computation of return type from options
armano2 Jan 23, 2020
da69d34
fix: correct typo in isNotCommentToken
armano2 Jan 23, 2020
e9cbe19
Merge branch 'master' into type-improvement
armano2 Jan 23, 2020
552a5d7
fix(eslint-plugin): remove unnecessary type casting
armano2 Jan 23, 2020
0d45972
chore(eslint-plugin): remove one more type cast
armano2 Jan 23, 2020
499c705
chore: add missing type guards to astUtils
armano2 Jan 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintignore
View file
Open in desktop
Expand Up @@ -4,5 +4,6 @@ jest.config.js
fixtures
shared-fixtures
coverage
__snapshots__

packages/eslint-plugin-tslint/tests
25 changes: 10 additions & 15 deletions packages/eslint-plugin/src/rules/comma-spacing.ts
View file
Open in desktop
Expand Up @@ -54,7 +54,7 @@ export default createRule<Options, MessageIds>({
create(context, [{ before: spaceBefore, after: spaceAfter }]) {
const sourceCode = context.getSourceCode();
const tokensAndComments = sourceCode.tokensAndComments;
const ignoredTokens = new Set<TSESTree.Token>();
const ignoredTokens = new Set<TSESTree.PunctuatorToken>();

/**
* Adds null elements of the ArrayExpression or ArrayPattern node to the ignore list
Expand All @@ -67,7 +67,7 @@ export default createRule<Options, MessageIds>({
for (const element of node.elements) {
let token: TSESTree.Token | null;
if (element === null) {
token = sourceCode.getTokenAfter(previousToken as TSESTree.Token);
token = sourceCode.getTokenAfter(previousToken!);
if (token && isCommaToken(token)) {
ignoredTokens.add(token);
}
Expand Down Expand Up @@ -100,9 +100,9 @@ export default createRule<Options, MessageIds>({
* @param nextToken The first token after the comma
*/
function validateCommaSpacing(
commaToken: TSESTree.Token,
prevToken: TSESTree.Token | null,
nextToken: TSESTree.Token | null,
commaToken: TSESTree.PunctuatorToken,
prevToken: TSESTree.Token | TSESTree.Comment | null,
nextToken: TSESTree.Token | TSESTree.Comment | null,
): void {
if (
prevToken &&
Expand Down Expand Up @@ -166,20 +166,15 @@ export default createRule<Options, MessageIds>({
return;
}

if (token.type === AST_TOKEN_TYPES.JSXText) {
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const commaToken = token as TSESTree.Token;
const prevToken = tokensAndComments[i - 1] as TSESTree.Token;
const nextToken = tokensAndComments[i + 1] as TSESTree.Token;
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
const prevToken = tokensAndComments[i - 1];
const nextToken = tokensAndComments[i + 1];

validateCommaSpacing(
commaToken,
isCommaToken(prevToken) || ignoredTokens.has(commaToken)
token,
isCommaToken(prevToken) || ignoredTokens.has(token)
? null
: prevToken,
isCommaToken(nextToken) || ignoredTokens.has(commaToken)
isCommaToken(nextToken) || ignoredTokens.has(token)
? null
: nextToken,
);
Expand Down
10 changes: 6 additions & 4 deletions packages/eslint-plugin/src/util/astUtils.ts
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ const LINEBREAK_MATCHER = /\r\n|[\r\n\u2028\u2029]/;

function isOptionalChainPunctuator(
token: TSESTree.Token | TSESTree.Comment,
): boolean {
): token is TSESTree.PunctuatorToken & { value: '?.' } {
return token.type === AST_TOKEN_TYPES.Punctuator && token.value === '?.';
}
function isNotOptionalChainPunctuator(
Expand All @@ -19,7 +19,7 @@ function isNotOptionalChainPunctuator(

function isNonNullAssertionPunctuator(
token: TSESTree.Token | TSESTree.Comment,
): boolean {
): token is TSESTree.PunctuatorToken & { value: '!' } {
return token.type === AST_TOKEN_TYPES.Punctuator && token.value === '!';
}
function isNotNonNullAssertionPunctuator(
Expand All @@ -33,7 +33,7 @@ function isNotNonNullAssertionPunctuator(
*/
function isOptionalOptionalChain(
node: TSESTree.Node,
): node is TSESTree.OptionalCallExpression {
): node is TSESTree.OptionalCallExpression & { optional: true } {
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
return (
node.type === AST_NODE_TYPES.OptionalCallExpression &&
// this flag means the call expression itself is option
Expand All @@ -45,7 +45,9 @@ function isOptionalOptionalChain(
/**
* Returns true if and only if the node represents logical OR
*/
function isLogicalOrOperator(node: TSESTree.Node): boolean {
function isLogicalOrOperator(
node: TSESTree.Node,
): node is TSESTree.LogicalExpression & { operator: '||' } {
return (
node.type === AST_NODE_TYPES.LogicalExpression && node.operator === '||'
);
Expand Down
28 changes: 14 additions & 14 deletions packages/eslint-plugin/typings/eslint-utils.d.ts
View file
Open in desktop
Expand Up @@ -113,67 +113,67 @@ declare module 'eslint-utils' {

export function isArrowToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: '=>' };
Copy link
Member

Choose a reason for hiding this comment

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

question:
did we want to make this generic on the value?

Copy link
Member Author

Choose a reason for hiding this comment

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

i did that at begging but typescript was behaving in unexpected way and type-guard was not working as it should

export function isNotArrowToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isClosingBraceToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: '}' };
export function isNotClosingBraceToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isClosingBracketToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: ']' };
export function isNotClosingBracketToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isClosingParenToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: ')' };
export function isNotClosingParenToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isColonToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: ':' };
export function isNotColonToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isCommaToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: ',' };
export function isNotCommaToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isCommentToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isNotCommentToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.Comment;
export function isNotCommentToken<
T extends TSESTree.Token | TSESTree.Comment
>(token: T): token is Exclude<T, TSESTree.Comment>;
export function isOpeningBraceToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: '{' };
export function isNotOpeningBraceToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isOpeningBracketToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: '[' };
export function isNotOpeningBracketToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isOpeningParenToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: '(' };
export function isNotOpeningParenToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
export function isSemicolonToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
): token is TSESTree.PunctuatorToken & { value: ';' };
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
export function isNotSemicolonToken(
token: TSESTree.Token | TSESTree.Comment,
): boolean;
Expand Down
101 changes: 51 additions & 50 deletions packages/experimental-utils/src/ts-eslint/SourceCode.ts
View file
Open in desktop
Expand Up @@ -33,8 +33,8 @@ declare interface SourceCode {
getNodeByRangeIndex(index: number): TSESTree.Node | null;

isSpaceBetween(
first: TSESTree.Token | TSESTree.Node,
second: TSESTree.Token | TSESTree.Node,
first: TSESTree.Token | TSESTree.Comment | TSESTree.Node,
second: TSESTree.Token | TSESTree.Comment | TSESTree.Node,
): boolean;

/**
Expand All @@ -49,93 +49,90 @@ declare interface SourceCode {
// Inherited methods from TokenStore
// ---------------------------------

getTokenByRangeStart(
getTokenByRangeStart<T extends { includeComments?: boolean }>(
offset: number,
options?: { includeComments?: boolean },
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getFirstToken(
getFirstToken<T extends SourceCode.CursorWithSkipOptions>(
node: TSESTree.Node,
options?: SourceCode.CursorWithSkipOptions,
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getFirstTokens(
getFirstTokens<T extends SourceCode.CursorWithCountOptions>(
node: TSESTree.Node,
options?: SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getLastToken(
getLastToken<T extends SourceCode.CursorWithSkipOptions>(
node: TSESTree.Node,
options?: SourceCode.CursorWithSkipOptions,
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getLastTokens(
getLastTokens<T extends SourceCode.CursorWithCountOptions>(
node: TSESTree.Node,
options?: SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getTokenBefore(
getTokenBefore<T extends SourceCode.CursorWithSkipOptions>(
node: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithSkipOptions,
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getTokensBefore(
getTokensBefore<T extends SourceCode.CursorWithCountOptions>(
node: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getTokenAfter(
getTokenAfter<T extends SourceCode.CursorWithSkipOptions>(
node: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithSkipOptions,
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getTokensAfter(
getTokensAfter<T extends SourceCode.CursorWithCountOptions>(
node: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getFirstTokenBetween(
getFirstTokenBetween<T extends SourceCode.CursorWithSkipOptions>(
left: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
right: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithSkipOptions,
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getFirstTokensBetween(
getFirstTokensBetween<T extends SourceCode.CursorWithCountOptions>(
left: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
right: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getLastTokenBetween(
getLastTokenBetween<T extends SourceCode.CursorWithSkipOptions>(
left: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
right: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithSkipOptions,
): TSESTree.Token | null;
options?: T,
): SourceCode.ReturnTypeFromOptions<T> | null;

getLastTokensBetween(
getLastTokensBetween<T extends SourceCode.CursorWithCountOptions>(
left: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
right: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
options?: SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getTokensBetween(
getTokensBetween<T extends SourceCode.CursorWithCountOptions>(
left: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
right: TSESTree.Node | TSESTree.Token | TSESTree.Comment,
padding?:
| number
| SourceCode.FilterPredicate
| SourceCode.CursorWithCountOptions,
armano2 marked this conversation as resolved.
Show resolved Hide resolved
): TSESTree.Token[];
padding?: T,
): SourceCode.ReturnTypeFromOptions<T>[];

getTokens(
node: TSESTree.Node,
beforeCount?: number,
afterCount?: number,
): TSESTree.Token[];
getTokens(
getTokens<T extends SourceCode.CursorWithCountOptions>(
node: TSESTree.Node,
options: SourceCode.FilterPredicate | SourceCode.CursorWithCountOptions,
): TSESTree.Token[];
options: T,
): SourceCode.ReturnTypeFromOptions<T>[];

commentsExistBetween(
left: TSESTree.Node | TSESTree.Token,
Expand Down Expand Up @@ -175,6 +172,10 @@ namespace SourceCode {
tokenOrComment: TSESTree.Token | TSESTree.Comment,
) => boolean;

export type ReturnTypeFromOptions<T> = T extends { includeComments: true }
? TSESTree.Token | TSESTree.Comment
: TSESTree.Token;
bradzacher marked this conversation as resolved.
Show resolved Hide resolved

export type CursorWithSkipOptions =
| number
| FilterPredicate
Expand Down
6 changes: 3 additions & 3 deletions packages/typescript-estree/src/convert-comments.ts
View file
Open in desktop
@@ -1,7 +1,7 @@
import * as ts from 'typescript';
import { forEachComment } from 'tsutils/util/util';
import { getLocFor } from './node-utils';
import { TSESTree } from './ts-estree';
import { AST_TOKEN_TYPES, TSESTree } from './ts-estree';

/**
* Convert all comments for the given AST.
Expand All @@ -21,8 +21,8 @@ export function convertComments(
(_, comment) => {
const type =
comment.kind == ts.SyntaxKind.SingleLineCommentTrivia
? 'Line'
: 'Block';
? AST_TOKEN_TYPES.Line
: AST_TOKEN_TYPES.Block;
armano2 marked this conversation as resolved.
Show resolved Hide resolved
const range: TSESTree.Range = [comment.pos, comment.end];
const loc = getLocFor(range[0], range[1], ast);

Expand Down