From fd551de42aeabbbca9a5ebeebbd21902f7d3f2cd Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sun, 25 Jun 2023 22:43:28 -0700 Subject: [PATCH 01/11] Add roact dangling connection lint --- selene-lib/src/lib.rs | 1 + selene-lib/src/lints.rs | 3 + .../lints/roblox_roact_dangling_connection.rs | 254 ++++++++++++++++++ .../no_roact.lua | 74 +++++ .../no_roact.std.toml | 2 + .../no_roact.stderr | 0 .../roblox_roact_dangling_connection.lua | 30 +++ .../roblox_roact_dangling_connection.std.toml | 2 + .../roblox_roact_dangling_connection.stderr | 60 +++++ .../with_roact.lua | 76 ++++++ .../with_roact.std.toml | 2 + .../with_roact.stderr | 72 +++++ 12 files changed, 576 insertions(+) create mode 100644 selene-lib/src/lints/roblox_roact_dangling_connection.rs create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.lua create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.std.toml create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.stderr create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.std.toml create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.lua create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.std.toml create mode 100644 selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.stderr diff --git a/selene-lib/src/lib.rs b/selene-lib/src/lib.rs index 64d0c40c..fc4aa2dc 100644 --- a/selene-lib/src/lib.rs +++ b/selene-lib/src/lib.rs @@ -327,6 +327,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_roact_dangling_connection: lints::roblox_roact_dangling_connection::RoactDanglingConnectionLint, 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 27c30c09..520d757b 100644 --- a/selene-lib/src/lints.rs +++ b/selene-lib/src/lints.rs @@ -41,6 +41,9 @@ pub mod roblox_incorrect_color3_new_bounds; #[cfg(feature = "roblox")] pub mod roblox_incorrect_roact_usage; +#[cfg(feature = "roblox")] +pub mod roblox_roact_dangling_connection; + #[cfg(feature = "roblox")] pub mod roblox_suspicious_udim2_new; diff --git a/selene-lib/src/lints/roblox_roact_dangling_connection.rs b/selene-lib/src/lints/roblox_roact_dangling_connection.rs new file mode 100644 index 00000000..9d8b3b10 --- /dev/null +++ b/selene-lib/src/lints/roblox_roact_dangling_connection.rs @@ -0,0 +1,254 @@ +use super::*; +use crate::ast_util::range; +use std::{collections::HashMap, convert::Infallible}; + +use full_moon::{ + ast::{self, Ast, Expression}, + tokenizer::TokenReference, + visitors::Visitor, +}; + +pub struct RoactDanglingConnectionLint; + +impl Lint for RoactDanglingConnectionLint { + type Config = (); + type Error = Infallible; + + const SEVERITY: Severity = Severity::Error; + const LINT_TYPE: LintType = LintType::Correctness; + + fn new(_: Self::Config) -> Result { + Ok(RoactDanglingConnectionLint) + } + + fn pass(&self, ast: &Ast, context: &Context, _: &AstContext) -> Vec { + if !context.is_roblox() { + return Vec::new(); + } + + let mut visitor = RoactDanglingConnectionVisitor { + dangling_connections: Vec::new(), + has_roact_in_file: false, + assignments: HashMap::new(), + function_contexts: Vec::new(), + }; + + visitor.visit_ast(ast); + + let mut diagnostics = Vec::new(); + + for invalid_event in visitor.dangling_connections { + match invalid_event.function_context { + ConnectionContext::UseEffect => { + diagnostics.push(Diagnostic::new( + "roblox_roact_dangling_connection", + "disconnect the connection in the useEffect cleanup function".to_owned(), + Label::new(invalid_event.range), + )); + } + _ => { + diagnostics.push(Diagnostic::new( + "roblox_roact_dangling_connection", + "disconnect the connection where appropriate".to_owned(), + Label::new(invalid_event.range), + )); + } + } + } + + diagnostics + } +} + +fn get_last_function_call_suffix(prefix: &ast::Prefix, suffixes: &[&ast::Suffix]) -> String { + let last_suffix = match suffixes.last() { + Some(ast::Suffix::Call(ast::Call::MethodCall(_))) => suffixes.last(), + Some(ast::Suffix::Call(ast::Call::AnonymousCall(_))) => { + if suffixes.len() == 1 { + // a() + return if let ast::Prefix::Name(name) = prefix { + name.token().to_string() + } else { + "".to_owned() + }; + } else { + // In a.b(), b is the suffix before the last one + Some(&suffixes[suffixes.len() - 2]) + } + } + _ => return "".to_owned(), + }; + + last_suffix + .map(|suffix| match suffix { + ast::Suffix::Index(ast::Index::Dot { name, .. }) => name.token().to_string(), + ast::Suffix::Call(ast::Call::MethodCall(method_call)) => { + method_call.name().token().to_string() + } + ast::Suffix::Call(ast::Call::AnonymousCall(anonymous_call)) => { + anonymous_call.to_string() + } + _ => "".to_string(), + }) + .unwrap_or_else(|| "".to_owned()) +} + +#[derive(Debug, PartialEq, Clone, Copy)] +enum ConnectionContext { + Unknown, + UseEffect, +} + +#[derive(Debug, PartialEq, Clone, Copy)] +enum ConnectionContextType { + FunctionBody, + FunctionCall, +} + +#[derive(Debug)] +struct RoactDanglingConnectionVisitor { + dangling_connections: Vec, + has_roact_in_file: bool, + assignments: HashMap<(usize, usize), String>, + function_contexts: Vec<(ConnectionContextType, ConnectionContext)>, +} + +#[derive(Debug)] +struct DanglingConnection { + range: (usize, usize), + function_context: ConnectionContext, +} + +fn get_last_known_context( + function_contexts: &[(ConnectionContextType, ConnectionContext)], +) -> ConnectionContext { + match function_contexts + .iter() + .rev() + .find(|(_, connection_type)| *connection_type != ConnectionContext::Unknown) + { + Some(context) => context.1, + None => ConnectionContext::Unknown, + } +} + +impl RoactDanglingConnectionVisitor { + fn process_assignment(&mut self, name: &TokenReference, expression: &Expression) { + if ["Roact", "React"].contains( + &name + .token() + .to_string() + .trim_end_matches(char::is_whitespace), + ) { + self.has_roact_in_file = true; + } + + if let ast::Expression::Value { value, .. } = expression { + if let ast::Value::FunctionCall(_) = &**value { + self.assignments + .insert(range(value), name.token().to_string()); + } + } + } +} + +impl Visitor for RoactDanglingConnectionVisitor { + fn visit_function_call(&mut self, call: &ast::FunctionCall) { + let last_suffix = + get_last_function_call_suffix(call.prefix(), &call.suffixes().collect::>()); + + // Ignore cases like a(b:Connect()) where connection is passed as an argument + let is_immediately_in_function_call = + self.function_contexts.last().map_or(false, |context| { + context.0 == ConnectionContextType::FunctionCall + }); + + let is_call_assigned_to_variable = self.assignments.contains_key(&range(call)); + + if self.has_roact_in_file + && !is_immediately_in_function_call + && !is_call_assigned_to_variable + // Ignore connections on the top level as they are not in a Roact component + && !self.function_contexts.is_empty() + && ["Connect", "connect", "ConnectParallel", "Once"].contains(&last_suffix.as_str()) + { + self.dangling_connections.push(DanglingConnection { + range: range(call), + function_context: get_last_known_context(&self.function_contexts), + }); + } + + self.function_contexts.push(( + ConnectionContextType::FunctionCall, + match last_suffix.as_str() { + "useEffect" => ConnectionContext::UseEffect, + _ => ConnectionContext::Unknown, + }, + )); + } + + fn visit_function_call_end(&mut self, _node: &ast::FunctionCall) { + self.function_contexts.pop(); + } + + fn visit_assignment(&mut self, assignment: &ast::Assignment) { + for (var, expression) in assignment + .variables() + .iter() + .zip(assignment.expressions().iter()) + { + if let ast::Var::Name(name) = var { + self.process_assignment(name, expression); + } + } + } + + fn visit_local_assignment(&mut self, node: &ast::LocalAssignment) { + for (name, expression) in node.names().iter().zip(node.expressions().iter()) { + self.process_assignment(name, expression) + } + } + + fn visit_function_body(&mut self, _node: &ast::FunctionBody) { + self.function_contexts.push(( + ConnectionContextType::FunctionBody, + ConnectionContext::Unknown, + )); + } + + fn visit_function_body_end(&mut self, _node: &ast::FunctionBody) { + self.function_contexts.pop(); + } +} + +#[cfg(test)] +mod tests { + use super::{super::test_util::test_lint, *}; + + #[test] + fn test_no_roact() { + test_lint( + RoactDanglingConnectionLint::new(()).unwrap(), + "roblox_roact_dangling_connection", + "no_roact", + ); + } + + #[test] + fn test_roblox_roact_dangling_connection() { + test_lint( + RoactDanglingConnectionLint::new(()).unwrap(), + "roblox_roact_dangling_connection", + "roblox_roact_dangling_connection", + ); + } + + #[test] + fn test_with_roact() { + test_lint( + RoactDanglingConnectionLint::new(()).unwrap(), + "roblox_roact_dangling_connection", + "with_roact", + ); + } +} diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.lua b/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.lua new file mode 100644 index 00000000..d7fce0c6 --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.lua @@ -0,0 +1,74 @@ +a:Connect() +a.Connect() +a:connect() +a.connect() + +a.b:Connect() +a.b.Connect() +a.b:connect() +a.b.connect() + +a.b.c:Connect() +a.b.c.Connect() +a.b.c:connect() +a.b.c.connect() + +local foo = a:Connect() +local foo = a.Connect() +local foo = a:connect() +local foo = a.connect() + +foo = a:Connect() +foo = a.Connect() +foo = a:connect() +foo = a.connect() + +foo = a.b:Connect() +foo = a.b.Connect() +foo = a.b:connect() +foo = a.b.connect() + +foo = a.b.c:Connect() +foo = a.b.c.Connect() +foo = a.b.c:connect() +foo = a.b.c.connect() + +local function c() + a:Connect() + a:connect() + + a.b:Connect() + a.b:connect() + + a.b.c:Connect() + a.b.c:connect() + + foo = a:Connect() + foo = a:connect() + + foo = a.b:Connect() + foo = a.b:connect() + + foo = a.b.c:Connect() + foo = a.b.c:connect() +end + +function d:e() + a:Connect() + a:connect() + + a.b:Connect() + a.b:connect() + + a.b.c:Connect() + a.b.c:connect() + + foo = a:Connect() + foo = a:connect() + + foo = a.b:Connect() + foo = a.b:connect() + + foo = a.b.c:Connect() + foo = a.b.c:connect() +end diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.std.toml b/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.std.toml new file mode 100644 index 00000000..98886350 --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.std.toml @@ -0,0 +1,2 @@ +[selene] +name = "roblox" diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.stderr b/selene-lib/tests/lints/roblox_roact_dangling_connection/no_roact.stderr new file mode 100644 index 00000000..e69de29b diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua new file mode 100644 index 00000000..601aa11a --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua @@ -0,0 +1,30 @@ +local React = a + +a:connect() + +local function c() + a:Connect() + a:connect() + a:ConnectParallel() + a:Once() + + a.Connect() + a.connect() + a.ConnectParallel() + a.Once() + + a(b:Connect()) + a(function() end, b:Connect()) + + useEffect(function() + a:connect() + a(b:Connect()) + local b = a:connect() + end) + + React.useEffect(function() + a:connect() + a(b:Connect()) + local b = a:connect() + end) +end diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.std.toml b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.std.toml new file mode 100644 index 00000000..98886350 --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.std.toml @@ -0,0 +1,2 @@ +[selene] +name = "roblox" diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr new file mode 100644 index 00000000..97166142 --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr @@ -0,0 +1,60 @@ +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:6:5 + │ +6 │ a:Connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:7:5 + │ +7 │ a:connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:8:5 + │ +8 │ a:ConnectParallel() + │ ^^^^^^^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:9:5 + │ +9 │ a:Once() + │ ^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:11:5 + │ +11 │ a.Connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:12:5 + │ +12 │ a.connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:13:5 + │ +13 │ a.ConnectParallel() + │ ^^^^^^^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:14:5 + │ +14 │ a.Once() + │ ^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection in the useEffect cleanup function + ┌─ roblox_roact_dangling_connection.lua:20:9 + │ +20 │ a:connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection in the useEffect cleanup function + ┌─ roblox_roact_dangling_connection.lua:26:9 + │ +26 │ a:connect() + │ ^^^^^^^^^^^ + diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.lua b/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.lua new file mode 100644 index 00000000..2674f397 --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.lua @@ -0,0 +1,76 @@ +local Roact = a + +a:Connect() +a.Connect() +a:connect() +a.connect() + +a.b:Connect() +a.b.Connect() +a.b:connect() +a.b.connect() + +a.b.c:Connect() +a.b.c.Connect() +a.b.c:connect() +a.b.c.connect() + +local foo = a:Connect() +local foo = a.Connect() +local foo = a:connect() +local foo = a.connect() + +foo = a:Connect() +foo = a.Connect() +foo = a:connect() +foo = a.connect() + +foo = a.b:Connect() +foo = a.b.Connect() +foo = a.b:connect() +foo = a.b.connect() + +foo = a.b.c:Connect() +foo = a.b.c.Connect() +foo = a.b.c:connect() +foo = a.b.c.connect() + +local function c() + a:Connect() + a:connect() + + a.b:Connect() + a.b:connect() + + a.b.c:Connect() + a.b.c:connect() + + foo = a:Connect() + foo = a:connect() + + foo = a.b:Connect() + foo = a.b:connect() + + foo = a.b.c:Connect() + foo = a.b.c:connect() +end + +function d:e() + a:Connect() + a:connect() + + a.b:Connect() + a.b:connect() + + a.b.c:Connect() + a.b.c:connect() + + foo = a:Connect() + foo = a:connect() + + foo = a.b:Connect() + foo = a.b:connect() + + foo = a.b.c:Connect() + foo = a.b.c:connect() +end diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.std.toml b/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.std.toml new file mode 100644 index 00000000..98886350 --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.std.toml @@ -0,0 +1,2 @@ +[selene] +name = "roblox" diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.stderr b/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.stderr new file mode 100644 index 00000000..3d01451f --- /dev/null +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/with_roact.stderr @@ -0,0 +1,72 @@ +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:39:5 + │ +39 │ a:Connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:40:5 + │ +40 │ a:connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:42:5 + │ +42 │ a.b:Connect() + │ ^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:43:5 + │ +43 │ a.b:connect() + │ ^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:45:5 + │ +45 │ a.b.c:Connect() + │ ^^^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:46:5 + │ +46 │ a.b.c:connect() + │ ^^^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:59:5 + │ +59 │ a:Connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:60:5 + │ +60 │ a:connect() + │ ^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:62:5 + │ +62 │ a.b:Connect() + │ ^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:63:5 + │ +63 │ a.b:connect() + │ ^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:65:5 + │ +65 │ a.b.c:Connect() + │ ^^^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ with_roact.lua:66:5 + │ +66 │ a.b.c:connect() + │ ^^^^^^^^^^^^^^^ + From 1fd05c14b61adf0e796848fdcd67fa54572689d1 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sun, 25 Jun 2023 22:52:02 -0700 Subject: [PATCH 02/11] Update docs and changelog --- CHANGELOG.md | 1 + docs/src/SUMMARY.md | 1 + .../lints/roblox_roact_dangling_connection.md | 24 +++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 docs/src/lints/roblox_roact_dangling_connection.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f47f66..2ed1b990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `roblox_incorrect_roact_usage` now lints for illegal `Name` property - Added `ignore_pattern` config to `global_usage`, which will ignore any global variables with names that match the pattern - `roblox_incorrect_roact_usage` now checks for incorrect Roact17's `createElement` usage on variables named `React`. For Roact17 only, `key`, `children`, and `ref` are valid properties to Roblox instances. +- Added new [`roblox_roact_dangling_connection`](https://kampfkarren.github.io/selene/lints/roblox_roact_dangling_connection.html), which will check for connections made in components without getting cleaned up. ### Fixed - `string.pack` and `string.unpack` now have proper function signatures in the Lua 5.3 standard library. diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 7d0d0618..6dcf0747 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -32,6 +32,7 @@ - [parenthese_conditions](./lints/parenthese_conditions.md) - [roblox_incorrect_color3_new_bounds](./lints/roblox_incorrect_color3_new_bounds.md) - [roblox_incorrect_roact_usage](./lints/roblox_incorrect_roact_usage.md) + - [roblox_roact_dangling_connection](./lints/roblox_roact_dangling_connection.md) - [roblox_suspicious_udim2_new](./lints/roblox_suspicious_udim2_new.md) - [shadowing](./lints/shadowing.md) - [suspicious_reverse_loop](./lints/suspicious_reverse_loop.md) diff --git a/docs/src/lints/roblox_roact_dangling_connection.md b/docs/src/lints/roblox_roact_dangling_connection.md new file mode 100644 index 00000000..96074487 --- /dev/null +++ b/docs/src/lints/roblox_roact_dangling_connection.md @@ -0,0 +1,24 @@ +# roblox_roact_dangling_connection +## What it does +Checks for connections in Roact components that are either not assigned to a variable or passed as arguments. + +## Why this is bad +This indicates a memory leak and can cause unexpected behaviors. + +## Example +```lua +local function MyComponent() + useEffect(function() + a:Connect() + end, {}) +end +``` + +## Remarks +This lint is active if the file has a variable named `Roact` or `React` and that the connection is made within a function. + +This checks for connections by identifying the following keywords: +* Connect +* connect +* ConnectParallel +* Once From bfd8a04e7e7987e29a5d1b9b679629571b0e051d Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Tue, 27 Jun 2023 22:35:19 -0700 Subject: [PATCH 03/11] Fix scope_manager.function_calls missing method names --- selene-lib/src/ast_util/scopes.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/selene-lib/src/ast_util/scopes.rs b/selene-lib/src/ast_util/scopes.rs index 832a9ef1..02a65e67 100644 --- a/selene-lib/src/ast_util/scopes.rs +++ b/selene-lib/src/ast_util/scopes.rs @@ -239,6 +239,10 @@ fn get_name_path_from_call(call: &ast::FunctionCall) -> Option> { path.push(name.token().to_string()); } + ast::Suffix::Call(ast::Call::MethodCall(method_call)) => { + path.push(method_call.name().token().to_string()); + } + ast::Suffix::Index(ast::Index::Brackets { .. }) => { return None; } From d5e2123e20f8cb4da4c84ac709c2eef11c9c38a7 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Tue, 27 Jun 2023 22:35:35 -0700 Subject: [PATCH 04/11] Use scope manager to get dangling connections --- .../lints/roblox_roact_dangling_connection.rs | 120 ++++++++---------- 1 file changed, 50 insertions(+), 70 deletions(-) diff --git a/selene-lib/src/lints/roblox_roact_dangling_connection.rs b/selene-lib/src/lints/roblox_roact_dangling_connection.rs index 9d8b3b10..89473341 100644 --- a/selene-lib/src/lints/roblox_roact_dangling_connection.rs +++ b/selene-lib/src/lints/roblox_roact_dangling_connection.rs @@ -1,10 +1,9 @@ use super::*; use crate::ast_util::range; -use std::{collections::HashMap, convert::Infallible}; +use std::{collections::HashSet, convert::Infallible}; use full_moon::{ - ast::{self, Ast, Expression}, - tokenizer::TokenReference, + ast::{self, Ast}, visitors::Visitor, }; @@ -18,18 +17,45 @@ impl Lint for RoactDanglingConnectionLint { const LINT_TYPE: LintType = LintType::Correctness; fn new(_: Self::Config) -> Result { - Ok(RoactDanglingConnectionLint) + Ok(Self) } - fn pass(&self, ast: &Ast, context: &Context, _: &AstContext) -> Vec { + fn pass( + &self, + ast: &Ast, + context: &Context, + AstContext { scope_manager, .. }: &AstContext, + ) -> Vec { if !context.is_roblox() { return Vec::new(); } + if scope_manager.variables.iter().all(|(_, variable)| { + !["Roact", "React"].contains(&variable.name.trim_end_matches(char::is_whitespace)) + }) { + return Vec::new(); + } + let mut visitor = RoactDanglingConnectionVisitor { dangling_connections: Vec::new(), - has_roact_in_file: false, - assignments: HashMap::new(), + dangling_connection_start_ranges: scope_manager + .function_calls + .iter() + .filter_map(|(_, function_call_stmt)| { + function_call_stmt + .call_name_path + .last() + .and_then(|last_name| { + if ["Connect", "connect", "ConnectParallel", "Once"] + .contains(&last_name.as_str()) + { + Some(function_call_stmt.call_prefix_range.0) + } else { + None + } + }) + }) + .collect(), function_contexts: Vec::new(), }; @@ -90,7 +116,7 @@ fn get_last_function_call_suffix(prefix: &ast::Prefix, suffixes: &[&ast::Suffix] } _ => "".to_string(), }) - .unwrap_or_else(|| "".to_owned()) + .unwrap_or_default() } #[derive(Debug, PartialEq, Clone, Copy)] @@ -108,8 +134,7 @@ enum ConnectionContextType { #[derive(Debug)] struct RoactDanglingConnectionVisitor { dangling_connections: Vec, - has_roact_in_file: bool, - assignments: HashMap<(usize, usize), String>, + dangling_connection_start_ranges: HashSet, function_contexts: Vec<(ConnectionContextType, ConnectionContext)>, } @@ -132,50 +157,23 @@ fn get_last_known_context( } } -impl RoactDanglingConnectionVisitor { - fn process_assignment(&mut self, name: &TokenReference, expression: &Expression) { - if ["Roact", "React"].contains( - &name - .token() - .to_string() - .trim_end_matches(char::is_whitespace), - ) { - self.has_roact_in_file = true; - } - - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::FunctionCall(_) = &**value { - self.assignments - .insert(range(value), name.token().to_string()); - } - } - } -} - impl Visitor for RoactDanglingConnectionVisitor { fn visit_function_call(&mut self, call: &ast::FunctionCall) { let last_suffix = get_last_function_call_suffix(call.prefix(), &call.suffixes().collect::>()); - // Ignore cases like a(b:Connect()) where connection is passed as an argument - let is_immediately_in_function_call = - self.function_contexts.last().map_or(false, |context| { - context.0 == ConnectionContextType::FunctionCall - }); - - let is_call_assigned_to_variable = self.assignments.contains_key(&range(call)); - - if self.has_roact_in_file - && !is_immediately_in_function_call - && !is_call_assigned_to_variable - // Ignore connections on the top level as they are not in a Roact component - && !self.function_contexts.is_empty() - && ["Connect", "connect", "ConnectParallel", "Once"].contains(&last_suffix.as_str()) - { - self.dangling_connections.push(DanglingConnection { - range: range(call), - function_context: get_last_known_context(&self.function_contexts), - }); + if !self.function_contexts.is_empty() { + if let Some(call_range) = call.range() { + if self + .dangling_connection_start_ranges + .contains(&call_range.0.bytes()) + { + self.dangling_connections.push(DanglingConnection { + range: range(call), + function_context: get_last_known_context(&self.function_contexts), + }); + } + } } self.function_contexts.push(( @@ -187,36 +185,18 @@ impl Visitor for RoactDanglingConnectionVisitor { )); } - fn visit_function_call_end(&mut self, _node: &ast::FunctionCall) { + fn visit_function_call_end(&mut self, _: &ast::FunctionCall) { self.function_contexts.pop(); } - fn visit_assignment(&mut self, assignment: &ast::Assignment) { - for (var, expression) in assignment - .variables() - .iter() - .zip(assignment.expressions().iter()) - { - if let ast::Var::Name(name) = var { - self.process_assignment(name, expression); - } - } - } - - fn visit_local_assignment(&mut self, node: &ast::LocalAssignment) { - for (name, expression) in node.names().iter().zip(node.expressions().iter()) { - self.process_assignment(name, expression) - } - } - - fn visit_function_body(&mut self, _node: &ast::FunctionBody) { + fn visit_function_body(&mut self, _: &ast::FunctionBody) { self.function_contexts.push(( ConnectionContextType::FunctionBody, ConnectionContext::Unknown, )); } - fn visit_function_body_end(&mut self, _node: &ast::FunctionBody) { + fn visit_function_body_end(&mut self, _: &ast::FunctionBody) { self.function_contexts.pop(); } } From e3ba0269b76d9bbd5b591c36c4703b4693e50c81 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Tue, 27 Jun 2023 22:57:04 -0700 Subject: [PATCH 05/11] Fix non-exhaustive pattern --- selene-lib/src/ast_util/scopes.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/selene-lib/src/ast_util/scopes.rs b/selene-lib/src/ast_util/scopes.rs index 02a65e67..d6ff0b54 100644 --- a/selene-lib/src/ast_util/scopes.rs +++ b/selene-lib/src/ast_util/scopes.rs @@ -239,15 +239,15 @@ fn get_name_path_from_call(call: &ast::FunctionCall) -> Option> { path.push(name.token().to_string()); } - ast::Suffix::Call(ast::Call::MethodCall(method_call)) => { - path.push(method_call.name().token().to_string()); - } - ast::Suffix::Index(ast::Index::Brackets { .. }) => { return None; } - ast::Suffix::Call(_) => { + ast::Suffix::Call(call) => { + if let ast::Call::MethodCall(method_call) = call { + path.push(method_call.name().token().to_string()); + } + if index + 1 == length { return Some(path); } else { From 959d019a429387481f4cca3fdec76edb5b9af3d3 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sat, 19 Aug 2023 12:22:29 -0700 Subject: [PATCH 06/11] Change match to if let --- .../lints/roblox_roact_dangling_connection.rs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/selene-lib/src/lints/roblox_roact_dangling_connection.rs b/selene-lib/src/lints/roblox_roact_dangling_connection.rs index 89473341..7282b6b4 100644 --- a/selene-lib/src/lints/roblox_roact_dangling_connection.rs +++ b/selene-lib/src/lints/roblox_roact_dangling_connection.rs @@ -64,21 +64,18 @@ impl Lint for RoactDanglingConnectionLint { let mut diagnostics = Vec::new(); for invalid_event in visitor.dangling_connections { - match invalid_event.function_context { - ConnectionContext::UseEffect => { - diagnostics.push(Diagnostic::new( - "roblox_roact_dangling_connection", - "disconnect the connection in the useEffect cleanup function".to_owned(), - Label::new(invalid_event.range), - )); - } - _ => { - diagnostics.push(Diagnostic::new( - "roblox_roact_dangling_connection", - "disconnect the connection where appropriate".to_owned(), - Label::new(invalid_event.range), - )); - } + if let ConnectionContext::UseEffect = invalid_event.function_context { + diagnostics.push(Diagnostic::new( + "roblox_roact_dangling_connection", + "disconnect the connection in the useEffect cleanup function".to_owned(), + Label::new(invalid_event.range), + )); + } else { + diagnostics.push(Diagnostic::new( + "roblox_roact_dangling_connection", + "disconnect the connection where appropriate".to_owned(), + Label::new(invalid_event.range), + )); } } From 7e43d847f34324faf4a50867fd9df42c0e1f63e3 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sun, 27 Aug 2023 09:53:08 -0700 Subject: [PATCH 07/11] Require react prefix to useEffect to display useEffect error --- .../lints/roblox_roact_dangling_connection.rs | 46 ++++++++++++++++++- .../roblox_roact_dangling_connection.lua | 4 +- .../roblox_roact_dangling_connection.stderr | 44 +++++++++--------- 3 files changed, 70 insertions(+), 24 deletions(-) diff --git a/selene-lib/src/lints/roblox_roact_dangling_connection.rs b/selene-lib/src/lints/roblox_roact_dangling_connection.rs index 7282b6b4..64c7ea4a 100644 --- a/selene-lib/src/lints/roblox_roact_dangling_connection.rs +++ b/selene-lib/src/lints/roblox_roact_dangling_connection.rs @@ -6,6 +6,7 @@ use full_moon::{ ast::{self, Ast}, visitors::Visitor, }; +use if_chain::if_chain; pub struct RoactDanglingConnectionLint; @@ -57,6 +58,7 @@ impl Lint for RoactDanglingConnectionLint { }) .collect(), function_contexts: Vec::new(), + definitions_of_roact_functions: HashSet::new(), }; visitor.visit_ast(ast); @@ -133,6 +135,7 @@ struct RoactDanglingConnectionVisitor { dangling_connections: Vec, dangling_connection_start_ranges: HashSet, function_contexts: Vec<(ConnectionContextType, ConnectionContext)>, + definitions_of_roact_functions: HashSet, } #[derive(Debug)] @@ -154,6 +157,15 @@ fn get_last_known_context( } } +fn is_roact_function(prefix: &ast::Prefix) -> bool { + if let ast::Prefix::Name(prefix_token) = prefix { + ["roact", "react", "hooks"] + .contains(&prefix_token.token().to_string().to_lowercase().as_str()) + } else { + false + } +} + impl Visitor for RoactDanglingConnectionVisitor { fn visit_function_call(&mut self, call: &ast::FunctionCall) { let last_suffix = @@ -173,10 +185,29 @@ impl Visitor for RoactDanglingConnectionVisitor { } } + // Check if caller is Roact. or a variable defined to it + let mut suffixes = call.suffixes().collect::>(); + suffixes.pop(); + + let mut is_this_roact_function = false; + + if suffixes.is_empty() { + // Call is foo(), not foo.bar() + if let ast::Prefix::Name(name) = call.prefix() { + is_this_roact_function = self + .definitions_of_roact_functions + .get(&name.token().to_string()) + .is_some(); + } + } else if suffixes.len() == 1 { + // Call is foo.bar() + is_this_roact_function = is_roact_function(call.prefix()); + } + self.function_contexts.push(( ConnectionContextType::FunctionCall, match last_suffix.as_str() { - "useEffect" => ConnectionContext::UseEffect, + "useEffect" if is_this_roact_function => ConnectionContext::UseEffect, _ => ConnectionContext::Unknown, }, )); @@ -196,6 +227,19 @@ impl Visitor for RoactDanglingConnectionVisitor { fn visit_function_body_end(&mut self, _: &ast::FunctionBody) { self.function_contexts.pop(); } + + 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::Value { value, .. } = expr; + if let ast::Value::Var(ast::Var::Expression(var_expr)) = &**value; + if is_roact_function(var_expr.prefix()); + then { + self.definitions_of_roact_functions.insert(name.token().to_string()); + } + }; + } + } } #[cfg(test)] diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua index 601aa11a..8124467c 100644 --- a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.lua @@ -1,4 +1,5 @@ -local React = a +local React +local useEffect = React.useEffect a:connect() @@ -13,6 +14,7 @@ local function c() a.ConnectParallel() a.Once() + -- Ignore since `a` might take ownership of connections a(b:Connect()) a(function() end, b:Connect()) diff --git a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr index 97166142..aa48824b 100644 --- a/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr +++ b/selene-lib/tests/lints/roblox_roact_dangling_connection/roblox_roact_dangling_connection.stderr @@ -1,60 +1,60 @@ -error[roblox_roact_dangling_connection]: disconnect the connection where appropriate - ┌─ roblox_roact_dangling_connection.lua:6:5 - │ -6 │ a:Connect() - │ ^^^^^^^^^^^ - error[roblox_roact_dangling_connection]: disconnect the connection where appropriate ┌─ roblox_roact_dangling_connection.lua:7:5 │ -7 │ a:connect() +7 │ a:Connect() │ ^^^^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection where appropriate ┌─ roblox_roact_dangling_connection.lua:8:5 │ -8 │ a:ConnectParallel() - │ ^^^^^^^^^^^^^^^^^^^ +8 │ a:connect() + │ ^^^^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection where appropriate ┌─ roblox_roact_dangling_connection.lua:9:5 │ -9 │ a:Once() - │ ^^^^^^^^ +9 │ a:ConnectParallel() + │ ^^^^^^^^^^^^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection where appropriate - ┌─ roblox_roact_dangling_connection.lua:11:5 + ┌─ roblox_roact_dangling_connection.lua:10:5 │ -11 │ a.Connect() - │ ^^^^^^^^^^^ +10 │ a:Once() + │ ^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection where appropriate ┌─ roblox_roact_dangling_connection.lua:12:5 │ -12 │ a.connect() +12 │ a.Connect() │ ^^^^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection where appropriate ┌─ roblox_roact_dangling_connection.lua:13:5 │ -13 │ a.ConnectParallel() - │ ^^^^^^^^^^^^^^^^^^^ +13 │ a.connect() + │ ^^^^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection where appropriate ┌─ roblox_roact_dangling_connection.lua:14:5 │ -14 │ a.Once() +14 │ a.ConnectParallel() + │ ^^^^^^^^^^^^^^^^^^^ + +error[roblox_roact_dangling_connection]: disconnect the connection where appropriate + ┌─ roblox_roact_dangling_connection.lua:15:5 + │ +15 │ a.Once() │ ^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection in the useEffect cleanup function - ┌─ roblox_roact_dangling_connection.lua:20:9 + ┌─ roblox_roact_dangling_connection.lua:22:9 │ -20 │ a:connect() +22 │ a:connect() │ ^^^^^^^^^^^ error[roblox_roact_dangling_connection]: disconnect the connection in the useEffect cleanup function - ┌─ roblox_roact_dangling_connection.lua:26:9 + ┌─ roblox_roact_dangling_connection.lua:28:9 │ -26 │ a:connect() +28 │ a:connect() │ ^^^^^^^^^^^ From 8f66ac0e7ab09ec6c34d5e87c7c01081381be2ed Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sat, 11 Nov 2023 08:48:27 -0800 Subject: [PATCH 08/11] Fix test --- .../lints/roblox_roact_dangling_connection.rs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/selene-lib/src/lints/roblox_roact_dangling_connection.rs b/selene-lib/src/lints/roblox_roact_dangling_connection.rs index 64c7ea4a..8f4d27bf 100644 --- a/selene-lib/src/lints/roblox_roact_dangling_connection.rs +++ b/selene-lib/src/lints/roblox_roact_dangling_connection.rs @@ -6,7 +6,6 @@ use full_moon::{ ast::{self, Ast}, visitors::Visitor, }; -use if_chain::if_chain; pub struct RoactDanglingConnectionLint; @@ -47,13 +46,9 @@ impl Lint for RoactDanglingConnectionLint { .call_name_path .last() .and_then(|last_name| { - if ["Connect", "connect", "ConnectParallel", "Once"] + ["Connect", "connect", "ConnectParallel", "Once"] .contains(&last_name.as_str()) - { - Some(function_call_stmt.call_prefix_range.0) - } else { - None - } + .then_some(function_call_stmt.call_prefix_range.0) }) }) .collect(), @@ -230,14 +225,12 @@ impl Visitor for RoactDanglingConnectionVisitor { 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::Value { value, .. } = expr; - if let ast::Value::Var(ast::Var::Expression(var_expr)) = &**value; - if is_roact_function(var_expr.prefix()); - then { - self.definitions_of_roact_functions.insert(name.token().to_string()); + if let ast::Expression::Var(ast::Var::Expression(var_expr)) = expr { + if is_roact_function(var_expr.prefix()) { + self.definitions_of_roact_functions + .insert(name.token().to_string()); } - }; + } } } } From b5dff3b190412f41910619208a169edf4da8e0a8 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sat, 11 Nov 2023 08:51:22 -0800 Subject: [PATCH 09/11] Remove file --- .pre-commit-hooks.yaml | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 .pre-commit-hooks.yaml diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml deleted file mode 100644 index e4cb7416..00000000 --- a/.pre-commit-hooks.yaml +++ /dev/null @@ -1,15 +0,0 @@ -- id: selene-system - name: selene (system) - description: An opinionated Lua code linter - entry: selene - language: system - types: - - lua - -- id: selene-docker - name: selene (docker) - description: An opinionated Lua code linter - entry: /selene - language: docker - types: - - lua \ No newline at end of file From e3d680aa208cf91f517184b2998d1ca31bd5c078 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sat, 11 Nov 2023 08:54:16 -0800 Subject: [PATCH 10/11] Remove change --- Dockerfile | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index 3e3ac56a..5e3abffa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,14 +12,14 @@ RUN apt-get update && \ apt-get install g++ && \ cargo install --no-default-features --branch main --git https://github.com/Kampfkarren/selene selene -FROM rust:${RUST_VERSION}-alpine AS selene-light-musl-builder -RUN apk add g++ && \ - cargo install --no-default-features --branch main --git https://github.com/Kampfkarren/selene selene - FROM rust:${RUST_VERSION}-alpine AS selene-musl-builder RUN apk add g++ && \ cargo install --branch main --git https://github.com/Kampfkarren/selene selene +FROM rust:${RUST_VERSION}-alpine AS selene-light-musl-builder +RUN apk add g++ && \ + cargo install --no-default-features --branch main --git https://github.com/Kampfkarren/selene selene + FROM bash AS selene COPY --from=selene-builder /usr/local/cargo/bin/selene / CMD ["/selene"] @@ -28,10 +28,10 @@ FROM bash AS selene-light COPY --from=selene-light-builder /usr/local/cargo/bin/selene / CMD ["/selene"] -FROM bash AS selene-light-musl -COPY --from=selene-light-musl-builder /usr/local/cargo/bin/selene / -CMD ["/selene"] - FROM bash AS selene-musl COPY --from=selene-musl-builder /usr/local/cargo/bin/selene / +CMD ["/selene"] + +FROM bash AS selene-light-musl +COPY --from=selene-light-musl-builder /usr/local/cargo/bin/selene / CMD ["/selene"] \ No newline at end of file From a1c0bcdfbbc5cf58ae67038f76935a9a51ce85a6 Mon Sep 17 00:00:00 2001 From: Christopher Chang <51393127+chriscerie@users.noreply.github.com> Date: Sat, 11 Nov 2023 11:48:20 -0800 Subject: [PATCH 11/11] Fix clippy --- .../src/lints/roblox_roact_dangling_connection.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/selene-lib/src/lints/roblox_roact_dangling_connection.rs b/selene-lib/src/lints/roblox_roact_dangling_connection.rs index 8f4d27bf..969e3fa7 100644 --- a/selene-lib/src/lints/roblox_roact_dangling_connection.rs +++ b/selene-lib/src/lints/roblox_roact_dangling_connection.rs @@ -1,4 +1,4 @@ -use super::*; +use super::{AstContext, Context, Diagnostic, Label, Lint, LintType, Node, Severity}; use crate::ast_util::range; use std::{collections::HashSet, convert::Infallible}; @@ -16,7 +16,7 @@ impl Lint for RoactDanglingConnectionLint { const SEVERITY: Severity = Severity::Error; const LINT_TYPE: LintType = LintType::Correctness; - fn new(_: Self::Config) -> Result { + fn new((): Self::Config) -> Result { Ok(Self) } @@ -89,14 +89,14 @@ fn get_last_function_call_suffix(prefix: &ast::Prefix, suffixes: &[&ast::Suffix] return if let ast::Prefix::Name(name) = prefix { name.token().to_string() } else { - "".to_owned() + String::new() }; } else { // In a.b(), b is the suffix before the last one Some(&suffixes[suffixes.len() - 2]) } } - _ => return "".to_owned(), + _ => return String::new(), }; last_suffix @@ -108,7 +108,7 @@ fn get_last_function_call_suffix(prefix: &ast::Prefix, suffixes: &[&ast::Suffix] ast::Suffix::Call(ast::Call::AnonymousCall(anonymous_call)) => { anonymous_call.to_string() } - _ => "".to_string(), + _ => String::new(), }) .unwrap_or_default() }