Skip to content

Commit 118dcdb

Browse files
committed
fix: filter out unsupported versions when getting the solar compiler
from the output project parser
1 parent 83835ae commit 118dcdb

File tree

6 files changed

+70
-31
lines changed

6 files changed

+70
-31
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: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,17 @@ pub fn configure_pcx(
8383
Ok(())
8484
}
8585

86-
/// Configures a [`ParsingContext`] from a [`ProjectCompileOutput`] and [`SolcVersionedInput`].
86+
/// Extracts Solar-compatible sources from a [`ProjectCompileOutput`].
8787
///
8888
/// # Note:
8989
/// uses `output.graph().source_files()` and `output.artifact_ids()` rather than `output.sources()`
9090
/// because sources aren't populated when build is skipped when there are no changes in the source
9191
/// code. <https://github.com/foundry-rs/foundry/issues/12018>
92-
pub fn configure_pcx_from_compile_output(
93-
pcx: &mut ParsingContext<'_>,
92+
pub fn get_solar_sources_from_compile_output(
9493
config: &Config,
9594
output: &ProjectCompileOutput,
9695
target_paths: Option<&[PathBuf]>,
97-
) -> Result<()> {
96+
) -> Result<SolcVersionedInput> {
9897
let is_solidity_file = |path: &Path| -> bool {
9998
path.extension().and_then(|s| s.to_str()).is_some_and(|ext| SOLC_EXTENSIONS.contains(&ext))
10099
};
@@ -159,6 +158,17 @@ pub fn configure_pcx_from_compile_output(
159158
version,
160159
);
161160

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)?;
162172
configure_pcx_from_solc(pcx, &config.project_paths(), &solc, true);
163173
Ok(())
164174
}

crates/forge/src/cmd/build.rs

Lines changed: 18 additions & 5 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};
@@ -117,7 +117,6 @@ impl BuildArgs {
117117
if config.lint.lint_on_build && !output.output().errors.iter().any(|e| e.is_error()) {
118118
self.lint(&project, &config, self.paths.as_deref(), &mut output)
119119
.wrap_err("Lint failed")?;
120-
121120
}
122121

123122
Ok(output)
@@ -175,10 +174,24 @@ impl BuildArgs {
175174
})
176175
.collect::<Vec<_>>();
177176

178-
if !input_files.is_empty() {
179-
let compiler = output.parser_mut().solc_mut().compiler_mut();
180-
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(());
181186
}
187+
188+
let compiler = output.parser_mut().solc_mut().compiler_mut();
189+
compiler.enter_mut(|compiler| {
190+
compiler.drop_asts();
191+
let mut pcx = compiler.parse();
192+
configure_pcx_from_solc(&mut pcx, &config.project_paths(), &solar_sources, true);
193+
});
194+
linter.lint(&input_files, config.deny, compiler)?;
182195
}
183196

184197
Ok(())

crates/forge/src/cmd/lint.rs

Lines changed: 16 additions & 4 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)
@@ -107,7 +107,19 @@ impl LintArgs {
107107
.with_mixed_case_exceptions(&config.lint.mixed_case_exceptions);
108108

109109
let mut 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+
110117
let compiler = output.parser_mut().solc_mut().compiler_mut();
118+
compiler.enter_mut(|compiler| {
119+
compiler.drop_asts();
120+
let mut pcx = compiler.parse();
121+
configure_pcx_from_solc(&mut pcx, &config.project_paths(), &solar_sources, true);
122+
});
111123
linter.lint(&input, config.deny, compiler)?;
112124

113125
Ok(())

crates/forge/tests/cli/lint.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -796,23 +796,23 @@ fn ensure_no_privileged_lint_id() {
796796
}
797797
}
798798

799-
forgetest!(build_allows_linting_for_old_solidity_versions, |prj, cmd| {
799+
forgetest!(skips_linting_for_old_solidity_versions, |prj, cmd| {
800800
prj.wipe_contracts();
801-
801+
802802
// Add a contract with Solidity 0.7.x which has lint issues
803803
const OLD_CONTRACT: &str = r#"
804804
// SPDX-License-Identifier: MIT
805805
pragma solidity ^0.7.0;
806806
807807
contract OldContract {
808808
uint256 VARIABLE_MIXED_CASE_INFO;
809-
809+
810810
function FUNCTION_MIXED_CASE_INFO() public {}
811811
}
812812
"#;
813-
813+
814814
prj.add_source("OldContract", OLD_CONTRACT);
815-
815+
816816
// Configure linter to show all severities
817817
prj.update_config(|config| {
818818
config.lint = LinterConfig {
@@ -823,15 +823,15 @@ contract OldContract {
823823
..Default::default()
824824
};
825825
});
826-
827-
// Run forge build - should NOT show linting output because version < 0.8.0
828-
// Should show a warning about skipping linting
829-
let output = cmd.arg("build").assert_success();
830-
let stderr = String::from_utf8_lossy(&output.get_output().stderr);
831-
832-
// Should contain warning about skipping linting
833-
assert!(stderr.contains("mutable variables should use mixedCase"));
834-
835-
// Should NOT contain any lint warnings
836-
// assert!(!stderr.contains("mixed-case"));
826+
827+
// Run forge build - should SUCCEED without linting
828+
cmd.arg("build").assert_success().stderr_eq(
829+
"Warning: unable to lint. Solar only supports Solidity versions prior to 0.8.0\n",
830+
);
831+
832+
// Run forge lint - should FAIL
833+
cmd.forge_fuse()
834+
.arg("lint")
835+
.assert_failure()
836+
.stderr_eq("Error: unable to lint. Solar only supports Solidity versions prior to 0.8.0\n");
837837
});

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)