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

Commit

Permalink
fmt and clippy issues
Browse files Browse the repository at this point in the history
  • Loading branch information
xunilrj committed Oct 12, 2022
1 parent b6c4212 commit 1fe5806
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@ use crate::semantic_services::Semantic;
use crate::utils::react::*;
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_semantic::{Capture, SemanticModel};
use rome_js_syntax::{
JsAnyCallArgument, JsAnyExpression, JsArrayExpression, JsArrowFunctionExpression,
JsCallExpression, JsIdentifierBinding, JsSyntaxKind, TextRange,
};
use rome_js_semantic::Capture;
use rome_js_syntax::{JsCallExpression, JsIdentifierBinding, JsSyntaxKind, TextRange};
use rome_rowan::{AstNode, SyntaxNodeCast};
use serde::{Serialize, Deserialize};
use std::{collections::{HashMap, HashSet, BTreeMap}, borrow::Cow};
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, HashMap, HashSet};

declare_rule! {
/// Enforce all dependencies are correctly specified.
Expand All @@ -25,15 +22,14 @@ lazy_static::lazy_static! {
static ref OPTIONS: ReactExtensiveDependenciesOptions = ReactExtensiveDependenciesOptions::new();
}


#[derive(Clone, Serialize, Deserialize)]
pub struct ReactExtensiveDependenciesOptions {
hooks: HashMap<String, ReactHookClosureDependenciesPosition>,
stables: HashSet<ReactHookStable>,
}

impl ReactExtensiveDependenciesOptions {
pub fn new() -> Self {
pub fn new() -> Self {
let hooks = HashMap::from_iter([
("useEffect".to_string(), (0, 1).into()),
("useLayoutEffect".to_string(), (0, 1).into()),
Expand All @@ -58,8 +54,8 @@ impl ReactExtensiveDependenciesOptions {
}

pub enum Problem {
MissingDependency (TextRange, Vec<Capture>),
ExtraDependency(TextRange, TextRange)
MissingDependency(TextRange, Vec<Capture>),
ExtraDependency(TextRange, TextRange),
}

impl Rule for ReactExtensiveDependencies {
Expand All @@ -75,7 +71,8 @@ impl Rule for ReactExtensiveDependencies {
if let Some(result) = react_hook_with_dependency(node, &OPTIONS.hooks) {
let model = ctx.model();

let captures: Vec<_> = result.all_captures(model)
let captures: Vec<_> = result
.all_captures(model)
.into_iter()
.filter_map(|capture| {
capture.declaration().and_then(|declaration| {
Expand All @@ -99,12 +96,15 @@ impl Rule for ReactExtensiveDependencies {
.map(|x| (x.node().text_trimmed().to_string(), x))
.collect();

let deps: Vec<(String, TextRange)> = result.all_dependencies()
.into_iter()
.map(|dep| (
dep.syntax().text_trimmed().to_string(),
dep.syntax().text_trimmed_range(),
))
let deps: Vec<(String, TextRange)> = result
.all_dependencies()
.into_iter()
.map(|dep| {
(
dep.syntax().text_trimmed().to_string(),
dep.syntax().text_trimmed_range(),
)
})
.collect();

let mut add_deps: BTreeMap<String, Vec<Capture>> = BTreeMap::new();
Expand All @@ -127,11 +127,17 @@ impl Rule for ReactExtensiveDependencies {

// Generate signals
for (_, captures) in add_deps {
signals.push(Problem::MissingDependency(result.function_name_range, captures));
signals.push(Problem::MissingDependency(
result.function_name_range,
captures,
));
}

for dep_range in remove_deps {
signals.push(Problem::ExtraDependency(result.function_name_range, dep_range));
signals.push(Problem::ExtraDependency(
result.function_name_range,
dep_range,
));
}
}

Expand All @@ -148,31 +154,29 @@ impl Rule for ReactExtensiveDependencies {
"This useEffect has missing dependencies"
},
);

let mut diag = diag;

for capture in captures.iter() {
let node = capture.node();
diag = diag.secondary(
node.text_trimmed_range(),
"This capture is not in the dependency list",
);
}

Some(diag)
},
}
Problem::ExtraDependency(use_effect_range, dep_range) => {
let diag = RuleDiagnostic::new(
rule_category!(),
use_effect_range,
markup! {
"This useEffect has dependencies that were not captured"
},
).secondary(
dep_range,
"This dependecy is not being captured",
);

)
.secondary(dep_range, "This dependecy is not being captured");

Some(diag)
}
}
Expand Down
72 changes: 45 additions & 27 deletions crates/rome_js_analyze/src/utils/react.rs
Original file line number Diff line number Diff line change
@@ -1,65 +1,83 @@
use std::collections::{HashSet, HashMap};
use std::collections::{HashMap, HashSet};

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

pub(crate) struct ReactCallWithDependencyResult {
pub(crate) function_name_range: TextRange,
pub(crate) closure_node: Option<JsAnyExpression>,
pub(crate) dependencies_node: Option<JsAnyExpression>
pub(crate) dependencies_node: Option<JsAnyExpression>,
}

impl ReactCallWithDependencyResult {
pub fn all_captures(&self, model: &SemanticModel) -> Vec<Capture> {
self.closure_node.as_ref()
.and_then(|node|
Some(node.as_js_arrow_function_expression()?
.closure(model)
.all_captures()
.collect::<Vec<_>>()
)).unwrap_or_default()
self.closure_node
.as_ref()
.and_then(|node| {
Some(
node.as_js_arrow_function_expression()?
.closure(model)
.all_captures()
.collect::<Vec<_>>(),
)
})
.unwrap_or_default()
}

pub fn all_dependencies(&self) -> Vec<JsAnyExpression> {
self.dependencies_node.as_ref()
.and_then(|x|
Some(x.as_js_array_expression()?
pub fn all_dependencies(&self) -> Vec<JsAnyExpression> {
self.dependencies_node
.as_ref()
.and_then(|x| {
Some(
x.as_js_array_expression()?
.elements()
.into_iter()
.filter_map(|x| x.ok()?.as_js_any_expression().cloned())
.collect::<Vec<_>>())
).unwrap_or_default()
.collect::<Vec<_>>(),
)
})
.unwrap_or_default()
}
}

fn get_call_expression(call: &JsCallExpression, i: usize) -> Option<JsAnyExpression> {
let args = call.arguments().ok()?;
let args = args.args().into_iter();
args.skip(i).next()?.ok()?.as_js_any_expression().cloned()
let mut args = args.args().into_iter();
args.nth(i)?.ok()?.as_js_any_expression().cloned()
}

#[derive(Copy, Clone, Serialize, Deserialize)]
pub struct ReactHookClosureDependenciesPosition {
closure: usize,
dependencies: usize
dependencies: usize,
}

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

pub(crate) fn react_hook_with_dependency(call: &JsCallExpression, hooks: &HashMap<String, ReactHookClosureDependenciesPosition>) -> Option<ReactCallWithDependencyResult> {
let name = call.callee().ok()?
pub(crate) fn react_hook_with_dependency(
call: &JsCallExpression,
hooks: &HashMap<String, ReactHookClosureDependenciesPosition>,
) -> Option<ReactCallWithDependencyResult> {
let name = call
.callee()
.ok()?
.as_js_identifier_expression()?
.name().ok()?
.value_token().ok()?;
.name()
.ok()?
.value_token()
.ok()?;
let function_name_range = name.text_trimmed_range();
let name = name.text_trimmed();

Expand Down

0 comments on commit 1fe5806

Please sign in to comment.