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

feat(eslint-plugin): [no-invalid-void-type] allow union of void and allowInGenericTypeArguments #1960

Merged
merged 10 commits into from May 17, 2020
Merged

feat(eslint-plugin): [no-invalid-void-type] allow union of void and allowInGenericTypeArguments #1960

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a3b75c3
feat(eslint-plugin): allow union of void and Promise<void>
sinyovercosy May 1, 2020
f0aff27
fix: add original test code referred by issue
sinyovercosy May 1, 2020
6d9ec5a
test: added reversed union test case
angelayi May 2, 2020
c4c8637
fix: added safety checks for triple parent calling
angelayi May 2, 2020
f16d326
Merge branch 'master' into no-invalid-void-type
angelayi May 2, 2020
0c161c5
fix: [in progress] trying to add allowInGenericTypeArguments
angelayi May 4, 2020
34fcf4e
fix: allow some union types to work with allowGenericTypeArguments
sinyovercosy May 4, 2020
83608bf
docs: added union type behavior to documentation, some tests
sinyovercosy May 4, 2020
0662e55
fix: [coverage] ignore hard-to-enter check block
sinyovercosy May 4, 2020
9d58fe8
Merge branch 'master' into no-invalid-void-type
bradzacher May 17, 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
52 changes: 52 additions & 0 deletions packages/eslint-plugin/src/rules/no-invalid-void-type.ts
View file
Open in desktop
Expand Up @@ -61,6 +61,40 @@ export default util.createRule<[Options], MessageIds>({
AST_NODE_TYPES.Identifier,
];

/*
* isValidUnionType: checks that each member of the union is either a void
* or a Promise<void>
*/

function isValidUnionType(node: TSESTree.TSUnionType): boolean {
function isValidMemberType(member: TSESTree.TypeNode): boolean {
if (member.type === AST_NODE_TYPES.TSVoidKeyword) {
return true;
}

if (
member.type === AST_NODE_TYPES.TSTypeReference &&
member.typeParameters?.type ===
AST_NODE_TYPES.TSTypeParameterInstantiation
) {
const sourceCode = context.getSourceCode();
const fullyQualifiedName = sourceCode
.getText(member.typeName)
.replace(/ /gu, '');

return (
fullyQualifiedName === 'Promise' &&
member.typeParameters.params.length === 1 &&
member.typeParameters.params[0].type ===
AST_NODE_TYPES.TSVoidKeyword
);
}
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm.
I think this should follow the same rule as the base check.
i.e. instead of special casing Promise, it should allow all generic types if allowInGenericTypeArguments: true, or just the whitelisted ones if allowInGenericTypeArguments: string[].


return node.types.every(isValidMemberType);
}

if (allowInGenericTypeArguments === true) {
validParents.push(AST_NODE_TYPES.TSTypeParameterInstantiation);
}
Expand All @@ -72,6 +106,24 @@ export default util.createRule<[Options], MessageIds>({
return;
}

// handling void inside union
if (
node.parent.type === AST_NODE_TYPES.TSUnionType &&
isValidUnionType(node.parent)
) {
return;
}

// handling Promise<void> inside union
if (
node.parent.type === AST_NODE_TYPES.TSTypeParameterInstantiation &&
node.parent.parent.type === AST_NODE_TYPES.TSTypeReference &&
node.parent.parent.parent?.type === AST_NODE_TYPES.TSUnionType &&
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
isValidUnionType(node.parent.parent.parent)
) {
return;
}

if (
validParents.includes(node.parent.type) &&
!invalidGrandParents.includes(node.parent.parent.type)
Expand Down
17 changes: 17 additions & 0 deletions packages/eslint-plugin/tests/rules/no-invalid-void-type.test.ts
View file
Open in desktop
Expand Up @@ -11,6 +11,23 @@ ruleTester.run('allowInGenericTypeArguments: false', rule, {
code: 'type Generic<T> = [T];',
options: [{ allowInGenericTypeArguments: false }],
},
{
code: 'type voidPromiseUnion = void | Promise<void>;',
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
options: [{ allowInGenericTypeArguments: false }],
},
{
code: 'type voidPromiseUnion = Promise<void> | void;',
options: [{ allowInGenericTypeArguments: false }],
},
{
// https://github.com/typescript-eslint/typescript-eslint/issues/1956
code: `
async function foo(bar: () => void | Promise<void>) {
await bar();
}
`,
options: [{ allowInGenericTypeArguments: false }],
},
],
invalid: [
{
Expand Down