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

Lint against approximated constants #574

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 [`approximated_constants` lint](https://kampfkarren.github.io/selene/lints/approximated_constants.html), which will check for number literals that approximate constants.

## [0.27.1](https://github.com/Kampfkarren/selene/releases/tag/0.27.1) - 2024-04-28
### Fixed
Expand Down
1 change: 1 addition & 0 deletions docs/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- [Contributing](./contributing.md)
- [Lints](./lints/index.md)
- [almost_swapped](./lints/almost_swapped.md)
- [approximated_constants](./lints/approximated_constants.md)
- [constant_table_comparison](./lints/constant_table_comparison.md)
- [deprecated](./lints/deprecated.md)
- [divide_by_zero](./lints/divide_by_zero.md)
Expand Down
17 changes: 17 additions & 0 deletions docs/src/lints/approximated_constants.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# approximated_constants
## What it does
Checks for number literals that approximate constants.

## Why this is bad
Using constants provided by the Lua standard library is more precise.

## Example
```lua
local x = 3.14
```

...should be written as...

```lua
local x = math.pi
```
1 change: 1 addition & 0 deletions selene-lib/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use full_moon::{ast::Ast, node::Node};
use serde::de::DeserializeOwned;

pub mod almost_swapped;
pub mod approximated_constants;
pub mod bad_string_escape;
pub mod compare_nan;
pub mod constant_table_comparison;
Expand Down
87 changes: 87 additions & 0 deletions selene-lib/src/lints/approximated_constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
use super::*;
use std::convert::Infallible;

use full_moon::{ast::Ast, tokenizer::TokenType, visitors::Visitor};

pub struct ApproximatedConstantsLint;

impl Lint for ApproximatedConstantsLint {
type Config = ();
type Error = Infallible;

const SEVERITY: Severity = Severity::Warning;
const LINT_TYPE: LintType = LintType::Correctness;

fn new((): Self::Config) -> Result<Self, Self::Error> {
Ok(ApproximatedConstantsLint)
}

fn pass(&self, ast: &Ast, _: &Context, _: &AstContext) -> Vec<Diagnostic> {
let mut visitor = ApproxConstantVisitor {
approximated_constants: Vec::new(),
};

visitor.visit_ast(ast);

visitor
.approximated_constants
.iter()
.map(|constant| {
Diagnostic::new(
"approximated_constants",
format!("`{}` is more precise", constant.constant),
Label::new(constant.range),
)
})
.collect()
}
}

struct ApproxConstantVisitor {
approximated_constants: Vec<ApproximatedConstant>,
}

struct ApproximatedConstant {
range: (usize, usize),
constant: String,
}

impl Visitor for ApproxConstantVisitor {
fn visit_number(&mut self, token: &full_moon::tokenizer::Token) {
if let TokenType::Number { text } = token.token_type() {
if is_approx_const(std::f64::consts::PI, text, 3) {
self.approximated_constants.push(ApproximatedConstant {
range: (token.start_position().bytes(), token.end_position().bytes()),
constant: "math.pi".to_string(),
});
}
}
}
}

#[must_use]
fn is_approx_const(constant: f64, value: &str, min_digits: usize) -> bool {
if value.len() <= min_digits {
false
} else if constant.to_string().starts_with(value) {
// The value is a truncated constant
true
} else {
let round_const = format!("{constant:.*}", value.len() - 2);
value == round_const
}
}

#[cfg(test)]
mod tests {
use super::{super::test_util::test_lint, *};

#[test]
fn test_approximated_constants() {
test_lint(
ApproximatedConstantsLint::new(()).unwrap(),
"approximated_constants",
"approximated_constants",
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
local good = 3
local good = 3.1
local good = 3.13
local good = 3.15
local good = -3.15
local good = 3.1417
local good = 3.14159266

local good = 0x314

local bad = 3.14
local bad = 3.141
local bad = -3.141
local bad = 3.142
local bad = 3.1415
local bad = 3.14159265

local bad = 3.14 + 1
local bad = f(3.14 + 1)
local bad = f(-3.14 + 1)
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:11:13
11 │ local bad = 3.14
│ ^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:12:13
12 │ local bad = 3.141
│ ^^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:13:14
13 │ local bad = -3.141
│ ^^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:14:13
14 │ local bad = 3.142
│ ^^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:15:13
15 │ local bad = 3.1415
│ ^^^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:16:13
16 │ local bad = 3.14159265
│ ^^^^^^^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:18:13
18 │ local bad = 3.14 + 1
│ ^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:19:15
19 │ local bad = f(3.14 + 1)
│ ^^^^

error[approximated_constants]: `math.pi` is more precise
┌─ approximated_constants.lua:20:16
20 │ local bad = f(-3.14 + 1)
│ ^^^^

Loading