Skip to content

Commit 3e32767

Browse files
0xrusowsky0xrusowsky
authored
feat(lint): mixedCase exceptions (#11330)
* feat(lint): mixedCase exceptions * refactor: clippy + use slices * fix: config mixed case exceptions for build * fix: unit tests * fix: config test
1 parent a31d4b7 commit 3e32767

File tree

10 files changed

+193
-36
lines changed

10 files changed

+193
-36
lines changed

‎crates/config/src/lint.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,21 @@ pub struct LinterConfig {
2525
///
2626
/// Defaults to true. Set to false to disable automatic linting during builds.
2727
pub lint_on_build: bool,
28+
29+
/// Configurable patterns that should be excluded when performing `mixedCase` lint checks.
30+
///
31+
/// Default's to ["ERC"] to allow common names like `rescueERC20` or `ERC721TokenReceiver`.
32+
pub mixed_case_exceptions: Vec<String>,
2833
}
34+
2935
impl Default for LinterConfig {
3036
fn default() -> Self {
3137
Self {
3238
lint_on_build: true,
3339
severity: Vec::new(),
3440
exclude_lints: Vec::new(),
3541
ignore: Vec::new(),
42+
mixed_case_exceptions: vec!["ERC".to_string()],
3643
}
3744
}
3845
}

‎crates/forge/src/cmd/build.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ impl BuildArgs {
143143
.filter_map(|s| forge_lint::sol::SolLint::try_from(s.as_str()).ok())
144144
.collect(),
145145
)
146-
});
146+
})
147+
.with_mixed_case_exceptions(&config.lint.mixed_case_exceptions);
147148

148149
// Expand ignore globs and canonicalize from the get go
149150
let ignored = expand_globs(&config.root, config.lint.ignore.iter())?

