diff --git a/Cargo.lock b/Cargo.lock index c5a1a9f9..3a27e915 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -792,6 +792,7 @@ dependencies = [ "cairo-lang-diagnostics", "cairo-lang-filesystem", "cairo-lang-formatter", + "cairo-lang-lowering", "cairo-lang-parser", "cairo-lang-semantic", "cairo-lang-syntax", diff --git a/Cargo.toml b/Cargo.toml index 2bde8375..240529a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ cairo-lang-defs = "*" cairo-lang-diagnostics = "*" cairo-lang-filesystem = "*" cairo-lang-formatter = "*" +cairo-lang-lowering = "*" cairo-lang-parser = "*" cairo-lang-semantic = "*" cairo-lang-syntax = "*" diff --git a/src/context.rs b/src/context.rs index 5a82885d..d9d133bb 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,4 +1,6 @@ use crate::fixer::InternalFix; +use crate::lints::assert_on_const::AssertOnConst; +use crate::lints::assert_on_const::check_assert_on_const; use crate::lints::bitwise_for_parity_check::BitwiseForParity; use crate::lints::bitwise_for_parity_check::check_bitwise_for_parity; use crate::lints::bool_comparison::BoolComparison; @@ -403,6 +405,10 @@ impl LintContext { lints: vec![Box::new(ManualUnwrapOrElse)], check_function: check_manual_unwrap_or_else, }, + LintRuleGroup { + lints: vec![Box::new(AssertOnConst)], + check_function: check_assert_on_const, + }, ] } diff --git a/src/fixer/db.rs b/src/fixer/db.rs index c8d1c047..b557cd27 100644 --- a/src/fixer/db.rs +++ b/src/fixer/db.rs @@ -1,7 +1,7 @@ use cairo_lang_defs::db::{defs_group_input, init_external_files}; use cairo_lang_filesystem::db::files_group_input; +use cairo_lang_lowering::{db::init_lowering_group, optimizations::config::Optimizations}; use cairo_lang_semantic::db::semantic_group_input; - use salsa::{Database, Setter}; #[salsa::db] @@ -15,6 +15,13 @@ impl salsa::Database for FixerDatabase {} impl FixerDatabase { pub fn new_from(db: &dyn Database) -> Self { let mut new_db = Self::new(); + + init_lowering_group( + &mut new_db, + Optimizations::enabled_with_minimal_movable_functions(), + None, + ); + // SemanticGroup salsa inputs. semantic_group_input(&new_db) .set_default_analyzer_plugins(&mut new_db) diff --git a/src/lang/db.rs b/src/lang/db.rs index 28d20408..c300eb62 100644 --- a/src/lang/db.rs +++ b/src/lang/db.rs @@ -15,6 +15,7 @@ use cairo_lang_filesystem::{ flag::Flag, ids::FlagLongId, }; +use cairo_lang_lowering::{db::init_lowering_group, optimizations::config::Optimizations}; use cairo_lang_semantic::{ db::{init_semantic_group, semantic_group_input}, ids::AnalyzerPluginLongId, @@ -44,6 +45,11 @@ impl LinterAnalysisDatabase { init_defs_group(&mut res); init_semantic_group(&mut res); init_external_files(&mut res); + init_lowering_group( + &mut res, + Optimizations::enabled_with_minimal_movable_functions(), + None, + ); default_plugin_suite.add(cairo_lint_allow_plugin_suite()); @@ -74,7 +80,6 @@ impl LinterAnalysisDatabase { .map(AnalyzerPluginLongId) .collect(), )); - res } } diff --git a/src/lints/assert_on_const.rs b/src/lints/assert_on_const.rs new file mode 100644 index 00000000..2b99fdcb --- /dev/null +++ b/src/lints/assert_on_const.rs @@ -0,0 +1,428 @@ +use cairo_lang_defs::{ + ids::{FunctionWithBodyId, LanguageElementId, ModuleId, ModuleItemId}, + plugin::PluginDiagnostic, +}; +use cairo_lang_diagnostics::Severity; +use cairo_lang_filesystem::ids::FileLongId; +use cairo_lang_lowering::{ + self as lowering, Lowered, LoweringStage, Statement, StatementCall, db::LoweringGroup, +}; +use cairo_lang_parser::db::ParserGroup; +use cairo_lang_semantic::{ + self as semantic, FunctionId, + corelib::{core_bool_enum, unit_ty}, + helper::ModuleHelper, + items::{free_function::FreeFunctionSemantic, imp::ImplSemantic, trt::TraitSemantic}, + lsp_helpers::LspHelpers, +}; +use cairo_lang_syntax::node::{ + SyntaxNode, Terminal, TypedSyntaxNode, + ast::{ExprInlineMacro, ExprUnary, PathSegment}, +}; +use cairo_lang_utils::Intern; +use itertools::Itertools; +use salsa::Database; + +use crate::{ + context::{CairoLintKind, Lint}, + queries::get_all_inline_macro_calls, +}; + +pub struct AssertOnConst; + +/// ## What it does +/// +/// Checks for assertions on boolean literals, constants and expressions +/// which are simplified to constants by the compiler. +/// +/// ## Example +/// +/// ```cairo +/// fn main() { +/// // Bool consts: +/// const C: bool = true; +/// assert!(C); // Always passes +/// +/// // Bool literals: +/// assert!(true); // Always passes +/// assert!(false); // Never passes +/// +/// // Bool expressions: +/// assert!(true && false); // Never passes +/// assert!((1 == 1) || (2 == 2)); // Always passes +/// } +/// ``` +impl Lint for AssertOnConst { + fn allowed_name(&self) -> &'static str { + "assert_on_const" + } + + fn diagnostic_message(&self) -> &'static str { + "Unnecessary assert on a const value detected." + } + + fn kind(&self) -> crate::context::CairoLintKind { + CairoLintKind::RedundantOperation + } +} + +/// Checks for `assert!`s called on const boolean expressions. +/// +/// This function implements an algorithm which allows us to determine whether an `assert!` +/// is called with an argument which can be simplified to a const expression by the compiler during const folding. +/// +/// # Expansion of the `assert!` macro: +/// `assert!(condition, error_message)` generates the following code: +/// ```cairo +/// if !(condition) { +/// // panic with error_message +/// } +/// ``` +/// +/// This expansion has two important properties in two layers of abstraction: +/// 1. Syntax: AST of the expansion **always** contains a `ExprUnary` node as a top-level expression inside the `if` +/// 2. Semantic: This unary expression **always** translates to the call of `core::bool_not_impl` function on the condition. +/// +/// # Algorithm: +/// For each `assert!`: +/// 1. Obtain its expansion and the top-level unary expression. +/// 2. Obtain the lowered representation of the function. +/// 3. Collect all boolean consts and lowering-level variables which have unconditional, constant values. +/// 4. Collect all calls to `core::bool_not_impl` on those consts and const variables. **Important note**: Lowering representation contains their location in the code. +/// 5. Check if among the collected calls to `core::bool_not_impl` **exactly one** has a span identical to the unary expression from the `assert!` expansion. +/// +/// This way, we make absolutely sure that the expression we call `assert!` on can be const-folded. +#[tracing::instrument(skip_all, level = "trace")] +pub fn check_assert_on_const<'db>( + db: &'db dyn Database, + item: &ModuleItemId<'db>, + diagnostics: &mut Vec>, +) { + let functions_with_body = match item { + ModuleItemId::FreeFunction(free_function_id) => { + vec![FunctionWithBodyId::Free(*free_function_id)] + } + ModuleItemId::Impl(impl_def_id) => { + let Ok(impl_functions) = db.impl_functions(*impl_def_id) else { + return; + }; + + impl_functions + .values() + .map(|impl_function_id| FunctionWithBodyId::Impl(*impl_function_id)) + .collect_vec() + } + ModuleItemId::Trait(trait_id) => { + let Ok(trait_functions) = db.trait_functions(*trait_id) else { + return; + }; + + trait_functions + .values() + .map(|trait_function_id| FunctionWithBodyId::Trait(*trait_function_id)) + .collect_vec() + } + _ => return, + }; + + for function_with_body_id in functions_with_body { + check_assert_on_const_for_function_with_body(db, item, function_with_body_id, diagnostics); + } +} + +fn check_assert_on_const_for_function_with_body<'db>( + db: &'db dyn Database, + module_item_id: &ModuleItemId<'db>, + function_with_body_id: FunctionWithBodyId<'db>, + diagnostics: &mut Vec>, +) { + let Some(function_body_lowering) = get_function_body_lowering(db, function_with_body_id) else { + return; + }; + + let bool_not_impl_calls_on_const_exprs = + find_bool_not_impl_calls_on_const_values(db, function_body_lowering); + + let module_id = module_item_id.parent_module(db); + + for assert_call in get_assert_macro_calls(db, module_item_id) { + let Some(expansion_syntax) = get_inline_macro_expansion_syntax(db, &assert_call, module_id) + else { + continue; + }; + + let Some(unary_expression) = expansion_syntax + .descendants(db) + .find_map(|node| node.cast::(db)) + else { + continue; + }; + + if is_unary_expr_a_bool_not_impl_call_on_const( + db, + unary_expression, + &bool_not_impl_calls_on_const_exprs, + ) { + diagnostics.push(PluginDiagnostic { + stable_ptr: assert_call.as_syntax_node().stable_ptr(db), + message: AssertOnConst.diagnostic_message().to_string(), + severity: Severity::Warning, + inner_span: None, + error_code: None, + }) + } + } +} + +/// Returns a lowered representation of the function after applying baseline optimizations, +/// including the const folding, which is essential for this lint. +fn get_function_body_lowering<'db>( + db: &'db dyn Database, + function_with_body_id: FunctionWithBodyId<'db>, +) -> Option<&'db Lowered<'db>> { + // `LoweringGroup::lowered_body` can only be applied to functions that have no generic parameters in scope. + if has_generic_params(db, function_with_body_id) { + return None; + } + + // `ConcreteFunctionWithBodyId::from_generic` only changes the semantic representation from generic to + // concrete and does not actually check if the function is concrete. + // The check above ensures that the obtained concrete function is lowerable. + let semantic_concrete_function_with_body_id = + semantic::ConcreteFunctionWithBodyId::from_generic(db, function_with_body_id).ok()?; + + let lowering_concrete_function_with_body_id = + lowering::ids::ConcreteFunctionWithBodyLongId::Semantic( + semantic_concrete_function_with_body_id, + ) + .intern(db); + + db.lowered_body( + lowering_concrete_function_with_body_id, + LoweringStage::PostBaseline, + ) + .ok() +} + +/// Checks whether the function has any generic parameters in scope - either its own or from the enclosing impl or trait. +fn has_generic_params<'db>( + db: &'db dyn Database, + function_with_body_id: FunctionWithBodyId<'db>, +) -> bool { + let is_empty = |params: cairo_lang_diagnostics::Maybe<&[semantic::GenericParam<'db>]>| { + params.is_ok_and(|params| params.is_empty()) + }; + + match function_with_body_id { + FunctionWithBodyId::Free(free_function_id) => { + !is_empty(db.free_function_generic_params(free_function_id)) + } + FunctionWithBodyId::Impl(impl_function_id) => { + !is_empty(db.impl_function_generic_params(impl_function_id)) + || !is_empty(db.impl_def_generic_params(impl_function_id.impl_def_id(db))) + } + FunctionWithBodyId::Trait(trait_function_id) => { + !is_empty(db.trait_function_generic_params(trait_function_id)) + || !is_empty(db.trait_generic_params(trait_function_id.trait_id(db))) + } + } +} + +/// Finds all statements in the lowered representation which are calls to `core::bool_not_impl`. +/// Returns only those which have **constant arguments**. +fn find_bool_not_impl_calls_on_const_values<'db>( + db: &'db dyn Database, + function_body_lowering: &'db Lowered<'db>, +) -> Vec<&'db StatementCall<'db>> { + // Const statements can be collected from all blocks. + // They are rather unlikely to appear outside the block they are defined in though. + let mut const_statements = vec![]; + + // We collect all these calls from all the blocks because + // we never know which of them is a part of the assert! macro. + let mut bool_not_impl_calls_on_const_exprs: Vec<&'db StatementCall<'db>> = vec![]; + + for (_, block) in function_body_lowering.blocks.iter() { + // Unit structs and bool enums should be collected separately for each block. + // If variables with those are used across two blocks, + // it always means that their values are conditional. + let mut unit_structs = vec![]; + let mut bool_enum_constructs = vec![]; + + for statement in block.statements.iter() { + match statement { + // We obviously need to collect all bool constants. + // Those are defined explicitly by the user and aren't results of the const folding. + Statement::Const(const_statement) => { + const_statements.push(const_statement); + } + + // Bool enums are always constructed from unit structs, + // that's why we need to collect those. + Statement::StructConstruct(struct_construct) => { + let output_variable = function_body_lowering + .variables + .get(struct_construct.output) + .expect("function body lowering should contain variable from it own block"); + + let output_type = output_variable.ty.long(db); + + if output_type == unit_ty(db).long(db) { + unit_structs.push(struct_construct); + } + } + + // Collect all bool enum instantiations. + Statement::EnumConstruct(enum_construct) => { + let input_variable = enum_construct.input; + + let concrete_enum_id = enum_construct.variant.concrete_enum_id; + let is_bool = concrete_enum_id == core_bool_enum(db); + + // Bool enum can also be constructed from other boolean variable. + // That means it is not constant. + let is_constructed_from_unit_struct = unit_structs + .iter() + .any(|unit_struct| unit_struct.output == input_variable.var_id); + + if is_constructed_from_unit_struct && is_bool { + bool_enum_constructs.push(enum_construct); + } + } + + // Collect all calls to `core::bool_not_impl`. + Statement::Call(call) => { + let Some(function_id) = + try_semantic_function_id_from_lowering(db, call.function) + else { + continue; + }; + + if !is_core_bool_not_impl(db, function_id) { + continue; + } + + // This function is guaranteed to have exactly one argument. + let input = call + .inputs + .first() + .expect("bool_not_impl should have exactly one argument"); + + // Check if the function is called on a bool constant (defined as Statement::Const). + let is_input_const = const_statements + .iter() + .any(|const_statement| const_statement.output == input.var_id); + + // Check if the function is called on a bool variable, created using `EnumConstruct`. + let is_input_bool_literal = bool_enum_constructs + .iter() + .any(|bool_enum| bool_enum.output == input.var_id); + + // Check if the function is called on an output of other `bool_not_impl` captured before. + // This occurs when `assert!` contains a negated expression. + let is_input_other_bool_not_impl = bool_not_impl_calls_on_const_exprs + .iter() + .any(|call| call.outputs.contains(&input.var_id)); + + if !is_input_const && !is_input_bool_literal && !is_input_other_bool_not_impl { + continue; + } + + bool_not_impl_calls_on_const_exprs.push(call); + } + + _ => {} + } + } + } + + bool_not_impl_calls_on_const_exprs +} + +/// Transforms a function ID from the lowering representation to the corresponding semantic representation. +fn try_semantic_function_id_from_lowering<'db>( + db: &'db dyn Database, + function_id: lowering::ids::FunctionId<'db>, +) -> Option> { + match function_id.long(db) { + lowering::ids::FunctionLongId::Semantic(function_id) => Some(*function_id), + _ => None, + } +} + +/// Checks if the given function is `core::bool_not_impl`. +fn is_core_bool_not_impl<'db>(db: &'db dyn Database, function_id: FunctionId<'db>) -> bool { + let bool_not_impl = ModuleHelper::core(db).extern_function_id("bool_not_impl"); + function_id.try_get_extern_function_id(db) == Some(bool_not_impl) +} + +/// Returns all `assert!` calls from the given module item. +fn get_assert_macro_calls<'db>( + db: &'db dyn Database, + module_item: &ModuleItemId<'db>, +) -> impl Iterator> { + get_all_inline_macro_calls(db, module_item) + .into_iter() + .filter(|call| { + let path_elements = call.path(db).segments(db).elements(db).collect::>(); + match &path_elements[..] { + [PathSegment::Simple(path_segment)] => { + path_segment.ident(db).text(db).long(db) == "assert" + } + _ => false, + } + }) +} + +/// Returns a syntax node which is a root of the syntax tree +/// constructed during expansion of the given inline macro. +fn get_inline_macro_expansion_syntax<'db>( + db: &'db dyn Database, + inline_macro: &ExprInlineMacro<'db>, + module: ModuleId<'db>, +) -> Option> { + let expansion_virtual_file = + db.inline_macro_expansion_files(module) + .iter() + .find_map(|&file_id| { + let FileLongId::Virtual(virtual_file) = file_id.long(db) else { + return None; + }; + + let span = virtual_file.parent?; + + inline_macro + .as_syntax_node() + .span(db) + .contains(span.span) + .then_some(file_id) + })?; + + db.file_syntax(expansion_virtual_file).ok() +} + +/// Checks if the given list of calls to `core::bool_not_impl` function +/// contains exactly one call which was generated by the `assert!` macro. +fn is_unary_expr_a_bool_not_impl_call_on_const<'db>( + db: &'db dyn Database, + unary_expr: ExprUnary<'db>, + bool_not_impl_calls: &[&'db StatementCall<'db>], +) -> bool { + let bool_not_calls_inside_assert = bool_not_impl_calls + .iter() + .filter(|call| { + call.inputs + .first() + .expect("bool_not_impl should have exactly one argument") + .location + .all_locations(db) + .iter() + .any(|location| { + location.span_in_file(db) + == unary_expr.as_syntax_node().stable_ptr(db).span_in_file(db) + }) + }) + .collect::>(); + + bool_not_calls_inside_assert.len() == 1 +} diff --git a/src/lints/mod.rs b/src/lints/mod.rs index be415c05..1275db41 100644 --- a/src/lints/mod.rs +++ b/src/lints/mod.rs @@ -3,6 +3,7 @@ use cairo_lang_semantic::FunctionId; use cairo_lang_semantic::items::imp::ImplSemantic; use salsa::Database; +pub mod assert_on_const; pub mod bitwise_for_parity_check; pub mod bool_comparison; pub mod breaks; diff --git a/src/queries.rs b/src/queries.rs index 151d825d..c5f1460c 100644 --- a/src/queries.rs +++ b/src/queries.rs @@ -7,7 +7,7 @@ use cairo_lang_semantic::{ ExprWhile, FunctionBody, Pattern, Statement, StatementBreak, }; use cairo_lang_syntax::node::TypedSyntaxNode; -use cairo_lang_syntax::node::ast::ExprParenthesized; +use cairo_lang_syntax::node::ast::{ExprInlineMacro, ExprParenthesized}; use cairo_lang_syntax::node::kind::SyntaxKind; use cairo_lang_syntax::node::{SyntaxNode, TypedStablePtr}; use if_chain::if_chain; @@ -203,6 +203,25 @@ pub fn get_all_while_expressions<'db>( .collect() } +#[tracing::instrument(skip_all, level = "trace")] +pub fn get_all_inline_macro_calls<'db>( + db: &'db dyn Database, + item: &ModuleItemId<'db>, +) -> Vec> { + let function_nodes = match item { + ModuleItemId::Constant(id) => id.stable_ptr(db).lookup(db).as_syntax_node(), + ModuleItemId::FreeFunction(id) => id.stable_ptr(db).lookup(db).as_syntax_node(), + ModuleItemId::Impl(id) => id.stable_ptr(db).lookup(db).as_syntax_node(), + ModuleItemId::Trait(id) => id.stable_ptr(db).lookup(db).as_syntax_node(), + _ => return vec![], + } + .descendants(db); + + function_nodes + .filter_map(|node| node.cast::(db)) + .collect() +} + #[tracing::instrument(skip_all, level = "trace")] pub fn get_all_break_statements<'db>( function_body: &'db FunctionBody<'db>, diff --git a/tests/assert_on_const/mod.rs b/tests/assert_on_const/mod.rs new file mode 100644 index 00000000..2bc351aa --- /dev/null +++ b/tests/assert_on_const/mod.rs @@ -0,0 +1,267 @@ +use crate::test_lint_diagnostics; + +const BOOL_LITERAL: &str = r#" +fn foo() { + assert!(true, "message"); +} +"#; + +const BOOL_CONST: &str = r#" +const C: bool = false; +fn foo() { + assert!(C, "message"); +} +"#; + +const BOOL_CONST_NEGATED: &str = r#" +const C: bool = false; +fn foo() { + assert!(!C, "message"); +} +"#; + +const BOOL_EXPR_SIMPLE: &str = r#" +fn foo() { + assert!(1 == 1, "message"); +} +"#; + +const BOOL_EXPR_SIMPLE_NEGATED: &str = r#" +fn foo() { + assert!(!((1 == 1) && (2 == 2)), "message"); +} +"#; + +const BOOL_EXPR_ARITHMETIC: &str = r#" +fn foo() { + assert!(1 == (1 + 1), "message"); +} +"#; + +const BOOL_EXPR_ARITHMETIC_COMPLEX: &str = r#" +fn foo() { +assert!((1 == 1) && (5 - 5 == 3 - 3)); +} +"#; + +const BOOL_EXPR_WITH_CONST: &str = r#" +const C: bool = false; +const D: bool = false; +fn foo() { + assert!(C && D, "message"); +} +"#; + +const BOOL_EXPR_SIMPLE_WITH_FUNCTION_CALL: &str = r#" +fn bar(x: felt252) -> bool { + x == 10 +} +fn foo() { + assert!(true || bar(5), "message"); +} +"#; + +const BOOL_EXPR_COMPLEX_WITH_FUNCTION_CALL: &str = r#" +fn bar(x: felt252) -> bool { + x == 10 +} +fn foo() { + assert!(((1 == 1) && (5 == 5)) || bar(5), "message"); +} +"#; + +/// This is a negative example. Lint should not trigger +/// because the expression we assert on is not constant. +const BOOL_EXPR_NON_CONST_WITH_FUNCTION_CALL: &str = r#" +fn bar(x: felt252) -> bool { + x == 10 +} +fn foo(x: felt252) { + assert!(bar(x), "message"); +} +"#; + +const BOOL_CONST_IN_IMPL_FUNCTION: &str = r#" +trait Trait { + fn foo(); +} + +impl Impl of Trait { + fn foo() { + assert!(true, "message"); + } +} +"#; + +/// A trait function with a default body which is not fully concrete (its impl is the abstract `Self` impl) +/// but has no generic parameters, so the lint should handle it. +const BOOL_CONST_IN_TRAIT_FUNCTION: &str = r#" +trait Trait { + fn foo() { + assert!(true, "message"); + } +} +"#; + +/// A trait function with a default body which is not fully concrete (its impl is the abstract `Self` impl) +/// and additionally pulls an associated type into a body variable. +/// Lint should fail gracefully when trying to retrieve the lowering of the function and not panic. +const BOOL_CONST_IN_TRAIT_FUNCTION_WITH_ASSOCIATED_TYPE: &str = r#" +trait Trait { + type Item; + fn item() -> Self::Item; + fn foo() { + let _item = Self::item(); + assert!(true, "message"); + } +} +"#; + +/// A trait function which is not fully concrete and has a generic parameter - it silently takes a generic impl of PartialEq as a param. +/// Lint should fail gracefully when trying to retrieve the lowering of the function and not panic. +const BOOL_EXPR_NOT_FULLY_CONCRETE_FUNCTION: &str = r#" +pub trait MyTrait { + fn foo<+PartialEq>(a: @T, b: @T); +} + +impl MyTraitImpl of MyTrait { + fn foo<+PartialEq>(a: @T, b: @T) { + assert!(a == b, "message"); + } +} +"#; + +#[test] +fn bool_literal_diagnostics() { + test_lint_diagnostics!(BOOL_LITERAL, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:3:5 + assert!(true, "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_const_diagnostics() { + test_lint_diagnostics!(BOOL_CONST, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:4:5 + assert!(C, "message"); + ^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_const_negated_diagnostics() { + test_lint_diagnostics!(BOOL_CONST_NEGATED, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:4:5 + assert!(!C, "message"); + ^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_expr_simple_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_SIMPLE, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:3:5 + assert!(1 == 1, "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_expr_simple_negated_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_SIMPLE_NEGATED, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:3:5 + assert!(!((1 == 1) && (2 == 2)), "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_expr_arithmetic_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_ARITHMETIC, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:3:5 + assert!(1 == (1 + 1), "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_expr_arithmetic_complex_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_ARITHMETIC_COMPLEX, @r" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:3:1 + assert!((1 == 1) && (5 - 5 == 3 - 3)); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ") +} + +#[test] +fn bool_expr_with_const_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_WITH_CONST, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:5:5 + assert!(C && D, "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_expr_simple_with_function_call_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_SIMPLE_WITH_FUNCTION_CALL, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:6:5 + assert!(true || bar(5), "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_expr_complex_with_function_call_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_COMPLEX_WITH_FUNCTION_CALL, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:6:5 + assert!(((1 == 1) && (5 == 5)) || bar(5), "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn non_const_bool_expr_with_function_call_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_NON_CONST_WITH_FUNCTION_CALL, @"") +} + +#[test] +fn bool_const_in_impl_function_diagnostics() { + test_lint_diagnostics!(BOOL_CONST_IN_IMPL_FUNCTION, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:8:9 + assert!(true, "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_const_in_trait_function_diagnostics() { + test_lint_diagnostics!(BOOL_CONST_IN_TRAIT_FUNCTION, @r#" + Plugin diagnostic: Unnecessary assert on a const value detected. + --> lib.cairo:4:9 + assert!(true, "message"); + ^^^^^^^^^^^^^^^^^^^^^^^^ + "#) +} + +#[test] +fn bool_const_in_trait_function_with_associated_type_diagnostics() { + test_lint_diagnostics!(BOOL_CONST_IN_TRAIT_FUNCTION_WITH_ASSOCIATED_TYPE, @"") +} + +#[test] +fn bool_expr_not_fully_concrete_function_diagnostics() { + test_lint_diagnostics!(BOOL_EXPR_NOT_FULLY_CONCRETE_FUNCTION, @"") +} diff --git a/tests/ifs/collapsible_if.rs b/tests/ifs/collapsible_if.rs index 414e01e7..ba6b2d1c 100644 --- a/tests/ifs/collapsible_if.rs +++ b/tests/ifs/collapsible_if.rs @@ -75,8 +75,8 @@ fn main() { "#; const COLLAPSIBLE_IF_WITH_FUNCTION_CALLS: &str = r#" -fn is_valid(_a: bool) -> bool { true } -fn is_ready(_b: bool) -> bool { true } +fn is_valid(_a: bool) -> bool { true } +fn is_ready(_b: bool) -> bool { true } fn main() { if is_valid(true) { @@ -126,7 +126,7 @@ fn main() { if x || z { if y && z { println!("Hello"); - } + } } else { println!("World"); } @@ -163,9 +163,9 @@ fn main() { "#; const IF_LET_TO_IGNORE_WITH_ASSERT: &str = r#" -fn main() { - let x = Option::Some(true); - let y = Option::Some(true); +fn main(n: felt252) { + let x = Option::Some(n); + let y = Option::Some(1); if let Option::Some(_z) = x { assert!(x == y); @@ -270,8 +270,8 @@ fn main() { "#; const IF_LET_WITH_ASSERT: &str = r#" -fn main() { - let x = Some(42); +fn main(n: felt252) { + let x = Some(n); if let Some(y) = x { assert!(y == 42); @@ -577,9 +577,9 @@ fn if_let_to_ignore_with_assert_diagnostic() { #[test] fn if_let_to_ignore_fixer() { test_lint_fixer!(IF_LET_TO_IGNORE_WITH_ASSERT, @r" - fn main() { - let x = Option::Some(true); - let y = Option::Some(true); + fn main(n: felt252) { + let x = Option::Some(n); + let y = Option::Some(1); if let Option::Some(_z) = x { assert!(x == y); diff --git a/tests/main.rs b/tests/main.rs index 9d122fc2..79469270 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -1,3 +1,4 @@ +mod assert_on_const; mod bitwise_for_parity_check; mod bool_comparison; mod breaks;