Add unnecessary-lint-allow detector

detectors/unnecessary-lint-allow/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+edition = "2021"
+name = "unnecessary-lint-allow"
+version = "0.1.0"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+clippy_utils = { workspace = true }
+clippy_wrappers = { workspace = true }
+dylint_linting = { workspace = true }
+if_chain = { workspace = true }
+serde = { version = "1", features = ["derive"] }
+serde_json = "=1.0.120"
+
+[package.metadata.rust-analyzer]
+rustc_private = true

detectors/unnecessary-lint-allow/src/lib.rs

@@ -0,0 +1,99 @@
+#![feature(rustc_private)]
+
+extern crate rustc_ast;
+extern crate rustc_hir;
+extern crate rustc_span;
+
+mod processor;
+pub use processor::process_findings;
+
+use clippy_wrappers::span_lint_and_help;
+use if_chain::if_chain;
+use rustc_ast::{
+    token::{Delimiter, Token, TokenKind},
+    tokenstream::{TokenStream, TokenTree},
+    AttrArgs, AttrKind, Attribute, Item, ItemKind,
+};
+use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_span::{Span, Symbol};
+use std::collections::VecDeque;
+
+const LINT_MESSAGE: &str = "This `#[scout_allow]` attribute may be unnecessary. Consider removing it if the lint is no longer triggered.";
+
+dylint_linting::declare_pre_expansion_lint! {
+    pub UNNECESSARY_LINT_ALLOW,
+    Warn,
+    LINT_MESSAGE,
+    {
+        name: "Unnecessary Lint Allow",
+        long_message: "The `#[scout_allow]` attribute may be unnecessary. Consider removing it if the lint is no longer triggered.",
+        severity: "Enhancement",
+        help: "https://coinfabrik.github.io/scout-soroban/docs/detectors/unnecessary-lint-allow",
+        vulnerability_class: "Code Quality",
+    }
+}
+
+impl UnnecessaryLintAllow {
+    fn extract_lint_names(&self, tokens: &TokenStream) -> Vec<String> {
+        let mut lint_names = Vec::new();
+        let mut stack = VecDeque::from([tokens]);
+
+        while let Some(current_stream) = stack.pop_front() {
+            for tree in current_stream.trees() {
+                match tree {
+                    TokenTree::Token(
+                        Token {
+                            kind: TokenKind::Ident(ident, _),
+                            ..
+                        },
+                        _,
+                    ) => {
+                        lint_names.push(ident.to_string());
+                    }
+                    TokenTree::Delimited(_, _, Delimiter::Parenthesis, inner_stream) => {
+                        stack.push_back(inner_stream);
+                    }
+                    _ => {}
+                }
+            }
+        }
+
+        lint_names
+    }
+
+    fn check_scout_allow_attrs(&self, cx: &EarlyContext<'_>, attrs: &[Attribute], span: Span) {
+        for attr in attrs {
+            if_chain! {
+                if !attr.span.from_expansion();
+                if attr.has_name(Symbol::intern("scout_allow"));
+                if let AttrKind::Normal(item) = &attr.kind;
+                if let AttrArgs::Delimited(delimited_args) = &item.item.args;
+                then {
+                    let lint_names = self.extract_lint_names(&delimited_args.tokens);
+                    for lint_name in lint_names {
+                        span_lint_and_help(
+                            cx,
+                            UNNECESSARY_LINT_ALLOW,
+                            span,
+                            LINT_MESSAGE,
+                            None,
+                            format!("The detector `{}` is no longer triggered. Consider removing the `#[scout_allow({})]` attribute if the lint is no longer triggered.", lint_name, lint_name)
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
+
+impl EarlyLintPass for UnnecessaryLintAllow {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        self.check_scout_allow_attrs(cx, &item.attrs, item.span);
+
+        if let ItemKind::Impl(impl_) = &item.kind {
+            for impl_item in &impl_.items {
+                self.check_scout_allow_attrs(cx, &impl_item.attrs, impl_item.span);
+            }
+        }
+    }
+}

detectors/unnecessary-lint-allow/src/processor.rs

@@ -0,0 +1,201 @@
+use serde::de::Error;
+use serde_json::Value;
+use std::{collections::HashMap, ffi::CStr, ffi::CString, os::raw::c_char};
+
+#[derive(Debug, Clone)]
+struct Finding {
+    detector: String,
+    file_name: String,
+    span: (usize, usize),
+    allowed_lint: Option<String>,
+}
+
+struct FileFindings {
+    unnecessary_allows: Vec<Finding>,
+    other_findings: Vec<Finding>,
+}
+
+struct FindingsCache {
+    by_file: HashMap<String, FileFindings>,
+}
+
+impl FindingsCache {
+    fn new(all_findings: &[Value]) -> Self {
+        let mut by_file: HashMap<String, FileFindings> = HashMap::new();
+
+        for finding in all_findings {
+            if let Some(parsed) = parse_finding(finding) {
+                by_file
+                    .entry(parsed.file_name.clone())
+                    .or_insert_with(|| FileFindings {
+                        unnecessary_allows: Vec::new(),
+                        other_findings: Vec::new(),
+                    })
+                    .add_finding(parsed);
+            }
+        }
+
+        FindingsCache { by_file }
+    }
+}
+
+impl FileFindings {
+    fn add_finding(&mut self, finding: Finding) {
+        if finding.detector == "unnecessary_lint_allow" {
+            self.unnecessary_allows.push(finding);
+        } else {
+            self.other_findings.push(finding);
+        }
+    }
+}
+
+fn parse_finding(finding: &Value) -> Option<Finding> {
+    let detector = finding.get("code")?.get("code")?.as_str()?;
+    let span = finding.get("spans")?.as_array()?.first()?;
+    let file_name = span.get("file_name")?.as_str()?;
+    let line_start = span.get("line_start")?.as_u64()?;
+    let line_end = span.get("line_end")?.as_u64()?;
+
+    let allowed_lint = if detector == "unnecessary_lint_allow" {
+        finding
+            .get("children")?
+            .get(0)?
+            .get("message")?
+            .as_str()
+            .and_then(|msg| msg.split('`').nth(1).map(String::from))
+    } else {
+        None
+    };
+
+    let start = usize::try_from(line_start).ok()?;
+    let end = usize::try_from(line_end).ok()?;
+
+    Some(Finding {
+        detector: detector.to_owned(),
+        file_name: file_name.to_owned(),
+        span: (start, end),
+        allowed_lint,
+    })
+}
+
+fn spans_overlap(span1: (usize, usize), span2: (usize, usize)) -> bool {
+    span1.0 <= span2.1 && span2.0 <= span1.1
+}
+
+fn process_findings_impl(
+    successful_findings: Vec<Value>,
+    output: Vec<Value>,
+    inside_vscode: bool,
+) -> (Vec<Value>, String) {
+    let findings_cache = FindingsCache::new(&successful_findings);
+
+    let console_findings: Vec<_> = successful_findings
+        .into_iter()
+        .filter(|finding| should_include_finding_impl(finding, &findings_cache))
+        .collect();
+
+    let output_vscode: Vec<_> = if inside_vscode {
+        output
+            .into_iter()
+            .filter(|val| {
+                val.get("message")
+                    .map(|message| should_include_finding_impl(message, &findings_cache))
+                    .unwrap_or(true)
+            })
+            .collect()
+    } else {
+        Vec::new()
+    };
+
+    let output_string_vscode = output_vscode
+        .into_iter()
+        .filter_map(|finding| serde_json::to_string(&finding).ok())
+        .collect::<Vec<_>>()
+        .join("\n");
+
+    (console_findings, output_string_vscode)
+}
+
+fn should_include_finding_impl(finding: &Value, cache: &FindingsCache) -> bool {
+    let current_finding = match parse_finding(finding) {
+        Some(f) => f,
+        None => return false, // If we can't parse the finding, we don't include it
+    };
+
+    if let Some(file_findings) = cache.by_file.get(&current_finding.file_name) {
+        if current_finding.detector == "unnecessary_lint_allow" {
+            if let Some(allowed_lint) = &current_finding.allowed_lint {
+                !file_findings.other_findings.iter().any(|f| {
+                    &f.detector == allowed_lint && spans_overlap(f.span, current_finding.span)
+                })
+            } else {
+                true // Include if we can't determine the allowed lint
+            }
+        } else {
+            !file_findings.unnecessary_allows.iter().any(|allow| {
+                allow
+                    .allowed_lint
+                    .as_ref()
+                    .map_or(false, |lint| lint == &current_finding.detector)
+                    && spans_overlap(allow.span, current_finding.span)
+            })
+        }
+    } else {
+        true // If we can't find the file, we include it by default
+    }
+}
+
+/// Process the findings to filter out unnecessary findings.
+///
+/// # Safety
+///
+/// This function is marked as unsafe because it deals with raw pointers.
+/// The caller is responsible for ensuring the safety of the pointers passed as arguments.
+#[no_mangle]
+pub unsafe extern "C" fn process_findings(
+    successful_findings_json: *const c_char,
+    output_json: *const c_char,
+    inside_vscode: bool,
+) -> *mut c_char {
+    let successful_findings = match parse_json(successful_findings_json) {
+        Ok(Value::Array(v)) => v,
+        _ => return std::ptr::null_mut(),
+    };
+
+    let output = match parse_json(output_json) {
+        Ok(Value::Array(v)) => v,
+        _ => return std::ptr::null_mut(),
+    };
+
+    let (console_findings, output_string_vscode) =
+        process_findings_impl(successful_findings, output, inside_vscode);
+
+    let result = serde_json::json!({
+        "console_findings": console_findings,
+        "output_string_vscode": output_string_vscode
+    });
+
+    match serde_json::to_string(&result) {
+        Ok(result_string) => match CString::new(result_string) {
+            Ok(c_str) => c_str.into_raw(),
+            Err(_) => std::ptr::null_mut(),
+        },
+        Err(_) => std::ptr::null_mut(),
+    }
+}
+
+unsafe fn parse_json(input: *const c_char) -> Result<Value, serde_json::Error> {
+    let c_str = CStr::from_ptr(input);
+    let input_str = c_str
+        .to_str()
+        .map_err(|_| serde_json::Error::custom("Invalid UTF-8"))?;
+    serde_json::from_str(input_str)
+}
+
+// Free the string allocated by the `process_findings` function. Should be called after processing everything
+#[no_mangle]
+pub unsafe extern "C" fn free_string(ptr: *mut c_char) {
+    if !ptr.is_null() {
+        let _ = CString::from_raw(ptr);
+    }
+}

docs/docs/detectors/12-soroban-version.md

@@ -1,19 +1,19 @@
 # Soroban version
 
-## Description 
+## Description
 
 - Category: `Best practices`
 - Severity: `Enhacement`
 - Detector: [`soroban-version`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version)
-- Test Cases: [`soroban-version-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1) 
+- Test Cases: [`soroban-version-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1)
 
 Using an outdated version of Soroban can lead to issues in our contract. It's a good practice to use the latest version.
 
-## Why is this bad? 
+## Why is this bad?
 
 Using an old version of Soroban can be dangerous, as it may have bugs or security issues.
 
-## Issue example 
+## Issue example
 
 
 Consider the following `Cargo.toml`:
@@ -39,7 +39,7 @@ The code example can be found [here](https://github.com/CoinFabrik/scout-soroban
     soroban-sdk = { workspace = true }
 
     [dev_dependencies]
-    soroban-sdk = { workspace = true, features = ["testutils"] }    
+    soroban-sdk = { workspace = true, features = ["testutils"] }
 ```
 
 The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1/remediated-example)
@@ -53,4 +53,4 @@ Warns you if you are using an old version of Soroban in the `Cargo.toml`.
 - [Floating Pragma](https://swcregistry.io/docs/SWC-103/)
 - [outdated Compiler Version](https://swcregistry.io/docs/SWC-102/)
 
-    
+

test-cases/unnecessary-lint-allow/Cargo.toml

@@ -0,0 +1,21 @@
+[workspace]
+exclude = [".cargo", "target"]
+members = ["unnecessary-lint-allow-*/*"]
+resolver = "2"
+
+[workspace.dependencies]
+soroban-sdk = { version = "=21.4.0" }
+
+[profile.release]
+codegen-units = 1
+debug = 0
+debug-assertions = false
+lto = true
+opt-level = "z"
+overflow-checks = true
+panic = "abort"
+strip = "symbols"
+
+[profile.release-with-logs]
+debug-assertions = true
+inherits = "release"