Merge pull request #353 from CoinFabrik/main

Avoid panic error detector fix
CoinFabrik · Aug 31, 2024 · 30a14ab · 30a14ab
2 parents 6aa4ec5 + de0613a
commit 30a14ab
Show file tree

Hide file tree

Showing 15 changed files with 139 additions and 102 deletions.
diff --git a/detectors/avoid-panic-error/src/lib.rs b/detectors/avoid-panic-error/src/lib.rs
@@ -3,8 +3,9 @@
 extern crate rustc_ast;
 extern crate rustc_span;
 
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_wrappers::span_lint_and_help;
 use rustc_ast::{
+    ptr::P,
     tokenstream::TokenTree,
     visit::{walk_expr, Visitor},
     AssocItemKind, AttrArgs, AttrKind, Block, Expr, ExprKind, FnRetTy, Item, ItemKind, MacCall,
@@ -93,16 +94,12 @@ impl EarlyLintPass for AvoidPanicError {
             ItemKind::Impl(impl_item) => {
                 for assoc_item in &impl_item.items {
                     if let AssocItemKind::Fn(fn_item) = &assoc_item.kind {
-                        self.check_function(
-                            cx,
-                            &fn_item.sig.decl.output,
-                            fn_item.body.as_ref().unwrap(),
-                        );
+                        self.check_function(cx, &fn_item.sig.decl.output, &fn_item.body);
                     }
                 }
             }
             ItemKind::Fn(fn_item) => {
-                self.check_function(cx, &fn_item.sig.decl.output, fn_item.body.as_ref().unwrap());
+                self.check_function(cx, &fn_item.sig.decl.output, &fn_item.body);
             }
             ItemKind::Mod(_, ModKind::Loaded(items, _, _)) => {
                 for item in items {
@@ -112,11 +109,7 @@ impl EarlyLintPass for AvoidPanicError {
             ItemKind::Trait(trait_item) => {
                 for item in &trait_item.items {
                     if let AssocItemKind::Fn(fn_item) = &item.kind {
-                        self.check_function(
-                            cx,
-                            &fn_item.sig.decl.output,
-                            fn_item.body.as_ref().unwrap(),
-                        );
+                        self.check_function(cx, &fn_item.sig.decl.output, &fn_item.body);
                     }
                 }
             }
@@ -126,10 +119,12 @@ impl EarlyLintPass for AvoidPanicError {
 }
 
 impl AvoidPanicError {
-    fn check_function(&self, cx: &EarlyContext, output: &FnRetTy, body: &Block) {
-        if is_result_type(output) {
-            let mut visitor = PanicVisitor { cx };
-            visitor.visit_block(body);
+    fn check_function(&self, cx: &EarlyContext, output: &FnRetTy, body: &Option<P<Block>>) {
+        if let Some(body) = body {
+            if is_result_type(output) {
+                let mut visitor = PanicVisitor { cx };
+                visitor.visit_block(body);
+            }
         }
     }
 }

diff --git a/detectors/dynamic-storage/Cargo.toml b/detectors/dynamic-storage/Cargo.toml
@@ -7,7 +7,7 @@ version = "0.1.0"
 crate-type = ["cdylib"]
 
 [dependencies]
-clippy_utils = { workspace = true }
+clippy_wrappers = { workspace = true }
 dylint_linting = { workspace = true }
 if_chain = { workspace = true }
 utils = { workspace = true }

diff --git a/detectors/dynamic-storage/src/lib.rs b/detectors/dynamic-storage/src/lib.rs
@@ -4,7 +4,7 @@ extern crate rustc_hir;
 extern crate rustc_middle;
 extern crate rustc_span;
 
-use clippy_utils::diagnostics::span_lint;
+use clippy_wrappers::span_lint;
 use if_chain::if_chain;
 use rustc_hir::{
     intravisit::{walk_expr, FnKind, Visitor},

diff --git a/detectors/integer-overflow-or-underflow/Cargo.toml b/detectors/integer-overflow-or-underflow/Cargo.toml
@@ -7,7 +7,7 @@ version = "0.1.0"
 crate-type = ["cdylib"]
 
 [dependencies]
-clippy_utils = { workspace = true }
+clippy_wrappers = { workspace = true }
 dylint_linting = { workspace = true }
 if_chain = { workspace = true }
 utils = { workspace = true }

diff --git a/detectors/integer-overflow-or-underflow/src/lib.rs b/detectors/integer-overflow-or-underflow/src/lib.rs
@@ -3,7 +3,7 @@
 extern crate rustc_hir;
 extern crate rustc_span;
 
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_wrappers::span_lint_and_help;
 use rustc_hir::{
     intravisit::{walk_expr, FnKind, Visitor},
     BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp,

diff --git a/detectors/iterators-over-indexing/src/lib.rs b/detectors/iterators-over-indexing/src/lib.rs
@@ -20,6 +20,7 @@ use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{TyCtxt, TyKind};
 use rustc_span::{symbol::Ident, Span};
 use rustc_type_ir::Interner;
+use std::collections::HashSet;
 use utils::get_node_type;
 
 const LINT_MESSAGE: &str =
@@ -39,7 +40,7 @@ dylint_linting::declare_late_lint! {
 }
 
 struct ForLoopVisitor<'a, 'b> {
-    span_constant: Vec<Span>,
+    span_constant: HashSet<Span>,
     cx: &'b LateContext<'a>,
 }
 struct VectorAccessVisitor<'a, 'b> {
@@ -351,7 +352,7 @@ fn handle_expr<'a>(me: &mut ForLoopVisitor<'a, '_>, expr: &'a Expr<'a>) -> Resul
     }
 
     if visitor.has_vector_access {
-        me.span_constant.push(expr.span);
+        me.span_constant.insert(expr.span);
     }
 
     Ok(())
@@ -377,7 +378,7 @@ impl<'tcx> LateLintPass<'tcx> for IteratorOverIndexing {
         if let FnKind::Method(_ident, _sig) = kind {
             let span_constant = {
                 let mut visitor = ForLoopVisitor {
-                    span_constant: vec![],
+                    span_constant: HashSet::new(),
                     cx,
                 };
                 walk_expr(&mut visitor, body.value);

diff --git a/detectors/storage-change-events/Cargo.toml b/detectors/storage-change-events/Cargo.toml
@@ -7,7 +7,7 @@ version = "0.1.0"
 crate-type = ["cdylib"]
 
 [dependencies]
-clippy_utils = { workspace = true }
+clippy_wrappers = { workspace = true }
 dylint_linting = { workspace = true }
 if_chain = { workspace = true }
 utils = { workspace = true }

diff --git a/detectors/storage-change-events/src/lib.rs b/detectors/storage-change-events/src/lib.rs
@@ -1,26 +1,21 @@
 #![feature(rustc_private)]
 
 extern crate rustc_hir;
-extern crate rustc_middle;
 extern crate rustc_span;
 
-use clippy_utils::diagnostics::span_lint_and_help;
-
+use clippy_wrappers::span_lint_and_help;
 use rustc_hir::{
     intravisit::{walk_expr, Visitor},
     Expr, ExprKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
-
-use rustc_span::Span;
-
-use std::collections::HashMap;
-use std::collections::HashSet;
-use std::vec;
+use rustc_span::{def_id::DefId, Span};
+use std::{
+    collections::{HashMap, HashSet},
+    vec,
+};
 use utils::{is_soroban_function, FunctionCallVisitor};
 
-use rustc_span::def_id::DefId;
-
 const LINT_MESSAGE: &str = "Consider emiting an event when storage is modified";
 
 dylint_linting::impl_late_lint! {

diff --git a/detectors/token-interface-events/Cargo.toml b/detectors/token-interface-events/Cargo.toml
@@ -7,7 +7,7 @@ version = "0.1.0"
 crate-type = ["cdylib"]
 
 [dependencies]
-clippy_utils = { workspace = true }
+clippy_wrappers = { workspace = true }
 dylint_linting = { workspace = true }
 if_chain = { workspace = true }
 utils = { workspace = true }

diff --git a/detectors/token-interface-events/src/lib.rs b/detectors/token-interface-events/src/lib.rs
@@ -1,26 +1,21 @@
 #![feature(rustc_private)]
 
 extern crate rustc_hir;
-extern crate rustc_middle;
 extern crate rustc_span;
 
-use clippy_utils::diagnostics::span_lint_and_help;
-
+use clippy_wrappers::span_lint_and_help;
 use rustc_hir::{
     intravisit::{walk_expr, Visitor},
     Expr, ExprKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
-
-use rustc_span::Span;
-
-use std::collections::HashMap;
-use std::collections::HashSet;
-use std::vec;
+use rustc_span::{def_id::DefId, Span};
+use std::{
+    collections::{HashMap, HashSet},
+    vec,
+};
 use utils::{verify_token_interface_function, FunctionCallVisitor};
 
-use rustc_span::def_id::DefId;
-
 const LINT_MESSAGE: &str = "This function belongs to the Token Interface and should emit an event";
 
 dylint_linting::impl_late_lint! {

diff --git a/detectors/unsafe-expect/Cargo.toml b/detectors/unsafe-expect/Cargo.toml
@@ -11,6 +11,7 @@ clippy_utils = { workspace = true }
 clippy_wrappers = { workspace = true }
 dylint_linting = { workspace = true }
 if_chain = { workspace = true }
+utils = { workspace = true }
 
 [package.metadata.rust-analyzer]
 rustc_private = true
diff --git a/detectors/unsafe-expect/src/lib.rs b/detectors/unsafe-expect/src/lib.rs
@@ -4,9 +4,7 @@
 extern crate rustc_hir;
 extern crate rustc_span;
 
-use std::{collections::HashSet, hash::Hash};
-
-use clippy_utils::higher;
+use clippy_utils::higher::IfOrIfLet;
 use clippy_wrappers::span_lint_and_help;
 use if_chain::if_chain;
 use rustc_hir::{
@@ -17,6 +15,8 @@ use rustc_hir::{
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_span::{sym, Span, Symbol};
+use std::{collections::HashSet, hash::Hash};
+use utils::fn_returns;
 
 const LINT_MESSAGE: &str = "Unsafe usage of `expect`";
 const PANIC_INDUCING_FUNCTIONS: [&str; 2] = ["panic", "bail"];
@@ -149,6 +149,7 @@ struct UnsafeExpectVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     conditional_checker: HashSet<ConditionalChecker>,
     checked_exprs: HashSet<HirId>,
+    linted_spans: HashSet<Span>,
 }
 
 impl UnsafeExpectVisitor<'_, '_> {
@@ -174,21 +175,23 @@ impl UnsafeExpectVisitor<'_, '_> {
         None
     }
 
-    fn set_conditional_checker(&mut self, conditional_checkers: &HashSet<ConditionalChecker>) {
-        for checker in conditional_checkers {
-            self.conditional_checker.insert(*checker);
-            if checker.check_type.is_safe_to_expect() {
-                self.checked_exprs.insert(checker.checked_expr_hir_id);
-            }
-        }
-    }
-
-    fn reset_conditional_checker(&mut self, conditional_checkers: HashSet<ConditionalChecker>) {
+    fn update_conditional_checker(
+        &mut self,
+        conditional_checkers: &HashSet<ConditionalChecker>,
+        set: bool,
+    ) {
         for checker in conditional_checkers {
-            if checker.check_type.is_safe_to_expect() {
-                self.checked_exprs.remove(&checker.checked_expr_hir_id);
+            if set {
+                self.conditional_checker.insert(*checker);
+                if checker.check_type.is_safe_to_expect() {
+                    self.checked_exprs.insert(checker.checked_expr_hir_id);
+                }
+            } else {
+                if checker.check_type.is_safe_to_expect() {
+                    self.checked_exprs.remove(&checker.checked_expr_hir_id);
+                }
+                self.conditional_checker.remove(checker);
             }
-            self.conditional_checker.remove(&checker);
         }
     }
 
@@ -236,6 +239,7 @@ impl UnsafeExpectVisitor<'_, '_> {
                 .get_expect_info(receiver)
                 .map_or(true, |id| !self.checked_exprs.contains(&id));
         }
+
         args.iter().any(|arg| self.contains_unsafe_method_call(arg))
             || self.contains_unsafe_method_call(receiver)
     }
@@ -252,7 +256,9 @@ impl UnsafeExpectVisitor<'_, '_> {
     fn check_expr_for_unsafe_expect(&mut self, expr: &Expr) {
         match &expr.kind {
             ExprKind::MethodCall(path_segment, receiver, args, _) => {
-                if self.is_method_call_unsafe(path_segment, receiver, args) {
+                if self.is_method_call_unsafe(path_segment, receiver, args)
+                    && !self.linted_spans.contains(&expr.span)
+                {
                     span_lint_and_help(
                         self.cx,
                         UNSAFE_EXPECT,
@@ -261,6 +267,7 @@ impl UnsafeExpectVisitor<'_, '_> {
                         None,
                         "Please, use a custom error instead of `expect`",
                     );
+                    self.linted_spans.insert(expr.span);
                 }
             }
             ExprKind::Call(func, args) => {
@@ -312,17 +319,17 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafeExpectVisitor<'a, 'tcx> {
         }
 
         // Find `if` or `if let` expressions
-        if let Some(higher::IfOrIfLet {
+        if let Some(IfOrIfLet {
             cond,
             then: if_expr,
             r#else: _,
-        }) = higher::IfOrIfLet::hir(expr)
+        }) = IfOrIfLet::hir(expr)
         {
             // If we are interested in the condition (if it is a CheckType) we traverse the body.
             let conditional_checker = ConditionalChecker::from_expression(cond);
-            self.set_conditional_checker(&conditional_checker);
+            self.update_conditional_checker(&conditional_checker, true);
             walk_expr(self, if_expr);
-            self.reset_conditional_checker(conditional_checker);
+            self.update_conditional_checker(&conditional_checker, false);
             return;
         }
 
@@ -338,20 +345,23 @@ impl<'tcx> LateLintPass<'tcx> for UnsafeExpect {
         &mut self,
         cx: &LateContext<'tcx>,
         _: FnKind<'tcx>,
-        _: &'tcx FnDecl<'tcx>,
+        fn_decl: &'tcx FnDecl<'tcx>,
         body: &'tcx Body<'tcx>,
         span: Span,
         _: LocalDefId,
     ) {
-        // If the function comes from a macro expansion, we don't want to analyze it.
-        if span.from_expansion() {
+        // If the function comes from a macro expansion or does not return a Result<(), ()> or Option<()>, we don't want to analyze it.
+        if span.from_expansion()
+            || !fn_returns(fn_decl, sym::Result) && !fn_returns(fn_decl, sym::Option)
+        {
             return;
         }
 
         let mut visitor = UnsafeExpectVisitor {
             cx,
             checked_exprs: HashSet::new(),
             conditional_checker: HashSet::new(),
+            linted_spans: HashSet::new(),
         };
 
         walk_expr(&mut visitor, body.value);

diff --git a/detectors/unsafe-unwrap/Cargo.toml b/detectors/unsafe-unwrap/Cargo.toml
@@ -11,6 +11,7 @@ clippy_utils = { workspace = true }
 clippy_wrappers = { workspace = true }
 dylint_linting = { workspace = true }
 if_chain = { workspace = true }
+utils = { workspace = true }
 
 [package.metadata.rust-analyzer]
 rustc_private = true