Skip to content

Commit

Permalink
fix(lint): fix noConfusingVoidType to accept generic types in a uni…
Browse files Browse the repository at this point in the history
…on type (#611)
  • Loading branch information
unvalley authored Oct 29, 2023
1 parent 8d61c37 commit 96061a8
Show file tree
Hide file tree
Showing 8 changed files with 611 additions and 140 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom

### Analyzer

#### Bug fixes

- Fix [#604](https://github.com/biomejs/biome/issues/604) which made [noConfusingVoidType](https://biomejs.dev/linter/rules/no-confusing-void-type) report false positives when the `void` type is used in a generic type parameter. Contributed by @unvalley

### CLI

### Configuration
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_js_syntax::{AnyTsType, JsSyntaxKind};
use biome_js_syntax::{JsLanguage, JsSyntaxKind, TsVoidType};
use biome_rowan::{AstNode, SyntaxNode};

declare_rule! {
/// Disallow `void` type outside of generic or return types.
///
/// `void` in TypeScript refers to a function return that is meant to be ignored. Attempting to use a void type outside of a return type or generic type argument is often a sign of programmer error. void can also be misleading for other developers even if used correctly.
/// `void` in TypeScript refers to a function return that is meant to be ignored. Attempting to use a void type outside of a return type or generic type argument is often a sign of programmer error. `void` can also be misleading for other developers even if used correctly.
///
/// > The `void` type means cannot be mixed with any other types, other than `never`, which accepts all types.
/// > If you think you need this then you probably want the undefined type instead.
/// > If you think you need this then you probably want the `undefined` type instead.
///
/// ## Examples
///
/// ### Invalid
///
/// ```ts,expect_diagnostic
Expand Down Expand Up @@ -44,7 +45,6 @@ declare_rule! {
///
/// ```ts
/// function printArg<T = void>(arg: T) {}
/// printArg<void>(undefined);
/// ```
pub(crate) NoConfusingVoidType {
version: "1.2.0",
Expand All @@ -53,88 +53,100 @@ declare_rule! {
}
}

type Language = <AnyTsType as AstNode>::Language;

// We only focus on union type
pub enum VoidTypeIn {
/// We only focus on union type
pub enum VoidTypeContext {
Union,
Unknown,
}

impl Rule for NoConfusingVoidType {
type Query = Ast<AnyTsType>;
type State = VoidTypeIn;
type Query = Ast<TsVoidType>;
type State = VoidTypeContext;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();

if let AnyTsType::TsVoidType(node) = node {
let result = node_in(node.syntax());
return result;
}

None
decide_void_type_context(node.syntax())
}

fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
return Some(
RuleDiagnostic::new(
rule_category!(),
node.range(),
markup! {{match_message(state)}},
)
.note(markup! {
"Remove "<Emphasis>"void"</Emphasis>
}),
);
let message = match state {
VoidTypeContext::Union => "void is not valid as a constituent in a union type",
VoidTypeContext::Unknown => {
"void is only valid as a return type or a type argument in generic type"
}
};

Some(
RuleDiagnostic::new(rule_category!(), node.range(), markup! {{message}}).note(
markup! {
"Remove "<Emphasis>"void"</Emphasis>
},
),
)
}
}

fn node_in(node: &SyntaxNode<Language>) -> Option<VoidTypeIn> {
fn decide_void_type_context(node: &SyntaxNode<JsLanguage>) -> Option<VoidTypeContext> {
for parent in node.parent()?.ancestors() {
match parent.kind() {
JsSyntaxKind::TS_UNION_TYPE_VARIANT_LIST => {
// checks if the union type contains a generic type has a void type as argument
for child in parent.descendants() {
if child.kind() == JsSyntaxKind::TS_TYPE_ARGUMENT_LIST {
let found_void_type = child
.descendants()
.any(|descendant| descendant.kind() == JsSyntaxKind::TS_VOID_TYPE);
if found_void_type {
return None;
}
}
}
}

// (string | void)
// string | void
// string & void
// arg: void
// fn<T = void>() {}
JsSyntaxKind::TS_PARENTHESIZED_TYPE
| JsSyntaxKind::TS_UNION_TYPE_VARIANT_LIST
| JsSyntaxKind::TS_INTERSECTION_TYPE_ELEMENT_LIST
| JsSyntaxKind::TS_TYPE_ANNOTATION
| JsSyntaxKind::TS_DEFAULT_TYPE_CLAUSE => {
continue;
}

JsSyntaxKind::TS_UNION_TYPE => {
return Some(VoidTypeIn::Union);
return Some(VoidTypeContext::Union);
}

// Promise<void>
JsSyntaxKind::TS_TYPE_ARGUMENT_LIST => {
// handle generic function that has a void type argument
// e.g. functionGeneric<void>(undefined);
let Some(grand_parent) = parent.grand_parent() else {
continue;
};
if grand_parent.kind() == JsSyntaxKind::JS_CALL_EXPRESSION {
return Some(VoidTypeContext::Unknown);
}
return None;
}

// function fn(this: void) {}
// fn(): void;
// fn<T = void>() {}
// Promise<void>
JsSyntaxKind::TS_THIS_PARAMETER
| JsSyntaxKind::TS_RETURN_TYPE_ANNOTATION
| JsSyntaxKind::TS_TYPE_PARAMETER
| JsSyntaxKind::TS_FUNCTION_TYPE
| JsSyntaxKind::TS_TYPE_ARGUMENT_LIST => {
| JsSyntaxKind::TS_FUNCTION_TYPE => {
return None;
}

_ => return Some(VoidTypeIn::Unknown),
_ => return Some(VoidTypeContext::Unknown),
}
}

Some(VoidTypeIn::Unknown)
}

fn match_message(node: &VoidTypeIn) -> String {
if matches!(node, VoidTypeIn::Union) {
return "void is not valid as a constituent in a union type".into();
}

"void is only valid as a return type or a type argument in generic type".into()
Some(VoidTypeContext::Unknown)
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,45 @@
type PossibleValues = string | number | void;
type MorePossibleValues = string | ((number & any) | (string | void));

function logSomething(thing: void) {}
function printArg<T = void>(arg: T) {}
logAndReturn<void>(undefined);

let voidPromise: Promise<void> = new Promise<void>(() => { });
let voidMap: Map<string, void> = new Map<string, void>();
// ref: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/tests/rules/no-invalid-void-type.test.ts
function takeVoid(thing: void) {}
const arrowGeneric = <T extends void>(arg: T) => {};
const arrowGeneric2 = <T extends void = void>(arg: T) => {};
function functionGeneric<T extends void>(arg: T) {}
function functionGeneric2<T extends void = void>(arg: T) {}
declare function functionDeclaration<T extends void>(arg: T): void;
declare function functionDeclaration2<T extends void = void>(arg: T): void;
functionGeneric<void>(undefined);
declare function voidArray(args: void[]): void[];
let value = undefined as void;
let value = <void>undefined;
function takesThings(...things: void[]): void {}
type KeyofVoid = keyof void;

interface Interface {
prop: void;
lambda: () => void;
voidProp: void;
}

class MyClass {
private readonly propName: void;
class ClassName {
private readonly propName: void;
}
let letVoid: void;
type VoidType = void;
class OtherClassName {
private propName: VoidType;
}

let foo: void;
let bar = 1 as unknown as void;
let baz = 1 as unknown as void | string;
type UnionType = string | number | void;
type UnionType = string | ((number & any) | (string | void));
declare function test(): number | void;
declare function test<T extends number | void>(): T;
type IntersectionType = string & number & void;

type MappedType<T> = {
[K in keyof T]: void;
};

type ConditionalType<T> = {
[K in keyof T]: T[K] extends string ? void : string;
};
type ManyVoid = readonly void[];
function foo(arr: readonly void[]) {}
type invalidVoidUnion = void | Map<string, number>;
Loading

0 comments on commit 96061a8

Please sign in to comment.