Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: disable next ast item
  • Loading branch information
0xrusowsky committed Jun 16, 2025
commit 9db437a845f259e78b5775fd28a8e53baef58fb2
108 changes: 74 additions & 34 deletions crates/lint/src/inline_config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use solar_parse::{ast::Span, lexer::token::RawTokenKind};
use std::{collections::HashMap, fmt, str::FromStr};
use solar_ast::{visit::Visit, Item, SourceUnit};
use solar_parse::ast::Span;
use std::{collections::HashMap, fmt, marker::PhantomData, ops::ControlFlow, str::FromStr};

/// An inline config item
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -80,14 +81,16 @@ impl InlineConfig {
/// # Panics
///
/// Panics if `items` is not sorted in ascending order of [`Span`]s.
pub fn new(
pub fn new<'ast>(
items: impl IntoIterator<Item = (Span, InlineConfigItem)>,
lints: &[&'static str],
ast: &'ast SourceUnit<'ast>,
src: &str,
) -> (Self, Vec<(Vec<String>, Span)>) {
let mut invalid_ids: Vec<(Vec<String>, Span)> = Vec::new();
let mut disabled_ranges: HashMap<String, Vec<DisabledRange>> = HashMap::new();
let mut disabled_blocks: HashMap<String, (usize, usize)> = HashMap::new();

let mut prev_sp = Span::DUMMY;
for (sp, item) in items {
if cfg!(debug_assertions) {
Expand All @@ -96,37 +99,23 @@ impl InlineConfig {
}

match item {
// TODO: figure out why it doesn't work.
InlineConfigItem::DisableNextItem(ids) => {
let lints = match validate_ids(&mut invalid_ids, lints, ids, sp) {
InlineConfigItem::DisableNextItem(lint) => {
let lints = match validate_ids(&mut invalid_ids, lints, lint, sp) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move this to InlineConfigItem::parse? This is specific to the inline config parser not the disabled ranges. And this way we don't need to have 2 separate error handling in parse_inline_config

Some(lints) => lints,
None => continue,
};

use RawTokenKind::*;
let offset = sp.hi().to_usize();
let mut idx = offset;
let mut tokens = solar_parse::Cursor::new(&src[offset..])
.map(|token| {
let start = idx;
idx += token.len as usize;
(start, token)
})
.filter(|(_, t)| {
!matches!(t.kind, LineComment { .. } | BlockComment { .. })
})
.skip_while(|(_, t)| matches!(t.kind, Whitespace));
if let Some((mut start, _)) = tokens.next() {
start += offset;
let end = tokens
.find(|(_, t)| !matches!(t.kind, Whitespace))
.map(|(idx, _)| idx)
.unwrap_or(src.len());
let range = DisabledRange { start, end, loose: true };
let comment_end = sp.hi().to_usize();

if let Some(next_item) = NextItemFinder::new(comment_end).find(&ast) {
for lint in lints {
disabled_ranges.entry(lint).or_default().push(range)
disabled_ranges.entry(lint).or_default().push(DisabledRange {
start: next_item.lo().to_usize(),
end: next_item.hi().to_usize(),
loose: false,
});
}
}
};
}
InlineConfigItem::DisableLine(lint) => {
let lints = match validate_ids(&mut invalid_ids, lints, lint, sp) {
Expand All @@ -145,9 +134,12 @@ impl InlineConfig {
src[end_offset..].char_indices().skip_while(|(_, ch)| *ch != '\n');
let end =
end_offset + next_newline.next().map(|(idx, _)| idx).unwrap_or_default();
let range = DisabledRange { start, end, loose: false };
for lint in lints {
disabled_ranges.entry(lint).or_default().push(range)
disabled_ranges.entry(lint).or_default().push(DisabledRange {
start,
end,
loose: false,
})
}
}
InlineConfigItem::DisableNextLine(ids) => {
Expand All @@ -165,9 +157,12 @@ impl InlineConfig {
.find(|(_, ch)| *ch == '\n')
.map(|(idx, _)| offset + idx + 1)
.unwrap_or(src.len());
let range = DisabledRange { start, end, loose: false };
for lint in lints {
disabled_ranges.entry(lint).or_default().push(range)
disabled_ranges.entry(lint).or_default().push(DisabledRange {
start,
end,
loose: false,
})
}
}
}
Expand Down Expand Up @@ -208,8 +203,11 @@ impl InlineConfig {
}

for (lint, (start, _)) in disabled_blocks {
let range = DisabledRange { start, end: src.len(), loose: false };
disabled_ranges.entry(lint).or_default().push(range);
disabled_ranges.entry(lint).or_default().push(DisabledRange {
start,
end: src.len(),
loose: false,
});
}

(Self { disabled_ranges }, invalid_ids)
Expand Down Expand Up @@ -258,3 +256,45 @@ fn validate_ids(
None
}
}

/// An AST visitor that finds the first `Item` that starts after a given offset.
#[derive(Debug, Default)]
struct NextItemFinder<'ast> {
/// The offset to search after.
offset: usize,
/// The span of the found item, if any.
found_span: Option<Span>,
_pd: PhantomData<&'ast ()>,
}

impl<'ast> NextItemFinder<'ast> {
fn new(offset: usize) -> Self {
Self { offset, found_span: None, _pd: PhantomData }
}

/// Finds the next AST item which a span that begins after the `threashold`.
fn find(&mut self, ast: &'ast SourceUnit<'ast>) -> Option<Span> {
for item in ast.items.iter() {
if let ControlFlow::Break(()) = self.visit_item(item) {
break;
}
}

self.found_span
}
}

impl<'ast> Visit<'ast> for NextItemFinder<'ast> {
type BreakValue = ();

fn visit_item(&mut self, item: &'ast Item<'ast>) -> ControlFlow<Self::BreakValue> {
// Check if this item starts after the offset.
if item.span.lo().to_usize() > self.offset {
self.found_span = Some(item.span);
return ControlFlow::Break(());
}

// Otherwise, continue traversing inside this item.
self.walk_item(item)
}
}
27 changes: 15 additions & 12 deletions crates/lint/src/sol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
use foundry_compilers::{solc::SolcLanguage, ProjectPathsConfig};
use foundry_config::lint::Severity;
use rayon::iter::{IntoParallelIterator, ParallelIterator};
use solar_ast::{visit::Visit, Arena};
use solar_ast::{visit::Visit, Arena, SourceUnit};
use solar_interface::{
diagnostics::{self, DiagCtxt, JsonEmitter},
Session, SourceMap,
Expand Down Expand Up @@ -77,6 +77,14 @@ impl SolidityLinter {
let arena = Arena::new();

let _ = sess.enter(|| -> Result<(), diagnostics::ErrorGuaranteed> {
// Initialize the parser, get the AST, and process the inline-config
let mut parser = solar_parse::Parser::from_file(sess, &arena, path)?;
let file = sess
.source_map()
.load_file(path)
.map_err(|e| sess.dcx.err(e.to_string()).emit())?;
let ast = parser.parse_file().map_err(|e| e.emit())?;

// Declare all available passes and lints
let mut passes_and_lints = Vec::new();
passes_and_lints.extend(high::create_lint_passes());
Expand Down Expand Up @@ -117,16 +125,10 @@ impl SolidityLinter {
})
.collect();

// Initialize the parser, get the AST, and process the inline-config
let mut parser = solar_parse::Parser::from_file(sess, &arena, path)?;
let file = sess
.source_map()
.load_file(path)
.map_err(|e| sess.dcx.err(e.to_string()).emit())?;
// Process the inline-config
let source = file.src.as_str();
let comments = Comments::new(&file);
let ast = parser.parse_file().map_err(|e| e.emit())?;
let inline_config = parse_inline_config(sess, &comments, &lints, source);
let inline_config = parse_inline_config(sess, &comments, &lints, &ast, source);

// Initialize and run the visitor
let ctx = LintContext::new(sess, self.with_description, inline_config);
Expand Down Expand Up @@ -170,10 +172,11 @@ impl Linter for SolidityLinter {
}
}

fn parse_inline_config(
fn parse_inline_config<'ast>(
sess: &Session,
comments: &Comments,
lints: &[&'static str],
ast: &'ast SourceUnit<'ast>,
src: &str,
) -> InlineConfig {
let items = comments.iter().filter_map(|comment| {
Expand All @@ -184,7 +187,7 @@ fn parse_inline_config(
if let Some(suffix) = comment.suffix() {
item = item.strip_suffix(suffix).unwrap_or(item);
}
let item = item.trim_start().strip_prefix("forgelint:")?.trim();
let item = item.trim_start().strip_prefix("forge-lint:")?.trim();
let span = comment.span;
match item.parse::<InlineConfigItem>() {
Ok(item) => Some((span, item)),
Expand All @@ -195,7 +198,7 @@ fn parse_inline_config(
}
});

let (inline_config, invalid_lints) = InlineConfig::new(items, lints, src);
let (inline_config, invalid_lints) = InlineConfig::new(items, lints, &ast, src);

for (ids, span) in &invalid_lints {
let msg = format!("unknown lint id: '{}'", ids.join("', '"));
Expand Down
31 changes: 24 additions & 7 deletions crates/lint/testdata/Keccak256.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,24 @@ contract AsmKeccak256 {
bytes32 constant OTHER_HASH = keccak256(1234);

constructor(uint256 a, uint256 b) {
// forgelint: disable-next-line(asm-keccak256)
// forge-lint: disable-next-line(asm-keccak256)
keccak256(abi.encodePacked(a, b));

keccak256(abi.encodePacked(a, b)); // forgelint: disable-line(asm-keccak256)
keccak256(abi.encodePacked(a, b)); // forge-lint: disable-line(asm-keccak256)

// forgelint: disable-start(asm-keccak256) ------
// forge-lint: disable-start(asm-keccak256) ------
keccak256(abi.encodePacked(a, b)); // |
// |
// forgelint: disable-start(asm-keccak256) --- |
// forge-lint: disable-start(asm-keccak256) --- |
keccak256(abi.encodePacked(a, b)); // | |
// | |
// forgelint: disable-end(asm-keccak256) ----- |
// forgelint: disable-end(asm-keccak256) --------
// forge-lint: disable-end(asm-keccak256) ----- |
// forge-lint: disable-end(asm-keccak256) --------

keccak256(abi.encodePacked(a, b));
}

// forgelint: disable-next-item(asm-keccak256)
// forge-lint: disable-next-item(asm-keccak256)
function solidityHashDisabled(uint256 a, uint256 b) public view returns (bytes32) {
bytes32 hash = keccak256(a);
return keccak256(abi.encodePacked(a, b));
Expand All @@ -42,3 +42,20 @@ contract AsmKeccak256 {
}
}
}

// forge-lint: disable-next-item(asm-keccak256)
contract OtherAsmKeccak256 {
function contratDisabledHash(uint256 a, uint256 b) public view returns (bytes32) {
return keccak256(abi.encodePacked(a, b));
}

function contratDisabledHash2(uint256 a, uint256 b) public view returns (bytes32) {
return keccak256(abi.encodePacked(a, b));
}
}

contract YetAnotherAsmKeccak256 {
function nonDisabledHash(uint256 a, uint256 b) public view returns (bytes32) {
return keccak256(abi.encodePacked(a, b)); //~NOTE: hash using inline assembly to save gas
}
}
9 changes: 9 additions & 0 deletions crates/lint/testdata/Keccak256.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,12 @@ note[asm-keccak256]: hash using inline assembly to save gas
| ---------
|
= help: https://book.getfoundry.sh/reference/forge/forge-lint#asm-keccak256

note[asm-keccak256]: hash using inline assembly to save gas
--> ROOT/testdata/Keccak256.sol:LL:CC
|
59 | return keccak256(abi.encodePacked(a, b));
| ---------
|
= help: https://book.getfoundry.sh/reference/forge/forge-lint#asm-keccak256