Skip to content

Commit

Permalink
Handle function calls
Browse files Browse the repository at this point in the history
  • Loading branch information
chriscerie committed Sep 20, 2023
1 parent 89e0e98 commit e2d36a1
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 36 deletions.
37 changes: 27 additions & 10 deletions selene-lib/src/ast_util/scopes.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{borrow::Cow, collections::HashSet};

use full_moon::{
ast::{self, VarExpression},
ast::{self},
node::Node,
tokenizer::{Symbol, TokenKind, TokenReference, TokenType},
visitors::Visitor,
Expand Down Expand Up @@ -90,8 +90,7 @@ pub struct Reference {
pub within_function_stmt: Option<WithinFunctionStmt>,

// x.y["z"] produces ["y", "z"]
// x.y.z().w is None currently, but could change if necessary.
// If that change is made, ensure unused_variable is adjusted for write_only.
// x.y.z().w produce ["y", "z", "w"]
pub indexing: Option<Vec<IndexEntry>>,
}

Expand Down Expand Up @@ -131,6 +130,7 @@ pub struct WithinFunctionStmt {
pub struct IndexEntry {
pub index: Range,
pub static_name: Option<TokenReference>,
pub is_function_call: bool,
}

#[derive(Debug, Default)]
Expand Down Expand Up @@ -349,6 +349,8 @@ impl ScopeVisitor {

ast::Value::FunctionCall(call) => {
self.read_prefix(call.prefix());
self.adjust_indexing(call.suffixes(), range(call));

for suffix in call.suffixes() {
self.read_suffix(suffix);
}
Expand Down Expand Up @@ -492,7 +494,7 @@ impl ScopeVisitor {
match var {
ast::Var::Expression(var_expr) => {
self.read_prefix(var_expr.prefix());
self.adjust_indexing(var_expr);
self.adjust_indexing(var_expr.suffixes(), range(var_expr));

for suffix in var_expr.suffixes() {
self.read_suffix(suffix);
Expand Down Expand Up @@ -701,17 +703,24 @@ impl ScopeVisitor {
}
}

fn adjust_indexing(&mut self, var_expr: &VarExpression) {
let mut index_entries = Vec::new();
fn adjust_indexing<'a, I: Iterator<Item = &'a ast::Suffix>>(
&mut self,
suffixes: I,
expr_range: (usize, usize),
) {
let mut index_entries: Vec<IndexEntry> = Vec::new();

for suffix in var_expr.suffixes() {
for suffix in suffixes {
#[cfg_attr(
feature = "force_exhaustive_checks",
deny(non_exhaustive_omitted_patterns)
)]
let static_name = match suffix {
ast::Suffix::Call(_) => {
return;
if let Some(last_entry) = index_entries.last_mut() {
last_entry.is_function_call = true;
}
continue;
}

ast::Suffix::Index(ast::Index::Brackets { expression, .. }) => {
Expand All @@ -728,14 +737,15 @@ impl ScopeVisitor {
index_entries.push(IndexEntry {
index: range(suffix),
static_name: static_name.cloned(),
is_function_call: false,
})
}

if index_entries.is_empty() {
return;
}

let Some(reference) = self.scope_manager.reference_at_byte_mut(range(var_expr).0) else {
let Some(reference) = self.scope_manager.reference_at_byte_mut(expr_range.0) else {
return;
};

Expand Down Expand Up @@ -1137,7 +1147,13 @@ mod tests {
.as_ref()
.expect("indexing was None")
.iter()
.map(|index| index.static_name.as_ref().map(|token| token.to_string()))
.map(|index| index.static_name.as_ref().map(|token| {
if index.is_function_call {
format!("{}()", token)
} else {
token.to_string()
}
}))
.collect::<Vec<Option<String>>>()
);
}
Expand All @@ -1147,5 +1163,6 @@ mod tests {
}

test_indexing("x.y.z", &[Some("y"), Some("z")]);
test_indexing("a.b.c().d", &[Some("b"), Some("c()"), Some("d")]);
}
}
54 changes: 30 additions & 24 deletions selene-lib/src/lints/roblox_roact_non_exhaustive_deps.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::*;
use crate::ast_util::range;
use crate::ast_util::{range, scopes::Reference};
use std::{
collections::{HashMap, HashSet},
convert::Infallible,
Expand Down Expand Up @@ -214,6 +214,34 @@ struct Upvalue {
}

impl Upvalue {
fn new(reference: &Reference, resolved_start_range: &Option<usize>) -> Self {
let indexing_identifiers = reference
.indexing
.as_ref()
.unwrap_or(&vec![])
.iter()
.map_while(|index_entry| {
if index_entry.is_function_call {
return None;
}

index_entry.static_name.as_ref().and_then(|static_name| {
if let TokenType::Identifier { identifier } = static_name.token().token_type() {
Some(identifier.to_string())
} else {
None
}
})
})
.collect::<Vec<_>>();

Upvalue {
prefix: reference.name.clone(),
indexing_identifiers,
resolved_start_range: *resolved_start_range,
}
}

/// `a.b["c"].d`, `a.b.[c].d`, and `a.b.c().d` yield `a.b`
/// `a` just yields `a`
fn indexing_expression_name(&self) -> String {
Expand Down Expand Up @@ -286,29 +314,7 @@ impl RoactMissingDependencyVisitor<'_> {
variable.identifiers.last().map(|(start, _)| *start)
});

let indexing_identifiers = reference
.indexing
.as_ref()
.unwrap_or(&vec![])
.iter()
.filter_map(|index_entry| {
index_entry.static_name.as_ref().and_then(|static_name| {
if let TokenType::Identifier { identifier } =
static_name.token().token_type()
{
Some(identifier.to_string())
} else {
None
}
})
})
.collect::<Vec<_>>();

Some(Upvalue {
prefix: reference.name.clone(),
indexing_identifiers,
resolved_start_range,
})
Some(Upvalue::new(reference, &resolved_start_range))
} else {
None
}
Expand Down
1 change: 1 addition & 0 deletions selene-lib/src/lints/unused_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ impl Lint for UnusedVariableLint {
if is_static_table
&& indexing.len() == 1 // This restriction can be lifted someday, but only once we can verify that the value has no side effects/is its own static table
&& indexing.iter().any(|index| index.static_name.is_some())
&& indexing.iter().all(|index| !index.is_function_call)
{
return AnalyzedReference::ObservedWrite(Label::new_with_message(
reference.identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ local function Component2()
end, { a.b })
end

local function Component3()
local function Component()
local a = {}
React.useEffect(function()
print(a) -- Bad
Expand All @@ -30,11 +30,21 @@ local function Component3()
end, { a.b.c })
end

local function Component3()
local function Component()
local a = {}
React.useEffect(function()
print(a.b) -- Bad
print(a.b.c) -- Bad, but already reported with `a.b`
print(a.b.c.d)
end, { a.b.c.d })
end

local function Component()
local a = {}
local d = {}
React.useEffect(function()
print(a.b.c()) -- Bad
print(a.b.d()) -- Bad, but already reported
print(d.e.f.g()) -- Good
end, { a.b.c, d.e.f })
end
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,11 @@ error[roblox_roact_non_exhaustive_deps]: react hook useEffect has missing depend
= help: either include it or remove the dependency array

error[roblox_roact_non_exhaustive_deps]: react hook useEffect has missing dependency: 'a.b'
┌─ complex_deps.lua:49:10
49 │ end, { a.b.c, d.e.f })
│ ^^^^^^^^^^^^^^^^
= help: either include it or remove the dependency array

0 comments on commit e2d36a1

Please sign in to comment.