diff --git a/CHANGELOG.md b/CHANGELOG.md index 7caf7b34..7611692b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/Kampfkarren/selene/compare/0.29.0...HEAD) +- Added new [`roblox_react_exhaustive_deps` lint](https://kampfkarren.github.io/selene/lints/roblox_react_exhaustive_deps.html), which will warn when React hook dependencies are missing or unnecessary. ## [0.29.0](https://github.com/Kampfkarren/selene/releases/tag/0.29.0) - 2025-07-23 - Added `Instance.fromExisting` to the Roblox standard library diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index f70b8b69..1a14c140 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -34,6 +34,7 @@ - [restricted_module_paths](./lints/restricted_module_paths.md) - [roblox_incorrect_color3_new_bounds](./lints/roblox_incorrect_color3_new_bounds.md) - [roblox_incorrect_roact_usage](./lints/roblox_incorrect_roact_usage.md) + - [roblox_react_exhaustive_deps](./lints/roblox_react_exhaustive_deps.md) - [roblox_manual_fromscale_or_fromoffset](./lints/roblox_manual_fromscale_or_fromoffset.md) - [roblox_suspicious_udim2_new](./lints/roblox_suspicious_udim2_new.md) - [shadowing](./lints/shadowing.md) diff --git a/docs/src/lints/roblox_react_exhaustive_deps.md b/docs/src/lints/roblox_react_exhaustive_deps.md new file mode 100644 index 00000000..a14dd6ee --- /dev/null +++ b/docs/src/lints/roblox_react_exhaustive_deps.md @@ -0,0 +1,88 @@ +# roblox_react_exhaustive_deps + +## What it does +Checks that all dependencies used inside React/Roact hooks like `useEffect`, `useCallback`, and `useMemo` are correctly specified in the dependency array. + +## Why this is bad +Missing dependencies can cause your effects or memoized values to use stale data, leading to bugs. Unnecessary dependencies can cause unnecessary re-renders or re-computations, hurting performance. + +When you reference a variable inside a React hook callback, React expects you to declare it as a dependency so it knows when to re-run the effect or recompute the value. Failing to do so can lead to: + +1. **Stale closures**: The callback captures the variable's value from when it was created, not its current value +2. **Missing updates**: Changes to dependencies won't trigger the effect to re-run +3. **Hard-to-debug issues**: The behavior might work initially but break when component re-renders occur in different orders + +## Example + +### Missing dependencies +```lua +local React = require(Packages.React) + +local function Component(props) + -- Bad: props.userId is used but not in dependencies + React.useEffect(function() + fetchUser(props.userId) + end, {}) + + -- Good: all dependencies are listed + React.useEffect(function() + fetchUser(props.userId) + end, { props.userId }) +end +``` + +### Unnecessary dependencies +```lua +-- Bad: count is not used in the effect +React.useEffect(function() + print("Hello") +end, { count }) + +-- Good: empty dependencies since nothing is used +React.useEffect(function() + print("Hello") +end, {}) +``` + +### Property access dependencies +```lua +-- Bad: props.user.name is used but not in dependencies +React.useEffect(function() + setTitle(props.user.name) +end, {}) + +-- Good: property access is correctly tracked +React.useEffect(function() + setTitle(props.user.name) +end, { props.user.name }) +``` + +## Supported hooks +This lint checks the following React/Roact hooks: +- `useEffect` - dependencies are the second parameter +- `useLayoutEffect` - dependencies are the second parameter +- `useCallback` - dependencies are the second parameter +- `useMemo` - dependencies are the second parameter +- `useImperativeHandle` - dependencies are the third parameter + +## Remarks +This lint works with both the legacy Roact API (using `Roact.useEffect`) and the new React-like API (using `React.useEffect`). + +This lint is only active when using the Roblox standard library. + +The lint analyzes variable references within the hook callback and compares them against the declared dependency array. It will: + +1. Report missing dependencies that are used in the callback but not listed +2. Report unnecessary dependencies that are listed but not used +3. Track property access (e.g., `props.value`) as separate dependencies +4. Ignore built-in Lua globals and Roblox APIs (like `print`, `game`, `workspace`, etc.) + +### Limitations +- The lint assumes variables are stable and doesn't track complex control flow +- It doesn't detect dependencies in nested function definitions +- Complex dependency expressions (computed property access with brackets) may not be analyzed correctly +- setState functions and refs are currently not detected as stable (unlike in React) + +## Configuration +This lint does not have any configuration options. + diff --git a/selene-lib/src/lints.rs b/selene-lib/src/lints.rs index dd80bfd4..d3649e2f 100644 --- a/selene-lib/src/lints.rs +++ b/selene-lib/src/lints.rs @@ -43,6 +43,9 @@ pub mod roblox_incorrect_color3_new_bounds; #[cfg(feature = "roblox")] pub mod roblox_incorrect_roact_usage; +#[cfg(feature = "roblox")] +pub mod roblox_react_exhaustive_deps; + #[cfg(feature = "roblox")] pub mod roblox_manual_fromscale_or_fromoffset; diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs new file mode 100644 index 00000000..b2e6aafa --- /dev/null +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -0,0 +1,886 @@ +use super::*; +use crate::ast_util::{range, strip_parentheses}; +use std::{ + collections::{HashMap, HashSet}, + convert::Infallible, +}; + +use full_moon::{ + ast::{self, Ast}, + visitors::Visitor, +}; +use if_chain::if_chain; + +pub struct ReactExhaustiveDepsLint; + +impl Lint for ReactExhaustiveDepsLint { + type Config = (); + type Error = Infallible; + + const SEVERITY: Severity = Severity::Warning; + const LINT_TYPE: LintType = LintType::Correctness; + + fn new(_: Self::Config) -> Result { + Ok(ReactExhaustiveDepsLint) + } + + fn pass(&self, ast: &Ast, context: &Context, ast_context: &AstContext) -> Vec { + if !context.is_roblox() { + return Vec::new(); + } + + let mut visitor = ReactExhaustiveDepsVisitor { + scope_manager: &ast_context.scope_manager, + definitions_of_hooks: HashMap::new(), + issues: Vec::new(), + current_scope_id: None, + }; + + visitor.visit_ast(ast); + + visitor.issues + } +} + +struct ReactExhaustiveDepsVisitor<'a> { + scope_manager: &'a crate::ast_util::scopes::ScopeManager, + definitions_of_hooks: HashMap, + issues: Vec, + current_scope_id: Option>, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(clippy::enum_variant_names)] +enum HookType { + UseEffect, + UseCallback, + UseMemo, + UseLayoutEffect, + UseImperativeHandle, +} + +impl HookType { + fn name(&self) -> &'static str { + match self { + HookType::UseEffect => "useEffect", + HookType::UseCallback => "useCallback", + HookType::UseMemo => "useMemo", + HookType::UseLayoutEffect => "useLayoutEffect", + HookType::UseImperativeHandle => "useImperativeHandle", + } + } + + fn deps_position(&self) -> usize { + match self { + HookType::UseEffect + | HookType::UseCallback + | HookType::UseMemo + | HookType::UseLayoutEffect => 1, + HookType::UseImperativeHandle => 2, + } + } + + fn callback_position(&self) -> usize { + match self { + HookType::UseEffect + | HookType::UseCallback + | HookType::UseMemo + | HookType::UseLayoutEffect => 0, + HookType::UseImperativeHandle => 1, + } + } +} + +fn is_react_hook_call(prefix: &ast::Prefix, suffixes: &[&ast::Suffix]) -> Option { + if_chain! { + if let ast::Prefix::Name(prefix_token) = prefix; + if ["React", "Roact"].contains(&&*prefix_token.token().to_string()); + if suffixes.len() == 1; + if let ast::Suffix::Index(ast::Index::Dot { name, .. }) = suffixes[0]; + then { + match &*name.token().to_string() { + "useEffect" => Some(HookType::UseEffect), + "useCallback" => Some(HookType::UseCallback), + "useMemo" => Some(HookType::UseMemo), + "useLayoutEffect" => Some(HookType::UseLayoutEffect), + "useImperativeHandle" => Some(HookType::UseImperativeHandle), + _ => None, + } + } else { + None + } + } +} + +fn extract_dependencies_from_array( + table: &ast::TableConstructor, +) -> Option> { + let mut deps = Vec::new(); + + for field in table.fields() { + match field { + ast::Field::NoKey(expr) => { + if let Some(dep_name) = extract_dependency_name(expr) { + deps.push((dep_name, range(expr))); + } else { + // Complex expression in deps array - we can't analyze it + return None; + } + } + _ => { + // Key-value pairs in deps array are invalid + return None; + } + } + } + + Some(deps) +} + +fn extract_dependency_name(expr: &ast::Expression) -> Option { + let expr = strip_parentheses(expr); + + match expr { + ast::Expression::Var(var) => { + if let ast::Var::Name(name) = var { + Some(name.token().to_string()) + } else if let ast::Var::Expression(var_expr) = var { + // Handle property access like props.value or obj.field + extract_property_chain(var_expr) + } else { + None + } + } + _ => None, + } +} + +fn extract_property_chain(var_expr: &ast::VarExpression) -> Option { + let mut parts = Vec::new(); + + // Get the base name + if let ast::Prefix::Name(name) = var_expr.prefix() { + parts.push(name.token().to_string()); + } else { + return None; + } + + // Get all property accesses + for suffix in var_expr.suffixes() { + if let ast::Suffix::Index(ast::Index::Dot { name, .. }) = suffix { + parts.push(name.token().to_string()); + } else { + // Complex access like brackets, we can't analyze simply + return None; + } + } + + Some(parts.join(".")) +} + +fn collect_referenced_variables( + expr: &ast::Expression, + scope_id: id_arena::Id, + scope_manager: &crate::ast_util::scopes::ScopeManager, +) -> HashSet { + let mut collector = VariableCollector { + variables: HashSet::new(), + scope_id, + scope_manager, + }; + + collector.visit_expression(expr); + collector.variables +} + +struct VariableCollector<'a> { + variables: HashSet, + #[allow(dead_code)] + scope_id: id_arena::Id, + #[allow(dead_code)] + scope_manager: &'a crate::ast_util::scopes::ScopeManager, +} + +impl<'a> VariableCollector<'a> { + fn visit_expression(&mut self, expr: &ast::Expression) { + let expr = strip_parentheses(expr); + + match expr { + ast::Expression::Var(var) => { + self.visit_var(var); + } + ast::Expression::BinaryOperator { lhs, rhs, .. } => { + self.visit_expression(lhs); + self.visit_expression(rhs); + } + ast::Expression::UnaryOperator { expression, .. } => { + self.visit_expression(expression); + } + ast::Expression::Function(_) => { + // Don't traverse into nested functions - they have their own scope + } + ast::Expression::FunctionCall(call) => { + self.visit_function_call(call); + } + ast::Expression::TableConstructor(table) => { + self.visit_table_constructor(table); + } + // Note: We skip Luau-specific expressions like IfExpression and InterpolatedString + // as they require more complex handling + _ => {} + } + } + + fn visit_var(&mut self, var: &ast::Var) { + match var { + ast::Var::Name(name) => { + let var_name = name.token().to_string(); + + // Check if this variable is defined in the current scope or parent scopes + if self.is_external_variable(&var_name) { + self.variables.insert(var_name); + } + } + ast::Var::Expression(var_expr) => { + // Handle property access like props.value + if let Some(chain) = extract_property_chain(var_expr) { + // Get the base variable name + if let Some(base) = chain.split('.').next() { + if self.is_external_variable(base) { + self.variables.insert(chain); + } + } + } else { + // Fallback to visiting the prefix and suffixes + self.visit_prefix(var_expr.prefix()); + for suffix in var_expr.suffixes() { + self.visit_suffix(suffix); + } + } + } + _ => {} + } + } + + fn visit_prefix(&mut self, prefix: &ast::Prefix) { + match prefix { + ast::Prefix::Name(name) => { + let var_name = name.token().to_string(); + if self.is_external_variable(&var_name) { + self.variables.insert(var_name); + } + } + ast::Prefix::Expression(expr) => { + self.visit_expression(expr); + } + _ => {} + } + } + + fn visit_suffix(&mut self, suffix: &ast::Suffix) { + match suffix { + ast::Suffix::Index(index) => match index { + ast::Index::Brackets { expression, .. } => { + self.visit_expression(expression); + } + ast::Index::Dot { .. } => { + // Property access, handled in extract_property_chain + } + _ => {} + }, + ast::Suffix::Call(call) => match call { + ast::Call::AnonymousCall(args) => match args { + ast::FunctionArgs::Parentheses { arguments, .. } => { + for arg in arguments { + self.visit_expression(arg); + } + } + ast::FunctionArgs::String(_) => {} + ast::FunctionArgs::TableConstructor(table) => { + self.visit_table_constructor(table); + } + _ => {} + }, + ast::Call::MethodCall(method_call) => { + if let ast::FunctionArgs::Parentheses { arguments, .. } = method_call.args() { + for arg in arguments { + self.visit_expression(arg); + } + } + } + _ => {} + }, + _ => {} + } + } + + fn visit_function_call(&mut self, call: &ast::FunctionCall) { + self.visit_prefix(call.prefix()); + for suffix in call.suffixes() { + self.visit_suffix(suffix); + } + } + + fn visit_table_constructor(&mut self, table: &ast::TableConstructor) { + for field in table.fields() { + match field { + ast::Field::ExpressionKey { key, value, .. } => { + self.visit_expression(key); + self.visit_expression(value); + } + ast::Field::NameKey { value, .. } => { + self.visit_expression(value); + } + ast::Field::NoKey(expr) => { + self.visit_expression(expr); + } + _ => {} + } + } + } + + fn is_external_variable(&self, name: &str) -> bool { + // Skip built-in globals and common constants + if ["true", "false", "nil", "self"].contains(&name) { + return false; + } + + // Check if this variable is defined in the current function scope + // For now, we'll use a simple heuristic: if it starts with lowercase and + // is a simple identifier, it's likely an external variable + // TODO: Use scope_manager to properly check if variable is external + true + } +} + +fn visit_block_for_variables( + block: &ast::Block, + scope_id: id_arena::Id, + scope_manager: &crate::ast_util::scopes::ScopeManager, +) -> HashSet { + let mut variables = HashSet::new(); + + for stmt in block.stmts() { + variables.extend(collect_variables_from_stmt(stmt, scope_id, scope_manager)); + } + + if let Some(ast::LastStmt::Return(return_stmt)) = block.last_stmt() { + for expr in return_stmt.returns() { + variables.extend(collect_referenced_variables(expr, scope_id, scope_manager)); + } + } + + variables +} + +fn collect_variables_from_stmt( + stmt: &ast::Stmt, + scope_id: id_arena::Id, + scope_manager: &crate::ast_util::scopes::ScopeManager, +) -> HashSet { + let mut variables = HashSet::new(); + + match stmt { + ast::Stmt::Assignment(assignment) => { + for expr in assignment.expressions() { + variables.extend(collect_referenced_variables(expr, scope_id, scope_manager)); + } + // Don't traverse into the variables being assigned to + } + ast::Stmt::LocalAssignment(assignment) => { + for expr in assignment.expressions() { + variables.extend(collect_referenced_variables(expr, scope_id, scope_manager)); + } + } + ast::Stmt::FunctionCall(call) => { + variables.extend(collect_variables_from_function_call( + call, + scope_id, + scope_manager, + )); + } + ast::Stmt::If(if_stmt) => { + variables.extend(collect_referenced_variables( + if_stmt.condition(), + scope_id, + scope_manager, + )); + variables.extend(visit_block_for_variables( + if_stmt.block(), + scope_id, + scope_manager, + )); + + if let Some(else_ifs) = if_stmt.else_if() { + for else_if in else_ifs { + variables.extend(collect_referenced_variables( + else_if.condition(), + scope_id, + scope_manager, + )); + variables.extend(visit_block_for_variables( + else_if.block(), + scope_id, + scope_manager, + )); + } + } + + if let Some(else_block) = if_stmt.else_block() { + variables.extend(visit_block_for_variables( + else_block, + scope_id, + scope_manager, + )); + } + } + ast::Stmt::While(while_stmt) => { + variables.extend(collect_referenced_variables( + while_stmt.condition(), + scope_id, + scope_manager, + )); + variables.extend(visit_block_for_variables( + while_stmt.block(), + scope_id, + scope_manager, + )); + } + ast::Stmt::Repeat(repeat_stmt) => { + variables.extend(visit_block_for_variables( + repeat_stmt.block(), + scope_id, + scope_manager, + )); + variables.extend(collect_referenced_variables( + repeat_stmt.until(), + scope_id, + scope_manager, + )); + } + ast::Stmt::Do(do_stmt) => { + variables.extend(visit_block_for_variables( + do_stmt.block(), + scope_id, + scope_manager, + )); + } + ast::Stmt::GenericFor(for_stmt) => { + for expr in for_stmt.expressions() { + variables.extend(collect_referenced_variables(expr, scope_id, scope_manager)); + } + variables.extend(visit_block_for_variables( + for_stmt.block(), + scope_id, + scope_manager, + )); + } + ast::Stmt::NumericFor(for_stmt) => { + variables.extend(collect_referenced_variables( + for_stmt.start(), + scope_id, + scope_manager, + )); + variables.extend(collect_referenced_variables( + for_stmt.end(), + scope_id, + scope_manager, + )); + if let Some(step) = for_stmt.step() { + variables.extend(collect_referenced_variables(step, scope_id, scope_manager)); + } + variables.extend(visit_block_for_variables( + for_stmt.block(), + scope_id, + scope_manager, + )); + } + _ => {} + } + + variables +} + +fn collect_variables_from_function_call( + call: &ast::FunctionCall, + scope_id: id_arena::Id, + scope_manager: &crate::ast_util::scopes::ScopeManager, +) -> HashSet { + let mut variables = HashSet::new(); + + // Visit the prefix + match call.prefix() { + ast::Prefix::Name(name) => { + let var_name = name.token().to_string(); + // Check if it's an external variable + if !["true", "false", "nil", "self"].contains(&&*var_name) { + variables.insert(var_name); + } + } + ast::Prefix::Expression(expr) => { + variables.extend(collect_referenced_variables(expr, scope_id, scope_manager)); + } + _ => {} + } + + // Visit suffixes + for suffix in call.suffixes() { + match suffix { + ast::Suffix::Call(call) => match call { + ast::Call::AnonymousCall(args) => match args { + ast::FunctionArgs::Parentheses { arguments, .. } => { + for arg in arguments { + variables.extend(collect_referenced_variables( + arg, + scope_id, + scope_manager, + )); + } + } + ast::FunctionArgs::TableConstructor(table) => { + for field in table.fields() { + match field { + ast::Field::ExpressionKey { key, value, .. } => { + variables.extend(collect_referenced_variables( + key, + scope_id, + scope_manager, + )); + variables.extend(collect_referenced_variables( + value, + scope_id, + scope_manager, + )); + } + ast::Field::NameKey { value, .. } => { + variables.extend(collect_referenced_variables( + value, + scope_id, + scope_manager, + )); + } + ast::Field::NoKey(expr) => { + variables.extend(collect_referenced_variables( + expr, + scope_id, + scope_manager, + )); + } + _ => {} + } + } + } + _ => {} + }, + ast::Call::MethodCall(method) => { + if let ast::FunctionArgs::Parentheses { arguments, .. } = method.args() { + for arg in arguments { + variables.extend(collect_referenced_variables( + arg, + scope_id, + scope_manager, + )); + } + } + } + _ => {} + }, + ast::Suffix::Index(ast::Index::Brackets { expression, .. }) => { + variables.extend(collect_referenced_variables( + expression, + scope_id, + scope_manager, + )); + } + _ => {} + } + } + + variables +} + +impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { + fn visit_function_call(&mut self, call: &ast::FunctionCall) { + // Check if this is a React hook call + let mut suffixes = call.suffixes().collect::>(); + let call_suffix = suffixes.pop(); + + let mut hook_type = None; + + if suffixes.is_empty() { + // Call is foo(), not foo.bar() + // Check if foo is a variable for a React hook + if let ast::Prefix::Name(name) = call.prefix() { + let name_str = name.token().to_string(); + if let Some(stored_hook) = self.definitions_of_hooks.get(&name_str) { + hook_type = Some(*stored_hook); + } + } + } else if suffixes.len() == 1 { + // Call is foo.bar() + // Check if foo.bar is a React hook + if let Some(detected_hook) = is_react_hook_call(call.prefix(), &suffixes) { + hook_type = Some(detected_hook); + } + } + + let hook_type = match hook_type { + Some(ht) => ht, + None => return, + }; + + // Extract arguments + if_chain! { + if let Some(ast::Suffix::Call(ast::Call::AnonymousCall( + ast::FunctionArgs::Parentheses { arguments, .. } + ))) = call_suffix; + then { + let args: Vec<_> = arguments.iter().collect(); + + // Get callback and deps based on hook type + let callback_pos = hook_type.callback_position(); + let deps_pos = hook_type.deps_position(); + + if args.len() <= callback_pos { + return; + } + + let callback = args[callback_pos]; + let deps_arg = if args.len() > deps_pos { + Some(args[deps_pos]) + } else { + None + }; + + // Extract the function body to analyze + let callback_body = match strip_parentheses(callback) { + ast::Expression::Function(func) => func.body().block(), + _ => return, // Not a function literal + }; + + // Collect variables referenced in the callback + let callback_range: (usize, usize) = range(callback); + + // We need a valid scope ID - if we don't have one, skip this analysis + let Some(scope_id) = self.current_scope_id.or(self.scope_manager.initial_scope) else { + return; + }; + + let referenced_vars = visit_block_for_variables( + callback_body, + scope_id, + self.scope_manager, + ); + + // Extract declared dependencies + let declared_deps = if let Some(deps_expr) = deps_arg { + match strip_parentheses(deps_expr) { + ast::Expression::TableConstructor(table) => { + extract_dependencies_from_array(table) + } + _ => None, // Not a table, can't analyze + } + } else { + Some(Vec::new()) // No deps array means empty dependencies + }; + + if let Some(declared_deps) = declared_deps { + let declared_set: HashSet = declared_deps + .iter() + .map(|(name, _)| name.clone()) + .collect(); + + // Filter out variables that should be ignored + let filtered_referenced: HashSet = referenced_vars + .into_iter() + .filter(|var| { + // Ignore built-in globals + !["print", "warn", "error", "assert", "type", "typeof", + "pairs", "ipairs", "next", "tonumber", "tostring", + "table", "string", "math", "coroutine", "debug", + "require", "game", "workspace", "script", "task", + "_G", "_VERSION"].contains(&&**var) + }) + .collect(); + + // Find missing dependencies + let mut missing: Vec = filtered_referenced + .difference(&declared_set) + .cloned() + .collect(); + missing.sort(); + + // Find unnecessary dependencies + let mut unnecessary: Vec = declared_set + .difference(&filtered_referenced) + .cloned() + .collect(); + unnecessary.sort(); + + // Report if there are issues + if !missing.is_empty() || !unnecessary.is_empty() { + let deps_range = deps_arg.map(range); + + let mut message = format!("react hook {} has", hook_type.name()); + if !missing.is_empty() { + message.push_str(&format!( + " missing dependenc{}: {}", + if missing.len() == 1 { "y" } else { "ies" }, + missing.join(", ") + )); + } + if !missing.is_empty() && !unnecessary.is_empty() { + message.push_str(", and"); + } + if !unnecessary.is_empty() { + message.push_str(&format!( + " unnecessary dependenc{}: {}", + if unnecessary.len() == 1 { "y" } else { "ies" }, + unnecessary.join(", ") + )); + } + + let label_range = deps_range.unwrap_or(callback_range); + + let mut notes = Vec::new(); + if !missing.is_empty() { + notes.push(format!( + "help: either include {} in the dependency array or remove the dependency", + if missing.len() == 1 { + format!("'{}'", missing[0]) + } else { + format!("them ({})", missing.join(", ")) + } + )); + } + if !unnecessary.is_empty() { + notes.push(format!( + "help: remove {} from the dependency array", + if unnecessary.len() == 1 { + format!("'{}'", unnecessary[0]) + } else { + format!("them ({})", unnecessary.join(", ")) + } + )); + } + + self.issues.push(Diagnostic::new_complete( + "roblox_react_exhaustive_deps", + message, + Label::new(label_range), + notes, + Vec::new(), + )); + } + } + } + } + } + + fn visit_local_assignment(&mut self, node: &ast::LocalAssignment) { + for (name, expr) in node.names().iter().zip(node.expressions().iter()) { + if_chain! { + if let ast::Expression::Var(ast::Var::Expression(var_expr)) = expr; + if let Some(hook) = is_react_hook_call(var_expr.prefix(), &var_expr.suffixes().collect::>()); + then { + self.definitions_of_hooks.insert(name.token().to_string(), hook); + } + }; + } + } +} + +#[cfg(test)] +mod tests { + use super::{super::test_util::test_lint, *}; + + #[test] + fn test_missing_dependencies() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "missing_dependencies", + ); + } + + #[test] + fn test_unnecessary_dependencies() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "unnecessary_dependencies", + ); + } + + #[test] + fn test_correct_dependencies() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "correct_dependencies", + ); + } + + #[test] + fn test_property_access() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "property_access", + ); + } + + #[test] + fn test_use_callback() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "use_callback", + ); + } + + #[test] + fn test_use_memo() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "use_memo", + ); + } + + #[test] + fn test_known_stable_vars() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "known_stable_vars", + ); + } + + #[test] + fn test_implicit_self() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "implicit_self", + ); + } + + #[test] + fn test_complex_deps() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "complex_deps", + ); + } + + #[test] + fn test_outer_scope() { + test_lint( + ReactExhaustiveDepsLint::new(()).unwrap(), + "roblox_react_exhaustive_deps", + "outer_scope", + ); + } +} diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.lua new file mode 100644 index 00000000..2b4572e7 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.lua @@ -0,0 +1,57 @@ +-- Test cases for complex dependency patterns + +local React = { + useEffect = function() end, +} + +-- Bracket indexing with string literals +local function Component1() + local a = {} + + React.useEffect(function() + print(a["b c"]["d"]) + end, { a["b c"]["d"] }) +end + +-- Missing bracket-indexed dependency +local function Component2() + local a = {} + + React.useEffect(function() + print(a["key"]) + end, {}) +end + +-- Hierarchical: using a.b.c but only a.b in deps +local function Component3() + local a = {} + + React.useEffect(function() + print(a) + print(a.b) + print(a.b.c) + end, { a.b }) +end + +-- Hierarchical: using a but only a.b in deps (should warn about a) +local function Component4() + local a = {} + + React.useEffect(function() + print(a) + print(a.b) + end, { a.b }) +end + +-- Multiple property paths +local function Component5() + local a = {} + local d = {} + + React.useEffect(function() + print(a.b.c()) + print(a.b.d()) + print(d.e.f.g()) + end, { a.b.c, d.e.f }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.stderr new file mode 100644 index 00000000..42b5f9f0 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.stderr @@ -0,0 +1,33 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: a + ┌─ complex_deps.lua:22:10 + │ +22 │ end, {}) + │ ^^ + │ + = help: either include 'a' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: a, a.b.c + ┌─ complex_deps.lua:33:10 + │ +33 │ end, { a.b }) + │ ^^^^^^^ + │ + = help: either include them (a, a.b.c) in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: a + ┌─ complex_deps.lua:43:10 + │ +43 │ end, { a.b }) + │ ^^^^^^^ + │ + = help: either include 'a' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: a, d, and unnecessary dependencies: a.b.c, d.e.f + ┌─ complex_deps.lua:55:10 + │ +55 │ end, { a.b.c, d.e.f }) + │ ^^^^^^^^^^^^^^^^ + │ + = help: either include them (a, d) in the dependency array or remove the dependency + = help: remove them (a.b.c, d.e.f) from the dependency array + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.lua new file mode 100644 index 00000000..3ce62216 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.lua @@ -0,0 +1,54 @@ +-- Test cases for correct dependencies (should have no warnings) + +local React = { + useEffect = function() end, +} + +-- Correct single dependency +local function Component1(props) + React.useEffect(function() + print(props.value) + end, { props.value }) +end + +-- Correct multiple dependencies +local function Component2(props) + local count = 5 + + React.useEffect(function() + print(props.value) + print(count) + end, { props.value, count }) +end + +-- No dependencies needed +local function Component3() + React.useEffect(function() + print("hello") + end, {}) +end + +-- No dependencies array (runs every render) +local function Component4(props) + React.useEffect(function() + print(props.value) + end) +end + +-- Using only constants +local function Component5() + React.useEffect(function() + print(1 + 2) + print("constant") + end, {}) +end + +-- Using built-in globals +local function Component6() + React.useEffect(function() + print("test") + local t = { 1, 2, 3 } + table.insert(t, 4) + end, {}) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.stderr new file mode 100644 index 00000000..d8e38d6b --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.stderr @@ -0,0 +1,19 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.value + ┌─ correct_dependencies.lua:33:21 + │ +33 │ React.useEffect(function() + │ ╭─────────────────────^ +34 │ │ print(props.value) +35 │ │ end) + │ ╰───────^ + │ + = help: either include 'props.value' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: t + ┌─ correct_dependencies.lua:52:10 + │ +52 │ end, {}) + │ ^^ + │ + = help: either include 't' in the dependency array or remove the dependency + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.lua new file mode 100644 index 00000000..af2127be --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.lua @@ -0,0 +1,36 @@ +-- Test cases for implicit self in method calls + +local React = { + useEffect = function() end, +} + +-- Method call with colon passes self implicitly +local function Component1() + local a = {} + + React.useEffect(function() + -- a.b:c() implicitly passes a.b as self + -- So a.b should be in deps, not a.b.c + local _ = a.b:c() + end, { a.b.c }) +end + +-- Regular function call (dot) doesn't pass self +local function Component2() + local a = {} + + React.useEffect(function() + -- a.b.c() is just a function call, a.b.c is correct dep + local _ = a.b.c() + end, { a.b.c }) +end + +-- Method call with correct deps +local function Component3() + local a = {} + + React.useEffect(function() + local _ = a.b:c() + end, { a.b }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.stderr new file mode 100644 index 00000000..19207538 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.stderr @@ -0,0 +1,27 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: a, and unnecessary dependency: a.b.c + ┌─ implicit_self.lua:15:10 + │ +15 │ end, { a.b.c }) + │ ^^^^^^^^^ + │ + = help: either include 'a' in the dependency array or remove the dependency + = help: remove 'a.b.c' from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: a, and unnecessary dependency: a.b.c + ┌─ implicit_self.lua:25:10 + │ +25 │ end, { a.b.c }) + │ ^^^^^^^^^ + │ + = help: either include 'a' in the dependency array or remove the dependency + = help: remove 'a.b.c' from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: a, and unnecessary dependency: a.b + ┌─ implicit_self.lua:34:10 + │ +34 │ end, { a.b }) + │ ^^^^^^^ + │ + = help: either include 'a' in the dependency array or remove the dependency + = help: remove 'a.b' from the dependency array + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.lua new file mode 100644 index 00000000..e7527a43 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.lua @@ -0,0 +1,58 @@ +-- Test cases for known stable variables (setState, refs, etc.) + +local React = { + useEffect = function() end, + useState = function() end, + useRef = function() end, + useBinding = function() end, +} + +-- setState from useState should be stable and not required in deps +local function Component1() + local state, setState = React.useState() + + React.useEffect(function() + -- setState is stable, shouldn't need to be in deps + setState("new value") + end, {}) +end + +-- state from useState SHOULD be in deps +local function Component2() + local state, setState = React.useState() + + React.useEffect(function() + -- state is NOT stable, should be in deps + print(state) + end, {}) +end + +-- ref from useRef should be stable +local function Component3() + local ref = React.useRef() + + React.useEffect(function() + -- ref is stable, shouldn't need to be in deps + print(ref.current) + end, {}) +end + +-- setBinding from useBinding should be stable +local function Component4() + local binding, setBinding = React.useBinding() + + React.useEffect(function() + -- setBinding is stable + setBinding("new value") + end, {}) +end + +-- Passing stable vars in deps shouldn't warn about them being unnecessary +local function Component5() + local state, setState = React.useState() + + React.useEffect(function() + setState("value") + end, { setState }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.stderr new file mode 100644 index 00000000..6b8399cb --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.stderr @@ -0,0 +1,32 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: setState + ┌─ known_stable_vars.lua:17:10 + │ +17 │ end, {}) + │ ^^ + │ + = help: either include 'setState' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: state + ┌─ known_stable_vars.lua:27:10 + │ +27 │ end, {}) + │ ^^ + │ + = help: either include 'state' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: ref.current + ┌─ known_stable_vars.lua:37:10 + │ +37 │ end, {}) + │ ^^ + │ + = help: either include 'ref.current' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: setBinding + ┌─ known_stable_vars.lua:47:10 + │ +47 │ end, {}) + │ ^^ + │ + = help: either include 'setBinding' in the dependency array or remove the dependency + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.lua new file mode 100644 index 00000000..3bc6cfb7 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.lua @@ -0,0 +1,64 @@ +-- Test cases for missing dependencies + +local React = { + useEffect = function() end, +} + +-- Missing single dependency +local function Component1(props) + React.useEffect(function() + print(props.value) + end, {}) +end + +-- Missing multiple dependencies +local function Component2(props) + local count = 5 + + React.useEffect(function() + print(props.value) + print(count) + print(props.name) + end, {}) +end + +-- Missing dependency with some correct ones +local function Component3(props) + local count = 5 + local name = "test" + + React.useEffect(function() + print(props.value) + print(count) + print(name) + end, { count }) +end + +-- Missing dependency in nested function call +local function Component4(props) + local spawn = function() end + React.useEffect(function() + spawn(function() + print(props.value) + end) + end, {}) +end + +-- Missing dependency with property access +local function Component5(props) + React.useEffect(function() + print(props.data.value) + end, {}) +end + +-- Roact version +local Roact = { + useEffect = function() end, +} + +local function RoactComponent(props) + Roact.useEffect(function() + print(props.value) + end, {}) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.std.yml new file mode 100644 index 00000000..4ec59611 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.std.yml @@ -0,0 +1,5 @@ +--- +name: roblox +base: roblox + + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.stderr new file mode 100644 index 00000000..b7cccd0a --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.stderr @@ -0,0 +1,48 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.value + ┌─ missing_dependencies.lua:11:10 + │ +11 │ end, {}) + │ ^^ + │ + = help: either include 'props.value' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: count, props.name, props.value + ┌─ missing_dependencies.lua:22:10 + │ +22 │ end, {}) + │ ^^ + │ + = help: either include them (count, props.name, props.value) in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: name, props.value + ┌─ missing_dependencies.lua:34:10 + │ +34 │ end, { count }) + │ ^^^^^^^^^ + │ + = help: either include them (name, props.value) in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: spawn + ┌─ missing_dependencies.lua:44:10 + │ +44 │ end, {}) + │ ^^ + │ + = help: either include 'spawn' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.data.value + ┌─ missing_dependencies.lua:51:10 + │ +51 │ end, {}) + │ ^^ + │ + = help: either include 'props.data.value' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.value + ┌─ missing_dependencies.lua:62:10 + │ +62 │ end, {}) + │ ^^ + │ + = help: either include 'props.value' in the dependency array or remove the dependency + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.lua new file mode 100644 index 00000000..b2945ba8 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.lua @@ -0,0 +1,43 @@ +-- Test cases for outer scope variables + +local React = { + useEffect = function() end, +} + +-- Module-level variable (outer scope) +local moduleVar = {} + +local function Component1() + local reactiveVar = {} + + React.useEffect(function() + -- moduleVar is outer scope, doesn't need to be in deps + print(moduleVar) + -- reactiveVar is component scope, needs to be in deps + print(reactiveVar) + end, {}) +end + +-- Outer scope variable in deps should warn as unnecessary +local function Component2() + React.useEffect(function() + print(moduleVar) + end, { moduleVar }) +end + +-- Nested component - outer component's vars are outer scope +local function MakeComponent() + local outerVar = {} + + local function Component() + local innerVar = {} + + React.useEffect(function() + -- outerVar is from outer scope (not component scope) + print(outerVar) + -- innerVar is component scope + print(innerVar) + end, { innerVar }) + end +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.stderr new file mode 100644 index 00000000..61bff996 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.stderr @@ -0,0 +1,16 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: moduleVar, reactiveVar + ┌─ outer_scope.lua:18:10 + │ +18 │ end, {}) + │ ^^ + │ + = help: either include them (moduleVar, reactiveVar) in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: outerVar + ┌─ outer_scope.lua:40:14 + │ +40 │ end, { innerVar }) + │ ^^^^^^^^^^^^ + │ + = help: either include 'outerVar' in the dependency array or remove the dependency + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.lua new file mode 100644 index 00000000..fa5a481e --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.lua @@ -0,0 +1,38 @@ +-- Test cases for property access dependencies + +local React = { + useEffect = function() end, +} + +-- Property access - correct +local function Component1(props) + React.useEffect(function() + print(props.data.value) + end, { props.data.value }) +end + +-- Property access - missing +local function Component2(props) + React.useEffect(function() + print(props.user.name) + print(props.user.age) + end, {}) +end + +-- Nested property access +local function Component3(props) + React.useEffect(function() + print(props.data.user.profile.name) + end, { props.data.user.profile.name }) +end + +-- Mixed property and direct access +local function Component4(props) + local count = 5 + + React.useEffect(function() + print(props.data.value) + print(count) + end, { count }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.stderr new file mode 100644 index 00000000..4d24f962 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.stderr @@ -0,0 +1,16 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: props.user.age, props.user.name + ┌─ property_access.lua:19:10 + │ +19 │ end, {}) + │ ^^ + │ + = help: either include them (props.user.age, props.user.name) in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.data.value + ┌─ property_access.lua:36:10 + │ +36 │ end, { count }) + │ ^^^^^^^^^ + │ + = help: either include 'props.data.value' in the dependency array or remove the dependency + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.lua new file mode 100644 index 00000000..c42e452d --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.lua @@ -0,0 +1,39 @@ +-- Test cases for unnecessary dependencies + +local React = { + useEffect = function() end, +} + +-- Single unnecessary dependency +local function Component1(props) + React.useEffect(function() + print("hello") + end, { props.value }) +end + +-- Multiple unnecessary dependencies +local function Component2(props) + React.useEffect(function() + print("hello") + end, { props.value, props.name, props.id }) +end + +-- Mix of correct and unnecessary dependencies +local function Component3(props) + local count = 5 + + React.useEffect(function() + print(count) + end, { count, props.value }) +end + +-- All dependencies unnecessary +local function Component4() + local a = 1 + local b = 2 + + React.useEffect(function() + print("constant") + end, { a, b }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr new file mode 100644 index 00000000..bb86b3ca --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr @@ -0,0 +1,32 @@ +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependency: props.value + ┌─ unnecessary_dependencies.lua:11:10 + │ +11 │ end, { props.value }) + │ ^^^^^^^^^^^^^^^ + │ + = help: remove 'props.value' from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependencies: props.id, props.name, props.value + ┌─ unnecessary_dependencies.lua:18:10 + │ +18 │ end, { props.value, props.name, props.id }) + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + │ + = help: remove them (props.id, props.name, props.value) from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependency: props.value + ┌─ unnecessary_dependencies.lua:27:10 + │ +27 │ end, { count, props.value }) + │ ^^^^^^^^^^^^^^^^^^^^^^ + │ + = help: remove 'props.value' from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependencies: a, b + ┌─ unnecessary_dependencies.lua:37:10 + │ +37 │ end, { a, b }) + │ ^^^^^^^^ + │ + = help: remove them (a, b) from the dependency array + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.lua new file mode 100644 index 00000000..c6535b06 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.lua @@ -0,0 +1,43 @@ +-- Test cases for useCallback + +local React = { + useCallback = function() end, +} + +-- Missing dependency in useCallback +local function Component1(props) + local handleClick = React.useCallback(function() + print(props.value) + end, {}) +end + +-- Correct dependencies in useCallback +local function Component2(props) + local count = 5 + + local handleClick = React.useCallback(function() + print(props.value) + print(count) + end, { props.value, count }) +end + +-- Unnecessary dependency in useCallback +local function Component3() + local value = 5 + + local handleClick = React.useCallback(function() + print("constant") + end, { value }) +end + +-- Mixed issues +local function Component4(props) + local count = 5 + local unused = 10 + + local handleClick = React.useCallback(function() + print(props.value) + print(count) + end, { unused }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.stderr new file mode 100644 index 00000000..63caace7 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.stderr @@ -0,0 +1,25 @@ +error[roblox_react_exhaustive_deps]: react hook useCallback has missing dependency: props.value + ┌─ use_callback.lua:11:10 + │ +11 │ end, {}) + │ ^^ + │ + = help: either include 'props.value' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useCallback has unnecessary dependency: value + ┌─ use_callback.lua:30:10 + │ +30 │ end, { value }) + │ ^^^^^^^^^ + │ + = help: remove 'value' from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useCallback has missing dependencies: count, props.value, and unnecessary dependency: unused + ┌─ use_callback.lua:41:10 + │ +41 │ end, { unused }) + │ ^^^^^^^^^^ + │ + = help: either include them (count, props.value) in the dependency array or remove the dependency + = help: remove 'unused' from the dependency array + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.lua b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.lua new file mode 100644 index 00000000..49831eb9 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.lua @@ -0,0 +1,41 @@ +-- Test cases for useMemo + +local React = { + useMemo = function() end, +} + +-- Missing dependency in useMemo +local function Component1(props) + local computed = React.useMemo(function() + return props.value * 2 + end, {}) +end + +-- Correct dependencies in useMemo +local function Component2(props) + local multiplier = 5 + + local computed = React.useMemo(function() + return props.value * multiplier + end, { props.value, multiplier }) +end + +-- Unnecessary dependency in useMemo +local function Component3() + local unused = 5 + + local computed = React.useMemo(function() + return 10 * 2 + end, { unused }) +end + +-- Complex calculation with missing deps +local function Component4(props) + local base = 10 + + local computed = React.useMemo(function() + local result = props.value + base + return result * props.multiplier + end, { base }) +end + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.std.yml b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.std.yml new file mode 100644 index 00000000..a315ac51 --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.std.yml @@ -0,0 +1,4 @@ +--- +name: roblox +base: roblox + diff --git a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr new file mode 100644 index 00000000..a7aad4ca --- /dev/null +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr @@ -0,0 +1,24 @@ +error[roblox_react_exhaustive_deps]: react hook useMemo has missing dependency: props.value + ┌─ use_memo.lua:11:10 + │ +11 │ end, {}) + │ ^^ + │ + = help: either include 'props.value' in the dependency array or remove the dependency + +error[roblox_react_exhaustive_deps]: react hook useMemo has unnecessary dependency: unused + ┌─ use_memo.lua:29:10 + │ +29 │ end, { unused }) + │ ^^^^^^^^^^ + │ + = help: remove 'unused' from the dependency array + +error[roblox_react_exhaustive_deps]: react hook useMemo has missing dependencies: props.multiplier, props.value, result + ┌─ use_memo.lua:39:10 + │ +39 │ end, { base }) + │ ^^^^^^^^ + │ + = help: either include them (props.multiplier, props.value, result) in the dependency array or remove the dependency +