diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6a62ce..f0fb656a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased](https://github.com/Kampfkarren/selene/compare/0.27.1...HEAD) ### Added - Added `Path2DControlPoint.new` to the Roblox standard library +- Added new [`roblox_manual_fromscale_or_fromoffset` lint](https://kampfkarren.github.io/selene/lints/roblox_manual_fromscale_or_fromoffset.html), which will warn when the arguments could be simplified to `UDim2.fromScale` or `UDim2.fromOffset`. ## [0.27.1](https://github.com/Kampfkarren/selene/releases/tag/0.27.1) - 2024-04-28 ### Fixed diff --git a/docs/src/lints/roblox_manual_fromscale_or_fromoffset.md b/docs/src/lints/roblox_manual_fromscale_or_fromoffset.md new file mode 100644 index 00000000..38dce0c5 --- /dev/null +++ b/docs/src/lints/roblox_manual_fromscale_or_fromoffset.md @@ -0,0 +1,14 @@ +# roblox_manual_fromscale_or_fromoffset +## What it does +Checks for uses of `UDim2.new` where the arguments could be simplified to `UDim2.fromScale` or `UDim2.fromOffset`. + +## Why this is bad +This reduces readability of `UDim2.new()` construction. + +## Example +```lua +UDim2.new(1, 0, 1, 0) +``` + +## Remarks +This lint is only active if you are using the Roblox standard library. diff --git a/selene-lib/src/lib.rs b/selene-lib/src/lib.rs index 77bb09c3..8c743f96 100644 --- a/selene-lib/src/lib.rs +++ b/selene-lib/src/lib.rs @@ -326,6 +326,7 @@ use_lints! { #[cfg(feature = "roblox")] { + roblox_manual_fromscale_or_fromoffset: lints::roblox_manual_fromscale_or_fromoffset::ManualFromScaleOrFromOffsetLint, roblox_incorrect_color3_new_bounds: lints::roblox_incorrect_color3_new_bounds::Color3BoundsLint, roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint, 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 fa42553c..4d6b5bb2 100644 --- a/selene-lib/src/lints.rs +++ b/selene-lib/src/lints.rs @@ -36,6 +36,9 @@ pub mod undefined_variable; pub mod unscoped_variables; pub mod unused_variable; +#[cfg(feature = "roblox")] +pub mod roblox_manual_fromscale_or_fromoffset; + #[cfg(feature = "roblox")] pub mod roblox_incorrect_color3_new_bounds; diff --git a/selene-lib/src/lints/roblox_manual_fromscale_or_fromoffset.rs b/selene-lib/src/lints/roblox_manual_fromscale_or_fromoffset.rs new file mode 100644 index 00000000..b9fb8cfa --- /dev/null +++ b/selene-lib/src/lints/roblox_manual_fromscale_or_fromoffset.rs @@ -0,0 +1,156 @@ +use super::*; +use crate::ast_util::range; +use std::convert::Infallible; + +use full_moon::{ + ast::{self, Ast}, + visitors::Visitor, +}; + +pub struct ManualFromScaleOrFromOffsetLint; + +fn create_diagnostic(args: &UDim2ComplexArgs) -> Diagnostic { + let code = "roblox_manual_fromscale_or_fromoffset"; + let primary_label = Label::new(args.call_range); + + match args.complexity_type { + UDim2ConstructorType::OffsetOnly => Diagnostic::new_complete( + code, + "this UDim2.new call only sets offset, and can be simplified using UDim2.fromOffset" + .to_owned(), + primary_label, + vec![format!( + "try: UDim2.fromOffset({}, {})", + args.arg_0, args.arg_1 + )], + Vec::new(), + ), + UDim2ConstructorType::ScaleOnly => Diagnostic::new_complete( + code, + "this UDim2.new call only sets scale, and can be simplified using UDim2.fromScale" + .to_owned(), + primary_label, + vec![format!( + "try: UDim2.fromScale({}, {})", + args.arg_0, args.arg_1 + )], + Vec::new(), + ), + } +} + +impl Lint for ManualFromScaleOrFromOffsetLint { + type Config = (); + type Error = Infallible; + + const SEVERITY: Severity = Severity::Warning; + const LINT_TYPE: LintType = LintType::Style; + + fn new(_: Self::Config) -> Result { + Ok(ManualFromScaleOrFromOffsetLint) + } + + fn pass(&self, ast: &Ast, context: &Context, _: &AstContext) -> Vec { + if !context.is_roblox() { + return Vec::new(); + } + + let mut visitor = UDim2NewVisitor::default(); + + visitor.visit_ast(ast); + + visitor.args.iter().map(create_diagnostic).collect() + } +} + +#[derive(Default)] +struct UDim2NewVisitor { + args: Vec, +} + +#[derive(PartialEq)] +enum UDim2ConstructorType { + ScaleOnly, + OffsetOnly, +} + +struct UDim2ComplexArgs { + complexity_type: UDim2ConstructorType, + call_range: (usize, usize), + arg_0: String, + arg_1: String, +} + +impl Visitor for UDim2NewVisitor { + fn visit_function_call(&mut self, call: &ast::FunctionCall) { + if_chain::if_chain! { + if let ast::Prefix::Name(token) = call.prefix(); + if token.token().to_string() == "UDim2"; + let mut suffixes = call.suffixes().collect::>(); + + if suffixes.len() == 2; // .new and () + let call_suffix = suffixes.pop().unwrap(); + let index_suffix = suffixes.pop().unwrap(); + + if let ast::Suffix::Index(ast::Index::Dot { name, .. }) = index_suffix; + if name.token().to_string() == "new"; + + if let ast::Suffix::Call(ast::Call::AnonymousCall( + ast::FunctionArgs::Parentheses { arguments, .. } + )) = call_suffix; + + then { + if arguments.len() != 4 { + return; + } + + let mut iter = arguments.iter(); + let x_scale = iter.next().unwrap().to_string(); + let x_offset = iter.next().unwrap().to_string(); + let y_scale = iter.next().unwrap().to_string(); + let y_offset = iter.next().unwrap().to_string(); + + let only_offset = x_scale.parse::() == Ok(0.0) && y_scale.parse::() == Ok(0.0); + let only_scale = x_offset.parse::() == Ok(0.0) && y_offset.parse::() == Ok(0.0); + + if only_offset && only_scale + { + // Skip linting if all are zero + return; + } + else if only_offset + { + self.args.push(UDim2ComplexArgs { + call_range: range(call), + arg_0: x_offset, + arg_1: y_offset, + complexity_type: UDim2ConstructorType::OffsetOnly, + }); + } + else if only_scale + { + self.args.push(UDim2ComplexArgs { + call_range: range(call), + arg_0: x_scale, + arg_1: y_scale, + complexity_type: UDim2ConstructorType::ScaleOnly, + }); + } + } + } + } +} + +#[cfg(test)] +mod tests { + use super::{super::test_util::test_lint, *}; + + #[test] + fn test_manual_fromscale_or_fromoffset() { + test_lint( + ManualFromScaleOrFromOffsetLint::new(()).unwrap(), + "roblox_manual_fromscale_or_fromoffset", + "roblox_manual_fromscale_or_fromoffset", + ); + } +} diff --git a/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.lua b/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.lua new file mode 100644 index 00000000..0dc4db84 --- /dev/null +++ b/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.lua @@ -0,0 +1,27 @@ +UDim2.new(0) +UDim2.new(0.5) +UDim2.new(1) +UDim2.new(2) +UDim2.new(a) +UDim2.new(1, 1) +UDim2.new(1, 2) +UDim2.new(a, b) +UDim2.new(1, a) +UDim2.new(1, 1, 1) +UDim2.new(a, b, c) +UDim2.new(1, 1, 1, 1) +UDim2.new(a, b, c, d) +UDim2.fromOffset(1, 1) +UDim2.fromScale(1, 1) +UDim2.new() +UDim2.new(1, 0, 1, 0) +UDim2.new(0, 0, 1, 0) +UDim2.new(0, 5, 1, 5) +UDim2.new(0, 5, 0, 1) +UDim2.new(0, 0, 0, 1) +UDim2.new(0, 1, 0, 1) +UDim2.new(0, a, 0, 1) +UDim2.new(0, a, 0, b) +UDim2.new(1, a, 0, b) +UDim2.new(a, 0, 0, 0) +UDim2.new(a, 0.0, 0, 0.0) diff --git a/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.std.toml b/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.std.toml new file mode 100644 index 00000000..98886350 --- /dev/null +++ b/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.std.toml @@ -0,0 +1,2 @@ +[selene] +name = "roblox" diff --git a/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.stderr b/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.stderr new file mode 100644 index 00000000..a30fdf32 --- /dev/null +++ b/selene-lib/tests/lints/roblox_manual_fromscale_or_fromoffset/roblox_manual_fromscale_or_fromoffset.stderr @@ -0,0 +1,72 @@ +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets scale, and can be simplified using UDim2.fromScale + ┌─ roblox_manual_fromscale_or_fromoffset.lua:17:1 + │ +17 │ UDim2.new(1, 0, 1, 0) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromScale(1, 1) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets scale, and can be simplified using UDim2.fromScale + ┌─ roblox_manual_fromscale_or_fromoffset.lua:18:1 + │ +18 │ UDim2.new(0, 0, 1, 0) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromScale(0, 1) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets offset, and can be simplified using UDim2.fromOffset + ┌─ roblox_manual_fromscale_or_fromoffset.lua:20:1 + │ +20 │ UDim2.new(0, 5, 0, 1) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromOffset(5, 1) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets offset, and can be simplified using UDim2.fromOffset + ┌─ roblox_manual_fromscale_or_fromoffset.lua:21:1 + │ +21 │ UDim2.new(0, 0, 0, 1) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromOffset(0, 1) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets offset, and can be simplified using UDim2.fromOffset + ┌─ roblox_manual_fromscale_or_fromoffset.lua:22:1 + │ +22 │ UDim2.new(0, 1, 0, 1) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromOffset(1, 1) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets offset, and can be simplified using UDim2.fromOffset + ┌─ roblox_manual_fromscale_or_fromoffset.lua:23:1 + │ +23 │ UDim2.new(0, a, 0, 1) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromOffset(a, 1) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets offset, and can be simplified using UDim2.fromOffset + ┌─ roblox_manual_fromscale_or_fromoffset.lua:24:1 + │ +24 │ UDim2.new(0, a, 0, b) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromOffset(a, b) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets scale, and can be simplified using UDim2.fromScale + ┌─ roblox_manual_fromscale_or_fromoffset.lua:26:1 + │ +26 │ UDim2.new(a, 0, 0, 0) + │ ^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromScale(a, 0) + +error[roblox_manual_fromscale_or_fromoffset]: this UDim2.new call only sets scale, and can be simplified using UDim2.fromScale + ┌─ roblox_manual_fromscale_or_fromoffset.lua:27:1 + │ +27 │ UDim2.new(a, 0.0, 0, 0.0) + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ + │ + = try: UDim2.fromScale(a, 0) +