Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
126 changes: 123 additions & 3 deletions src/check/inline_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@
DisableStart,
/// Disables formatting for any code that precedes this and after the previous "disable-start"
DisableEnd,
/// Ignores the next code item for linting rules
IgnoreNextItem,
/// Ignores the current line for linting rules
IgnoreLine,
/// Ignores the next line for linting rules
IgnoreNextLine,
/// Ignores linting rules for any code that follows this and before the next "ignore-end"
IgnoreStart,
/// Ignores linting rules for any code that precedes this and after the previous "ignore-start"
IgnoreEnd,
}

impl FromStr for InlineConfigItem {
Expand All @@ -34,6 +44,11 @@
"disable-next-line" => InlineConfigItem::DisableNextLine,
"disable-start" => InlineConfigItem::DisableStart,
"disable-end" => InlineConfigItem::DisableEnd,
"ignore-next-item" => InlineConfigItem::IgnoreNextItem,
"ignore-line" => InlineConfigItem::IgnoreLine,
"ignore-next-line" => InlineConfigItem::IgnoreNextLine,
"ignore-start" => InlineConfigItem::IgnoreStart,
"ignore-end" => InlineConfigItem::IgnoreEnd,
s => return Err(InvalidInlineConfigItem(s.into())),
})
}
Expand Down Expand Up @@ -64,23 +79,45 @@
}
}

/// An inline config. Keeps track of disabled ranges.
///
/// An ignored formatting range. `loose` designates that the range includes any loc which
/// may start in between start and end, whereas the strict version requires that
/// `range.start >= loc.start <=> loc.end <= range.end`
#[derive(Debug)]
struct IgnoredRange {
start: usize,
end: usize,
loose: bool,
}

impl IgnoredRange {
fn includes(&self, loc: Loc) -> bool {
loc.start() >= self.start && (if self.loose { loc.start() } else { loc.end() } <= self.end)
}
}

/// This is a list of Inline Config items for locations in a source file. This is acquired by
/// parsing the comments for `scopelint:` items. See [`Comments::parse_inline_config_items`] for

Check warning on line 99 in src/check/inline_config.rs

View workflow job for this annotation

GitHub Actions / lint

unresolved link to `Comments::parse_inline_config_items`
/// details.
#[derive(Default, Debug)]
pub struct InlineConfig {
disabled_ranges: Vec<DisabledRange>,
ignored_ranges: Vec<IgnoredRange>,
}

impl InlineConfig {
/// Build a new inline config with an iterator of inline config items and their locations in a
/// source file
pub fn new(items: impl IntoIterator<Item = (Loc, InlineConfigItem)>, src: &str) -> Self {
// Disable ranges (for formatting)
let mut disabled_ranges = vec![];
let mut disabled_range_start = None;
let mut disabled_depth = 0usize;

// Ignore ranges (for linting)
let mut ignored_ranges = vec![];
let mut ignored_range_start = None;
let mut ignored_depth = 0usize;

for (loc, item) in items.into_iter().sorted_by_key(|(loc, _)| loc.start()) {
match item {
InlineConfigItem::DisableNextItem => {
Expand Down Expand Up @@ -145,16 +182,99 @@
}
}
}
InlineConfigItem::IgnoreNextItem => {
let offset = loc.end();
let mut char_indices = src[offset..]
.comment_state_char_indices()
.filter_map(|(state, idx, ch)| match state {
CommentState::None => Some((idx, ch)),
_ => None,
})
.skip_while(|(_, ch)| ch.is_whitespace());
if let Some((mut start, _)) = char_indices.next() {
start += offset;
// Find the end of the function declaration by looking for the closing brace
let mut brace_count = 0;
let mut found_function_start = false;
let mut end = src.len();

for (idx, ch) in src[start..].char_indices() {
if ch == '{' {
brace_count += 1;
found_function_start = true;
} else if ch == '}' {
brace_count -= 1;
if found_function_start && brace_count == 0 {
end = start + idx + 1;
break;
}
}
}
ignored_ranges.push(IgnoredRange { start, end, loose: true });
}
}
InlineConfigItem::IgnoreLine => {
let mut prev_newline =
src[..loc.start()].char_indices().rev().skip_while(|(_, ch)| *ch != '\n');
let start = prev_newline.next().map(|(idx, _)| idx).unwrap_or_default();

let end_offset = loc.end();
let mut next_newline =
src[end_offset..].char_indices().skip_while(|(_, ch)| *ch != '\n');
let end =
end_offset + next_newline.next().map(|(idx, _)| idx).unwrap_or_default();

ignored_ranges.push(IgnoredRange { start, end, loose: false });
}
InlineConfigItem::IgnoreNextLine => {
let offset = loc.end();
let mut char_indices =
src[offset..].char_indices().skip_while(|(_, ch)| *ch != '\n').skip(1);
if let Some((mut start, _)) = char_indices.next() {
start += offset;
let end = char_indices
.find(|(_, ch)| *ch == '\n')
.map(|(idx, _)| offset + idx + 1)
.unwrap_or(src.len());
ignored_ranges.push(IgnoredRange { start, end, loose: false });
}
}
InlineConfigItem::IgnoreStart => {
if ignored_depth == 0 {
ignored_range_start = Some(loc.end());
}
ignored_depth += 1;
}
InlineConfigItem::IgnoreEnd => {
ignored_depth = ignored_depth.saturating_sub(1);
if ignored_depth == 0 {
if let Some(start) = ignored_range_start.take() {
ignored_ranges.push(IgnoredRange {
start,
end: loc.start(),
loose: false,
})
}
}
}
}
}
if let Some(start) = disabled_range_start.take() {
disabled_ranges.push(DisabledRange { start, end: src.len(), loose: false })
}
Self { disabled_ranges }
if let Some(start) = ignored_range_start.take() {
ignored_ranges.push(IgnoredRange { start, end: src.len(), loose: false })
}
Self { disabled_ranges, ignored_ranges }
}

