From a6b24eec1d007e421109288dd44e2b06b2c0f98b Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Mon, 29 Aug 2022 12:09:44 +0100 Subject: [PATCH] feat(rome_js_analyze): no unused starting with underscore; small hack for React and removing suggested fix (#3124) * no unused starting with an underscore, react and no fix --- .../js/no_unused_variables.rs | 84 ++++---- .../js/noUnusedVariables/noUnusedVariables.js | 8 +- .../noUnusedVariables.js.snap | 180 +++--------------- .../noUnusedVariables.ts.snap | 22 --- .../src/docs/lint/rules/noUnusedVariables.md | 59 ++---- 5 files changed, 87 insertions(+), 266 deletions(-) diff --git a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs index cc1a74b1eba..4dc02a0957d 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs @@ -1,16 +1,13 @@ -use crate::{semantic_services::Semantic, utils::batch::JsBatchMutation, JsRuleAction}; -use rome_analyze::{ - context::RuleContext, declare_rule, ActionCategory, Rule, RuleCategory, RuleDiagnostic, -}; +use crate::semantic_services::Semantic; +use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleCategory, RuleDiagnostic}; use rome_console::markup; -use rome_diagnostics::Applicability; use rome_js_semantic::{AllReferencesExtensions, SemanticScopeExtensions}; use rome_js_syntax::{ - JsCatchDeclaration, JsConstructorParameterList, JsConstructorParameters, JsFormalParameter, - JsFunctionDeclaration, JsIdentifierBinding, JsParameterList, JsParameters, JsSyntaxKind, + JsClassExpression, JsConstructorParameterList, JsConstructorParameters, JsFunctionDeclaration, + JsFunctionExpression, JsIdentifierBinding, JsParameterList, JsParameters, JsSyntaxKind, JsVariableDeclarator, }; -use rome_rowan::{AstNode, BatchMutationExt}; +use rome_rowan::AstNode; declare_rule! { /// Disallow unused variables. @@ -65,6 +62,20 @@ declare_rule! { /// }; /// foo(); /// ``` + /// + /// ```js + /// function foo(_unused) { + /// }; + /// foo(); + /// ``` + /// + /// ```jsx + /// import React from 'react'; + /// function foo() { + /// return
; + /// }; + /// foo(); + /// ``` pub(crate) NoUnusedVariables { version: "0.9.0", name: "noUnusedVariables", @@ -117,12 +128,28 @@ impl Rule for NoUnusedVariables { fn run(ctx: &RuleContext) -> Option { let binding = ctx.query(); - let model = ctx.model(); + let name = binding.name_token().ok()?; + let name = name.token_text_trimmed(); + let name = name.text(); + + // Old code import React but do not used directly + // only indirectly after transpiling JSX. + if name.starts_with('_') || name == "React" { + return None; + } + + // Ignore expressions + if binding.parent::().is_some() + || binding.parent::().is_some() + { + return None; + } if is_typescript_unused_ok(binding).is_some() { return None; } + let model = ctx.model(); if model.is_exported(binding) { return None; } @@ -198,43 +225,4 @@ impl Rule for NoUnusedVariables { Some(diag) } - - fn action(ctx: &RuleContext, _: &Self::State) -> Option { - let root = ctx.root(); - let binding = ctx.query(); - - let mut batch = root.begin(); - - // Try to remove node - let removed = if let Some(declaration) = binding.parent::() { - batch.remove_node(declaration); - true - } else if let Some(variable_declarator) = binding.parent::() { - batch.remove_js_variable_declarator(&variable_declarator) - } else if let Some(formal_parameter) = binding.parent::() { - batch.remove_js_formal_parameter(&formal_parameter) - } else if let Some(catch_declaration) = binding.parent::() { - batch.remove_node(catch_declaration); - true - } else { - false - }; - - if removed { - let symbol_type = match binding.syntax().parent().unwrap().kind() { - JsSyntaxKind::JS_FORMAL_PARAMETER => "parameter", - JsSyntaxKind::JS_FUNCTION_DECLARATION => "function", - _ => "variable", - }; - - Some(JsRuleAction { - category: ActionCategory::QuickFix, - applicability: Applicability::MaybeIncorrect, - message: markup! { "Remove this " {symbol_type} "." }.to_owned(), - mutation: batch, - }) - } else { - None - } - } } diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js index 39a0d7ca46e..977617aa07a 100644 --- a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js +++ b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js @@ -1,3 +1,5 @@ +import React from 'react'; + const a = 1; const b = 2, c = 3; @@ -47,4 +49,8 @@ let value; function Button() {} console.log(