Skip to content

Commit 56aadcb

Browse files
authored
fix(lint): avoid ANSI chars with JSON emitter (#11470)
1 parent 30cc56e commit 56aadcb

File tree

5 files changed

+143
-19
lines changed

5 files changed

+143
-19
lines changed

crates/forge/tests/cli/lint.rs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,94 @@ Compiler run successful!
530530
"#]]);
531531
});
532532

533+
// <https://github.com/foundry-rs/foundry/issues/11460>
534+
forgetest!(lint_json_output_no_ansi_escape_codes, |prj, cmd| {
535+
prj.wipe_contracts();
536+
prj.add_source(
537+
"UnwrappedModifierTest",
538+
r#"
539+
// SPDX-License-Identifier: MIT
540+
pragma solidity ^0.8.0;
541+
542+
contract UnwrappedModifierTest {
543+
mapping(address => bool) isOwner;
544+
545+
modifier onlyOwner() {
546+
require(isOwner[msg.sender], "Not owner");
547+
require(msg.sender != address(0), "Zero address");
548+
_;
549+
}
550+
551+
function doSomething() public onlyOwner {}
552+
}
553+
"#,
554+
);
555+
556+
prj.update_config(|config| {
557+
config.lint = LinterConfig {
558+
severity: vec![LintSeverity::CodeSize],
559+
exclude_lints: vec![],
560+
ignore: vec![],
561+
lint_on_build: true,
562+
..Default::default()
563+
};
564+
});
565+
566+
// should produce clean JSON without ANSI escape sequences (for the url nor the snippets)
567+
cmd.arg("lint").arg("--json").assert_json_stderr(true,
568+
str![[r#"
569+
{
570+
"$message_type": "diag",
571+
"message": "wrap modifier logic to reduce code size",
572+
"code": {
573+
"code": "unwrapped-modifier-logic",
574+
"explanation": null
575+
},
576+
"level": "note",
577+
"spans": [
578+
{
579+
"file_name": "[..]",
580+
"byte_start": 183,
581+
"byte_end": 192,
582+
"line_start": 8,
583+
"line_end": 8,
584+
"column_start": 22,
585+
"column_end": 31,
586+
"is_primary": true,
587+
"text": [
588+
{
589+
"text": " modifier onlyOwner() {",
590+
"highlight_start": 22,
591+
"highlight_end": 31
592+
}
593+
],
594+
"label": null
595+
}
596+
],
597+
"children": [
598+
{
599+
"message": "wrap modifier logic to reduce code size\n\n- modifier onlyOwner() {\n- require(isOwner[msg.sender], \"Not owner\");\n- require(msg.sender != address(0), \"Zero address\");\n- _;\n- }\n+ modifier onlyOwner() {\n+ _onlyOwner();\n+ _;\n+ }\n+ \n+ function _onlyOwner() internal {\n+ require(isOwner[msg.sender], \"Not owner\");\n+ require(msg.sender != address(0), \"Zero address\");\n+ }\n\n",
600+
"code": null,
601+
"level": "note",
602+
"spans": [],
603+
"children": [],
604+
"rendered": null
605+
},
606+
{
607+
"message": "https://book.getfoundry.sh/reference/forge/forge-lint#unwrapped-modifier-logic",
608+
"code": null,
609+
"level": "help",
610+
"spans": [],
611+
"children": [],
612+
"rendered": null
613+
}
614+
],
615+
"rendered": "note[unwrapped-modifier-logic]: wrap modifier logic to reduce code size\n |\n8 | modifier onlyOwner() {\n | ---------\n |\n = note: wrap modifier logic to reduce code size\n \n - modifier onlyOwner() {\n - require(isOwner[msg.sender], \"Not owner\");\n - require(msg.sender != address(0), \"Zero address\");\n - _;\n - }\n + modifier onlyOwner() {\n + _onlyOwner();\n + _;\n + }\n + \n + function _onlyOwner() internal {\n + require(isOwner[msg.sender], \"Not owner\");\n + require(msg.sender != address(0), \"Zero address\");\n + }\n \n = help: https://book.getfoundry.sh/reference/forge/forge-lint#unwrapped-modifier-logic\n\n --> [..]\n"
616+
}
617+
"#]],
618+
);
619+
});
620+
533621
// ------------------------------------------------------------------------------------------------
534622

535623
#[tokio::test]

crates/lint/src/linter/mod.rs

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ pub trait Lint {
5454
pub struct LintContext<'s, 'c> {
5555
sess: &'s Session,
5656
with_description: bool,
57+
with_json_emitter: bool,
5758
pub config: LinterConfig<'c>,
5859
active_lints: Vec<&'static str>,
5960
}
@@ -67,10 +68,11 @@ impl<'s, 'c> LintContext<'s, 'c> {
6768
pub fn new(
6869
sess: &'s Session,
6970
with_description: bool,
71+
with_json_emitter: bool,
7072
config: LinterConfig<'c>,
7173
active_lints: Vec<&'static str>,
7274
) -> Self {
73-
Self { sess, with_description, config, active_lints }
75+
Self { sess, with_description, with_json_emitter, config, active_lints }
7476
}
7577

7678
pub fn session(&self) -> &'s Session {
@@ -92,13 +94,19 @@ impl<'s, 'c> LintContext<'s, 'c> {
9294
}
9395

9496
let desc = if self.with_description { lint.description() } else { "" };
95-
let diag: DiagBuilder<'_, ()> = self
97+
let mut diag: DiagBuilder<'_, ()> = self
9698
.sess
9799
.dcx
98100
.diag(lint.severity().into(), desc)
99101
.code(DiagId::new_str(lint.id()))
100-
.span(MultiSpan::from_span(span))
101-
.help(lint.help());
102+
.span(MultiSpan::from_span(span));
103+
104+
// Avoid ANSI characters when using a JSON emitter
105+
if self.with_json_emitter {
106+
diag = diag.help(lint.help());
107+
} else {
108+
diag = diag.help(hyperlink(lint.help()));
109+
}
102110

103111
diag.emit();
104112
}
@@ -131,14 +139,21 @@ impl<'s, 'c> LintContext<'s, 'c> {
131139
};
132140

133141
let desc = if self.with_description { lint.description() } else { "" };
134-
let diag: DiagBuilder<'_, ()> = self
142+
let mut diag: DiagBuilder<'_, ()> = self
135143
.sess
136144
.dcx
137145
.diag(lint.severity().into(), desc)
138146
.code(DiagId::new_str(lint.id()))
139-
.span(MultiSpan::from_span(span))
140-
.highlighted_note(snippet.to_note(self))
141-
.help(lint.help());
147+
.span(MultiSpan::from_span(span));
148+
149+
// Avoid ANSI characters when using a JSON emitter
150+
if self.with_json_emitter {
151+
diag = diag
152+
.note(snippet.to_note(self).iter().map(|l| l.0.as_str()).collect::<String>())
153+
.help(lint.help());
154+
} else {
155+
diag = diag.highlighted_note(snippet.to_note(self)).help(hyperlink(lint.help()));
156+
}
142157

143158
diag.emit();
144159
}
@@ -253,3 +268,8 @@ impl Snippet {
253268
&s[byte_offset..]
254269
}
255270
}
271+
272+
/// Creates a hyperlink of the input url.
273+
fn hyperlink(url: &'static str) -> String {
274+
format!("\x1b]8;;{url}\x1b\\{url}\x1b]8;;\x1b\\")
275+
}

crates/lint/src/sol/macros.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
#[macro_export]
2-
macro_rules! link {
3-
($url:expr) => {
4-
concat!("\x1b]8;;", $url, "\x1b\\", $url, "\x1b]8;;\x1b\\")
5-
};
6-
}
7-
81
/// Macro for defining lints and relevant metadata for the Solidity linter.
92
///
103
/// # Parameters
@@ -27,7 +20,7 @@ macro_rules! declare_forge_lint {
2720
id: $str_id,
2821
severity: $severity,
2922
description: $desc,
30-
help: link!(concat!("https://book.getfoundry.sh/reference/forge/forge-lint#", $str_id)),
23+
help: concat!("https://book.getfoundry.sh/reference/forge/forge-lint#", $str_id),
3124
};
3225
};
3326

