Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support linting complex udim2 constructors that can be simplified #601

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions docs/src/lints/roblox_manual_fromscale_or_fromoffset.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions selene-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment on lines +329 to 330
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should be alphabetical order. Same with lints.rs.

roblox_incorrect_roact_usage: lints::roblox_incorrect_roact_usage::IncorrectRoactUsageLint,
roblox_suspicious_udim2_new: lints::roblox_suspicious_udim2_new::SuspiciousUDim2NewLint,
Expand Down
3 changes: 3 additions & 0 deletions selene-lib/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
156 changes: 156 additions & 0 deletions selene-lib/src/lints/roblox_manual_fromscale_or_fromoffset.rs
Original file line number Diff line number Diff line change
@@ -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<Self, Self::Error> {
Ok(ManualFromScaleOrFromOffsetLint)
}

fn pass(&self, ast: &Ast, context: &Context, _: &AstContext) -> Vec<Diagnostic> {
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<UDim2ComplexArgs>,
}

#[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::<Vec<_>>();

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::<f32>() == Ok(0.0) && y_scale.parse::<f32>() == Ok(0.0);
let only_scale = x_offset.parse::<f32>() == Ok(0.0) && y_offset.parse::<f32>() == 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",
);
}
}
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[selene]
name = "roblox"
Original file line number Diff line number Diff line change
@@ -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)

Loading