From 925a373eef431afb0c50b32aabd7469e0bf58e75 Mon Sep 17 00:00:00 2001 From: kaykdm Date: Mon, 4 Nov 2024 19:48:53 +0900 Subject: [PATCH] feat(useExplicitType): support explicit function argument types --- .../migrate/eslint_any_rule_to_biome.rs | 8 + .../src/analyzer/linter/rules.rs | 2 +- .../src/lint/nursery/use_explicit_type.rs | 211 ++++++++++++++---- .../specs/nursery/useExplicitType/invalid.ts | 14 +- .../nursery/useExplicitType/invalid.ts.snap | 92 ++++++++ .../specs/nursery/useExplicitType/valid.d.ts | 7 + .../nursery/useExplicitType/valid.d.ts.snap | 7 + .../@biomejs/backend-jsonrpc/src/workspace.ts | 2 +- .../@biomejs/biome/configuration_schema.json | 2 +- 9 files changed, 301 insertions(+), 44 deletions(-) diff --git a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs index 1701be91588f..1a8286f123c8 100644 --- a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs +++ b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs @@ -147,6 +147,14 @@ pub(crate) fn migrate_eslint_any_rule( .get_or_insert(Default::default()); rule.set_level(rule_severity.into()); } + "@typescript-eslint/explicit-module-boundary-types" => { + if !options.include_nursery { + return false; + } + let group = rules.nursery.get_or_insert_with(Default::default); + let rule = group.use_explicit_type.get_or_insert(Default::default()); + rule.set_level(rule_severity.into()); + } "@typescript-eslint/naming-convention" => { if !options.include_inspired { results.has_inspired_rules = true; diff --git a/crates/biome_configuration/src/analyzer/linter/rules.rs b/crates/biome_configuration/src/analyzer/linter/rules.rs index db931a9c76c8..0079fc1de15d 100644 --- a/crates/biome_configuration/src/analyzer/linter/rules.rs +++ b/crates/biome_configuration/src/analyzer/linter/rules.rs @@ -3418,7 +3418,7 @@ pub struct Nursery { #[serde(skip_serializing_if = "Option::is_none")] pub use_deprecated_reason: Option>, - #[doc = "Require explicit return types on functions and class methods."] + #[doc = "Require explicit argument and return types on functions and class methods."] #[serde(skip_serializing_if = "Option::is_none")] pub use_explicit_type: Option>, #[doc = "Enforces the use of a recommended display strategy with Google Fonts."] diff --git a/crates/biome_js_analyze/src/lint/nursery/use_explicit_type.rs b/crates/biome_js_analyze/src/lint/nursery/use_explicit_type.rs index dd4bda33b777..d1fd7d3f3abb 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_explicit_type.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_explicit_type.rs @@ -1,13 +1,13 @@ use biome_analyze::{ context::RuleContext, declare_lint_rule, Ast, Rule, RuleDiagnostic, RuleSource, }; -use biome_console::markup; +use biome_console::{markup, MarkupBuf}; use biome_js_semantic::HasClosureAstNode; use biome_js_syntax::{ AnyJsBinding, AnyJsExpression, AnyJsFunctionBody, AnyJsStatement, AnyTsType, JsCallExpression, JsFileSource, JsFormalParameter, JsInitializerClause, JsLanguage, JsObjectExpression, - JsParenthesizedExpression, JsPropertyClassMember, JsPropertyObjectMember, JsStatementList, - JsSyntaxKind, JsVariableDeclarator, + JsParameters, JsParenthesizedExpression, JsPropertyClassMember, JsPropertyObjectMember, + JsStatementList, JsSyntaxKind, JsVariableDeclarator, }; use biome_js_syntax::{ AnyJsFunction, JsGetterClassMember, JsGetterObjectMember, JsMethodClassMember, @@ -16,16 +16,16 @@ use biome_js_syntax::{ use biome_rowan::{declare_node_union, AstNode, SyntaxNode, SyntaxNodeOptionExt, TextRange}; declare_lint_rule! { - /// Require explicit return types on functions and class methods. + /// Require explicit argument and return types on functions and class methods. /// /// Functions in TypeScript often don't need to be given an explicit return type annotation. /// Leaving off the return type is less code to read or write and allows the compiler to infer it from the contents of the function. /// - /// However, explicit return types do make it visually more clear what type is returned by a function. + /// However, explicit argument and return types make it visually more clear what types a function accepts and returns. /// They can also speed up TypeScript type checking performance in large codebases with many large functions. - /// Explicit return types also reduce the chance of bugs by asserting the return type, and it avoids surprising "action at a distance," where changing the body of one function may cause failures inside another function. + /// Explicit types also reduce the chance of bugs by asserting both input and output types, and it avoids surprising "action at a distance," where changing the body of one function may cause failures inside another function. /// - /// This rule enforces that functions do have an explicit return type annotation. + /// This rule enforces that functions have explicit type annotations for both their arguments and return type. /// /// ## Examples /// @@ -105,6 +105,32 @@ declare_lint_rule! { /// } /// ``` /// + /// The following pattern is considered incorrect code for missing an argument type on an function: + /// + /// ```ts,expect_diagnostic + /// export function test(a: number, b): void { + /// return; + /// } + /// ``` + /// + /// ```ts,expect_diagnostic + /// export const test = (a): void => { + /// return; + /// } + /// ``` + /// + /// ```ts,expect_diagnostic + /// export default function test(a): void { + /// return; + /// } + /// ``` + /// + /// ```ts,expect_diagnostic + /// export default (a): void => { + /// return; + /// } + /// ``` + /// /// ### Valid /// ```ts /// // No return value should be expected (void) @@ -115,8 +141,8 @@ declare_lint_rule! { /// /// ```ts /// // A return value of type number - /// var fn = function (): number { - /// return 1; + /// var fn = function (a: number): number { + /// return a + 1; /// } /// ``` /// @@ -202,22 +228,87 @@ declare_lint_rule! { /// } /// ``` /// + /// The following pattern is considered correct code because it includes both a return type and an argument type on an function: + /// + /// ```ts + /// export function test(a: number, b: number): void { + /// return; + /// } + /// ``` + /// + /// ```ts + /// export default function test(obj: {a: string}): void { + /// return; + /// } + /// ``` + /// + /// ```ts + /// export default (a: SomeType): void => { + /// return; + /// } + /// ``` + /// pub UseExplicitType { version: "1.9.3", name: "useExplicitType", language: "ts", recommended: false, - sources: &[RuleSource::EslintTypeScript("explicit-function-return-type")], + sources: &[RuleSource::EslintTypeScript("explicit-function-return-type"), RuleSource::EslintTypeScript("explicit-module-boundary-types")], } } declare_node_union! { - pub AnyJsFunctionWithReturnType = AnyJsFunction | JsMethodClassMember | JsMethodObjectMember | JsGetterClassMember | JsGetterObjectMember + pub AnyJsFunctionWithReturnTypeOrJsParameters = AnyJsFunction | JsMethodClassMember | JsMethodObjectMember | JsGetterClassMember | JsGetterObjectMember | JsParameters +} + +pub struct UseExplicitTypeState { + range: TextRange, + cause: UseExplicitTypeCause, +} + +enum UseExplicitTypeCause { + MissingReturnType, + MissingArgumentnType(String), +} + +impl UseExplicitTypeState { + fn title(&self) -> MarkupBuf { + match &self.cause { + UseExplicitTypeCause::MissingReturnType => { + (markup! {"Missing return type on function."}).to_owned() + } + UseExplicitTypeCause::MissingArgumentnType(name) => { + (markup! {"Argument '"{name}"' should be typed."}).to_owned() + } + } + } + + fn note_reason(&self) -> MarkupBuf { + match &self.cause { + UseExplicitTypeCause::MissingReturnType => { + (markup! {"Declaring the return type makes the code self-documenting and can speed up TypeScript type checking."}).to_owned() + } + UseExplicitTypeCause::MissingArgumentnType(_) => { + (markup! {"Declaring the argument types makes the code self-documenting and can speed up TypeScript type checking."}).to_owned() + } + } + } + + fn note_action(&self) -> MarkupBuf { + match &self.cause { + UseExplicitTypeCause::MissingReturnType => { + (markup! {"Add a return type annotation."}).to_owned() + } + UseExplicitTypeCause::MissingArgumentnType(_) => { + (markup! {"Add type annotations to the function arguments."}).to_owned() + } + } + } } impl Rule for UseExplicitType { - type Query = Ast; - type State = TextRange; + type Query = Ast; + type State = UseExplicitTypeState; type Signals = Option; type Options = (); @@ -229,7 +320,7 @@ impl Rule for UseExplicitType { let node = ctx.query(); match node { - AnyJsFunctionWithReturnType::AnyJsFunction(func) => { + AnyJsFunctionWithReturnTypeOrJsParameters::AnyJsFunction(func) => { if func.return_type_annotation().is_some() { return None; } @@ -256,60 +347,71 @@ impl Rule for UseExplicitType { let func_range = func.syntax().text_range(); if let Ok(Some(AnyJsBinding::JsIdentifierBinding(id))) = func.id() { - return Some(TextRange::new( - func_range.start(), - id.syntax().text_range().end(), - )); + return Some(UseExplicitTypeState { + range: TextRange::new(func_range.start(), id.syntax().text_range().end()), + cause: UseExplicitTypeCause::MissingReturnType, + }); } - Some(func_range) + Some(UseExplicitTypeState { + range: func_range, + cause: UseExplicitTypeCause::MissingReturnType, + }) } - AnyJsFunctionWithReturnType::JsMethodClassMember(method) => { + AnyJsFunctionWithReturnTypeOrJsParameters::JsMethodClassMember(method) => { if method.return_type_annotation().is_some() { return None; } - Some(method.node_text_range()) + Some(UseExplicitTypeState { + range: method.node_text_range(), + cause: UseExplicitTypeCause::MissingReturnType, + }) } - AnyJsFunctionWithReturnType::JsGetterClassMember(getter) => { + AnyJsFunctionWithReturnTypeOrJsParameters::JsGetterClassMember(getter) => { if getter.return_type().is_some() { return None; } - Some(getter.node_text_range()) + Some(UseExplicitTypeState { + range: getter.node_text_range(), + cause: UseExplicitTypeCause::MissingReturnType, + }) } - AnyJsFunctionWithReturnType::JsMethodObjectMember(method) => { + AnyJsFunctionWithReturnTypeOrJsParameters::JsMethodObjectMember(method) => { if method.return_type_annotation().is_some() { return None; } - Some(method.node_text_range()) + Some(UseExplicitTypeState { + range: method.node_text_range(), + cause: UseExplicitTypeCause::MissingReturnType, + }) } - AnyJsFunctionWithReturnType::JsGetterObjectMember(getter) => { + AnyJsFunctionWithReturnTypeOrJsParameters::JsGetterObjectMember(getter) => { if getter.return_type().is_some() { return None; } - Some(getter.node_text_range()) + Some(UseExplicitTypeState { + range: getter.node_text_range(), + cause: UseExplicitTypeCause::MissingReturnType, + }) + } + AnyJsFunctionWithReturnTypeOrJsParameters::JsParameters(params) => { + if let Some(params_diagnostic) = check_function_parameters_type(params) { + return Some(params_diagnostic); + } + None } } } fn diagnostic(_: &RuleContext, state: &Self::State) -> Option { Some( - RuleDiagnostic::new( - rule_category!(), - state, - markup! { - "Missing return type on function." - }, - ) - .note(markup! { - "Declaring the return type makes the code self-documenting and can speed up TypeScript type checking." - }) - .note(markup! { - "Add a return type annotation." - }), + RuleDiagnostic::new(rule_category!(), state.range, state.title()) + .note(state.note_reason()) + .note(state.note_action()), ) } } @@ -550,3 +652,32 @@ fn is_type_assertion(syntax: &SyntaxNode) -> bool { } }) } + +/// Checks if all parameters in a function have explicit type annotations. +/// +/// This function iterates over the provided function parameters and examines each one +/// to determine if it has a type annotation. If any parameter is missing a type annotation, +/// the function returns a `Some(UseExplicitTypeState)` indicating the range of the parameter +/// and the cause of the issue. If all parameters have type annotations, it returns `None`. +/// +/// # Arguments +/// +/// * `parameters` - A reference to a `JsParameters` object representing the parameters of a function. +/// +/// # Returns +/// +/// * `Option` - `Some(UseExplicitTypeState)` if a parameter is missing a type annotation, +/// or `None` if all parameters have explicit type annotations. +fn check_function_parameters_type(parameters: &JsParameters) -> Option { + for p in parameters.items() { + let param = p.ok()?; + let formal_param = param.as_any_js_formal_parameter()?; + if formal_param.type_annotation().is_none() { + return Some(UseExplicitTypeState { + range: formal_param.range(), + cause: UseExplicitTypeCause::MissingArgumentnType(formal_param.text()), + }); + } + } + None +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts index 1cb6ba5f394e..8efebe87b2f3 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts +++ b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts @@ -92,4 +92,16 @@ function fn() { } const x = { prop: () => {} } -const x = { bar: { prop: () => {} } } \ No newline at end of file +const x = { bar: { prop: () => {} } } + +export default (a): void => { + return; +} +export function test(a: number, b) { + return; +} +export class Test { + method(a): void { + return; + } +} \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts.snap index 6f34976d42c3..ca9dd98ceab6 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/invalid.ts.snap @@ -1,6 +1,7 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs expression: invalid.ts +snapshot_kind: text --- # Input ```ts @@ -99,6 +100,18 @@ function fn() { const x = { prop: () => {} } const x = { bar: { prop: () => {} } } + +export default (a): void => { + return; +} +export function test(a: number, b) { + return; +} +export class Test { + method(a): void { + return; + } +} ``` # Diagnostics @@ -541,6 +554,7 @@ invalid.ts:94:19 lint/nursery/useExplicitType ━━━━━━━━━━━ > 94 │ const x = { prop: () => {} } │ ^^^^^^^^^ 95 │ const x = { bar: { prop: () => {} } } + 96 │ i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking. @@ -557,6 +571,8 @@ invalid.ts:95:26 lint/nursery/useExplicitType ━━━━━━━━━━━ 94 │ const x = { prop: () => {} } > 95 │ const x = { bar: { prop: () => {} } } │ ^^^^^^^^^ + 96 │ + 97 │ export default (a): void => { i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking. @@ -564,3 +580,79 @@ invalid.ts:95:26 lint/nursery/useExplicitType ━━━━━━━━━━━ ``` + +``` +invalid.ts:97:17 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Argument 'a' should be typed. + + 95 │ const x = { bar: { prop: () => {} } } + 96 │ + > 97 │ export default (a): void => { + │ ^ + 98 │ return; + 99 │ } + + i Declaring the argument types makes the code self-documenting and can speed up TypeScript type checking. + + i Add type annotations to the function arguments. + + +``` + +``` +invalid.ts:100:8 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Missing return type on function. + + 98 │ return; + 99 │ } + > 100 │ export function test(a: number, b) { + │ ^^^^^^^^^^^^^ + 101 │ return; + 102 │ } + + i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking. + + i Add a return type annotation. + + +``` + +``` +invalid.ts:100:33 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Argument 'b' should be typed. + + 98 │ return; + 99 │ } + > 100 │ export function test(a: number, b) { + │ ^ + 101 │ return; + 102 │ } + + i Declaring the argument types makes the code self-documenting and can speed up TypeScript type checking. + + i Add type annotations to the function arguments. + + +``` + +``` +invalid.ts:104:10 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Argument 'a' should be typed. + + 102 │ } + 103 │ export class Test { + > 104 │ method(a): void { + │ ^ + 105 │ return; + 106 │ } + + i Declaring the argument types makes the code self-documenting and can speed up TypeScript type checking. + + i Add type annotations to the function arguments. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts index 812eafccb024..18ff40085575 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts +++ b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts @@ -46,3 +46,10 @@ declare function test(): void; declare var fn: () => number; declare var arrowFn: () => string; + +export default function test(obj: {a: string}): void { + return; +} +export function add(a: number, b: number): number { + return a + b; +} \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts.snap index ea35339a16f3..b00fc3569c32 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/useExplicitType/valid.d.ts.snap @@ -1,6 +1,7 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs expression: valid.d.ts +snapshot_kind: text --- # Input ```ts @@ -53,4 +54,10 @@ declare var fn: () => number; declare var arrowFn: () => string; +export default function test(obj: {a: string}): void { + return; +} +export function add(a: number, b: number): number { + return a + b; +} ``` diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index 819e60042f2b..cfbfac52caa0 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -1391,7 +1391,7 @@ export interface Nursery { */ useDeprecatedReason?: RuleConfiguration_for_Null; /** - * Require explicit return types on functions and class methods. + * Require explicit argument and return types on functions and class methods. */ useExplicitType?: RuleConfiguration_for_Null; /** diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index eee69adde827..eedf7d9872b4 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -2430,7 +2430,7 @@ ] }, "useExplicitType": { - "description": "Require explicit return types on functions and class methods.", + "description": "Require explicit argument and return types on functions and class methods.", "anyOf": [ { "$ref": "#/definitions/RuleConfiguration" }, { "type": "null" }