/// Check if the location is in a disabled range
pub fn is_disabled(&self, loc: Loc) -> bool {
self.disabled_ranges.iter().any(|range| range.includes(loc))
}

/// Check if the location is in an ignored range
pub fn is_ignored(&self, loc: Loc) -> bool {
self.ignored_ranges.iter().any(|range| range.includes(loc))
}
}
4 changes: 2 additions & 2 deletions src/check/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ impl fmt::Display for Report {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result {
self.invalid_items
.iter()
.filter(|item| !item.is_disabled)
.filter(|item| !item.is_disabled && !item.is_ignored)
.sorted_unstable()
.try_for_each(|item| writeln!(f, "{}", item.description()))
}
Expand All @@ -33,6 +33,6 @@ impl Report {
/// Returns true if no issues were found.
#[must_use]
pub fn is_valid(&self) -> bool {
!self.invalid_items.iter().any(|item| !item.is_disabled)
!self.invalid_items.iter().any(|item| !item.is_disabled && !item.is_ignored)
}
}
4 changes: 3 additions & 1 deletion src/check/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub struct InvalidItem {
pub text: String, // Details to show about the invalid item.
pub line: usize, // Line number.
pub is_disabled: bool, // Whether the invalid item is in a disabled region.
pub is_ignored: bool, // Whether the invalid item is in an ignored region.
}

impl InvalidItem {
Expand All @@ -45,7 +46,8 @@ impl InvalidItem {
let Parsed { file, src, inline_config, .. } = parsed;
let line = offset_to_line(src, loc.start());
let is_disabled = inline_config.is_disabled(loc);
Self { kind, file: file.display().to_string(), text, line, is_disabled }
let is_ignored = inline_config.is_ignored(loc);
Self { kind, file: file.display().to_string(), text, line, is_disabled, is_ignored }
}

#[must_use]
Expand Down
30 changes: 30 additions & 0 deletions tests/check-proj1-AllFindings/src/CounterIgnored4.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Test file showing differences between ignore directives
pragma solidity ^0.8.17;

contract CounterIgnored4 {
// Test 1: ignore-next-item (ignores entire function declaration, even multiline)
// scopelint: ignore-next-item
function multiLineFunction(
address user,
uint256 amount
) internal {
// complex function body
}

// Test 2: ignore-next-line (ignores only the next line)
// scopelint: ignore-next-line
function singleLineFunction() internal {}

// Test 3: ignore-line (ignores only this comment line, NOT the function)
function functionOnSameLine() internal {} // scopelint: ignore-line

// Test 4: ignore-start/ignore-end (ignores multiple items)
// scopelint: ignore-start
function batchFunction1() internal {}
function batchFunction2() private {}
function batchFunction3() internal {}
// scopelint: ignore-end

// Control test: this should be flagged (no ignore directive)
function missingLeadingUnderscoreAndNotIgnored() internal {}
}
1 change: 1 addition & 0 deletions tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ fn test_check_proj1_all_findings() {
"Invalid constant or immutable name in ./test/Counter.t.sol on line 7: testVal",
"Invalid src method name in ./src/Counter.sol on line 23: internalShouldHaveLeadingUnderscore",
"Invalid src method name in ./src/Counter.sol on line 25: privateShouldHaveLeadingUnderscore",
"Invalid src method name in ./src/CounterIgnored4.sol on line 29: missingLeadingUnderscoreAndNotIgnored",
"Invalid test name in ./test/Counter.t.sol on line 16: testIncrementBadName",
"Invalid directive in ./src/Counter.sol: Invalid inline config item: this directive is invalid",
"error: Convention checks failed, see details above",
Expand Down
Loading