refactor(noUnusedVariables): don't report unused function params

biomejs · Aug 6, 2024 · 70bdcd2 · 70bdcd2
1 parent f90e50c
commit 70bdcd2
Show file tree

Hide file tree

Showing 13 changed files with 89 additions and 387 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -220,6 +220,12 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
   Contributed by @Conaclos
 
+- [noUnusedVariables](https://biomejs.dev/linter/rules/no-unused-variables/) no longer reports unused function parameters.
+
+  To report unused function parameter, use [noUnusedFunctionParameters](https://biomejs.dev/linter/rules/no-unused-function-parameters/) instead.
+
+  Contributed by @Conaclos
+
 #### Bug fixes
 
 - Don't request alt text for elements hidden from assistive technologies ([#3316](https://github.com/biomejs/biome/issues/3316)). Contributed by @robintown

diff --git a/crates/biome_js_analyze/src/lint/correctness/no_unused_variables.rs b/crates/biome_js_analyze/src/lint/correctness/no_unused_variables.rs
@@ -6,9 +6,7 @@ use biome_analyze::{
 };
 use biome_console::markup;
 use biome_js_semantic::ReferencesExtensions;
-use biome_js_syntax::binding_ext::{
-    AnyJsBindingDeclaration, AnyJsIdentifierBinding, AnyJsParameterParentFunction,
-};
+use biome_js_syntax::binding_ext::{AnyJsBindingDeclaration, AnyJsIdentifierBinding};
 use biome_js_syntax::declaration_ext::is_in_ambient_context;
 use biome_js_syntax::{
     AnyJsExpression, JsClassExpression, JsFileSource, JsForStatement, JsFunctionExpression,
@@ -113,31 +111,6 @@ pub enum SuggestedFix {
     PrefixUnderscore,
 }
 
-fn is_function_that_is_ok_parameter_not_be_used(
-    parent_function: &Option<AnyJsParameterParentFunction>,
-) -> bool {
-    matches!(
-        parent_function,
-        Some(
-            // bindings in signatures are ok to not be used
-            AnyJsParameterParentFunction::TsMethodSignatureClassMember(_)
-            | AnyJsParameterParentFunction::TsCallSignatureTypeMember(_)
-            | AnyJsParameterParentFunction::TsConstructSignatureTypeMember(_)
-            | AnyJsParameterParentFunction::TsConstructorSignatureClassMember(_)
-            | AnyJsParameterParentFunction::TsMethodSignatureTypeMember(_)
-            | AnyJsParameterParentFunction::TsSetterSignatureClassMember(_)
-            | AnyJsParameterParentFunction::TsSetterSignatureTypeMember(_)
-            | AnyJsParameterParentFunction::TsIndexSignatureClassMember(_)
-            // bindings in function types are ok to not be used
-            | AnyJsParameterParentFunction::TsFunctionType(_)
-            | AnyJsParameterParentFunction::TsConstructorType(_)
-            // binding in declare are ok to not be used
-            | AnyJsParameterParentFunction::TsDeclareFunctionDeclaration(_)
-            | AnyJsParameterParentFunction::TsDeclareFunctionExportDefaultDeclaration(_)
-        )
-    )
-}
-
 fn suggestion_for_binding(binding: &AnyJsIdentifierBinding) -> Option<SuggestedFix> {
     if binding.is_under_object_pattern_binding()? {
         Some(SuggestedFix::NoSuggestion)
@@ -176,27 +149,12 @@ fn suggested_fix_if_unused(binding: &AnyJsIdentifierBinding) -> Option<Suggested
         | AnyJsBindingDeclaration::JsFunctionExpression(_)
         | AnyJsBindingDeclaration::TsIndexSignatureParameter(_)
         | AnyJsBindingDeclaration::TsMappedType(_)
-        | AnyJsBindingDeclaration::TsEnumMember(_) => None,
-
-        // Some parameters are ok to not be used
-        AnyJsBindingDeclaration::JsArrowFunctionExpression(_) => {
-            suggestion_for_binding(binding)
-        }
-        AnyJsBindingDeclaration::TsPropertyParameter(_) => None,
-        AnyJsBindingDeclaration::JsFormalParameter(parameter) => {
-            if is_function_that_is_ok_parameter_not_be_used(&parameter.parent_function()) {
-                None
-            } else {
-                suggestion_for_binding(binding)
-            }
-        }
-        AnyJsBindingDeclaration::JsRestParameter(parameter) => {
-            if is_function_that_is_ok_parameter_not_be_used(&parameter.parent_function()) {
-                None
-            } else {
-                suggestion_for_binding(binding)
-            }
-        }
+        | AnyJsBindingDeclaration::TsEnumMember(_)
+        // Unused parameters are reported by `noUnusedFunctionParameters`
+        | AnyJsBindingDeclaration::JsArrowFunctionExpression(_)
+        | AnyJsBindingDeclaration::TsPropertyParameter(_)
+        | AnyJsBindingDeclaration::JsFormalParameter(_)
+        | AnyJsBindingDeclaration::JsRestParameter(_)  => None,
         // declarations need to be check if they are under `declare`
         AnyJsBindingDeclaration::JsArrayBindingPatternElement(_)
         | AnyJsBindingDeclaration::JsArrayBindingPatternRestElement(_)
@@ -305,6 +263,7 @@ impl Rule for NoUnusedVariables {
         }
 
         // Ignore expressions
+        // e.e. `f` is "unused" in `export const g = function f(){}`.
         if binding.parent::<JsFunctionExpression>().is_some()
             || binding.parent::<JsClassExpression>().is_some()
         {

diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidClass.ts b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidClass.ts
@@ -1,5 +1,5 @@
 class D {
-	f(a: D): D | undefined { return; }
+	f(a: D): D | undefined { return a; }
 }
 
 export {}
diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidClass.ts.snap b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidClass.ts.snap
@@ -5,7 +5,7 @@ expression: invalidClass.ts
 # Input
 ```ts
 class D {
-	f(a: D): D | undefined { return; }
+	f(a: D): D | undefined { return a; }
 }
 
 export {}
@@ -19,36 +19,10 @@ invalidClass.ts:1:7 lint/correctness/noUnusedVariables ━━━━━━━━
   
   > 1 │ class D {
       │       ^
-    2 │ 	f(a: D): D | undefined { return; }
+    2 │ 	f(a: D): D | undefined { return a; }
     3 │ }
   
   i Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
   
 
 ```
-
-```
-invalidClass.ts:2:4 lint/correctness/noUnusedVariables  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
-
-  ! This parameter is unused.
-  
-    1 │ class D {
-  > 2 │ 	f(a: D): D | undefined { return; }
-      │ 	  ^
-    3 │ }
-    4 │ 
-  
-  i Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
-  
-  i Unsafe fix: If this is intentional, prepend a with an underscore.
-  
-    1 1 │   class D {
-    2   │ - → f(a:·D):·D·|·undefined·{·return;·}
-      2 │ + → f(_a:·D):·D·|·undefined·{·return;·}
-    3 3 │   }
-    4 4 │   
-  
-
-```
-
-
diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidFunction.ts b/crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidFunction.ts
@@ -13,19 +13,6 @@ function f3() {
     g();
 }
 
-// parameter a is not used
-{(function (a) { })}
-{(function ({a}) { })}
-{(function ([a]) { })}
-(function (a, b) {
-    console.log(b);
-})
-
-// parameter b is not used
-(function (a, b) {
-    console.log(a);
-})
-
 // f5 is not used
 const f5 = () => { };