Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): exhaustive deps for React hooks #3355

Merged
merged 36 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
7e62df2
react exhaustive deps lint rule
xunilrj Oct 5, 2022
ac857fc
fmt and clippy issues
xunilrj Oct 5, 2022
44b1451
support for stable react dependencies
xunilrj Oct 10, 2022
3bba4d1
filtering out declaration captures
xunilrj Oct 10, 2022
b9a83b0
support for stable captures
xunilrj Oct 10, 2022
0038818
fmt and clippy issues
xunilrj Oct 10, 2022
0c4c8f9
support for dependencies not captured
xunilrj Oct 11, 2022
db4d4ea
support for any react hook
xunilrj Oct 12, 2022
6e0d319
fmt and clippy issues
xunilrj Oct 12, 2022
898e530
version and options
xunilrj Oct 12, 2022
00496cd
rebase issues
xunilrj Oct 12, 2022
73d0f97
fmt and clippy issues
xunilrj Oct 12, 2022
9aee51d
update codegen
xunilrj Oct 12, 2022
e32a85b
update codegen
xunilrj Oct 12, 2022
b05dc45
support for inner closures
xunilrj Oct 12, 2022
bb4dc3c
support for imported dependencies
xunilrj Oct 12, 2022
3ba5731
support for imported dependencies
xunilrj Oct 12, 2022
f63df02
renaming the rule
xunilrj Oct 18, 2022
c257806
pr adjustments
xunilrj Oct 19, 2022
be1a789
fmt and clippy issues
xunilrj Oct 19, 2022
fe7638a
rule documentation
xunilrj Oct 19, 2022
a0a78df
codegen
xunilrj Oct 19, 2022
eeb0ac4
codegen
xunilrj Oct 19, 2022
e6ed92a
renaming trait CanBeImportedExported
xunilrj Oct 19, 2022
56ca7c3
better example on stable returns
xunilrj Oct 19, 2022
d0bbcbd
lint rules examples
xunilrj Oct 20, 2022
628d932
improve doc
xunilrj Oct 20, 2022
06e0182
removing with_capacity
xunilrj Oct 20, 2022
1bcf1e2
remove dependency fix being aggregated
xunilrj Oct 20, 2022
92fd6c7
all_dependencies returning lazy iterator
xunilrj Oct 20, 2022
fe33ac7
lazy all_captures
xunilrj Oct 20, 2022
6364daa
optimized argument retrieve using get_arguments_by_index
xunilrj Oct 21, 2022
1a4cc15
merging react modules
xunilrj Oct 21, 2022
03c5ee4
fmt and clippy issues
xunilrj Oct 21, 2022
2e3ebfc
codegen
xunilrj Oct 21, 2022
3b345a2
codegen
xunilrj Oct 25, 2022
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
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ define_dategories! {
"lint/nursery/noConstAssign": "https://rome.tools/docs/lint/rules/noConstAssign",
"lint/nursery/noExplicitAny": "https://rome.tools/docs/lint/rules/noExplicitAny",
"lint/nursery/useValidForDirection": "https://rome.tools/docs/lint/rules/useValidForDirection",
"lint/nursery/useExhaustiveDependencies": "https://rome.tools/docs/lint/rules/useExhaustiveDependencies",
"lint/style/noNegationElse": "https://rome.tools/docs/lint/rules/noNegationElse",
"lint/style/noShoutyConstants": "https://rome.tools/docs/lint/rules/noShoutyConstants",
"lint/style/useSelfClosingElements": "https://rome.tools/docs/lint/rules/useSelfClosingElements",
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/src/react.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! A series of AST utilities to work with the React library

pub mod hooks;

use rome_js_semantic::SemanticModel;
use rome_js_syntax::{
JsAnyCallArgument, JsAnyExpression, JsCallExpression, JsIdentifierBinding, JsImport,
Expand Down
218 changes: 218 additions & 0 deletions crates/rome_js_analyze/src/react/hooks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
use std::collections::{HashMap, HashSet};

use rome_js_semantic::{Capture, ClosureExtensions, SemanticModel};
use rome_js_syntax::{
JsAnyExpression, JsArrayBindingPattern, JsArrayBindingPatternElementList, JsCallExpression,
JsIdentifierBinding, JsVariableDeclarator, TextRange,
};
use rome_rowan::AstNode;
use serde::{Deserialize, Serialize};

/// Return result of [react_hook_with_dependency].
pub(crate) struct ReactCallWithDependencyResult {
pub(crate) function_name_range: TextRange,
pub(crate) closure_node: Option<JsAnyExpression>,
pub(crate) dependencies_node: Option<JsAnyExpression>,
}

impl ReactCallWithDependencyResult {
/// Returns all [Reference] captured by the closure argument of the React hook.
/// See [react_hook_with_dependency].
pub fn all_captures(&self, model: &SemanticModel) -> impl Iterator<Item = Capture> {
self.closure_node
.as_ref()
.and_then(|node| node.as_js_arrow_function_expression())
.map(|arrow_function| {
let closure = arrow_function.closure(model);
let range = *closure.closure_range();
closure
.descendents()
.flat_map(|closure| closure.all_captures())
.filter(move |capture| capture.declaration_range().intersect(range).is_none())
})
.into_iter()
.flatten()
}

/// Returns all dependencies of a React hook.
/// See [react_hook_with_dependency]
pub fn all_dependencies(&self) -> impl Iterator<Item = JsAnyExpression> {
self.dependencies_node
.as_ref()
.and_then(|x| Some(x.as_js_array_expression()?.elements().into_iter()))
.into_iter()
.flatten()
.filter_map(|x| x.ok()?.as_js_any_expression().cloned())
}
}

#[derive(Copy, Clone, Serialize, Deserialize)]
pub struct ReactHookConfiguration {
closure_index: usize,
dependencies_index: usize,
}

impl From<(usize, usize)> for ReactHookConfiguration {
fn from((closure, dependencies): (usize, usize)) -> Self {
Self {
closure_index: closure,
dependencies_index: dependencies,
}
}
}

/// Returns the [TextRange] of the hook name; the node of the
/// expression of the argument that correspond to the closure of
/// the hook; and the node of the dependency list of the hook.
///
/// Example:
/// ```js
/// useEffect(() => {}, []);
/// ^^ <- dependencies_node
/// ^^^^^^^^ <- closure_node
/// ^^^^^^^^^ <- function_name_range
/// ```
///
/// This function will use the parameter "hooks" with the configuration
/// of all function that are considered hooks. See [ReactHookConfiguration].
pub(crate) fn react_hook_with_dependency(
call: &JsCallExpression,
hooks: &HashMap<String, ReactHookConfiguration>,
) -> Option<ReactCallWithDependencyResult> {
let name = call
.callee()
.ok()?
.as_js_identifier_expression()?
.name()
.ok()?
.value_token()
.ok()?;
let function_name_range = name.text_trimmed_range();
let name = name.text_trimmed();

let hook = hooks.get(name)?;

let mut indices = [hook.closure_index, hook.dependencies_index];
indices.sort();
let [closure_node, dependencies_node] = call.get_arguments_by_index(indices);

Some(ReactCallWithDependencyResult {
function_name_range,
closure_node: closure_node.and_then(|x| x.as_js_any_expression().cloned()),
dependencies_node: dependencies_node.and_then(|x| x.as_js_any_expression().cloned()),
})
}

/// Specifies which, if any, of the returns of a React hook are stable.
/// See [is_binding_react_stable].
#[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct StableReactHookConfiguration {
/// Name of the React hook
hook_name: String,
/// Index of the position of the stable return, [None] if
/// none returns are stable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc issue: [None] if the entire return value is stable.

index: Option<usize>,
}

impl StableReactHookConfiguration {
pub fn new(hook_name: &str, index: Option<usize>) -> Self {
Self {
hook_name: hook_name.into(),
index,
}
}
}

/// Checks if the binding is bound to a stable React hook
/// return value. Stable returns do not need to be specified
/// as dependencies.
///
/// Example:
/// ```js
/// let [name, setName] = useState("");
/// useEffect(() => {
/// // name is NOT stable, so it must be specified as dependency
/// console.log(name);
/// // setName IS stable, so it must not be specified as dependency
/// console.log(setName("a"));
/// }, [name]);
/// ```
pub fn is_binding_react_stable(
binding: &JsIdentifierBinding,
stable_config: &HashSet<StableReactHookConfiguration>,
) -> bool {
fn array_binding_declarator_index(
binding: &JsIdentifierBinding,
) -> Option<(JsVariableDeclarator, Option<usize>)> {
let index = binding.syntax().index() / 2;
let declarator = binding
.parent::<JsArrayBindingPatternElementList>()?
.parent::<JsArrayBindingPattern>()?
.parent::<JsVariableDeclarator>()?;
Some((declarator, Some(index)))
}

fn assignment_declarator(
binding: &JsIdentifierBinding,
) -> Option<(JsVariableDeclarator, Option<usize>)> {
let declarator = binding.parent::<JsVariableDeclarator>()?;
Some((declarator, None))
}

array_binding_declarator_index(binding)
.or_else(|| assignment_declarator(binding))
.and_then(|(declarator, index)| {
let hook_name = declarator
.initializer()?
.expression()
.ok()?
.as_js_call_expression()?
.callee()
.ok()?
.as_js_identifier_expression()?
.name()
.ok()?
.value_token()
.ok()?
.token_text();

let stable = StableReactHookConfiguration {
hook_name: hook_name.to_string(),
index,
};

Some(stable_config.contains(&stable))
})
.unwrap_or(false)
}

#[cfg(test)]
mod test {
use super::*;
use rome_js_parser::FileId;
use rome_js_syntax::SourceType;
use rome_rowan::SyntaxNodeCast;

#[test]
pub fn ok_react_stable_captures() {
let r = rome_js_parser::parse(
"const ref = useRef();",
FileId::zero(),
SourceType::js_module(),
);
let node = r
.syntax()
.descendants()
.filter(|x| x.text_trimmed() == "ref")
.last()
.unwrap();
let set_name = node.cast::<JsIdentifierBinding>().unwrap();

let config = HashSet::from_iter([
StableReactHookConfiguration::new("useRef", None),
StableReactHookConfiguration::new("useState", Some(1)),
]);

assert!(is_binding_react_stable(&set_name, &config));
}
}
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/semantic_analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_semantic::{AllReferencesExtensions, IsExportedCanBeQueried, SemanticModel};
use rome_js_semantic::{AllReferencesExtensions, CanBeImportedExported, SemanticModel};
use rome_js_syntax::{
JsFormalParameter, JsFunctionDeclaration, JsFunctionExportDefaultDeclaration,
JsGetterClassMember, JsIdentifierBinding, JsLiteralMemberName, JsMethodClassMember,
Expand Down
Loading