From af06a89ea7f4c0178175a09f53c394d23e423add Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 18 Nov 2025 21:19:46 -0800 Subject: [PATCH 01/10] Add react missing dependencies linter rule --- docs/src/SUMMARY.md | 1 + .../src/lints/roblox_react_exhaustive_deps.md | 88 ++ selene-lib/src/lib.rs | 1 + selene-lib/src/lints.rs | 3 + .../src/lints/roblox_react_exhaustive_deps.rs | 856 ++++++++++++++++++ .../correct_dependencies.lua | 54 ++ .../correct_dependencies.std.yml | 4 + .../correct_dependencies.stderr | 19 + .../missing_dependencies.lua | 64 ++ .../missing_dependencies.std.yml | 5 + .../missing_dependencies.stderr | 48 + .../property_access.lua | 38 + .../property_access.std.yml | 4 + .../property_access.stderr | 16 + .../unnecessary_dependencies.lua | 39 + .../unnecessary_dependencies.std.yml | 4 + .../unnecessary_dependencies.stderr | 32 + .../use_callback.lua | 43 + .../use_callback.std.yml | 4 + .../use_callback.stderr | 25 + .../roblox_react_exhaustive_deps/use_memo.lua | 41 + .../use_memo.std.yml | 4 + .../use_memo.stderr | 24 + 23 files changed, 1417 insertions(+) create mode 100644 docs/src/lints/roblox_react_exhaustive_deps.md create mode 100644 selene-lib/src/lints/roblox_react_exhaustive_deps.rs create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr 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..882eb505 --- /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`). + +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) + +This lint is only active when using the Roblox standard library. + +## Configuration +This lint does not have any configuration options. + diff --git a/selene-lib/src/lib.rs b/selene-lib/src/lib.rs index 6a3952ae..0a7f4bf8 100644 --- a/selene-lib/src/lib.rs +++ b/selene-lib/src/lib.rs @@ -329,6 +329,7 @@ use_lints! { { roblox_incorrect_color3_new_bounds: lints::roblox_incorrect_color3_new_bounds::Color3BoundsLint, roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint, + roblox_react_exhaustive_deps: lints::roblox_react_exhaustive_deps::ReactExhaustiveDepsLint, roblox_manual_fromscale_or_fromoffset: lints::roblox_manual_fromscale_or_fromoffset::ManualFromScaleOrFromOffsetLint, roblox_suspicious_udim2_new: lints::roblox_suspicious_udim2_new::SuspiciousUDim2NewLint, }, 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..c3c254de --- /dev/null +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -0,0 +1,856 @@ +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)] +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().as_str()); + if suffixes.len() == 1; + if let ast::Suffix::Index(ast::Index::Dot { name, .. }) = suffixes[0]; + then { + match name.token().to_string().as_str() { + "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(last_stmt) = block.last_stmt() { + match last_stmt { + ast::LastStmt::Return(return_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.as_str()) { + 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(index) => match 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.1.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 scope_id = match self.current_scope_id { + Some(id) => id, + None => { + // Try to use the initial scope if available + match self.scope_manager.initial_scope { + Some(id) => id, + None => 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.as_str()) + }) + .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(|expr| range(expr)); + + 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!( + "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!( + "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", + ); + } +} + 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..639da366 --- /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) + │ ╰───────^ + │ + = 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, {}) + │ ^^ + │ + = Either include 't' 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..0cb92ac4 --- /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, {}) + │ ^^ + │ + = 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, {}) + │ ^^ + │ + = 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 }) + │ ^^^^^^^^^ + │ + = 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, {}) + │ ^^ + │ + = 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, {}) + │ ^^ + │ + = 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, {}) + │ ^^ + │ + = Either include 'props.value' 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..ff56c03b --- /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, {}) + │ ^^ + │ + = 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 }) + │ ^^^^^^^^^ + │ + = 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..5091781f --- /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 }) + │ ^^^^^^^^^^^^^^^ + │ + = 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 }) + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + │ + = 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 }) + │ ^^^^^^^^^^^^^^^^^^^^^^ + │ + = 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 }) + │ ^^^^^^^^ + │ + = 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..3ff54303 --- /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, {}) + │ ^^ + │ + = 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 }) + │ ^^^^^^^^^ + │ + = 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 }) + │ ^^^^^^^^^^ + │ + = Either include them (count, props.value) in the dependency array or remove the dependency + = 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..dcf3d66e --- /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, {}) + │ ^^ + │ + = 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 }) + │ ^^^^^^^^^^ + │ + = 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 }) + │ ^^^^^^^^ + │ + = Either include them (props.multiplier, props.value, result) in the dependency array or remove the dependency + From 8a7c824e811a6223f22b5963e1dadd8390bbdd66 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 18 Nov 2025 21:23:27 -0800 Subject: [PATCH 02/10] format --- .../src/lints/roblox_react_exhaustive_deps.rs | 60 +++++++++++-------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs index c3c254de..16b96d6a 100644 --- a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -234,7 +234,7 @@ impl<'a> VariableCollector<'a> { 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); @@ -534,8 +534,11 @@ fn collect_variables_from_function_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)); + variables.extend(collect_referenced_variables( + arg, + scope_id, + scope_manager, + )); } } ast::FunctionArgs::TableConstructor(table) => { @@ -576,8 +579,11 @@ fn collect_variables_from_function_call( 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)); + variables.extend(collect_referenced_variables( + arg, + scope_id, + scope_manager, + )); } } } @@ -585,8 +591,11 @@ fn collect_variables_from_function_call( }, ast::Suffix::Index(index) => match index { ast::Index::Brackets { expression, .. } => { - variables - .extend(collect_referenced_variables(expression, scope_id, scope_manager)); + variables.extend(collect_referenced_variables( + expression, + scope_id, + scope_manager, + )); } _ => {} }, @@ -634,31 +643,31 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { ))) = 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.1.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 scope_id = match self.current_scope_id { Some(id) => id, @@ -670,13 +679,13 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { } } }; - + 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) { @@ -688,44 +697,44 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { } 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", + !["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.as_str()) }) .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(|expr| range(expr)); - + let mut message = format!("React Hook {} has", hook_type.name()); if !missing.is_empty() { message.push_str(&format!( @@ -744,9 +753,9 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { unnecessary.join(", ") )); } - + let label_range = deps_range.unwrap_or(callback_range); - + let mut notes = Vec::new(); if !missing.is_empty() { notes.push(format!( @@ -768,7 +777,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { } )); } - + self.issues.push(Diagnostic::new_complete( "roblox_react_exhaustive_deps", message, @@ -853,4 +862,3 @@ mod tests { ); } } - From 4cf7b5634ec738db9f159e309bd05bb43826daa6 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 18 Nov 2025 21:30:10 -0800 Subject: [PATCH 03/10] format --- selene-lib/src/lints/roblox_react_exhaustive_deps.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs index 16b96d6a..13bf4593 100644 --- a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -93,11 +93,11 @@ impl HookType { 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().as_str()); + 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().as_str() { + match &*name.token().to_string() { "useEffect" => Some(HookType::UseEffect), "useCallback" => Some(HookType::UseCallback), "useMemo" => Some(HookType::UseMemo), @@ -517,7 +517,7 @@ fn collect_variables_from_function_call( 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.as_str()) { + if !["true", "false", "nil", "self"].contains(&&*var_name) { variables.insert(var_name); } } @@ -713,7 +713,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { "pairs", "ipairs", "next", "tonumber", "tostring", "table", "string", "math", "coroutine", "debug", "require", "game", "workspace", "script", "task", - "_G", "_VERSION"].contains(&var.as_str()) + "_G", "_VERSION"].contains(&&**var) }) .collect(); From c55edb744a4d596c553b11d57ecb7866f0f845c3 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:13:20 -0800 Subject: [PATCH 04/10] Update selene-lib/src/lints/roblox_react_exhaustive_deps.rs Co-authored-by: Chris Chang <51393127+chriscerie@users.noreply.github.com> --- selene-lib/src/lints/roblox_react_exhaustive_deps.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs index 13bf4593..239a0e56 100644 --- a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -669,15 +669,8 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { let callback_range: (usize, usize) = range(callback); // We need a valid scope ID - if we don't have one, skip this analysis - let scope_id = match self.current_scope_id { - Some(id) => id, - None => { - // Try to use the initial scope if available - match self.scope_manager.initial_scope { - Some(id) => id, - None => return, - } - } + let Some(scope_id) = self.current_scope_id.or(self.scope_manager.initial_scope) else { + return; }; let referenced_vars = visit_block_for_variables( From 400ea0853bbe73d0d64ca0505e2cab0bf39b0742 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:39:09 -0800 Subject: [PATCH 05/10] uhhh ok --- selene-lib/src/lib.rs | 668 +++++++++--------- .../src/lints/roblox_react_exhaustive_deps.rs | 33 +- 2 files changed, 345 insertions(+), 356 deletions(-) diff --git a/selene-lib/src/lib.rs b/selene-lib/src/lib.rs index 0a7f4bf8..bf21fe6e 100644 --- a/selene-lib/src/lib.rs +++ b/selene-lib/src/lib.rs @@ -1,336 +1,332 @@ -#![recursion_limit = "1000"] -#![cfg_attr( - feature = "force_exhaustive_checks", - feature(non_exhaustive_omitted_patterns_lint) -)] -use std::{collections::HashMap, error::Error, fmt}; - -use full_moon::ast::Ast; -use serde::{ - de::{DeserializeOwned, Deserializer}, - Deserialize, -}; - -mod ast_util; -mod lint_filtering; -pub mod lints; -mod possible_std; -pub mod standard_library; -mod text; - -#[cfg(test)] -mod test_util; - -#[cfg(test)] -mod test_full_runs; - -use lints::{AstContext, Context, Diagnostic, Lint, Severity}; -use standard_library::StandardLibrary; - -#[derive(Debug)] -pub struct CheckerError { - pub name: &'static str, - pub problem: CheckerErrorProblem, -} - -#[derive(Debug)] -pub enum CheckerErrorProblem { - ConfigDeserializeError(Box), - LintNewError(Box), -} - -impl fmt::Display for CheckerError { - fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - use CheckerErrorProblem::*; - - write!(formatter, "[{}] ", self.name)?; - - match &self.problem { - ConfigDeserializeError(error) => write!( - formatter, - "Configuration was incorrectly formatted: {error}" - ), - LintNewError(error) => write!(formatter, "{error}"), - } - } -} - -impl Error for CheckerError {} - -#[derive(Deserialize)] -#[serde(default)] -#[serde(rename_all = "kebab-case")] -#[serde(deny_unknown_fields)] -pub struct CheckerConfig { - pub config: HashMap, - #[serde(alias = "rules")] - pub lints: HashMap, - pub std: Option, - pub exclude: Vec, - - // Not locked behind Roblox feature so that selene.toml for Roblox will - // run even without it. - pub roblox_std_source: RobloxStdSource, -} - -impl CheckerConfig { - pub fn std(&self) -> &str { - self.std.as_deref().unwrap_or("lua51") - } -} - -impl Default for CheckerConfig { - fn default() -> Self { - CheckerConfig { - config: HashMap::new(), - lints: HashMap::new(), - std: None, - exclude: Vec::new(), - - roblox_std_source: RobloxStdSource::default(), - } - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub enum LintVariation { - Allow, - Deny, - Warn, -} - -impl LintVariation { - pub fn to_severity(self) -> Severity { - match self { - LintVariation::Allow => Severity::Allow, - LintVariation::Deny => Severity::Error, - LintVariation::Warn => Severity::Warning, - } - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub enum RobloxStdSource { - Floating, - Pinned, -} - -impl Default for RobloxStdSource { - fn default() -> Self { - Self::Floating - } -} - -macro_rules! use_lints { - { - $( - $lint_name:ident: $lint_path:ty, - )+ - - $( - #[$meta:meta] - { - $($meta_lint_name:ident: $meta_lint_path:ty,)+ - }, - )+ - } => { - lazy_static::lazy_static! { - static ref ALL_LINTS: Vec<&'static str> = vec![ - $( - stringify!($lint_name), - )+ - - $( - $( - #[$meta] - stringify!($meta_lint_name), - )+ - )+ - ]; - } - - pub struct Checker { - config: CheckerConfig, - context: Context, - - $( - $lint_name: $lint_path, - )+ - - $( - $( - #[$meta] - $meta_lint_name: $meta_lint_path, - )+ - )+ - } - - impl Checker { - // TODO: Be more strict about config? Make sure all keys exist - pub fn new( - mut config: CheckerConfig, - standard_library: StandardLibrary, - ) -> Result where V: for<'de> Deserializer<'de> { - macro_rules! lint_field { - ($name:ident, $path:ty) => {{ - let lint_name = stringify!($name); - - let lint = <$path>::new({ - match config.config.remove(lint_name) { - Some(entry_generic) => { - <$path as Lint>::Config::deserialize(entry_generic).map_err(|error| { - CheckerError { - name: lint_name, - problem: CheckerErrorProblem::ConfigDeserializeError(Box::new(error)), - } - })? - } - - None => { - <$path as Lint>::Config::default() - } - } - }).map_err(|error| { - CheckerError { - name: stringify!($name), - problem: CheckerErrorProblem::LintNewError(Box::new(error)), - } - })?; - - lint - }}; - } - - Ok(Self { - $( - $lint_name: { - lint_field!($lint_name, $lint_path) - }, - )+ - $( - $( - #[$meta] - $meta_lint_name: { - lint_field!($meta_lint_name, $meta_lint_path) - }, - )+ - )+ - - context: Context { - standard_library, - user_set_standard_library: config.std.as_ref().map(|std_text| { - std_text.split('+').map(ToOwned::to_owned).collect() - }), - }, - - config, - }) - } - - pub fn test_on(&self, ast: &Ast) -> Vec { - let mut diagnostics = Vec::new(); - - let ast_context = AstContext::from_ast(ast); - - macro_rules! check_lint { - ($name:ident) => { - let lint = &self.$name; - - let lint_pass = { - profiling::scope!(&format!("lint: {}", stringify!($name))); - lint.pass(ast, &self.context, &ast_context) - }; - - diagnostics.extend(&mut lint_pass.into_iter().map(|diagnostic| { - CheckerDiagnostic { - diagnostic, - severity: self.get_lint_severity(lint, stringify!($name)), - } - })); - }; - } - - $( - check_lint!($lint_name); - )+ - - $( - $( - #[$meta] - { - check_lint!($meta_lint_name); - } - )+ - )+ - - diagnostics = lint_filtering::filter_diagnostics( - ast, - diagnostics, - self.get_lint_severity(&self.invalid_lint_filter, "invalid_lint_filter"), - ); - - diagnostics - } - - fn get_lint_severity(&self, _lint: &R, name: &'static str) -> Severity { - match self.config.lints.get(name) { - Some(variation) => variation.to_severity(), - None => R::SEVERITY, - } - } - } - }; -} - -#[derive(Debug)] -pub struct CheckerDiagnostic { - pub diagnostic: Diagnostic, - pub severity: Severity, -} - -pub fn lint_exists(name: &str) -> bool { - ALL_LINTS.contains(&name) -} - -use_lints! { - almost_swapped: lints::almost_swapped::AlmostSwappedLint, - bad_string_escape: lints::bad_string_escape::BadStringEscapeLint, - compare_nan: lints::compare_nan::CompareNanLint, - constant_table_comparison: lints::constant_table_comparison::ConstantTableComparisonLint, - deprecated: lints::deprecated::DeprecatedLint, - divide_by_zero: lints::divide_by_zero::DivideByZeroLint, - duplicate_keys: lints::duplicate_keys::DuplicateKeysLint, - empty_if: lints::empty_if::EmptyIfLint, - empty_loop: lints::empty_loop::EmptyLoopLint, - global_usage: lints::global_usage::GlobalLint, - high_cyclomatic_complexity: lints::high_cyclomatic_complexity::HighCyclomaticComplexityLint, - if_same_then_else: lints::if_same_then_else::IfSameThenElseLint, - ifs_same_cond: lints::ifs_same_cond::IfsSameCondLint, - incorrect_standard_library_use: lints::standard_library::StandardLibraryLint, - invalid_lint_filter: lints::invalid_lint_filter::InvalidLintFilterLint, - manual_table_clone: lints::manual_table_clone::ManualTableCloneLint, - mismatched_arg_count: lints::mismatched_arg_count::MismatchedArgCountLint, - mixed_table: lints::mixed_table::MixedTableLint, - multiple_statements: lints::multiple_statements::MultipleStatementsLint, - must_use: lints::must_use::MustUseLint, - parenthese_conditions: lints::parenthese_conditions::ParentheseConditionsLint, - restricted_module_paths: lints::restricted_module_paths::RestrictedModulePathsLint, - shadowing: lints::shadowing::ShadowingLint, - suspicious_reverse_loop: lints::suspicious_reverse_loop::SuspiciousReverseLoopLint, - type_check_inside_call: lints::type_check_inside_call::TypeCheckInsideCallLint, - unbalanced_assignments: lints::unbalanced_assignments::UnbalancedAssignmentsLint, - undefined_variable: lints::undefined_variable::UndefinedVariableLint, - unscoped_variables: lints::unscoped_variables::UnscopedVariablesLint, - unused_variable: lints::unused_variable::UnusedVariableLint, - - #[cfg(feature = "roblox")] - { - roblox_incorrect_color3_new_bounds: lints::roblox_incorrect_color3_new_bounds::Color3BoundsLint, - roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint, - roblox_react_exhaustive_deps: lints::roblox_react_exhaustive_deps::ReactExhaustiveDepsLint, - roblox_manual_fromscale_or_fromoffset: lints::roblox_manual_fromscale_or_fromoffset::ManualFromScaleOrFromOffsetLint, - roblox_suspicious_udim2_new: lints::roblox_suspicious_udim2_new::SuspiciousUDim2NewLint, - }, -} +#![recursion_limit = "1000"] +#![cfg_attr( + feature = "force_exhaustive_checks", + feature(non_exhaustive_omitted_patterns_lint) +)] +use std::{collections::HashMap, error::Error, fmt}; + +use full_moon::ast::Ast; +use serde::{ + de::{DeserializeOwned, Deserializer}, + Deserialize, +}; + +mod ast_util; +mod lint_filtering; +pub mod lints; +mod possible_std; +pub mod standard_library; +mod text; + +#[cfg(test)] +mod test_util; + +#[cfg(test)] +mod test_full_runs; + +use lints::{AstContext, Context, Diagnostic, Lint, Severity}; +use standard_library::StandardLibrary; + +#[derive(Debug)] +pub struct CheckerError { + pub name: &'static str, + pub problem: CheckerErrorProblem, +} + +#[derive(Debug)] +pub enum CheckerErrorProblem { + ConfigDeserializeError(Box), + LintNewError(Box), +} + +impl fmt::Display for CheckerError { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + use CheckerErrorProblem::*; + + write!(formatter, "[{}] ", self.name)?; + + match &self.problem { + ConfigDeserializeError(error) => write!( + formatter, + "Configuration was incorrectly formatted: {error}" + ), + LintNewError(error) => write!(formatter, "{error}"), + } + } +} + +impl Error for CheckerError {} + +#[derive(Deserialize)] +#[serde(default)] +#[serde(rename_all = "kebab-case")] +#[serde(deny_unknown_fields)] +pub struct CheckerConfig { + pub config: HashMap, + #[serde(alias = "rules")] + pub lints: HashMap, + pub std: Option, + pub exclude: Vec, + + // Not locked behind Roblox feature so that selene.toml for Roblox will + // run even without it. + pub roblox_std_source: RobloxStdSource, +} + +impl CheckerConfig { + pub fn std(&self) -> &str { + self.std.as_deref().unwrap_or("lua51") + } +} + +impl Default for CheckerConfig { + fn default() -> Self { + CheckerConfig { + config: HashMap::new(), + lints: HashMap::new(), + std: None, + exclude: Vec::new(), + + roblox_std_source: RobloxStdSource::default(), + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum LintVariation { + Allow, + Deny, + Warn, +} + +impl LintVariation { + pub fn to_severity(self) -> Severity { + match self { + LintVariation::Allow => Severity::Allow, + LintVariation::Deny => Severity::Error, + LintVariation::Warn => Severity::Warning, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "kebab-case")] +#[derive(Default)] +pub enum RobloxStdSource { + #[default] + Floating, + Pinned, +} + +macro_rules! use_lints { + { + $( + $lint_name:ident: $lint_path:ty, + )+ + + $( + #[$meta:meta] + { + $($meta_lint_name:ident: $meta_lint_path:ty,)+ + }, + )+ + } => { + lazy_static::lazy_static! { + static ref ALL_LINTS: Vec<&'static str> = vec![ + $( + stringify!($lint_name), + )+ + + $( + $( + #[$meta] + stringify!($meta_lint_name), + )+ + )+ + ]; + } + + pub struct Checker { + config: CheckerConfig, + context: Context, + + $( + $lint_name: $lint_path, + )+ + + $( + $( + #[$meta] + $meta_lint_name: $meta_lint_path, + )+ + )+ + } + + impl Checker { + // TODO: Be more strict about config? Make sure all keys exist + pub fn new( + mut config: CheckerConfig, + standard_library: StandardLibrary, + ) -> Result where V: for<'de> Deserializer<'de> { + macro_rules! lint_field { + ($name:ident, $path:ty) => {{ + let lint_name = stringify!($name); + + let lint = <$path>::new({ + match config.config.remove(lint_name) { + Some(entry_generic) => { + <$path as Lint>::Config::deserialize(entry_generic).map_err(|error| { + CheckerError { + name: lint_name, + problem: CheckerErrorProblem::ConfigDeserializeError(Box::new(error)), + } + })? + } + + None => { + <$path as Lint>::Config::default() + } + } + }).map_err(|error| { + CheckerError { + name: stringify!($name), + problem: CheckerErrorProblem::LintNewError(Box::new(error)), + } + })?; + + lint + }}; + } + + Ok(Self { + $( + $lint_name: { + lint_field!($lint_name, $lint_path) + }, + )+ + $( + $( + #[$meta] + $meta_lint_name: { + lint_field!($meta_lint_name, $meta_lint_path) + }, + )+ + )+ + + context: Context { + standard_library, + user_set_standard_library: config.std.as_ref().map(|std_text| { + std_text.split('+').map(ToOwned::to_owned).collect() + }), + }, + + config, + }) + } + + pub fn test_on(&self, ast: &Ast) -> Vec { + let mut diagnostics = Vec::new(); + + let ast_context = AstContext::from_ast(ast); + + macro_rules! check_lint { + ($name:ident) => { + let lint = &self.$name; + + let lint_pass = { + profiling::scope!(&format!("lint: {}", stringify!($name))); + lint.pass(ast, &self.context, &ast_context) + }; + + diagnostics.extend(&mut lint_pass.into_iter().map(|diagnostic| { + CheckerDiagnostic { + diagnostic, + severity: self.get_lint_severity(lint, stringify!($name)), + } + })); + }; + } + + $( + check_lint!($lint_name); + )+ + + $( + $( + #[$meta] + { + check_lint!($meta_lint_name); + } + )+ + )+ + + diagnostics = lint_filtering::filter_diagnostics( + ast, + diagnostics, + self.get_lint_severity(&self.invalid_lint_filter, "invalid_lint_filter"), + ); + + diagnostics + } + + fn get_lint_severity(&self, _lint: &R, name: &'static str) -> Severity { + match self.config.lints.get(name) { + Some(variation) => variation.to_severity(), + None => R::SEVERITY, + } + } + } + }; +} + +#[derive(Debug)] +pub struct CheckerDiagnostic { + pub diagnostic: Diagnostic, + pub severity: Severity, +} + +pub fn lint_exists(name: &str) -> bool { + ALL_LINTS.contains(&name) +} + +use_lints! { + almost_swapped: lints::almost_swapped::AlmostSwappedLint, + bad_string_escape: lints::bad_string_escape::BadStringEscapeLint, + compare_nan: lints::compare_nan::CompareNanLint, + constant_table_comparison: lints::constant_table_comparison::ConstantTableComparisonLint, + deprecated: lints::deprecated::DeprecatedLint, + divide_by_zero: lints::divide_by_zero::DivideByZeroLint, + duplicate_keys: lints::duplicate_keys::DuplicateKeysLint, + empty_if: lints::empty_if::EmptyIfLint, + empty_loop: lints::empty_loop::EmptyLoopLint, + global_usage: lints::global_usage::GlobalLint, + high_cyclomatic_complexity: lints::high_cyclomatic_complexity::HighCyclomaticComplexityLint, + if_same_then_else: lints::if_same_then_else::IfSameThenElseLint, + ifs_same_cond: lints::ifs_same_cond::IfsSameCondLint, + incorrect_standard_library_use: lints::standard_library::StandardLibraryLint, + invalid_lint_filter: lints::invalid_lint_filter::InvalidLintFilterLint, + manual_table_clone: lints::manual_table_clone::ManualTableCloneLint, + mismatched_arg_count: lints::mismatched_arg_count::MismatchedArgCountLint, + mixed_table: lints::mixed_table::MixedTableLint, + multiple_statements: lints::multiple_statements::MultipleStatementsLint, + must_use: lints::must_use::MustUseLint, + parenthese_conditions: lints::parenthese_conditions::ParentheseConditionsLint, + restricted_module_paths: lints::restricted_module_paths::RestrictedModulePathsLint, + shadowing: lints::shadowing::ShadowingLint, + suspicious_reverse_loop: lints::suspicious_reverse_loop::SuspiciousReverseLoopLint, + type_check_inside_call: lints::type_check_inside_call::TypeCheckInsideCallLint, + unbalanced_assignments: lints::unbalanced_assignments::UnbalancedAssignmentsLint, + undefined_variable: lints::undefined_variable::UndefinedVariableLint, + unscoped_variables: lints::unscoped_variables::UnscopedVariablesLint, + unused_variable: lints::unused_variable::UnusedVariableLint, + + #[cfg(feature = "roblox")] + { + roblox_incorrect_color3_new_bounds: lints::roblox_incorrect_color3_new_bounds::Color3BoundsLint, + roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint, + roblox_react_exhaustive_deps: lints::roblox_react_exhaustive_deps::ReactExhaustiveDepsLint, + roblox_manual_fromscale_or_fromoffset: lints::roblox_manual_fromscale_or_fromoffset::ManualFromScaleOrFromOffsetLint, + roblox_suspicious_udim2_new: lints::roblox_suspicious_udim2_new::SuspiciousUDim2NewLint, + }, +} diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs index 13bf4593..6f3c4d43 100644 --- a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -50,6 +50,7 @@ struct ReactExhaustiveDepsVisitor<'a> { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(clippy::enum_variant_names)] enum HookType { UseEffect, UseCallback, @@ -363,14 +364,9 @@ fn visit_block_for_variables( variables.extend(collect_variables_from_stmt(stmt, scope_id, scope_manager)); } - if let Some(last_stmt) = block.last_stmt() { - match last_stmt { - ast::LastStmt::Return(return_stmt) => { - for expr in return_stmt.returns() { - variables.extend(collect_referenced_variables(expr, 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)); } } @@ -589,16 +585,13 @@ fn collect_variables_from_function_call( } _ => {} }, - ast::Suffix::Index(index) => match index { - ast::Index::Brackets { expression, .. } => { - variables.extend(collect_referenced_variables( - expression, - scope_id, - scope_manager, - )); - } - _ => {} - }, + ast::Suffix::Index(ast::Index::Brackets { expression, .. }) => { + variables.extend(collect_referenced_variables( + expression, + scope_id, + scope_manager, + )); + } _ => {} } } @@ -661,7 +654,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { // Extract the function body to analyze let callback_body = match strip_parentheses(callback) { - ast::Expression::Function(func) => func.1.block(), + ast::Expression::Function(func) => func.body().block(), _ => return, // Not a function literal }; @@ -733,7 +726,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { // Report if there are issues if !missing.is_empty() || !unnecessary.is_empty() { - let deps_range = deps_arg.map(|expr| range(expr)); + let deps_range = deps_arg.map(range); let mut message = format!("React Hook {} has", hook_type.name()); if !missing.is_empty() { From 60703f91a51d40b40df6811b94e655a0b34fd2ad Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:43:45 -0800 Subject: [PATCH 06/10] address PR feedback --- docs/src/lints/roblox_react_exhaustive_deps.md | 4 ++-- selene-lib/src/lints/roblox_react_exhaustive_deps.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/src/lints/roblox_react_exhaustive_deps.md b/docs/src/lints/roblox_react_exhaustive_deps.md index 882eb505..a14dd6ee 100644 --- a/docs/src/lints/roblox_react_exhaustive_deps.md +++ b/docs/src/lints/roblox_react_exhaustive_deps.md @@ -68,6 +68,8 @@ This lint checks the following React/Roact hooks: ## 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 @@ -81,8 +83,6 @@ The lint analyzes variable references within the hook callback and compares them - 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) -This lint is only active when using the Roblox standard library. - ## Configuration This lint does not have any configuration options. diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs index ea86dc5e..e516119f 100644 --- a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -721,7 +721,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { if !missing.is_empty() || !unnecessary.is_empty() { let deps_range = deps_arg.map(range); - let mut message = format!("React Hook {} has", hook_type.name()); + let mut message = format!("react hook {} has", hook_type.name()); if !missing.is_empty() { message.push_str(&format!( " missing dependenc{}: {}", @@ -745,7 +745,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { let mut notes = Vec::new(); if !missing.is_empty() { notes.push(format!( - "Either include {} in the dependency array or remove the dependency", + "help: either include {} in the dependency array or remove the dependency", if missing.len() == 1 { format!("'{}'", missing[0]) } else { @@ -755,7 +755,7 @@ impl<'a> Visitor for ReactExhaustiveDepsVisitor<'a> { } if !unnecessary.is_empty() { notes.push(format!( - "Remove {} from the dependency array", + "help: remove {} from the dependency array", if unnecessary.len() == 1 { format!("'{}'", unnecessary[0]) } else { From de5e1339016273be71782cc05b5238bc4ef80250 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:44:36 -0800 Subject: [PATCH 07/10] update the stderr stuff --- .../correct_dependencies.stderr | 8 +++---- .../missing_dependencies.stderr | 24 +++++++++---------- .../property_access.stderr | 8 +++---- .../unnecessary_dependencies.stderr | 16 ++++++------- .../use_callback.stderr | 14 +++++------ .../use_memo.stderr | 12 +++++----- 6 files changed, 41 insertions(+), 41 deletions(-) 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 index 639da366..d8e38d6b 100644 --- a/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.stderr +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/correct_dependencies.stderr @@ -1,4 +1,4 @@ -error[roblox_react_exhaustive_deps]: React Hook useEffect has missing dependency: props.value +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.value ┌─ correct_dependencies.lua:33:21 │ 33 │ React.useEffect(function() @@ -7,13 +7,13 @@ error[roblox_react_exhaustive_deps]: React Hook useEffect has missing dependency 35 │ │ end) │ ╰───────^ │ - = Either include 'props.value' in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: t ┌─ correct_dependencies.lua:52:10 │ 52 │ end, {}) │ ^^ │ - = Either include 't' in the dependency array or remove the dependency + = help: either include 't' in the dependency array or remove the dependency 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 index 0cb92ac4..b7cccd0a 100644 --- a/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.stderr +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/missing_dependencies.stderr @@ -1,48 +1,48 @@ -error[roblox_react_exhaustive_deps]: React Hook useEffect has missing dependency: props.value +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.value ┌─ missing_dependencies.lua:11:10 │ 11 │ end, {}) │ ^^ │ - = Either include 'props.value' in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: count, props.name, props.value ┌─ missing_dependencies.lua:22:10 │ 22 │ end, {}) │ ^^ │ - = Either include them (count, props.name, props.value) in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: name, props.value ┌─ missing_dependencies.lua:34:10 │ 34 │ end, { count }) │ ^^^^^^^^^ │ - = Either include them (name, props.value) in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: spawn ┌─ missing_dependencies.lua:44:10 │ 44 │ end, {}) │ ^^ │ - = Either include 'spawn' in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.data.value ┌─ missing_dependencies.lua:51:10 │ 51 │ end, {}) │ ^^ │ - = Either include 'props.data.value' in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.value ┌─ missing_dependencies.lua:62:10 │ 62 │ end, {}) │ ^^ │ - = Either include 'props.value' in the dependency array or remove the dependency + = help: either include 'props.value' in the dependency array or remove the dependency 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 index ff56c03b..4d24f962 100644 --- a/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.stderr +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/property_access.stderr @@ -1,16 +1,16 @@ -error[roblox_react_exhaustive_deps]: React Hook useEffect has missing dependencies: props.user.age, props.user.name +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependencies: props.user.age, props.user.name ┌─ property_access.lua:19:10 │ 19 │ end, {}) │ ^^ │ - = Either include them (props.user.age, props.user.name) in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has missing dependency: props.data.value ┌─ property_access.lua:36:10 │ 36 │ end, { count }) │ ^^^^^^^^^ │ - = Either include 'props.data.value' in the dependency array or remove the dependency + = 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.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr index 5091781f..bb86b3ca 100644 --- a/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/unnecessary_dependencies.stderr @@ -1,32 +1,32 @@ -error[roblox_react_exhaustive_deps]: React Hook useEffect has unnecessary dependency: props.value +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependency: props.value ┌─ unnecessary_dependencies.lua:11:10 │ 11 │ end, { props.value }) │ ^^^^^^^^^^^^^^^ │ - = Remove 'props.value' from the dependency array + = 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 +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 }) │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ │ - = Remove them (props.id, props.name, props.value) from the dependency array + = 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 +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependency: props.value ┌─ unnecessary_dependencies.lua:27:10 │ 27 │ end, { count, props.value }) │ ^^^^^^^^^^^^^^^^^^^^^^ │ - = Remove 'props.value' from the dependency array + = help: remove 'props.value' from the dependency array -error[roblox_react_exhaustive_deps]: React Hook useEffect has unnecessary dependencies: a, b +error[roblox_react_exhaustive_deps]: react hook useEffect has unnecessary dependencies: a, b ┌─ unnecessary_dependencies.lua:37:10 │ 37 │ end, { a, b }) │ ^^^^^^^^ │ - = Remove them (a, b) from the dependency array + = help: remove them (a, b) from the dependency array 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 index 3ff54303..63caace7 100644 --- a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.stderr +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_callback.stderr @@ -1,25 +1,25 @@ -error[roblox_react_exhaustive_deps]: React Hook useCallback has missing dependency: props.value +error[roblox_react_exhaustive_deps]: react hook useCallback has missing dependency: props.value ┌─ use_callback.lua:11:10 │ 11 │ end, {}) │ ^^ │ - = Either include 'props.value' in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useCallback has unnecessary dependency: value ┌─ use_callback.lua:30:10 │ 30 │ end, { value }) │ ^^^^^^^^^ │ - = Remove 'value' from the dependency array + = 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 +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 }) │ ^^^^^^^^^^ │ - = Either include them (count, props.value) in the dependency array or remove the dependency - = Remove 'unused' from the dependency array + = 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.stderr b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr index dcf3d66e..a7aad4ca 100644 --- a/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr +++ b/selene-lib/tests/lints/roblox_react_exhaustive_deps/use_memo.stderr @@ -1,24 +1,24 @@ -error[roblox_react_exhaustive_deps]: React Hook useMemo has missing dependency: props.value +error[roblox_react_exhaustive_deps]: react hook useMemo has missing dependency: props.value ┌─ use_memo.lua:11:10 │ 11 │ end, {}) │ ^^ │ - = Either include 'props.value' in the dependency array or remove the dependency + = 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 +error[roblox_react_exhaustive_deps]: react hook useMemo has unnecessary dependency: unused ┌─ use_memo.lua:29:10 │ 29 │ end, { unused }) │ ^^^^^^^^^^ │ - = Remove 'unused' from the dependency array + = help: remove 'unused' from the dependency array -error[roblox_react_exhaustive_deps]: React Hook useMemo has missing dependencies: props.multiplier, props.value, result +error[roblox_react_exhaustive_deps]: react hook useMemo has missing dependencies: props.multiplier, props.value, result ┌─ use_memo.lua:39:10 │ 39 │ end, { base }) │ ^^^^^^^^ │ - = Either include them (props.multiplier, props.value, result) in the dependency array or remove the dependency + = help: either include them (props.multiplier, props.value, result) in the dependency array or remove the dependency From 8dba752c550102683cd5af9996fecb50ac6624ad Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:47:11 -0800 Subject: [PATCH 08/10] Discard changes to selene-lib/src/lib.rs --- selene-lib/src/lib.rs | 667 +++++++++++++++++++++--------------------- 1 file changed, 335 insertions(+), 332 deletions(-) diff --git a/selene-lib/src/lib.rs b/selene-lib/src/lib.rs index bf21fe6e..6a3952ae 100644 --- a/selene-lib/src/lib.rs +++ b/selene-lib/src/lib.rs @@ -1,332 +1,335 @@ -#![recursion_limit = "1000"] -#![cfg_attr( - feature = "force_exhaustive_checks", - feature(non_exhaustive_omitted_patterns_lint) -)] -use std::{collections::HashMap, error::Error, fmt}; - -use full_moon::ast::Ast; -use serde::{ - de::{DeserializeOwned, Deserializer}, - Deserialize, -}; - -mod ast_util; -mod lint_filtering; -pub mod lints; -mod possible_std; -pub mod standard_library; -mod text; - -#[cfg(test)] -mod test_util; - -#[cfg(test)] -mod test_full_runs; - -use lints::{AstContext, Context, Diagnostic, Lint, Severity}; -use standard_library::StandardLibrary; - -#[derive(Debug)] -pub struct CheckerError { - pub name: &'static str, - pub problem: CheckerErrorProblem, -} - -#[derive(Debug)] -pub enum CheckerErrorProblem { - ConfigDeserializeError(Box), - LintNewError(Box), -} - -impl fmt::Display for CheckerError { - fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - use CheckerErrorProblem::*; - - write!(formatter, "[{}] ", self.name)?; - - match &self.problem { - ConfigDeserializeError(error) => write!( - formatter, - "Configuration was incorrectly formatted: {error}" - ), - LintNewError(error) => write!(formatter, "{error}"), - } - } -} - -impl Error for CheckerError {} - -#[derive(Deserialize)] -#[serde(default)] -#[serde(rename_all = "kebab-case")] -#[serde(deny_unknown_fields)] -pub struct CheckerConfig { - pub config: HashMap, - #[serde(alias = "rules")] - pub lints: HashMap, - pub std: Option, - pub exclude: Vec, - - // Not locked behind Roblox feature so that selene.toml for Roblox will - // run even without it. - pub roblox_std_source: RobloxStdSource, -} - -impl CheckerConfig { - pub fn std(&self) -> &str { - self.std.as_deref().unwrap_or("lua51") - } -} - -impl Default for CheckerConfig { - fn default() -> Self { - CheckerConfig { - config: HashMap::new(), - lints: HashMap::new(), - std: None, - exclude: Vec::new(), - - roblox_std_source: RobloxStdSource::default(), - } - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub enum LintVariation { - Allow, - Deny, - Warn, -} - -impl LintVariation { - pub fn to_severity(self) -> Severity { - match self { - LintVariation::Allow => Severity::Allow, - LintVariation::Deny => Severity::Error, - LintVariation::Warn => Severity::Warning, - } - } -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] -#[serde(rename_all = "kebab-case")] -#[derive(Default)] -pub enum RobloxStdSource { - #[default] - Floating, - Pinned, -} - -macro_rules! use_lints { - { - $( - $lint_name:ident: $lint_path:ty, - )+ - - $( - #[$meta:meta] - { - $($meta_lint_name:ident: $meta_lint_path:ty,)+ - }, - )+ - } => { - lazy_static::lazy_static! { - static ref ALL_LINTS: Vec<&'static str> = vec![ - $( - stringify!($lint_name), - )+ - - $( - $( - #[$meta] - stringify!($meta_lint_name), - )+ - )+ - ]; - } - - pub struct Checker { - config: CheckerConfig, - context: Context, - - $( - $lint_name: $lint_path, - )+ - - $( - $( - #[$meta] - $meta_lint_name: $meta_lint_path, - )+ - )+ - } - - impl Checker { - // TODO: Be more strict about config? Make sure all keys exist - pub fn new( - mut config: CheckerConfig, - standard_library: StandardLibrary, - ) -> Result where V: for<'de> Deserializer<'de> { - macro_rules! lint_field { - ($name:ident, $path:ty) => {{ - let lint_name = stringify!($name); - - let lint = <$path>::new({ - match config.config.remove(lint_name) { - Some(entry_generic) => { - <$path as Lint>::Config::deserialize(entry_generic).map_err(|error| { - CheckerError { - name: lint_name, - problem: CheckerErrorProblem::ConfigDeserializeError(Box::new(error)), - } - })? - } - - None => { - <$path as Lint>::Config::default() - } - } - }).map_err(|error| { - CheckerError { - name: stringify!($name), - problem: CheckerErrorProblem::LintNewError(Box::new(error)), - } - })?; - - lint - }}; - } - - Ok(Self { - $( - $lint_name: { - lint_field!($lint_name, $lint_path) - }, - )+ - $( - $( - #[$meta] - $meta_lint_name: { - lint_field!($meta_lint_name, $meta_lint_path) - }, - )+ - )+ - - context: Context { - standard_library, - user_set_standard_library: config.std.as_ref().map(|std_text| { - std_text.split('+').map(ToOwned::to_owned).collect() - }), - }, - - config, - }) - } - - pub fn test_on(&self, ast: &Ast) -> Vec { - let mut diagnostics = Vec::new(); - - let ast_context = AstContext::from_ast(ast); - - macro_rules! check_lint { - ($name:ident) => { - let lint = &self.$name; - - let lint_pass = { - profiling::scope!(&format!("lint: {}", stringify!($name))); - lint.pass(ast, &self.context, &ast_context) - }; - - diagnostics.extend(&mut lint_pass.into_iter().map(|diagnostic| { - CheckerDiagnostic { - diagnostic, - severity: self.get_lint_severity(lint, stringify!($name)), - } - })); - }; - } - - $( - check_lint!($lint_name); - )+ - - $( - $( - #[$meta] - { - check_lint!($meta_lint_name); - } - )+ - )+ - - diagnostics = lint_filtering::filter_diagnostics( - ast, - diagnostics, - self.get_lint_severity(&self.invalid_lint_filter, "invalid_lint_filter"), - ); - - diagnostics - } - - fn get_lint_severity(&self, _lint: &R, name: &'static str) -> Severity { - match self.config.lints.get(name) { - Some(variation) => variation.to_severity(), - None => R::SEVERITY, - } - } - } - }; -} - -#[derive(Debug)] -pub struct CheckerDiagnostic { - pub diagnostic: Diagnostic, - pub severity: Severity, -} - -pub fn lint_exists(name: &str) -> bool { - ALL_LINTS.contains(&name) -} - -use_lints! { - almost_swapped: lints::almost_swapped::AlmostSwappedLint, - bad_string_escape: lints::bad_string_escape::BadStringEscapeLint, - compare_nan: lints::compare_nan::CompareNanLint, - constant_table_comparison: lints::constant_table_comparison::ConstantTableComparisonLint, - deprecated: lints::deprecated::DeprecatedLint, - divide_by_zero: lints::divide_by_zero::DivideByZeroLint, - duplicate_keys: lints::duplicate_keys::DuplicateKeysLint, - empty_if: lints::empty_if::EmptyIfLint, - empty_loop: lints::empty_loop::EmptyLoopLint, - global_usage: lints::global_usage::GlobalLint, - high_cyclomatic_complexity: lints::high_cyclomatic_complexity::HighCyclomaticComplexityLint, - if_same_then_else: lints::if_same_then_else::IfSameThenElseLint, - ifs_same_cond: lints::ifs_same_cond::IfsSameCondLint, - incorrect_standard_library_use: lints::standard_library::StandardLibraryLint, - invalid_lint_filter: lints::invalid_lint_filter::InvalidLintFilterLint, - manual_table_clone: lints::manual_table_clone::ManualTableCloneLint, - mismatched_arg_count: lints::mismatched_arg_count::MismatchedArgCountLint, - mixed_table: lints::mixed_table::MixedTableLint, - multiple_statements: lints::multiple_statements::MultipleStatementsLint, - must_use: lints::must_use::MustUseLint, - parenthese_conditions: lints::parenthese_conditions::ParentheseConditionsLint, - restricted_module_paths: lints::restricted_module_paths::RestrictedModulePathsLint, - shadowing: lints::shadowing::ShadowingLint, - suspicious_reverse_loop: lints::suspicious_reverse_loop::SuspiciousReverseLoopLint, - type_check_inside_call: lints::type_check_inside_call::TypeCheckInsideCallLint, - unbalanced_assignments: lints::unbalanced_assignments::UnbalancedAssignmentsLint, - undefined_variable: lints::undefined_variable::UndefinedVariableLint, - unscoped_variables: lints::unscoped_variables::UnscopedVariablesLint, - unused_variable: lints::unused_variable::UnusedVariableLint, - - #[cfg(feature = "roblox")] - { - roblox_incorrect_color3_new_bounds: lints::roblox_incorrect_color3_new_bounds::Color3BoundsLint, - roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint, - roblox_react_exhaustive_deps: lints::roblox_react_exhaustive_deps::ReactExhaustiveDepsLint, - roblox_manual_fromscale_or_fromoffset: lints::roblox_manual_fromscale_or_fromoffset::ManualFromScaleOrFromOffsetLint, - roblox_suspicious_udim2_new: lints::roblox_suspicious_udim2_new::SuspiciousUDim2NewLint, - }, -} +#![recursion_limit = "1000"] +#![cfg_attr( + feature = "force_exhaustive_checks", + feature(non_exhaustive_omitted_patterns_lint) +)] +use std::{collections::HashMap, error::Error, fmt}; + +use full_moon::ast::Ast; +use serde::{ + de::{DeserializeOwned, Deserializer}, + Deserialize, +}; + +mod ast_util; +mod lint_filtering; +pub mod lints; +mod possible_std; +pub mod standard_library; +mod text; + +#[cfg(test)] +mod test_util; + +#[cfg(test)] +mod test_full_runs; + +use lints::{AstContext, Context, Diagnostic, Lint, Severity}; +use standard_library::StandardLibrary; + +#[derive(Debug)] +pub struct CheckerError { + pub name: &'static str, + pub problem: CheckerErrorProblem, +} + +#[derive(Debug)] +pub enum CheckerErrorProblem { + ConfigDeserializeError(Box), + LintNewError(Box), +} + +impl fmt::Display for CheckerError { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + use CheckerErrorProblem::*; + + write!(formatter, "[{}] ", self.name)?; + + match &self.problem { + ConfigDeserializeError(error) => write!( + formatter, + "Configuration was incorrectly formatted: {error}" + ), + LintNewError(error) => write!(formatter, "{error}"), + } + } +} + +impl Error for CheckerError {} + +#[derive(Deserialize)] +#[serde(default)] +#[serde(rename_all = "kebab-case")] +#[serde(deny_unknown_fields)] +pub struct CheckerConfig { + pub config: HashMap, + #[serde(alias = "rules")] + pub lints: HashMap, + pub std: Option, + pub exclude: Vec, + + // Not locked behind Roblox feature so that selene.toml for Roblox will + // run even without it. + pub roblox_std_source: RobloxStdSource, +} + +impl CheckerConfig { + pub fn std(&self) -> &str { + self.std.as_deref().unwrap_or("lua51") + } +} + +impl Default for CheckerConfig { + fn default() -> Self { + CheckerConfig { + config: HashMap::new(), + lints: HashMap::new(), + std: None, + exclude: Vec::new(), + + roblox_std_source: RobloxStdSource::default(), + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum LintVariation { + Allow, + Deny, + Warn, +} + +impl LintVariation { + pub fn to_severity(self) -> Severity { + match self { + LintVariation::Allow => Severity::Allow, + LintVariation::Deny => Severity::Error, + LintVariation::Warn => Severity::Warning, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum RobloxStdSource { + Floating, + Pinned, +} + +impl Default for RobloxStdSource { + fn default() -> Self { + Self::Floating + } +} + +macro_rules! use_lints { + { + $( + $lint_name:ident: $lint_path:ty, + )+ + + $( + #[$meta:meta] + { + $($meta_lint_name:ident: $meta_lint_path:ty,)+ + }, + )+ + } => { + lazy_static::lazy_static! { + static ref ALL_LINTS: Vec<&'static str> = vec![ + $( + stringify!($lint_name), + )+ + + $( + $( + #[$meta] + stringify!($meta_lint_name), + )+ + )+ + ]; + } + + pub struct Checker { + config: CheckerConfig, + context: Context, + + $( + $lint_name: $lint_path, + )+ + + $( + $( + #[$meta] + $meta_lint_name: $meta_lint_path, + )+ + )+ + } + + impl Checker { + // TODO: Be more strict about config? Make sure all keys exist + pub fn new( + mut config: CheckerConfig, + standard_library: StandardLibrary, + ) -> Result where V: for<'de> Deserializer<'de> { + macro_rules! lint_field { + ($name:ident, $path:ty) => {{ + let lint_name = stringify!($name); + + let lint = <$path>::new({ + match config.config.remove(lint_name) { + Some(entry_generic) => { + <$path as Lint>::Config::deserialize(entry_generic).map_err(|error| { + CheckerError { + name: lint_name, + problem: CheckerErrorProblem::ConfigDeserializeError(Box::new(error)), + } + })? + } + + None => { + <$path as Lint>::Config::default() + } + } + }).map_err(|error| { + CheckerError { + name: stringify!($name), + problem: CheckerErrorProblem::LintNewError(Box::new(error)), + } + })?; + + lint + }}; + } + + Ok(Self { + $( + $lint_name: { + lint_field!($lint_name, $lint_path) + }, + )+ + $( + $( + #[$meta] + $meta_lint_name: { + lint_field!($meta_lint_name, $meta_lint_path) + }, + )+ + )+ + + context: Context { + standard_library, + user_set_standard_library: config.std.as_ref().map(|std_text| { + std_text.split('+').map(ToOwned::to_owned).collect() + }), + }, + + config, + }) + } + + pub fn test_on(&self, ast: &Ast) -> Vec { + let mut diagnostics = Vec::new(); + + let ast_context = AstContext::from_ast(ast); + + macro_rules! check_lint { + ($name:ident) => { + let lint = &self.$name; + + let lint_pass = { + profiling::scope!(&format!("lint: {}", stringify!($name))); + lint.pass(ast, &self.context, &ast_context) + }; + + diagnostics.extend(&mut lint_pass.into_iter().map(|diagnostic| { + CheckerDiagnostic { + diagnostic, + severity: self.get_lint_severity(lint, stringify!($name)), + } + })); + }; + } + + $( + check_lint!($lint_name); + )+ + + $( + $( + #[$meta] + { + check_lint!($meta_lint_name); + } + )+ + )+ + + diagnostics = lint_filtering::filter_diagnostics( + ast, + diagnostics, + self.get_lint_severity(&self.invalid_lint_filter, "invalid_lint_filter"), + ); + + diagnostics + } + + fn get_lint_severity(&self, _lint: &R, name: &'static str) -> Severity { + match self.config.lints.get(name) { + Some(variation) => variation.to_severity(), + None => R::SEVERITY, + } + } + } + }; +} + +#[derive(Debug)] +pub struct CheckerDiagnostic { + pub diagnostic: Diagnostic, + pub severity: Severity, +} + +pub fn lint_exists(name: &str) -> bool { + ALL_LINTS.contains(&name) +} + +use_lints! { + almost_swapped: lints::almost_swapped::AlmostSwappedLint, + bad_string_escape: lints::bad_string_escape::BadStringEscapeLint, + compare_nan: lints::compare_nan::CompareNanLint, + constant_table_comparison: lints::constant_table_comparison::ConstantTableComparisonLint, + deprecated: lints::deprecated::DeprecatedLint, + divide_by_zero: lints::divide_by_zero::DivideByZeroLint, + duplicate_keys: lints::duplicate_keys::DuplicateKeysLint, + empty_if: lints::empty_if::EmptyIfLint, + empty_loop: lints::empty_loop::EmptyLoopLint, + global_usage: lints::global_usage::GlobalLint, + high_cyclomatic_complexity: lints::high_cyclomatic_complexity::HighCyclomaticComplexityLint, + if_same_then_else: lints::if_same_then_else::IfSameThenElseLint, + ifs_same_cond: lints::ifs_same_cond::IfsSameCondLint, + incorrect_standard_library_use: lints::standard_library::StandardLibraryLint, + invalid_lint_filter: lints::invalid_lint_filter::InvalidLintFilterLint, + manual_table_clone: lints::manual_table_clone::ManualTableCloneLint, + mismatched_arg_count: lints::mismatched_arg_count::MismatchedArgCountLint, + mixed_table: lints::mixed_table::MixedTableLint, + multiple_statements: lints::multiple_statements::MultipleStatementsLint, + must_use: lints::must_use::MustUseLint, + parenthese_conditions: lints::parenthese_conditions::ParentheseConditionsLint, + restricted_module_paths: lints::restricted_module_paths::RestrictedModulePathsLint, + shadowing: lints::shadowing::ShadowingLint, + suspicious_reverse_loop: lints::suspicious_reverse_loop::SuspiciousReverseLoopLint, + type_check_inside_call: lints::type_check_inside_call::TypeCheckInsideCallLint, + unbalanced_assignments: lints::unbalanced_assignments::UnbalancedAssignmentsLint, + undefined_variable: lints::undefined_variable::UndefinedVariableLint, + unscoped_variables: lints::unscoped_variables::UnscopedVariablesLint, + unused_variable: lints::unused_variable::UnusedVariableLint, + + #[cfg(feature = "roblox")] + { + roblox_incorrect_color3_new_bounds: lints::roblox_incorrect_color3_new_bounds::Color3BoundsLint, + roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint, + roblox_manual_fromscale_or_fromoffset: lints::roblox_manual_fromscale_or_fromoffset::ManualFromScaleOrFromOffsetLint, + roblox_suspicious_udim2_new: lints::roblox_suspicious_udim2_new::SuspiciousUDim2NewLint, + }, +} From 27f22452e861a043d703af89f2f454bce7eadb79 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:52:28 -0800 Subject: [PATCH 09/10] add tests from chris's pr --- .../src/lints/roblox_react_exhaustive_deps.rs | 36 ++++++++++++ .../complex_deps.lua | 57 ++++++++++++++++++ .../complex_deps.std.yml | 4 ++ .../complex_deps.stderr | 33 +++++++++++ .../implicit_self.lua | 36 ++++++++++++ .../implicit_self.std.yml | 4 ++ .../implicit_self.stderr | 27 +++++++++ .../known_stable_vars.lua | 58 +++++++++++++++++++ .../known_stable_vars.std.yml | 4 ++ .../known_stable_vars.stderr | 32 ++++++++++ .../outer_scope.lua | 43 ++++++++++++++ .../outer_scope.std.yml | 4 ++ .../outer_scope.stderr | 16 +++++ 13 files changed, 354 insertions(+) create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/complex_deps.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/implicit_self.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/known_stable_vars.stderr create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.lua create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.std.yml create mode 100644 selene-lib/tests/lints/roblox_react_exhaustive_deps/outer_scope.stderr diff --git a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs index e516119f..b2e6aafa 100644 --- a/selene-lib/src/lints/roblox_react_exhaustive_deps.rs +++ b/selene-lib/src/lints/roblox_react_exhaustive_deps.rs @@ -847,4 +847,40 @@ mod tests { "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/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/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 + From 00176ec68085f8a4ca8d945c36ef5ed10d361b61 Mon Sep 17 00:00:00 2001 From: Andrea Fletcher <50600470+BitwiseAndrea@users.noreply.github.com> Date: Tue, 9 Dec 2025 19:52:39 -0800 Subject: [PATCH 10/10] and add changelog of course! --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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