Skip to content

Commit be810cb

Browse files
marcvernet310xrusowskygrandizzy
authored
chore(forge): filter out unsupported solidity versions [solar] (#12065)
* chore: disable linter for solidity versions not supported by solar (<0.8.0) * chore(forge): filter out old solidity versions * chore: remove reduntant version checks + update tests * fix: filter out unsupported versions when getting the solar compiler from the output project parser * fix: use fresh compiler rather than reusing output parser * fix: avoid panic + update tests --------- Co-authored-by: 0xrusowsky <[email protected]> Co-authored-by: 0xrusowsky <[email protected]> Co-authored-by: grandizzy <[email protected]>
1 parent bd2ced1 commit be810cb

File tree

7 files changed

+130
-26
lines changed

7 files changed

+130
-26
lines changed

crates/cli/src/opts/build/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ mod paths;
99
pub use self::paths::ProjectPathOpts;
1010

1111
mod utils;
12-
pub use self::utils::{configure_pcx, configure_pcx_from_compile_output, configure_pcx_from_solc};
12+
pub use self::utils::*;
1313

1414
// A set of solc compiler settings that can be set via command line arguments, which are intended
1515
// to be merged into an existing `foundry_config::Config`.

crates/cli/src/opts/build/utils.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ use foundry_compilers::{
77
};
88
use foundry_config::{Config, semver::Version};
99
use rayon::prelude::*;
10-
use solar::sema::ParsingContext;
10+
use solar::{interface::MIN_SOLIDITY_VERSION as MSV, sema::ParsingContext};
1111
use std::{
1212
collections::{HashSet, VecDeque},
1313
path::{Path, PathBuf},
1414
};
1515

16+
const MIN_SUPPORTED_VERSION: Version = Version::new(MSV.0, MSV.1, MSV.2);
17+
1618
/// Configures a [`ParsingContext`] from [`Config`].
1719
///
1820
/// - Configures include paths, remappings
@@ -49,7 +51,7 @@ pub fn configure_pcx(
4951

5052
// Only process sources with latest Solidity version to avoid conflicts.
5153
let graph = Graph::<MultiCompilerParser>::resolve_sources(&project.paths, sources)?;
52-
let (version, sources, _) = graph
54+
let (version, sources) = graph
5355
// Resolve graph into mapping language -> version -> sources
5456
.into_sources_by_version(project)?
5557
.sources
@@ -59,9 +61,15 @@ pub fn configure_pcx(
5961
.ok_or_else(|| eyre::eyre!("no Solidity sources"))?
6062
.1
6163
.into_iter()
64+
// Filter unsupported versions
65+
.filter(|(v, _, _)| v >= &MIN_SUPPORTED_VERSION)
6266
// Always pick the latest version
6367
.max_by(|(v1, _, _), (v2, _, _)| v1.cmp(v2))
64-
.unwrap();
68+
.map_or((MIN_SUPPORTED_VERSION, Sources::default()), |(v, s, _)| (v, s));
69+
70+
if sources.is_empty() {
71+
sh_warn!("no files found. Solar doesn't support Solidity versions prior to 0.8.0")?;
72+
}
6573

6674
let solc = SolcVersionedInput::build(
6775
sources,
@@ -75,18 +83,17 @@ pub fn configure_pcx(
7583
Ok(())
7684
}
7785

78-
/// Configures a [`ParsingContext`] from a [`ProjectCompileOutput`] and [`SolcVersionedInput`].
86+
/// Extracts Solar-compatible sources from a [`ProjectCompileOutput`].
7987
///
8088
/// # Note:
8189
/// uses `output.graph().source_files()` and `output.artifact_ids()` rather than `output.sources()`
8290
/// because sources aren't populated when build is skipped when there are no changes in the source
8391
/// code. <https://github.com/foundry-rs/foundry/issues/12018>
84-
pub fn configure_pcx_from_compile_output(
85-
pcx: &mut ParsingContext<'_>,
92+
pub fn get_solar_sources_from_compile_output(
8693
config: &Config,
8794
output: &ProjectCompileOutput,
8895
target_paths: Option<&[PathBuf]>,
89-
) -> Result<()> {
96+
) -> Result<SolcVersionedInput> {
9097
let is_solidity_file = |path: &Path| -> bool {
9198
path.extension().and_then(|s| s.to_str()).is_some_and(|ext| SOLC_EXTENSIONS.contains(&ext))
9299
};
@@ -125,12 +132,14 @@ pub fn configure_pcx_from_compile_output(
125132

126133
// Read all sources and find the latest version.
127134
let (version, sources) = {
128-
let (mut max_version, mut sources) = (Version::new(0, 0, 0), Sources::new());
135+
let (mut max_version, mut sources) = (MIN_SUPPORTED_VERSION, Sources::new());
129136
for (id, _) in output.artifact_ids() {
130137
if let Ok(path) = dunce::canonicalize(&id.source)
131138
&& source_paths.remove(&path)
132139
{
133-
if id.version > max_version {
140+
if id.version < MIN_SUPPORTED_VERSION {
141+
continue;
142+
} else if max_version < id.version {
134143
max_version = id.version;
135144
};
136145

@@ -149,6 +158,17 @@ pub fn configure_pcx_from_compile_output(
149158
version,
150159
);
151160

161+
Ok(solc)
162+
}
163+
164+
/// Configures a [`ParsingContext`] from a [`ProjectCompileOutput`].
165+
pub fn configure_pcx_from_compile_output(
166+
pcx: &mut ParsingContext<'_>,
167+
config: &Config,
168+
output: &ProjectCompileOutput,
169+
target_paths: Option<&[PathBuf]>,
170+
) -> Result<()> {
171+
let solc = get_solar_sources_from_compile_output(config, output, target_paths)?;
152172
configure_pcx_from_solc(pcx, &config.project_paths(), &solc, true);
153173
Ok(())
154174
}

crates/forge/src/cmd/build.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clap::Parser;
33
use eyre::{Context, Result};
44
use forge_lint::{linter::Linter, sol::SolidityLinter};
55
use foundry_cli::{
6-
opts::BuildOpts,
6+
opts::{BuildOpts, configure_pcx_from_solc, get_solar_sources_from_compile_output},
77
utils::{LoadConfig, cache_local_signatures},
88
};
99
use foundry_common::{compile::ProjectCompiler, shell};
@@ -174,10 +174,31 @@ impl BuildArgs {
174174
})
175175
.collect::<Vec<_>>();
176176

177-
if !input_files.is_empty() {
178-
let compiler = output.parser_mut().solc_mut().compiler_mut();
179-
linter.lint(&input_files, config.deny, compiler)?;
177+
let solar_sources =
178+
get_solar_sources_from_compile_output(config, output, Some(&input_files))?;
179+
if solar_sources.input.sources.is_empty() {
180+
if !input_files.is_empty() {
181+
sh_warn!(
182+
"unable to lint. Solar only supports Solidity versions prior to 0.8.0"
183+
)?;
184+
}
185+
return Ok(());
180186
}
187+
188+
// NOTE(rusowsky): Once solar can drop unsupported versions, rather than creating a new
189+
// compiler, we should reuse the parser from the project output.
190+
let mut compiler = solar::sema::Compiler::new(
191+
solar::interface::Session::builder().with_stderr_emitter().build(),
192+
);
193+
194+
// Load the solar-compatible sources to the pcx before linting
195+
compiler.enter_mut(|compiler| {
196+
let mut pcx = compiler.parse();
197+
configure_pcx_from_solc(&mut pcx, &config.project_paths(), &solar_sources, true);
198+
pcx.set_resolve_imports(true);
199+
pcx.parse();
200+
});
201+
linter.lint(&input_files, config.deny, &mut compiler)?;
181202
}
182203

183204
Ok(())

crates/forge/src/cmd/lint.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use forge_lint::{
55
sol::{SolLint, SolLintError, SolidityLinter},
66
};
77
use foundry_cli::{
8-
opts::BuildOpts,
8+
opts::{BuildOpts, configure_pcx_from_solc, get_solar_sources_from_compile_output},
99
utils::{FoundryPathExt, LoadConfig},
1010
};
1111
use foundry_common::{compile::ProjectCompiler, shell};
@@ -69,15 +69,15 @@ impl LintArgs {
6969
} else if path.is_sol() {
7070
inputs.push(path.to_path_buf());
7171
} else {
72-
warn!("Cannot process path {}", path.display());
72+
warn!("cannot process path {}", path.display());
7373
}
7474
}
7575
inputs
7676
}
7777
};
7878

7979
if input.is_empty() {
80-
sh_println!("Nothing to lint")?;
80+
sh_println!("nothing to lint")?;
8181
return Ok(());
8282
}
8383

@@ -95,7 +95,7 @@ impl LintArgs {
9595
let severity = self.severity.unwrap_or(config.lint.severity.clone());
9696

9797
if project.compiler.solc.is_none() {
98-
return Err(eyre!("Linting not supported for this language"));
98+
return Err(eyre!("linting not supported for this language"));
9999
}
100100

101101
let linter = SolidityLinter::new(path_config)
@@ -106,9 +106,28 @@ impl LintArgs {
106106
.with_severity(if severity.is_empty() { None } else { Some(severity) })
107107
.with_mixed_case_exceptions(&config.lint.mixed_case_exceptions);
108108

109-
let mut output = ProjectCompiler::new().files(input.iter().cloned()).compile(&project)?;
110-
let compiler = output.parser_mut().solc_mut().compiler_mut();
111-
linter.lint(&input, config.deny, compiler)?;
109+
let output = ProjectCompiler::new().files(input.iter().cloned()).compile(&project)?;
110+
let solar_sources = get_solar_sources_from_compile_output(&config, &output, Some(&input))?;
111+
if solar_sources.input.sources.is_empty() {
112+
return Err(eyre!(
113+
"unable to lint. Solar only supports Solidity versions prior to 0.8.0"
114+
));
115+
}
116+
117+
// NOTE(rusowsky): Once solar can drop unsupported versions, rather than creating a new
118+
// compiler, we should reuse the parser from the project output.
119+
let mut compiler = solar::sema::Compiler::new(
120+
solar::interface::Session::builder().with_stderr_emitter().build(),
121+
);
122+
123+
// Load the solar-compatible sources to the pcx before linting
124+
compiler.enter_mut(|compiler| {
125+
let mut pcx = compiler.parse();
126+
pcx.set_resolve_imports(true);
127+
configure_pcx_from_solc(&mut pcx, &config.project_paths(), &solar_sources, true);
128+
pcx.parse();
129+
});
130+
linter.lint(&input, config.deny, &mut compiler)?;
112131

113132
Ok(())
114133
}

crates/forge/src/multi_runner.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -575,10 +575,7 @@ impl MultiContractRunnerBuilder {
575575
if files.is_empty() { None } else { Some(&files) },
576576
)?;
577577
pcx.parse();
578-
// Check if any sources exist, to avoid logging `error: no files found`
579-
if !compiler.sess().source_map().is_empty() {
580-
let _ = compiler.lower_asts();
581-
}
578+
let _ = compiler.lower_asts();
582579
Ok(())
583580
})?;
584581

crates/forge/tests/cli/lint.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ import { ContractWithLints } from "./ContractWithLints.sol";
4848
4949
import { _PascalCaseInfo } from "./ContractWithLints.sol";
5050
import "./ContractWithLints.sol";
51+
52+
contract Dummy {
53+
bool foo;
54+
}
5155
"#;
5256

5357
const COUNTER_A: &str = r#"
@@ -778,3 +782,42 @@ fn ensure_no_privileged_lint_id() {
778782
assert_ne!(lint.id(), "all", "lint-id 'all' is reserved. Please use a different id");
779783
}
780784
}
785+
786+
forgetest!(skips_linting_for_old_solidity_versions, |prj, cmd| {
787+
const OLD_CONTRACT: &str = r#"
788+
// SPDX-License-Identifier: MIT
789+
pragma solidity ^0.7.0;
790+
791+
contract OldContract {
792+
uint256 VARIABLE_MIXED_CASE_INFO;
793+
794+
function FUNCTION_MIXED_CASE_INFO() public {}
795+
}
796+
"#;
797+
798+
// Add a contract with Solidity 0.7.x which has lint issues
799+
prj.add_source("OldContract", OLD_CONTRACT);
800+
prj.update_config(|config| {
801+
config.lint = LinterConfig {
802+
severity: vec![],
803+
exclude_lints: vec![],
804+
ignore: vec![],
805+
lint_on_build: true,
806+
..Default::default()
807+
};
808+
});
809+
810+
// Run forge build - should SUCCEED without linting
811+
cmd.arg("build").assert_success().stderr_eq(str![[
812+
r#"Warning: unable to lint. Solar only supports Solidity versions prior to 0.8.0
813+
814+
"#
815+
]]);
816+
817+
// Run forge lint - should FAIL
818+
cmd.forge_fuse().arg("lint").assert_failure().stderr_eq(str![[
819+
r#"Error: unable to lint. Solar only supports Solidity versions prior to 0.8.0
820+
821+
"#
822+
]]);
823+
});

crates/lint/src/sol/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use foundry_common::{
88
inline_config::{InlineConfig, InlineConfigItem},
99
},
1010
errors::convert_solar_errors,
11+
sh_warn,
1112
};
1213
use foundry_compilers::{ProjectPathsConfig, solc::SolcLanguage};
1314
use foundry_config::{DenyLevel, lint::Severity};
@@ -258,7 +259,10 @@ impl<'a> Linter for SolidityLinter<'a> {
258259
input.par_iter().for_each(|path| {
259260
let path = &self.path_config.root.join(path);
260261
let Some((_, ast_source)) = gcx.get_ast_source(path) else {
261-
panic!("AST source not found for {}", path.display());
262+
// issue a warning rather than panicking, in case that some (but not all) of the
263+
// input files have old solidity versions which are not supported by solar.
264+
_ = sh_warn!("AST source not found for {}", path.display());
265+
return;
262266
};
263267
let Some(ast) = &ast_source.ast else {
264268
panic!("AST missing for {}", path.display());

0 commit comments

Comments
 (0)