Skip to content

Commit

Permalink
Implement stable hook configuration (#1979)
Browse files Browse the repository at this point in the history
  • Loading branch information
arendjr committed Mar 7, 2024
1 parent 465df11 commit f5dd086
Show file tree
Hide file tree
Showing 23 changed files with 647 additions and 129 deletions.
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,34 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

Contributed by @arendjr

- Implemented [#1128](https://github.com/biomejs/biome/issues/1128). User-provided React hooks can
now be configured to track stable results. For example:

```json
"useExhaustiveDependencies": {
"level": "error",
"options": {
"hooks": [{
"name": "useMyState",
"stableResult": [
1
]
}]
}
}
```

This will allow the following to be validated:

```js
const [myState, setMyState] = useMyState();
const toggleMyState = useCallback(() => {
setMyState(!myState);
}, [myState]); // Only `myState` needs to be specified here.
```

Contributed by @arendjr

#### Bug fixes

- Fix [#1748](https://github.com/biomejs/biome/issues/1748). Now for the following case we won't provide an unsafe fix
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_deserialize/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub trait DeserializableValidator {
/// Returns `true` if the instance passes validation and `false` when it
/// should be rejected.
fn validate(
&self,
&mut self,
name: &str,
range: TextRange,
diagnostics: &mut Vec<DeserializationDiagnostic>,
Expand Down
6 changes: 3 additions & 3 deletions crates/biome_deserialize_macros/src/deserializable_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ fn generate_deserializable_enum(

let validator = if data.with_validator {
quote! {
if !biome_deserialize::DeserializableValidator::validate(&result, name, range, diagnostics) {
if !biome_deserialize::DeserializableValidator::validate(&mut result, name, range, diagnostics) {
return None;
}
}
Expand Down Expand Up @@ -274,7 +274,7 @@ fn generate_deserializable_newtype(
) -> TokenStream {
let validator = if data.with_validator {
quote! {
if !biome_deserialize::DeserializableValidator::validate(&result, name, value.range(), diagnostics) {
if !biome_deserialize::DeserializableValidator::validate(&mut result, name, value.range(), diagnostics) {
return None;
}
}
Expand Down Expand Up @@ -436,7 +436,7 @@ fn generate_deserializable_struct(
let validator = if data.with_validator {
quote! {
#validator
if !biome_deserialize::DeserializableValidator::validate(&result, name, range, diagnostics) {
if !biome_deserialize::DeserializableValidator::validate(&mut result, name, range, diagnostics) {
return None;
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_deserialize_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ use syn::{parse_macro_input, DeriveInput};
/// }
///
/// impl DeserializableValidator for ValidatedStruct {
/// fn fn validate(
/// fn validate(
/// &self,
/// name: &str,
/// range: biome_rowan::TextRange,
Expand Down
11 changes: 4 additions & 7 deletions crates/biome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,8 @@ mod tests {
use biome_js_syntax::{JsFileSource, TextRange, TextSize};
use std::slice;

use crate::semantic_analyzers::correctness::use_exhaustive_dependencies::{
Hooks, HooksOptions,
};
use crate::react::hooks::StableHookResult;
use crate::semantic_analyzers::correctness::use_exhaustive_dependencies::{Hook, HooksOptions};
use crate::{analyze, AnalysisFilter, ControlFlow};

// #[ignore]
Expand All @@ -260,18 +259,16 @@ mod tests {

const SOURCE: &str = r#"<a href="class/html-css1/navigation/links#" onclick="window.location.href=index.html"> Home </a>
"#;
// const SOURCE: &str = r#"document.querySelector("foo").value = document.querySelector("foo").value
//
// "#;

let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default());

let mut error_ranges: Vec<TextRange> = Vec::new();
let mut options = AnalyzerOptions::default();
let hook = Hooks {
let hook = Hook {
name: "myEffect".to_string(),
closure_index: Some(0),
dependencies_index: Some(1),
stable_result: StableHookResult::None,
};
let rule_filter = RuleFilter::Rule("a11y", "useValidAnchor");

Expand Down
191 changes: 170 additions & 21 deletions crates/biome_js_analyze/src/react/hooks.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use crate::react::{is_react_call_api, ReactLibrary};

use biome_console::markup;
use biome_deserialize::{
DeserializableValue, DeserializationDiagnostic, DeserializationVisitor, VisitableType,
};
use biome_diagnostics::Severity;
use biome_js_semantic::{Capture, Closure, ClosureExtensions, SemanticModel};
use biome_js_syntax::binding_ext::AnyJsBindingDeclaration;
use biome_js_syntax::{
Expand All @@ -12,6 +17,9 @@ use biome_rowan::{AstNode, SyntaxToken};
use rustc_hash::{FxHashMap, FxHashSet};
use serde::{Deserialize, Serialize};

#[cfg(feature = "schemars")]
use schemars::JsonSchema;

/// Return result of [react_hook_with_dependency].
#[derive(Debug)]
pub struct ReactCallWithDependencyResult {
Expand Down Expand Up @@ -83,20 +91,20 @@ impl ReactCallWithDependencyResult {

#[derive(Default, Debug, Copy, Clone, Serialize, Deserialize)]
pub struct ReactHookConfiguration {
pub closure_index: Option<usize>,
pub dependencies_index: Option<usize>,
pub closure_index: u8,
pub dependencies_index: u8,

/// `true` if it's a built-in React hook. For built-in hooks, we verify
/// whether they are imported from the React library. For user-defined
/// hooks, we don't.
pub builtin: bool,
}

impl From<(usize, usize, bool)> for ReactHookConfiguration {
fn from((closure, dependencies, builtin): (usize, usize, bool)) -> Self {
impl From<(u8, u8, bool)> for ReactHookConfiguration {
fn from((closure_index, dependencies_index, builtin): (u8, u8, bool)) -> Self {
Self {
closure_index: Some(closure),
dependencies_index: Some(dependencies),
closure_index,
dependencies_index,
builtin,
}
}
Expand Down Expand Up @@ -182,8 +190,8 @@ pub(crate) fn react_hook_with_dependency(
return None;
}

let closure_index = hook.closure_index?;
let dependencies_index = hook.dependencies_index?;
let closure_index = hook.closure_index as usize;
let dependencies_index = hook.dependencies_index as usize;

let mut indices = [closure_index, dependencies_index];
indices.sort_unstable();
Expand All @@ -201,17 +209,148 @@ pub(crate) fn react_hook_with_dependency(
#[derive(Debug, 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
index: Option<usize>,
pub(crate) hook_name: String,

/// The kind of (stable) result returned by the hook.
pub(crate) result: StableHookResult,

/// `true` if it's a built-in React hook. For built-in hooks, we verify
/// whether they are imported from the React library. For user-defined
/// hooks, we don't.
pub(crate) builtin: bool,
}

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

#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, PartialEq, Serialize)]
#[cfg_attr(feature = "schemars", derive(JsonSchema))]
pub enum StableHookResult {
/// Used to indicate the hook does not have a stable result.
#[default]
None,

/// Used to indicate the identity of the result value is stable.
///
/// Note this does not imply internal stability. For instance, the ref
/// objects returned by React's `useRef()` always have a stable identity,
/// but their internal value may be mutable.
Identity,

/// Used to indicate the hook returns an array and some of its indices have
/// stable identities.
///
/// For example, React's `useState()` hook returns a stable function at
/// index 1.
Indices(Vec<u8>),
}

impl biome_deserialize::Deserializable for StableHookResult {
fn deserialize(
value: &impl DeserializableValue,
name: &str,
diagnostics: &mut Vec<biome_deserialize::DeserializationDiagnostic>,
) -> Option<Self> {
value.deserialize(StableResultVisitor, name, diagnostics)
}
}

struct StableResultVisitor;
impl DeserializationVisitor for StableResultVisitor {
type Output = StableHookResult;

const EXPECTED_TYPE: VisitableType = VisitableType::ARRAY
.union(VisitableType::BOOL)
.union(VisitableType::NUMBER);

fn visit_array(
self,
items: impl Iterator<Item = Option<impl DeserializableValue>>,
_range: TextRange,
_name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
let indices: Vec<u8> = items
.filter_map(|value| {
DeserializableValue::deserialize(&value?, StableResultIndexVisitor, "", diagnostics)
})
.collect();

Some(if indices.is_empty() {
StableHookResult::None
} else {
StableHookResult::Indices(indices)
})
}

fn visit_bool(
self,
value: bool,
range: TextRange,
_name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
match value {
true => Some(StableHookResult::Identity),
false => {
diagnostics.push(
DeserializationDiagnostic::new(
markup! { "This hook is configured to not have a stable result" },
)
.with_custom_severity(Severity::Warning)
.with_range(range),
);
Some(StableHookResult::None)
}
}
}

fn visit_number(
self,
value: biome_deserialize::TextNumber,
range: TextRange,
name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
StableResultIndexVisitor::visit_number(
StableResultIndexVisitor,
value,
range,
name,
diagnostics,
)
.map(|index| StableHookResult::Indices(vec![index]))
}
}

struct StableResultIndexVisitor;
impl DeserializationVisitor for StableResultIndexVisitor {
type Output = u8;

const EXPECTED_TYPE: VisitableType = VisitableType::NUMBER;

fn visit_number(
self,
value: biome_deserialize::TextNumber,
range: TextRange,
_name: &str,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<Self::Output> {
match value.parse::<u8>() {
Ok(index) => Some(index),
Err(_) => {
diagnostics.push(DeserializationDiagnostic::new_out_of_bound_integer(
0, 255, range,
));
None
}
}
}
}
Expand Down Expand Up @@ -243,7 +382,8 @@ pub fn is_binding_react_stable(
};
let index = binding
.parent::<JsArrayBindingPatternElement>()
.map(|parent| parent.syntax().index() / 2);
.map(|parent| parent.syntax().index() / 2)
.and_then(|index| index.try_into().ok());
let Some(callee) = declarator
.initializer()
.and_then(|initializer| initializer.expression().ok())
Expand All @@ -252,8 +392,17 @@ pub fn is_binding_react_stable(
return false;
};
stable_config.iter().any(|config| {
is_react_call_api(&callee, model, ReactLibrary::React, &config.hook_name)
&& index == config.index
if config.builtin
&& !is_react_call_api(&callee, model, ReactLibrary::React, &config.hook_name)
{
return false;
}

match (&config.result, index) {
(StableHookResult::Identity, index) => index.is_none(),
(StableHookResult::Indices(indices), Some(index)) => indices.contains(&index),
(_, _) => false,
}
})
}

Expand Down Expand Up @@ -319,8 +468,8 @@ mod test {
let set_name = AnyJsIdentifierBinding::cast(node).unwrap();

let config = FxHashSet::from_iter([
StableReactHookConfiguration::new("useRef", None),
StableReactHookConfiguration::new("useState", Some(1)),
StableReactHookConfiguration::new("useRef", StableHookResult::Identity, true),
StableReactHookConfiguration::new("useState", StableHookResult::Indices(vec![1]), true),
]);

assert!(is_binding_react_stable(
Expand Down Expand Up @@ -349,8 +498,8 @@ mod test {
let set_name = AnyJsIdentifierBinding::cast(node).unwrap();

let config = FxHashSet::from_iter([
StableReactHookConfiguration::new("useRef", None),
StableReactHookConfiguration::new("useState", Some(1)),
StableReactHookConfiguration::new("useRef", StableHookResult::Identity, true),
StableReactHookConfiguration::new("useState", StableHookResult::Indices(vec![1]), true),
]);

assert!(is_binding_react_stable(
Expand Down

0 comments on commit f5dd086

Please sign in to comment.