Skip to content

Commit

Permalink
Merge remote-tracking branch 'Github/release/0.2.16-nightly-2024-07-1…
Browse files Browse the repository at this point in the history
…1' into release/0.2.19-nightly-2024-07-11
  • Loading branch information
Helios-vmg committed Oct 25, 2024
2 parents 6b374dd + 30a14ab commit 558e079
Show file tree
Hide file tree
Showing 9 changed files with 420 additions and 6 deletions.
18 changes: 18 additions & 0 deletions detectors/unnecessary-lint-allow/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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
99 changes: 99 additions & 0 deletions detectors/unnecessary-lint-allow/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
}
201 changes: 201 additions & 0 deletions detectors/unnecessary-lint-allow/src/processor.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
12 changes: 6 additions & 6 deletions docs/docs/detectors/12-soroban-version.md
Original file line number Diff line number Diff line change
@@ -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`:
Expand All @@ -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)
Expand All @@ -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/)


21 changes: 21 additions & 0 deletions test-cases/unnecessary-lint-allow/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Loading

0 comments on commit 558e079

Please sign in to comment.