‎crates/forge/src/cmd/lint.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ impl LintArgs {
109109
.with_description(true)
110110
.with_lints(include)
111111
.without_lints(exclude)
112-
.with_severity(if severity.is_empty() { None } else { Some(severity) });
112+
.with_severity(if severity.is_empty() { None } else { Some(severity) })
113+
.with_mixed_case_exceptions(&config.lint.mixed_case_exceptions);
113114

114115
let sess = linter.init();
115116

‎crates/forge/tests/cli/config.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,7 @@ severity = []
10891089
exclude_lints = []
10901090
ignore = []
10911091
lint_on_build = true
1092+
mixed_case_exceptions = ["ERC"]
10921093
10931094
[doc]
10941095
out = "docs"
@@ -1317,7 +1318,10 @@ exclude = []
13171318
"severity": [],
13181319
"exclude_lints": [],
13191320
"ignore": [],
1320-
"lint_on_build": true
1321+
"lint_on_build": true,
1322+
"mixed_case_exceptions": [
1323+
"ERC"
1324+
]
13211325
},
13221326
"doc": {
13231327
"out": "docs",

‎crates/forge/tests/cli/lint.rs

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const OTHER_CONTRACT: &str = r#"
3333
import { ContractWithLints } from "./ContractWithLints.sol";
3434
3535
contract OtherContractWithLints {
36-
uint256 VARIABLE_MIXED_CASE_INFO;
36+
function functionMIXEDCaseInfo() public {}
3737
}
3838
"#;
3939

@@ -78,6 +78,7 @@ forgetest!(can_use_config, |prj, cmd| {
7878
exclude_lints: vec!["incorrect-shift".into()],
7979
ignore: vec![],
8080
lint_on_build: true,
81+
..Default::default()
8182
};
8283
});
8384
cmd.arg("lint").assert_success().stderr_eq(str![[r#"
@@ -105,16 +106,17 @@ forgetest!(can_use_config_ignore, |prj, cmd| {
105106
exclude_lints: vec![],
106107
ignore: vec!["src/ContractWithLints.sol".into()],
107108
lint_on_build: true,
109+
..Default::default()
108110
};
109111
});
110112
cmd.arg("lint").assert_success().stderr_eq(str![[r#"
111-
note[mixed-case-variable]: mutable variables should use mixedCase
112-
[FILE]:9:17
113+
note[mixed-case-function]: function names should use mixedCase
114+
[FILE]:9:18
113115
|
114-
9 | uint256 VARIABLE_MIXED_CASE_INFO;
115-
| ------------------------
116+
9 | function functionMIXEDCaseInfo() public {}
117+
| ---------------------
116118
|
117-
= help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-variable
119+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-function
118120
119121
120122
"#]]);
@@ -126,6 +128,25 @@ note[mixed-case-variable]: mutable variables should use mixedCase
126128
exclude_lints: vec![],
127129
ignore: vec!["src/ContractWithLints.sol".into(), "src/OtherContract.sol".into()],
128130
lint_on_build: true,
131+
..Default::default()
132+
};
133+
});
134+
cmd.arg("lint").assert_success().stderr_eq(str![[""]]);
135+
});
136+
137+
forgetest!(can_use_config_mixed_case_exception, |prj, cmd| {
138+
prj.wipe_contracts();
139+
prj.add_source("ContractWithLints", CONTRACT).unwrap();
140+
prj.add_source("OtherContract", OTHER_CONTRACT).unwrap();
141+
142+
// Check config for `ignore`
143+
prj.update_config(|config| {
144+
config.lint = LinterConfig {
145+
severity: vec![],
146+
exclude_lints: vec![],
147+
ignore: vec!["src/ContractWithLints.sol".into()],
148+
lint_on_build: true,
149+
mixed_case_exceptions: vec!["MIXED".to_string()],
129150
};
130151
});
131152
cmd.arg("lint").assert_success().stderr_eq(str![[""]]);
@@ -143,16 +164,17 @@ forgetest!(can_override_config_severity, |prj, cmd| {
143164
exclude_lints: vec![],
144165
ignore: vec!["src/ContractWithLints.sol".into()],
145166
lint_on_build: true,
167+
..Default::default()
146168
};
147169
});
148170
cmd.arg("lint").args(["--severity", "info"]).assert_success().stderr_eq(str![[r#"
149-
note[mixed-case-variable]: mutable variables should use mixedCase
150-
[FILE]:9:17
171+
note[mixed-case-function]: function names should use mixedCase
172+
[FILE]:9:18
151173
|
152-
9 | uint256 VARIABLE_MIXED_CASE_INFO;
153-
| ------------------------
174+
9 | function functionMIXEDCaseInfo() public {}
175+
| ---------------------
154176
|
155-
= help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-variable
177+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-function
156178
157179
158180
"#]]);
@@ -170,6 +192,7 @@ forgetest!(can_override_config_path, |prj, cmd| {
170192
exclude_lints: vec!["incorrect-shift".into()],
171193
ignore: vec!["src/ContractWithLints.sol".into()],
172194
lint_on_build: true,
195+
..Default::default()
173196
};
174197
});
175198
cmd.arg("lint").arg("src/ContractWithLints.sol").assert_success().stderr_eq(str![[r#"
@@ -197,6 +220,7 @@ forgetest!(can_override_config_lint, |prj, cmd| {
197220
exclude_lints: vec!["incorrect-shift".into()],
198221
ignore: vec![],
199222
lint_on_build: true,
223+
..Default::default()
200224
};
201225
});
202226
cmd.arg("lint").args(["--only-lint", "incorrect-shift"]).assert_success().stderr_eq(str![[
@@ -225,6 +249,7 @@ forgetest!(build_runs_linter_by_default, |prj, cmd| {
225249
exclude_lints: vec!["incorrect-shift".into()],
226250
ignore: vec![],
227251
lint_on_build: true,
252+
..Default::default()
228253
};
229254
});
230255

@@ -288,6 +313,7 @@ forgetest!(build_respects_quiet_flag_for_linting, |prj, cmd| {
288313
exclude_lints: vec!["incorrect-shift".into()],
289314
ignore: vec![],
290315
lint_on_build: true,
316+
..Default::default()
291317
};
292318
});
293319

@@ -306,6 +332,7 @@ forgetest!(build_with_json_uses_json_linter_output, |prj, cmd| {
306332
exclude_lints: vec!["incorrect-shift".into()],
307333
ignore: vec![],
308334
lint_on_build: true,
335+
..Default::default()
309336
};
310337
});
311338

@@ -334,6 +361,7 @@ forgetest!(build_respects_lint_on_build_false, |prj, cmd| {
334361
exclude_lints: vec!["incorrect-shift".into()],
335362
ignore: vec![],
336363
lint_on_build: false,
364+
..Default::default()
337365
};
338366
});
339367

‎crates/lint/src/linter/mod.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use crate::inline_config::InlineConfig;
3333
/// # Note:
3434
///
3535
/// - For `early_lint` and `late_lint`, the `ParsingContext` should have the sources pre-loaded.
36-
pub trait Linter: Send + Sync + Clone {
36+
pub trait Linter: Send + Sync {
3737
type Language: Language;
3838
type Lint: Lint;
3939

@@ -52,18 +52,23 @@ pub trait Lint {
5252
pub struct LintContext<'s> {
5353
sess: &'s Session,
5454
with_description: bool,
55-
pub inline_config: InlineConfig,
55+
pub config: LinterConfig<'s>,
5656
active_lints: Vec<&'static str>,
5757
}
5858

59+
pub struct LinterConfig<'s> {
60+
pub inline: InlineConfig,
61+
pub mixed_case_exceptions: &'s [String],
62+
}
63+
5964
impl<'s> LintContext<'s> {
6065
pub fn new(
6166
sess: &'s Session,
6267
with_description: bool,
63-
config: InlineConfig,
68+
config: LinterConfig<'s>,
6469
active_lints: Vec<&'static str>,
6570
) -> Self {
66-
Self { sess, with_description, inline_config: config, active_lints }
71+
Self { sess, with_description, config, active_lints }
6772
}
6873

6974
pub fn session(&self) -> &'s Session {
@@ -80,7 +85,7 @@ impl<'s> LintContext<'s> {
8085

8186
/// Helper method to emit diagnostics easily from passes
8287
pub fn emit<L: Lint>(&self, lint: &'static L, span: Span) {
83-
if self.inline_config.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) {
88+
if self.config.inline.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) {
8489
return;
8590
}
8691

@@ -101,7 +106,7 @@ impl<'s> LintContext<'s> {
101106
/// For Diff snippets, if no span is provided, it will use the lint's span.
102107
/// If unable to get code from the span, it will fall back to a Block snippet.
103108
pub fn emit_with_fix<L: Lint>(&self, lint: &'static L, span: Span, snippet: Snippet) {
104-
if self.inline_config.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) {
109+
if self.config.inline.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) {
105110
return;
106111
}
107112

‎crates/lint/src/sol/info/mixed_case.rs

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use super::{MixedCaseFunction, MixedCaseVariable};
22
use crate::{
33
linter::{EarlyLintPass, LintContext},
4-
sol::{Severity, SolLint},
4+
sol::{Severity, SolLint, info::screaming_snake_case::is_screaming_snake_case},
55
};
6-
use solar_ast::{ItemFunction, VariableDefinition};
6+
use solar_ast::{FunctionHeader, ItemFunction, VariableDefinition, Visibility};
77

88
declare_forge_lint!(
99
MIXED_CASE_FUNCTION,
@@ -15,7 +15,8 @@ declare_forge_lint!(
1515
impl<'ast> EarlyLintPass<'ast> for MixedCaseFunction {
1616
fn check_item_function(&mut self, ctx: &LintContext<'_>, func: &'ast ItemFunction<'ast>) {
1717
if let Some(name) = func.header.name
18-
&& !is_mixed_case(name.as_str(), true)
18+
&& !is_mixed_case(name.as_str(), true, ctx.config.mixed_case_exceptions)
19+
&& !is_constant_getter(&func.header)
1920
{
2021
ctx.emit(&MIXED_CASE_FUNCTION, name.span);
2122
}
@@ -37,28 +38,73 @@ impl<'ast> EarlyLintPass<'ast> for MixedCaseVariable {
3738
) {
3839
if var.mutability.is_none()
3940
&& let Some(name) = var.name
40-
&& !is_mixed_case(name.as_str(), false)
41+
&& !is_mixed_case(name.as_str(), false, ctx.config.mixed_case_exceptions)
4142
{
4243
ctx.emit(&MIXED_CASE_VARIABLE, name.span);
4344
}
4445
}
4546
}
4647

47-
/// Check if a string is mixedCase
48+
/// Checks if a string is mixedCase.
4849
///
4950
/// To avoid false positives like `fn increment()` or `uint256 counter`,
5051
/// lowercase strings are treated as mixedCase.
51-
pub fn is_mixed_case(s: &str, is_fn: bool) -> bool {
52+
pub fn is_mixed_case(s: &str, is_fn: bool, allowed_patterns: &[String]) -> bool {
5253
if s.len() <= 1 {
5354
return true;
5455
}
5556

56-
// Remove leading/trailing underscores like `heck` does
57-
if s.trim_matches('_') == format!("{}", heck::AsLowerCamelCase(s)).as_str() {
57+
// Remove leading/trailing underscores like `heck` does.
58+
if check_lower_mixed_case(s.trim_matches('_')) {
5859
return true;
5960
}
6061

62+
// Ignore user-defined infixes.
63+
for pattern in allowed_patterns {
64+
if let Some(pos) = s.find(pattern.as_str())
65+
&& check_lower_mixed_case(&s[..pos])
66+
&& check_upper_mixed_case_post_pattern(&s[pos + pattern.len()..])
67+
{
68+
return true;
69+
}
70+
}
71+
6172
// Ignore `fn test*`, `fn invariant_*`, and `fn statefulFuzz*` patterns, as they usually contain
6273
// (allowed) underscores.
6374
is_fn && (s.starts_with("test") || s.starts_with("invariant_") || s.starts_with("statefulFuzz"))
6475
}
76+
77+
fn check_lower_mixed_case(s: &str) -> bool {
78+
s == heck::AsLowerCamelCase(s).to_string().as_str()
79+
}
80+
81+
fn check_upper_mixed_case_post_pattern(s: &str) -> bool {
82+
// Find the index of the first character that is not a numeric digit.
83+
let Some(split_idx) = s.find(|c: char| !c.is_numeric()) else {
84+
return true;
85+
};
86+
87+
// Validate the characters preceding the initial numbers have the correct format.
88+
let trimmed = &s[split_idx..];
89+
if let Some(c) = trimmed.chars().next()
90+
&& !c.is_alphabetic()
91+
{
92+
return false;
93+
}
94+
trimmed == heck::AsUpperCamelCase(trimmed).to_string().as_str()
95+
}
96+
97+
/// Checks if a function getter is a valid constant getter with a heuristic:
98+
/// * name is `SCREAMING_SNAKE_CASE`
99+
/// * external view visibility and mutability.
100+
/// * zero parameters.
101+
/// * exactly one return value.
102+
/// * return value is an elementary type
103+
fn is_constant_getter(header: &FunctionHeader<'_>) -> bool {
104+
header.visibility().is_some_and(|v| matches!(v, Visibility::External))
105+
&& header.state_mutability().is_view()
106+
&& header.parameters.is_empty()
107+
&& header.returns().len() == 1
108+
&& header.returns().first().is_some_and(|ret| ret.ty.kind.is_elementary())
109+
&& is_screaming_snake_case(header.name.unwrap().as_str())
110+
}

0 commit comments

Comments
 (0)