crates/lint/src/sol/mod.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,13 @@ impl<'a> SolidityLinter<'a> {
153153
let inline_config = parse_inline_config(sess, &comments, InlineConfigSource::Ast(ast));
154154

155155
// Initialize and run the early lint visitor
156-
let ctx = LintContext::new(sess, self.with_description, self.config(inline_config), lints);
156+
let ctx = LintContext::new(
157+
sess,
158+
self.with_description,
159+
self.with_json_emitter,
160+
self.config(inline_config),
161+
lints,
162+
);
157163
let mut early_visitor = EarlyLintVisitor::new(&ctx, &mut passes);
158164
_ = early_visitor.visit_source_unit(ast);
159165
early_visitor.post_source_unit(ast);
@@ -207,8 +213,13 @@ impl<'a> SolidityLinter<'a> {
207213
);
208214

209215
// Run late lint visitor
210-
let ctx =
211-
LintContext::new(gcx.sess, self.with_description, self.config(inline_config), lints);
216+
let ctx = LintContext::new(
217+
gcx.sess,
218+
self.with_description,
219+
self.with_json_emitter,
220+
self.config(inline_config),
221+
lints,
222+
);
212223
let mut late_visitor = LateLintVisitor::new(&ctx, &mut passes, &gcx.hir);
213224

214225
// Visit this specific source

crates/test-utils/src/util.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,18 @@ impl TestCommand {
962962
assert_data_eq!(actual, expected);
963963
}
964964

965+
/// Runs the command and asserts that it resulted in the expected outcome and JSON data.
966+
#[track_caller]
967+
pub fn assert_json_stderr(&mut self, success: bool, expected: impl IntoData) {
968+
let expected = expected.is(snapbox::data::DataFormat::Json).unordered();
969+
let stderr = if success { self.assert_success() } else { self.assert_failure() }
970+
.get_output()
971+
.stderr
972+
.clone();
973+
let actual = stderr.into_data().is(snapbox::data::DataFormat::Json).unordered();
974+
assert_data_eq!(actual, expected);
975+
}
976+
965977
/// Runs the command and asserts that it **succeeded** nothing was printed to stdout.
966978
#[track_caller]
967979
pub fn assert_empty_stdout(&mut self) {

0 commit comments

Comments
 (0)