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.

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

feat(useExplicitType): implement support for methods and functions wi… #4535

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 107 additions & 8 deletions crates/biome_js_analyze/src/lint/nursery/use_explicit_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use biome_js_syntax::{
};
use biome_js_syntax::{
AnyJsFunction, JsGetterClassMember, JsGetterObjectMember, JsMethodClassMember,
JsMethodObjectMember,
JsMethodObjectMember, TsCallSignatureTypeMember, TsDeclareFunctionDeclaration,
TsDeclareFunctionExportDefaultDeclaration, TsGetterSignatureClassMember,
TsMethodSignatureClassMember, TsMethodSignatureTypeMember,
};
use biome_rowan::{declare_node_union, AstNode, SyntaxNode, SyntaxNodeOptionExt, TextRange};

Expand Down Expand Up @@ -105,6 +107,55 @@ declare_lint_rule! {
/// }
/// ```
///
/// The following pattern is considered incorrect code for an interface method without a return type:
///
/// ```ts,expect_diagnostic
/// interface Array<Type> {
/// method();
/// }
/// ```
///
/// The following pattern is considered incorrect code for a type declaration of a function without a return type:
///
/// ```ts,expect_diagnostic
/// type MyObject = {
/// (input: string);
/// propertyName: string;
/// };
/// ```
///
/// The following pattern is considered incorrect code for an abstract class method without a return type:
///
/// ```ts,expect_diagnostic
/// abstract class MyClass {
/// public abstract method();
/// }
/// ```
///
/// The following pattern is considered incorrect code for an abstract class getter without a return type:
///
/// ```ts,expect_diagnostic
/// abstract class P<T> {
/// abstract get poke();
/// }
/// ```
///
/// The following pattern is considered incorrect code for a function declaration in a namespace without a return type:
///
/// ```ts,expect_diagnostic
/// declare namespace myLib {
/// function makeGreeting(s: string);
/// }
/// ```
///
/// The following pattern is considered incorrect code for a module function export without a return type:
///
/// ```ts,expect_diagnostic
/// declare module "foo" {
/// export default function bar();
/// }
/// ```
///
/// ### Valid
/// ```ts
/// // No return value should be expected (void)
Expand Down Expand Up @@ -212,11 +263,22 @@ declare_lint_rule! {
}

declare_node_union! {
pub AnyJsFunctionWithReturnType = AnyJsFunction | JsMethodClassMember | JsMethodObjectMember | JsGetterClassMember | JsGetterObjectMember
pub AnyCallableWithReturn =
AnyJsFunction
| JsMethodClassMember
| JsMethodObjectMember
| JsGetterClassMember
| JsGetterObjectMember
| TsMethodSignatureTypeMember
| TsCallSignatureTypeMember
| TsMethodSignatureClassMember
| TsGetterSignatureClassMember
| TsDeclareFunctionDeclaration
| TsDeclareFunctionExportDefaultDeclaration
}

impl Rule for UseExplicitType {
type Query = Ast<AnyJsFunctionWithReturnType>;
type Query = Ast<AnyCallableWithReturn>;
type State = TextRange;
type Signals = Option<Self::State>;
type Options = ();
Expand All @@ -229,7 +291,7 @@ impl Rule for UseExplicitType {

let node = ctx.query();
match node {
AnyJsFunctionWithReturnType::AnyJsFunction(func) => {
AnyCallableWithReturn::AnyJsFunction(func) => {
if func.return_type_annotation().is_some() {
return None;
}
Expand Down Expand Up @@ -264,34 +326,71 @@ impl Rule for UseExplicitType {

Some(func_range)
}
AnyJsFunctionWithReturnType::JsMethodClassMember(method) => {
AnyCallableWithReturn::JsMethodClassMember(method) => {
if method.return_type_annotation().is_some() {
return None;
}

Some(method.node_text_range())
}
AnyJsFunctionWithReturnType::JsGetterClassMember(getter) => {
AnyCallableWithReturn::JsGetterClassMember(getter) => {
if getter.return_type().is_some() {
return None;
}

Some(getter.node_text_range())
}
AnyJsFunctionWithReturnType::JsMethodObjectMember(method) => {
AnyCallableWithReturn::JsMethodObjectMember(method) => {
if method.return_type_annotation().is_some() {
return None;
}

Some(method.node_text_range())
}
AnyJsFunctionWithReturnType::JsGetterObjectMember(getter) => {
AnyCallableWithReturn::JsGetterObjectMember(getter) => {
if getter.return_type().is_some() {
return None;
}

Some(getter.node_text_range())
}
AnyCallableWithReturn::TsMethodSignatureTypeMember(member) => {
if member.return_type_annotation().is_some() {
return None;
}

Some(member.range())
}
AnyCallableWithReturn::TsCallSignatureTypeMember(member) => {
if member.return_type_annotation().is_some() {
return None;
}
Some(member.range())
}
AnyCallableWithReturn::TsMethodSignatureClassMember(member) => {
if member.return_type_annotation().is_some() {
return None;
}
Some(member.range())
}
AnyCallableWithReturn::TsGetterSignatureClassMember(member) => {
if member.return_type().is_some() {
return None;
}
Some(member.range())
}
AnyCallableWithReturn::TsDeclareFunctionDeclaration(decl) => {
if decl.return_type_annotation().is_some() {
return None;
}
Some(decl.range())
}
AnyCallableWithReturn::TsDeclareFunctionExportDefaultDeclaration(decl) => {
if decl.return_type_annotation().is_some() {
return None;
}
Some(decl.range())
}
}
}

Expand Down
112 changes: 70 additions & 42 deletions crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,67 +29,95 @@ class Test {

const obj = {
method() {
return "test"
}
}
return "test";
},
};

const obj = {
get method() {
return "test"
},
get method() {
return "test";
},
};

const func = (value: number) => ({ type: 'X', value }) as any;
const func = (value: number) => ({ type: 'X', value }) as Action;
const func = (value: number) => ({ type: "X", value }) as any;
const func = (value: number) => ({ type: "X", value }) as Action;

export default () => {};
export default function () {}

// check higher order functions
const arrowFn = () => () => {};
const arrowFn = () => function() {}
const arrowFn = () => function () {};
const arrowFn = () => {
return () => { };
}
return () => {};
};

// does not support detecting a return of a function inside other statements like if, switch, etc.
// we check only the first statment
// we check only the first statment
const arrowFn = (a: number) => {
if (a === 1) {
return (): void => { };
} else {
return (): number => {
return a + 2
}
}
}
if (a === 1) {
return (): void => {};
} else {
return (): number => {
return a + 2;
};
}
};
const arrowFn = (a: number) => {
switch (a) {
case 1: {
return (): void => { };
}
case 2: {
return (): void => { };
}
default: {
return (): void => { };
}
}
}
switch (a) {
case 1: {
return (): void => {};
}
case 2: {
return (): void => {};
}
default: {
return (): void => {};
}
}
};

function f() {
if (x) {
return 0;
}
return (): void => {}
if (x) {
return 0;
}
return (): void => {};
}

function fn() {
let str = "hey";
return function (): string {
return str;
};
let str = "hey";
return function (): string {
return str;
};
}

const x = { prop: () => {} }
const x = { bar: { prop: () => {} } }
const x = { prop: () => {} };
const x = { bar: { prop: () => {} } };

interface Array<Type> {
pop(): Type | undefined;
push(...items: Type[]): number;
method();
}

type MyObject = {
(input: string);
propertyName: string;
};

abstract class MyClass {
public abstract method();
}

abstract class P<T> {
abstract method(): T;
abstract get poke();
}

declare namespace myLib {
function makeGreeting(s: string);
}

declare module "foo" {
export default function bar();
}
Loading