diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5d1376dd..0bb94c43 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,13 +12,13 @@ jobs: steps: - uses: actions/checkout@v3 - name: Run tests (selene, all features) - run: cargo test -- --nocapture + run: cargo test --release -- --nocapture working-directory: selene - name: Run tests (selene, no features) - run: cargo test --no-default-features -- --nocapture + run: cargo test --release --no-default-features -- --nocapture working-directory: selene - name: Run tests (selene-lib, all features) - run: cargo test + run: cargo test --release working-directory: selene-lib - name: Run tests (selene-lib, no features) run: cargo test --no-default-features diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 00000000..e4cb7416 --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,15 @@ +- 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 68c989f7..20904890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Added `SharedTable` to Roblox standard library - Added new [`--fix`](https://kampfkarren.github.io/selene/cli/usage.html#fix) flag, which will automatically apply lint suggestions. +### Changed +- Updated internal parser, which includes floor division (`//`), more correct parsing of string interpolation with double braces, and better parsing of `\z` escapes. + ### Fixed - `string.pack` and `string.unpack` now have proper function signatures in the Lua 5.3 standard library. - Moved `math.log` second argument addition from Lua 5.3 std lib to 5.2 std lib diff --git a/Cargo.lock b/Cargo.lock index 43985053..0debea82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -357,9 +357,9 @@ dependencies = [ [[package]] name = "full_moon" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97743bd3cf799444b6fa793fe7787e0ca1d838b0c9b27ebecb54495d37a4e840" +checksum = "24ef4f8ad0689d3a86bb483650422d72e6f79a37fdc83ed5426cafe96b776ce1" dependencies = [ "bytecount", "cfg-if", diff --git a/Cargo.toml b/Cargo.toml index f84cc876..6e20337b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [workspace] - members = ["selene", "selene-lib"] +resolver = "2" [workspace.package] version = "0.25.0" @@ -11,8 +11,8 @@ license = "MPL-2.0" repository = "https://github.com/Kampfkarren/selene" [workspace.dependencies] -full_moon = "0.18.0" +full_moon = "0.19.0" toml = "0.7.2" # Do not update this without confirming profiling uses the same version of tracy-client as selene -profiling = "1.0.7" \ No newline at end of file +profiling = "1.0.7" diff --git a/Dockerfile b/Dockerfile index 5e3abffa..3e3ac56a 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-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 rust:${RUST_VERSION}-alpine AS selene-musl-builder +RUN apk add g++ && \ + cargo install --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-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"] + +FROM bash AS selene-musl +COPY --from=selene-musl-builder /usr/local/cargo/bin/selene / CMD ["/selene"] \ No newline at end of file diff --git a/selene-lib/src/ast_util/extract_static_token.rs b/selene-lib/src/ast_util/extract_static_token.rs index 4dfb7be8..0b5bb79d 100644 --- a/selene-lib/src/ast_util/extract_static_token.rs +++ b/selene-lib/src/ast_util/extract_static_token.rs @@ -8,21 +8,23 @@ pub fn extract_static_token(expression: &ast::Expression) -> Option<&TokenRefere deny(non_exhaustive_omitted_patterns) )] match expression { - ast::Expression::BinaryOperator { .. } | ast::Expression::UnaryOperator { .. } => None, - ast::Expression::Parentheses { expression, .. } => extract_static_token(expression), - #[cfg_attr( - feature = "force_exhaustive_checks", - allow(non_exhaustive_omitted_patterns) - )] - ast::Expression::Value { value, .. } => match &**value { - ast::Value::Number(token) => Some(token), - ast::Value::String(token) => Some(token), - ast::Value::Symbol(token) => Some(token), + ast::Expression::Number(token) + | ast::Expression::String(token) + | ast::Expression::Symbol(token) => Some(token), + + ast::Expression::BinaryOperator { .. } + | ast::Expression::UnaryOperator { .. } + | ast::Expression::Function(_) + | ast::Expression::FunctionCall(_) + | ast::Expression::TableConstructor(_) + | ast::Expression::Var(_) => None, - _ => None, - }, + #[cfg(feature = "roblox")] + ast::Expression::IfExpression(_) + | ast::Expression::InterpolatedString(_) + | ast::Expression::TypeAssertion { .. } => None, _ => None, } diff --git a/selene-lib/src/ast_util/mod.rs b/selene-lib/src/ast_util/mod.rs index ff7ecf4e..98690f26 100644 --- a/selene-lib/src/ast_util/mod.rs +++ b/selene-lib/src/ast_util/mod.rs @@ -49,8 +49,7 @@ pub fn first_code(ast: &Ast) -> Option<(Position, Position)> { pub fn is_vararg(expression: &ast::Expression) -> bool { if_chain::if_chain! { - if let ast::Expression::Value { value, .. } = expression; - if let ast::Value::Symbol(token) = &**value; + if let ast::Expression::Symbol(token) = expression; if let tokenizer::TokenType::Symbol { symbol: tokenizer::Symbol::Ellipse, } = token.token().token_type(); @@ -63,21 +62,9 @@ pub fn is_vararg(expression: &ast::Expression) -> bool { } } -pub fn is_function_call(expression: &ast::Expression) -> bool { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::FunctionCall(_) = &**value { - return true; - } - } - - false -} - pub fn expression_to_ident(expression: &ast::Expression) -> Option<&TokenReference> { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::Var(ast::Var::Name(name)) = &**value { - return Some(name); - } + if let ast::Expression::Var(ast::Var::Name(name)) = expression { + return Some(name); } None diff --git a/selene-lib/src/ast_util/name_paths.rs b/selene-lib/src/ast_util/name_paths.rs index f2d943b5..448d641b 100644 --- a/selene-lib/src/ast_util/name_paths.rs +++ b/selene-lib/src/ast_util/name_paths.rs @@ -38,19 +38,15 @@ pub fn name_path_from_prefix_suffix<'a, S: Iterator>( } pub fn name_path(expression: &ast::Expression) -> Option> { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::Var(var) = &**value { - match var { - ast::Var::Expression(expression) => { - name_path_from_prefix_suffix(expression.prefix(), expression.suffixes()) - } + if let ast::Expression::Var(var) = expression { + match var { + ast::Var::Expression(expression) => { + name_path_from_prefix_suffix(expression.prefix(), expression.suffixes()) + } - ast::Var::Name(name) => Some(vec![name.to_string()]), + ast::Var::Name(name) => Some(vec![name.to_string()]), - _ => None, - } - } else { - None + _ => None, } } else { None diff --git a/selene-lib/src/ast_util/scopes.rs b/selene-lib/src/ast_util/scopes.rs index 832a9ef1..3e3b6ff2 100644 --- a/selene-lib/src/ast_util/scopes.rs +++ b/selene-lib/src/ast_util/scopes.rs @@ -201,9 +201,9 @@ fn get_name_path_from_call(call: &ast::FunctionCall) -> Option> { let mut all_suffixes = Vec::new(); let mut path = match call.prefix() { - ast::Prefix::Expression(ast::Expression::Value { value, .. }) => match &**value { - ast::Value::Var(ast::Var::Name(name)) => vec![name.token().to_string()], - ast::Value::Var(ast::Var::Expression(var_expression)) => { + ast::Prefix::Expression(expression) => match &**expression { + ast::Expression::Var(ast::Var::Name(name)) => vec![name.token().to_string()], + ast::Expression::Var(ast::Var::Expression(var_expression)) => { let mut path = Vec::new(); let name = if let ast::Prefix::Name(name) = var_expression.prefix() { @@ -220,7 +220,6 @@ fn get_name_path_from_call(call: &ast::FunctionCall) -> Option> { } _ => return None, }, - ast::Prefix::Expression(_) => return None, ast::Prefix::Name(name) => vec![name.token().to_string()], _ => return None, }; @@ -259,12 +258,10 @@ fn get_name_path_from_call(call: &ast::FunctionCall) -> Option> { } fn get_assigned_value(expression: &ast::Expression) -> Option { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::TableConstructor(table_constructor) = &**value { - return Some(AssignedValue::StaticTable { - has_fields: !table_constructor.fields().is_empty(), - }); - } + if let ast::Expression::TableConstructor(table_constructor) = expression { + return Some(AssignedValue::StaticTable { + has_fields: !table_constructor.fields().is_empty(), + }); } None @@ -342,62 +339,61 @@ impl ScopeVisitor { self.read_expression(rhs); } - ast::Expression::Value { value, .. } => match &**value { - ast::Value::Function((name, _)) => { - self.read_name(name); - } - - ast::Value::FunctionCall(call) => { - self.read_prefix(call.prefix()); - for suffix in call.suffixes() { - self.read_suffix(suffix); - } - } + ast::Expression::Function((name, _)) => { + self.read_name(name); + } - ast::Value::TableConstructor(table) => { - self.read_table_constructor(table); + ast::Expression::FunctionCall(call) => { + self.read_prefix(call.prefix()); + for suffix in call.suffixes() { + self.read_suffix(suffix); } + } - ast::Value::ParenthesesExpression(expression) => self.read_expression(expression), + ast::Expression::TableConstructor(table) => { + self.read_table_constructor(table); + } - ast::Value::Symbol(symbol) => { - if *symbol.token_type() - == (TokenType::Symbol { - symbol: Symbol::Ellipse, - }) - { - self.read_name(symbol); - } + ast::Expression::Symbol(symbol) => { + if *symbol.token_type() + == (TokenType::Symbol { + symbol: Symbol::Ellipse, + }) + { + self.read_name(symbol); } + } - ast::Value::Var(var) => self.read_var(var), + ast::Expression::Var(var) => self.read_var(var), - #[cfg(feature = "roblox")] - ast::Value::IfExpression(if_expression) => { - self.read_expression(if_expression.condition()); - self.read_expression(if_expression.if_expression()); + #[cfg(feature = "roblox")] + ast::Expression::IfExpression(if_expression) => { + self.read_expression(if_expression.condition()); + self.read_expression(if_expression.if_expression()); - if let Some(else_if_expressions) = if_expression.else_if_expressions() { - for else_if_expression in else_if_expressions { - self.read_expression(else_if_expression.condition()); - self.read_expression(else_if_expression.expression()); - } + if let Some(else_if_expressions) = if_expression.else_if_expressions() { + for else_if_expression in else_if_expressions { + self.read_expression(else_if_expression.condition()); + self.read_expression(else_if_expression.expression()); } - - self.read_expression(if_expression.else_expression()); } - #[cfg(feature = "roblox")] - ast::Value::InterpolatedString(interpolated_string) => { - for expression in interpolated_string.expressions() { - self.read_expression(expression); - } + self.read_expression(if_expression.else_expression()); + } + + #[cfg(feature = "roblox")] + ast::Expression::InterpolatedString(interpolated_string) => { + for expression in interpolated_string.expressions() { + self.read_expression(expression); } + } - ast::Value::Number(_) | ast::Value::String(_) => {} + #[cfg(feature = "roblox")] + ast::Expression::TypeAssertion { expression, .. } => { + self.read_expression(expression); + } - _ => {} - }, + ast::Expression::Number(_) | ast::Expression::String(_) => {} _ => {} } diff --git a/selene-lib/src/ast_util/side_effects.rs b/selene-lib/src/ast_util/side_effects.rs index 1cea53d6..f8af5aec 100644 --- a/selene-lib/src/ast_util/side_effects.rs +++ b/selene-lib/src/ast_util/side_effects.rs @@ -18,54 +18,12 @@ impl HasSideEffects for ast::Expression { } ast::Expression::Parentheses { expression, .. } | ast::Expression::UnaryOperator { expression, .. } => expression.has_side_effects(), - ast::Expression::Value { value, .. } => value.has_side_effects(), - _ => false, - } - } -} - -impl HasSideEffects for ast::Prefix { - fn has_side_effects(&self) -> bool { - #[cfg_attr( - feature = "force_exhaustive_checks", - deny(non_exhaustive_omitted_patterns) - )] - match self { - ast::Prefix::Expression(expression) => expression.has_side_effects(), - ast::Prefix::Name(_) => false, - _ => true, - } - } -} - -impl HasSideEffects for ast::Suffix { - fn has_side_effects(&self) -> bool { - #[cfg_attr( - feature = "force_exhaustive_checks", - deny(non_exhaustive_omitted_patterns) - )] - match self { - ast::Suffix::Call(_) => true, - ast::Suffix::Index(_) => false, - _ => true, - } - } -} - -impl HasSideEffects for ast::Value { - fn has_side_effects(&self) -> bool { - #[cfg_attr( - feature = "force_exhaustive_checks", - deny(non_exhaustive_omitted_patterns) - )] - match self { - ast::Value::Function(_) - | ast::Value::Number(_) - | ast::Value::String(_) - | ast::Value::Symbol(_) => false, - ast::Value::FunctionCall(_) => true, - ast::Value::ParenthesesExpression(expression) => expression.has_side_effects(), - ast::Value::TableConstructor(table_constructor) => table_constructor + ast::Expression::Function(_) + | ast::Expression::Number(_) + | ast::Expression::String(_) + | ast::Expression::Symbol(_) => false, + ast::Expression::FunctionCall(_) => true, + ast::Expression::TableConstructor(table_constructor) => table_constructor .fields() .into_iter() .any(|field| match field { @@ -79,10 +37,10 @@ impl HasSideEffects for ast::Value { _ => true, }), - ast::Value::Var(var) => var.has_side_effects(), + ast::Expression::Var(var) => var.has_side_effects(), #[cfg(feature = "roblox")] - ast::Value::IfExpression(if_expression) => { + ast::Expression::IfExpression(if_expression) => { if if_expression.if_expression().has_side_effects() || if_expression.condition().has_side_effects() || if_expression.else_expression().has_side_effects() @@ -104,7 +62,7 @@ impl HasSideEffects for ast::Value { } #[cfg(feature = "roblox")] - ast::Value::InterpolatedString(interpolated_string) => { + ast::Expression::InterpolatedString(interpolated_string) => { for expression in interpolated_string.expressions() { if expression.has_side_effects() { return true; @@ -114,6 +72,37 @@ impl HasSideEffects for ast::Value { false } + #[cfg(feature = "roblox")] + ast::Expression::TypeAssertion { expression, .. } => expression.has_side_effects(), + + _ => true, + } + } +} + +impl HasSideEffects for ast::Prefix { + fn has_side_effects(&self) -> bool { + #[cfg_attr( + feature = "force_exhaustive_checks", + deny(non_exhaustive_omitted_patterns) + )] + match self { + ast::Prefix::Expression(expression) => expression.has_side_effects(), + ast::Prefix::Name(_) => false, + _ => true, + } + } +} + +impl HasSideEffects for ast::Suffix { + fn has_side_effects(&self) -> bool { + #[cfg_attr( + feature = "force_exhaustive_checks", + deny(non_exhaustive_omitted_patterns) + )] + match self { + ast::Suffix::Call(_) => true, + ast::Suffix::Index(_) => false, _ => true, } } diff --git a/selene-lib/src/ast_util/visit_nodes.rs b/selene-lib/src/ast_util/visit_nodes.rs index 6560bf7f..163365ae 100644 --- a/selene-lib/src/ast_util/visit_nodes.rs +++ b/selene-lib/src/ast_util/visit_nodes.rs @@ -95,7 +95,6 @@ make_node_visitor!({ visit_suffix(Suffix), visit_table_constructor(TableConstructor), visit_un_op(UnOp), - visit_value(Value), visit_var(Var), visit_var_expression(VarExpression), visit_while(While), diff --git a/selene-lib/src/lints/bad_string_escape.rs b/selene-lib/src/lints/bad_string_escape.rs index f36f6ba8..4d433d3f 100644 --- a/selene-lib/src/lints/bad_string_escape.rs +++ b/selene-lib/src/lints/bad_string_escape.rs @@ -116,9 +116,9 @@ struct StringEscapeSequence { } impl Visitor for BadStringEscapeVisitor { - fn visit_value(&mut self, node: &ast::Value) { + fn visit_expression(&mut self, node: &ast::Expression) { if_chain::if_chain! { - if let ast::Value::String(token) = node; + if let ast::Expression::String(token) = node; if let tokenizer::TokenType::StringLiteral { literal, multi_line, quote_type } = token.token_type(); if multi_line.is_none(); then { diff --git a/selene-lib/src/lints/compare_nan.rs b/selene-lib/src/lints/compare_nan.rs index d7ed4c0c..1f11eba2 100644 --- a/selene-lib/src/lints/compare_nan.rs +++ b/selene-lib/src/lints/compare_nan.rs @@ -58,8 +58,8 @@ struct Comparison { range: (usize, usize), } -fn value_is_zero(value: &ast::Value) -> bool { - if let ast::Value::Number(token) = value { +fn value_is_zero(value: &ast::Expression) -> bool { + if let ast::Expression::Number(token) = value { token.token().to_string() == "0" } else { false @@ -69,11 +69,7 @@ fn value_is_zero(value: &ast::Value) -> bool { fn expression_is_nan(node: &ast::Expression) -> bool { if_chain::if_chain! { if let ast::Expression::BinaryOperator { lhs, binop: ast::BinOp::Slash(_), rhs } = node; - if let ast::Expression::Value { value, .. } = &**lhs; - if let ast::Expression::Value { - value: rhs_value, .. - } = &**rhs; - if value_is_zero(rhs_value) && value_is_zero(value); + if value_is_zero(lhs) && value_is_zero(rhs); then { return true; } @@ -85,8 +81,7 @@ impl Visitor for CompareNanVisitor { fn visit_expression(&mut self, node: &ast::Expression) { if_chain::if_chain! { if let ast::Expression::BinaryOperator { lhs, binop, rhs } = node; - if let ast::Expression::Value { value, .. } = &**lhs; - if let ast::Value::Var(_) = value.as_ref(); + if let ast::Expression::Var(_) = lhs.as_ref(); then { match binop { ast::BinOp::TildeEqual(_) => { @@ -94,7 +89,7 @@ impl Visitor for CompareNanVisitor { let range = node.range().unwrap(); self.comparisons.push( Comparison { - variable: value.to_string().trim().to_owned(), + variable: lhs.to_string().trim().to_owned(), operator: "==".to_owned(), range: (range.0.bytes(), range.1.bytes()), } @@ -106,7 +101,7 @@ impl Visitor for CompareNanVisitor { let range = node.range().unwrap(); self.comparisons.push( Comparison { - variable: value.to_string().trim().to_owned(), + variable: lhs.to_string().trim().to_owned(), operator: "~=".to_owned(), range: (range.0.bytes(), range.1.bytes()), } diff --git a/selene-lib/src/lints/constant_table_comparison.rs b/selene-lib/src/lints/constant_table_comparison.rs index 087a6dac..c1647f32 100644 --- a/selene-lib/src/lints/constant_table_comparison.rs +++ b/selene-lib/src/lints/constant_table_comparison.rs @@ -1,4 +1,4 @@ -use crate::ast_util::{purge_trivia, range, strip_parentheses}; +use crate::ast_util::{purge_trivia, range}; use super::*; use std::convert::Infallible; @@ -94,14 +94,12 @@ enum ConstantTableMatch { } fn constant_table_match(expression: &ast::Expression) -> Option { - if let ast::Expression::Value { value, .. } = strip_parentheses(expression) { - if let ast::Value::TableConstructor(table_constructor) = &**value { - return if table_constructor.fields().is_empty() { - Some(ConstantTableMatch::Empty) - } else { - Some(ConstantTableMatch::NotEmpty) - }; - } + if let ast::Expression::TableConstructor(table_constructor) = expression { + return if table_constructor.fields().is_empty() { + Some(ConstantTableMatch::Empty) + } else { + Some(ConstantTableMatch::NotEmpty) + }; } None diff --git a/selene-lib/src/lints/divide_by_zero.rs b/selene-lib/src/lints/divide_by_zero.rs index 1282afa4..1a10ab6e 100644 --- a/selene-lib/src/lints/divide_by_zero.rs +++ b/selene-lib/src/lints/divide_by_zero.rs @@ -47,8 +47,8 @@ struct DivideByZeroVisitor { positions: Vec<(usize, usize)>, } -fn value_is_zero(value: &ast::Value) -> bool { - if let ast::Value::Number(token) = value { +fn value_is_zero(value: &ast::Expression) -> bool { + if let ast::Expression::Number(token) = value { token.token().to_string() == "0" } else { false @@ -59,12 +59,8 @@ impl Visitor for DivideByZeroVisitor { fn visit_expression(&mut self, node: &ast::Expression) { if_chain::if_chain! { if let ast::Expression::BinaryOperator { lhs, binop, rhs, .. } = node; - if let ast::Expression::Value { value, .. } = &**lhs; if let ast::BinOp::Slash(_) = binop; - if let ast::Expression::Value { - value: rhs_value, .. - } = &**rhs; - if value_is_zero(rhs_value) && !value_is_zero(value); + if value_is_zero(rhs) && !value_is_zero(lhs); then { let range = node.range().unwrap(); self.positions.push((range.0.bytes(), range.1.bytes())); diff --git a/selene-lib/src/lints/duplicate_keys.rs b/selene-lib/src/lints/duplicate_keys.rs index 015b3f3c..9ad26619 100644 --- a/selene-lib/src/lints/duplicate_keys.rs +++ b/selene-lib/src/lints/duplicate_keys.rs @@ -78,20 +78,18 @@ struct DuplicateKeysVisitor { /// Also extracts `5` from `[5] = true`. /// Only works for string literal expression keys, or constant number keys. fn expression_to_key(expression: &ast::Expression) -> Option { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::String(token) | ast::Value::Number(token) = &**value { - return match token.token().token_type() { - tokenizer::TokenType::StringLiteral { literal, .. } => Some(Key { - key_type: KeyType::String, - name: literal.to_string(), - }), - tokenizer::TokenType::Number { text, .. } => Some(Key { - key_type: KeyType::Number, - name: text.to_string(), - }), - _ => None, - }; - } + if let ast::Expression::String(token) | ast::Expression::Number(token) = expression { + return match token.token().token_type() { + tokenizer::TokenType::StringLiteral { literal, .. } => Some(Key { + key_type: KeyType::String, + name: literal.to_string(), + }), + tokenizer::TokenType::Number { text, .. } => Some(Key { + key_type: KeyType::Number, + name: text.to_string(), + }), + _ => None, + }; } None diff --git a/selene-lib/src/lints/high_cyclomatic_complexity.rs b/selene-lib/src/lints/high_cyclomatic_complexity.rs index a0cd7158..4cf2d56e 100644 --- a/selene-lib/src/lints/high_cyclomatic_complexity.rs +++ b/selene-lib/src/lints/high_cyclomatic_complexity.rs @@ -184,70 +184,61 @@ fn count_expression_complexity(expression: &ast::Expression, starting_complexity count_expression_complexity(expression, complexity) } - #[cfg_attr( - feature = "force_exhaustive_checks", - deny(non_exhaustive_omitted_patterns) - )] - ast::Expression::Value { value, .. } => match &**value { - // visit_value already tracks this - ast::Value::Function(_) => complexity, + // visit_expression already tracks this + ast::Expression::Function(_) => complexity, - ast::Value::FunctionCall(call) => { - if let ast::Prefix::Expression(prefix_expression) = call.prefix() { - complexity = count_expression_complexity(prefix_expression, complexity) - } - for suffix in call.suffixes() { - complexity = count_suffix_complexity(suffix, complexity) - } - - complexity + ast::Expression::FunctionCall(call) => { + if let ast::Prefix::Expression(prefix_expression) = call.prefix() { + complexity = count_expression_complexity(prefix_expression, complexity) } - - ast::Value::ParenthesesExpression(paren_expression) => { - count_expression_complexity(paren_expression, complexity) + for suffix in call.suffixes() { + complexity = count_suffix_complexity(suffix, complexity) } - ast::Value::Number(_) => complexity, - ast::Value::String(_) => complexity, - ast::Value::Symbol(_) => complexity, + complexity + } + + ast::Expression::Number(_) => complexity, + ast::Expression::String(_) => complexity, + ast::Expression::Symbol(_) => complexity, - ast::Value::TableConstructor(table) => count_table_complexity(table, complexity), + ast::Expression::TableConstructor(table) => count_table_complexity(table, complexity), - ast::Value::Var(ast::Var::Expression(var_expression)) => { - for suffix in var_expression.suffixes() { - complexity = count_suffix_complexity(suffix, complexity) - } - complexity + ast::Expression::Var(ast::Var::Expression(var_expression)) => { + for suffix in var_expression.suffixes() { + complexity = count_suffix_complexity(suffix, complexity) } + complexity + } - ast::Value::Var(ast::Var::Name(_)) => complexity, + ast::Expression::Var(ast::Var::Name(_)) => complexity, - #[cfg(feature = "roblox")] - ast::Value::IfExpression(if_expression) => { - complexity += 1; - if let Some(else_if_expressions) = if_expression.else_if_expressions() { - for else_if_expression in else_if_expressions { - complexity += 1; - complexity = count_expression_complexity( - else_if_expression.expression(), - complexity, - ); - } + #[cfg(feature = "roblox")] + ast::Expression::IfExpression(if_expression) => { + complexity += 1; + if let Some(else_if_expressions) = if_expression.else_if_expressions() { + for else_if_expression in else_if_expressions { + complexity += 1; + complexity = + count_expression_complexity(else_if_expression.expression(), complexity); } - complexity } + complexity + } - #[cfg(feature = "roblox")] - ast::Value::InterpolatedString(interpolated_string) => { - for expression in interpolated_string.expressions() { - complexity = count_expression_complexity(expression, complexity) - } - - complexity + #[cfg(feature = "roblox")] + ast::Expression::InterpolatedString(interpolated_string) => { + for expression in interpolated_string.expressions() { + complexity = count_expression_complexity(expression, complexity) } - _ => complexity, - }, + complexity + } + + #[cfg(feature = "roblox")] + ast::Expression::TypeAssertion { expression, .. } => { + count_expression_complexity(expression, complexity) + } _ => complexity, } @@ -410,13 +401,13 @@ impl Visitor for HighCyclomaticComplexityVisitor { } } - fn visit_value(&mut self, value: &ast::Value) { - if let ast::Value::Function((_, function_body)) = value { + fn visit_expression(&mut self, expression: &ast::Expression) { + if let ast::Expression::Function((_, function_body)) = expression { let complexity = count_block_complexity(function_body.block(), 1); if complexity > self.config.maximum_complexity { self.positions.push(( ( - value.start_position().unwrap().bytes() as u32, + expression.start_position().unwrap().bytes() as u32, range(function_body.parameters_parentheses()).1, ), complexity, @@ -432,6 +423,7 @@ mod tests { #[test] #[cfg(feature = "roblox")] + #[cfg_attr(debug_assertions, ignore)] // Remove these with the full_moon parser rewrite fn test_high_cyclomatic_complexity() { test_lint( HighCyclomaticComplexityLint::new(HighCyclomaticComplexityConfig::default()).unwrap(), @@ -442,6 +434,7 @@ mod tests { #[test] #[cfg(feature = "roblox")] + #[cfg_attr(debug_assertions, ignore)] fn test_complex_var_expressions() { test_lint( HighCyclomaticComplexityLint::new(HighCyclomaticComplexityConfig::default()).unwrap(), diff --git a/selene-lib/src/lints/manual_table_clone.rs b/selene-lib/src/lints/manual_table_clone.rs index f2dc5e16..48a72b83 100644 --- a/selene-lib/src/lints/manual_table_clone.rs +++ b/selene-lib/src/lints/manual_table_clone.rs @@ -119,14 +119,8 @@ impl ManualTableCloneVisitor<'_> { let loop_expression = expressions.iter().next().unwrap(); let function_call = match strip_parentheses(loop_expression) { - Expression::Value { value, .. } => match &**value { - ast::Value::FunctionCall(function_call) => function_call, - _ if self.is_luau() => return Some((LoopType::Other, loop_expression)), - _ => return None, - }, - + Expression::FunctionCall(function_call) => function_call, _ if self.is_luau() => return Some((LoopType::Other, loop_expression)), - _ => return None, }; diff --git a/selene-lib/src/lints/mismatched_arg_count.rs b/selene-lib/src/lints/mismatched_arg_count.rs index 43ccd8f7..76abc03c 100644 --- a/selene-lib/src/lints/mismatched_arg_count.rs +++ b/selene-lib/src/lints/mismatched_arg_count.rs @@ -1,7 +1,7 @@ use super::*; use crate::{ ast_util::{ - is_function_call, is_vararg, range, + is_vararg, range, scopes::{Reference, ScopeManager, Variable}, }, text::plural, @@ -240,7 +240,9 @@ impl PassedArgumentCount { passed_argument_count += 1; if let ast::punctuated::Pair::End(expression) = argument { - if is_function_call(expression) || is_vararg(expression) { + if matches!(expression, ast::Expression::FunctionCall(_)) + || is_vararg(expression) + { return PassedArgumentCount::Variable(passed_argument_count); } } @@ -331,14 +333,12 @@ impl Visitor for MapFunctionDefinitionVisitor<'_> { .zip(local_assignment.expressions()); for (name_token, expression) in assignment_expressions { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::Function((_, function_body)) = &**value { - let identifier = range(name_token); + if let ast::Expression::Function((_, function_body)) = expression { + let identifier = range(name_token); - if let Some(id) = self.find_variable(identifier) { - self.definitions - .insert(id, ParameterCount::from_function_body(function_body)); - } + if let Some(id) = self.find_variable(identifier) { + self.definitions + .insert(id, ParameterCount::from_function_body(function_body)); } } } @@ -348,14 +348,12 @@ impl Visitor for MapFunctionDefinitionVisitor<'_> { let assignment_expressions = assignment.variables().iter().zip(assignment.expressions()); for (var, expression) in assignment_expressions { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::Function((_, function_body)) = &**value { - let identifier = range(var); + if let ast::Expression::Function((_, function_body)) = expression { + let identifier = range(var); - if let Some(reference) = self.find_reference(identifier) { - if let Some(variable) = reference.resolved { - self.verify_assignment(variable, function_body) - } + if let Some(reference) = self.find_reference(identifier) { + if let Some(variable) = reference.resolved { + self.verify_assignment(variable, function_body) } } } diff --git a/selene-lib/src/lints/parenthese_conditions.rs b/selene-lib/src/lints/parenthese_conditions.rs index f8fae46f..d82df1c1 100644 --- a/selene-lib/src/lints/parenthese_conditions.rs +++ b/selene-lib/src/lints/parenthese_conditions.rs @@ -49,15 +49,7 @@ struct ParentheseConditionsVisitor { impl ParentheseConditionsVisitor { fn lint_condition(&mut self, condition: &ast::Expression) { - let is_parentheses = match condition { - ast::Expression::Parentheses { .. } => true, - ast::Expression::Value { value, .. } => { - matches!(&**value, ast::Value::ParenthesesExpression(_)) - } - _ => false, - }; - - if is_parentheses { + if matches!(condition, ast::Expression::Parentheses { .. }) { self.positions.push(range(condition)); } } diff --git a/selene-lib/src/lints/roblox_incorrect_roact_usage.rs b/selene-lib/src/lints/roblox_incorrect_roact_usage.rs index 6aae7cb1..08a02c16 100644 --- a/selene-lib/src/lints/roblox_incorrect_roact_usage.rs +++ b/selene-lib/src/lints/roblox_incorrect_roact_usage.rs @@ -34,17 +34,14 @@ fn is_lua_valid_table_key_identifier(string: &String) -> bool { fn get_lua_table_key_format(expression: &ast::Expression) -> String { match expression { - ast::Expression::Value { value, .. } => match &**value { - ast::Value::String(token) => { - let string = token.to_string(); - if is_lua_valid_table_key_identifier(&string) { - string[1..string.len() - 1].to_string() - } else { - format!("[{}]", string) - } + ast::Expression::String(token) => { + let string = token.to_string(); + if is_lua_valid_table_key_identifier(&string) { + string[1..string.len() - 1].to_string() + } else { + format!("[{}]", string) } - _ => format!("[{}]", expression), - }, + } _ => format!("[{}]", expression), } } @@ -275,13 +272,11 @@ impl<'a> Visitor for IncorrectRoactUsageVisitor<'a> { // Get first argument, check if it is a Roblox class let name_arg = iter.next().unwrap(); - if let ast::Expression::Value { value, .. } = name_arg; - if let ast::Value::String(token) = &**value; + if let ast::Expression::String(token) = name_arg; if let Some((name, class)) = self.check_class_name(token); // Get second argument, check if it is a table - if let Some(ast::Expression::Value { value, .. }) = iter.next(); - if let ast::Value::TableConstructor(table) = &**value; + if let Some(ast::Expression::TableConstructor(table)) = iter.next(); then { ((name, class), table) @@ -318,8 +313,7 @@ impl<'a> Visitor for IncorrectRoactUsageVisitor<'a> { let key = strip_parentheses(key); if_chain::if_chain! { - if let ast::Expression::Value { value, .. } = key; - if let ast::Value::Var(ast::Var::Expression(var_expression)) = &**value; + if let ast::Expression::Var(ast::Var::Expression(var_expression)) = key; if let ast::Prefix::Name(constant_roact_name) = var_expression.prefix(); if ["Roact", "React"].contains(&constant_roact_name.token().to_string().as_str()); @@ -350,8 +344,7 @@ impl<'a> Visitor for IncorrectRoactUsageVisitor<'a> { 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 let ast::Expression::Var(ast::Var::Expression(var_expr)) = expr; if let Some(roact_or_react) = is_roact_or_react_create_element(var_expr.prefix(), &var_expr.suffixes().collect::>()); then { self.definitions_of_create_element.insert(name.token().to_string(), roact_or_react); diff --git a/selene-lib/src/lints/roblox_suspicious_udim2_new.rs b/selene-lib/src/lints/roblox_suspicious_udim2_new.rs index 59f801c1..81f670de 100644 --- a/selene-lib/src/lints/roblox_suspicious_udim2_new.rs +++ b/selene-lib/src/lints/roblox_suspicious_udim2_new.rs @@ -110,10 +110,7 @@ impl Visitor for UDim2CountVisitor { } let numbers_passed = arguments.iter().filter(|expression| { - match expression { - ast::Expression::Value { value, .. } => matches!(&**value, ast::Value::Number(_)), - _ => false, - } + matches!(expression, ast::Expression::Number(_)) }).count(); // Prevents false positives for UDim2.new(UDim.new(), UDim.new()) diff --git a/selene-lib/src/lints/standard_library.rs b/selene-lib/src/lints/standard_library.rs index 2a0795e8..68796673 100644 --- a/selene-lib/src/lints/standard_library.rs +++ b/selene-lib/src/lints/standard_library.rs @@ -61,73 +61,66 @@ fn get_argument_type(expression: &ast::Expression) -> Option } } - ast::Expression::Value { value, .. } => match &**value { - ast::Value::Function(_) => Some(ArgumentType::Function.into()), - ast::Value::FunctionCall(_) => None, - ast::Value::Number(_) => Some(ArgumentType::Number.into()), - ast::Value::ParenthesesExpression(expression) => get_argument_type(expression), - ast::Value::String(token) => { - Some(PassedArgumentType::from_string(token.token().to_string())) - } - #[cfg_attr( - feature = "force_exhaustive_checks", - allow(non_exhaustive_omitted_patterns) - )] - ast::Value::Symbol(symbol) => match *symbol.token_type() { - TokenType::Symbol { symbol } => match symbol { - Symbol::False => Some(ArgumentType::Bool.into()), - Symbol::True => Some(ArgumentType::Bool.into()), - Symbol::Nil => Some(ArgumentType::Nil.into()), - Symbol::Ellipse => Some(ArgumentType::Vararg.into()), - ref other => { - unreachable!("TokenType::Symbol was not expected ({:?})", other) - } - }, - - ref other => unreachable!( - "ast::Value::Symbol token_type != TokenType::Symbol ({:?})", - other - ), - }, - ast::Value::TableConstructor(_) => Some(ArgumentType::Table.into()), - ast::Value::Var(_) => None, - - #[cfg(feature = "roblox")] - ast::Value::IfExpression(if_expression) => { - // This could be a union type - let expected_type = get_argument_type(if_expression.if_expression())?; - - if let Some(else_if_expressions) = if_expression.else_if_expressions() { - for else_if_expression in else_if_expressions { - if !get_argument_type(else_if_expression.expression())? - .same_type(&expected_type) - { - return None; - } - } + ast::Expression::Function(_) => Some(ArgumentType::Function.into()), + ast::Expression::FunctionCall(_) => None, + ast::Expression::Number(_) => Some(ArgumentType::Number.into()), + ast::Expression::String(token) => { + Some(PassedArgumentType::from_string(token.token().to_string())) + } + #[cfg_attr( + feature = "force_exhaustive_checks", + allow(non_exhaustive_omitted_patterns) + )] + ast::Expression::Symbol(symbol) => match *symbol.token_type() { + TokenType::Symbol { symbol } => match symbol { + Symbol::False => Some(ArgumentType::Bool.into()), + Symbol::True => Some(ArgumentType::Bool.into()), + Symbol::Nil => Some(ArgumentType::Nil.into()), + Symbol::Ellipse => Some(ArgumentType::Vararg.into()), + ref other => { + unreachable!("TokenType::Symbol was not expected ({:?})", other) } + }, - if get_argument_type(if_expression.else_expression())?.same_type(&expected_type) { - Some(expected_type) - } else { - None + ref other => unreachable!( + "ast::Expression::Symbol token_type != TokenType::Symbol ({:?})", + other + ), + }, + ast::Expression::TableConstructor(_) => Some(ArgumentType::Table.into()), + ast::Expression::Var(_) => None, + + #[cfg(feature = "roblox")] + ast::Expression::IfExpression(if_expression) => { + // This could be a union type + let expected_type = get_argument_type(if_expression.if_expression())?; + + if let Some(else_if_expressions) = if_expression.else_if_expressions() { + for else_if_expression in else_if_expressions { + if !get_argument_type(else_if_expression.expression())? + .same_type(&expected_type) + { + return None; + } } } - #[cfg(feature = "roblox")] - ast::Value::InterpolatedString(interpolated_string) => { - if interpolated_string.expressions().next().is_some() { - Some(ArgumentType::String.into()) - } else { - // Simple string, aka `Workspace` - Some(PassedArgumentType::from_string( - interpolated_string.last_string().token().to_string(), - )) - } - } + get_argument_type(if_expression.else_expression())? + .same_type(&expected_type) + .then_some(expected_type) + } - _ => None, - }, + #[cfg(feature = "roblox")] + ast::Expression::InterpolatedString(interpolated_string) => { + if interpolated_string.expressions().next().is_some() { + Some(ArgumentType::String.into()) + } else { + // Simple string, aka `Workspace` + Some(PassedArgumentType::from_string( + interpolated_string.last_string().token().to_string(), + )) + } + } ast::Expression::BinaryOperator { lhs, binop, rhs, .. @@ -184,10 +177,25 @@ fn get_argument_type(expression: &ast::Expression) -> Option None } + #[cfg(feature = "roblox")] + ast::BinOp::DoubleSlash(_) => { + let lhs_type = get_argument_type(lhs); + let rhs_type = get_argument_type(rhs); + + if lhs_type == rhs_type { + lhs_type + } else { + None + } + } + _ => None, } } + #[cfg(feature = "roblox")] + ast::Expression::TypeAssertion { expression, .. } => get_argument_type(expression), + _ => None, } } @@ -524,15 +532,13 @@ impl Visitor for StandardLibraryVisitor<'_> { let mut maybe_more_arguments = false; if let ast::FunctionArgs::Parentheses { arguments, .. } = function_args { - if let Some(ast::punctuated::Pair::End(ast::Expression::Value { value, .. })) = - arguments.last() - { - match &**value { - ast::Value::FunctionCall(_) => { + if let Some(ast::punctuated::Pair::End(last_argument)) = arguments.last() { + match last_argument { + ast::Expression::FunctionCall(_) => { maybe_more_arguments = true; } - ast::Value::Symbol(token_ref) => { + ast::Expression::Symbol(token_ref) => { if let TokenType::Symbol { symbol } = token_ref.token().token_type() { if symbol == &full_moon::tokenizer::Symbol::Ellipse { maybe_more_arguments = true; diff --git a/selene-lib/src/lints/suspicious_reverse_loop.rs b/selene-lib/src/lints/suspicious_reverse_loop.rs index 4b070970..37ada923 100644 --- a/selene-lib/src/lints/suspicious_reverse_loop.rs +++ b/selene-lib/src/lints/suspicious_reverse_loop.rs @@ -57,8 +57,7 @@ impl Visitor for SuspiciousReverseLoopVisitor { unop: ast::UnOp::Hash(_), .. } = node.start(); - if let ast::Expression::Value { value, .. } = node.end(); - if let ast::Value::Number(number) = &**value; + if let ast::Expression::Number(number) = node.end(); if str::parse::(&number.token().to_string()).ok() <= Some(1.0); then { self.positions.push(( diff --git a/selene-lib/src/lints/type_check_inside_call.rs b/selene-lib/src/lints/type_check_inside_call.rs index f06fad59..c88a50da 100644 --- a/selene-lib/src/lints/type_check_inside_call.rs +++ b/selene-lib/src/lints/type_check_inside_call.rs @@ -67,8 +67,7 @@ impl Visitor for TypeCheckInsideCallVisitor { = arguments.iter().next(); // Check that rhs is a constant string - if let ast::Expression::Value { value: rhs_value, .. } = &**rhs; - if let ast::Value::String(_) = &**rhs_value; + if matches!(&**rhs, ast::Expression::String(_)); then { self.positions.push(range(call)); diff --git a/selene-lib/src/lints/unbalanced_assignments.rs b/selene-lib/src/lints/unbalanced_assignments.rs index 68d31f3d..67dbaf38 100644 --- a/selene-lib/src/lints/unbalanced_assignments.rs +++ b/selene-lib/src/lints/unbalanced_assignments.rs @@ -74,9 +74,7 @@ struct UnbalancedAssignmentsVisitor { fn expression_is_call(expression: &ast::Expression) -> bool { match expression { ast::Expression::Parentheses { expression, .. } => expression_is_call(expression), - ast::Expression::Value { value, .. } => { - matches!(&**value, ast::Value::FunctionCall(_)) - } + ast::Expression::FunctionCall(_) => true, _ => false, } @@ -85,15 +83,11 @@ fn expression_is_call(expression: &ast::Expression) -> bool { fn expression_is_nil(expression: &ast::Expression) -> bool { match expression { ast::Expression::Parentheses { expression, .. } => expression_is_call(expression), - ast::Expression::Value { value, .. } => { - if let ast::Value::Symbol(symbol) = &**value { - *symbol.token_type() - == TokenType::Symbol { - symbol: Symbol::Nil, - } - } else { - false - } + ast::Expression::Symbol(symbol) => { + *symbol.token_type() + == TokenType::Symbol { + symbol: Symbol::Nil, + } } _ => false, @@ -101,13 +95,11 @@ fn expression_is_nil(expression: &ast::Expression) -> bool { } fn expression_is_ellipsis(expression: &ast::Expression) -> bool { - if let ast::Expression::Value { value, .. } = expression { - if let ast::Value::Symbol(symbol) = &**value { - return *symbol.token_type() - == TokenType::Symbol { - symbol: Symbol::Ellipse, - }; - } + if let ast::Expression::Symbol(symbol) = expression { + return *symbol.token_type() + == TokenType::Symbol { + symbol: Symbol::Ellipse, + }; } false diff --git a/selene-lib/tests/lints/divide_by_zero/divide_by_zero.lua b/selene-lib/tests/lints/divide_by_zero/divide_by_zero.lua index 5e902c2b..9b9422ce 100644 --- a/selene-lib/tests/lints/divide_by_zero/divide_by_zero.lua +++ b/selene-lib/tests/lints/divide_by_zero/divide_by_zero.lua @@ -3,3 +3,4 @@ local _ = 5 / 0 local _ = 0 / 5 local _ = x / 0 local _ = 0 / 0 +local _ = -1 / 0 \ No newline at end of file diff --git a/selene-lib/tests/lints/divide_by_zero/divide_by_zero.stderr b/selene-lib/tests/lints/divide_by_zero/divide_by_zero.stderr index 246657c2..e6bb50c2 100644 --- a/selene-lib/tests/lints/divide_by_zero/divide_by_zero.stderr +++ b/selene-lib/tests/lints/divide_by_zero/divide_by_zero.stderr @@ -10,3 +10,9 @@ error[divide_by_zero]: dividing by zero is not readable 4 │ local _ = x / 0 │ ^^^^^ try: `math.huge()` +error[divide_by_zero]: dividing by zero is not allowed, use math.huge instead + ┌─ divide_by_zero.lua:6:11 + │ +6 │ local _ = -1 / 0 + │ ^^^^^^ +