diff --git a/compiler/base/orchestrator/src/coordinator.rs b/compiler/base/orchestrator/src/coordinator.rs index 15f51646f..549847c40 100644 --- a/compiler/base/orchestrator/src/coordinator.rs +++ b/compiler/base/orchestrator/src/coordinator.rs @@ -410,6 +410,54 @@ pub struct FormatResponse { pub code: String, } +#[derive(Debug, Clone)] +pub struct ClippyRequest { + pub channel: Channel, + pub crate_type: CrateType, + pub edition: Edition, + pub code: String, +} + +impl ClippyRequest { + pub(crate) fn delete_previous_main_request(&self) -> DeleteFileRequest { + delete_previous_primary_file_request(self.crate_type) + } + + pub(crate) fn write_main_request(&self) -> WriteFileRequest { + write_primary_file_request(self.crate_type, &self.code) + } + + pub(crate) fn execute_cargo_request(&self) -> ExecuteCommandRequest { + ExecuteCommandRequest { + cmd: "cargo".to_owned(), + args: vec!["clippy".to_owned()], + envs: Default::default(), + cwd: None, + } + } +} + +impl CargoTomlModifier for ClippyRequest { + fn modify_cargo_toml(&self, mut cargo_toml: toml::Value) -> toml::Value { + if self.edition == Edition::Rust2024 { + cargo_toml = modify_cargo_toml::set_feature_edition2024(cargo_toml); + } + + cargo_toml = modify_cargo_toml::set_edition(cargo_toml, self.edition.to_cargo_toml_key()); + + if let Some(crate_type) = self.crate_type.to_library_cargo_toml_key() { + cargo_toml = modify_cargo_toml::set_crate_type(cargo_toml, crate_type); + } + cargo_toml + } +} + +#[derive(Debug, Clone)] +pub struct ClippyResponse { + pub success: bool, + pub exit_detail: String, +} + #[derive(Debug, Clone)] pub struct WithOutput<T> { pub response: T, @@ -574,6 +622,33 @@ where .await } + pub async fn clippy( + &self, + request: ClippyRequest, + ) -> Result<WithOutput<ClippyResponse>, ClippyError> { + use clippy_error::*; + + self.select_channel(request.channel) + .await + .context(CouldNotStartContainerSnafu)? + .clippy(request) + .await + } + + pub async fn begin_clippy( + &self, + token: CancellationToken, + request: ClippyRequest, + ) -> Result<ActiveClippy, ClippyError> { + use clippy_error::*; + + self.select_channel(request.channel) + .await + .context(CouldNotStartContainerSnafu)? + .begin_clippy(token, request) + .await + } + pub async fn idle(&mut self) -> Result<()> { let Self { stable, @@ -929,6 +1004,78 @@ impl Container { }) } + async fn clippy( + &self, + request: ClippyRequest, + ) -> Result<WithOutput<ClippyResponse>, ClippyError> { + let token = Default::default(); + + let ActiveClippy { + task, + stdout_rx, + stderr_rx, + } = self.begin_clippy(token, request).await?; + + WithOutput::try_absorb(task, stdout_rx, stderr_rx).await + } + + async fn begin_clippy( + &self, + token: CancellationToken, + request: ClippyRequest, + ) -> Result<ActiveClippy, ClippyError> { + use clippy_error::*; + + let delete_previous_main = request.delete_previous_main_request(); + let write_main = request.write_main_request(); + let execute_cargo = request.execute_cargo_request(); + + let delete_previous_main = self.commander.one(delete_previous_main); + let write_main = self.commander.one(write_main); + let modify_cargo_toml = self.modify_cargo_toml.modify_for(&request); + + let (delete_previous_main, write_main, modify_cargo_toml) = + join!(delete_previous_main, write_main, modify_cargo_toml); + + delete_previous_main.context(CouldNotDeletePreviousCodeSnafu)?; + write_main.context(CouldNotWriteCodeSnafu)?; + modify_cargo_toml.context(CouldNotModifyCargoTomlSnafu)?; + + let SpawnCargo { + task, + stdin_tx, + stdout_rx, + stderr_rx, + } = self + .spawn_cargo_task(token, execute_cargo) + .await + .context(CouldNotStartCargoSnafu)?; + + drop(stdin_tx); + + let task = async move { + let ExecuteCommandResponse { + success, + exit_detail, + } = task + .await + .context(CargoTaskPanickedSnafu)? + .context(CargoFailedSnafu)?; + + Ok(ClippyResponse { + success, + exit_detail, + }) + } + .boxed(); + + Ok(ActiveClippy { + task, + stdout_rx, + stderr_rx, + }) + } + async fn spawn_cargo_task( &self, token: CancellationToken, @@ -1158,6 +1305,47 @@ pub enum FormatError { CodeNotUtf8 { source: std::string::FromUtf8Error }, } +pub struct ActiveClippy { + pub task: BoxFuture<'static, Result<ClippyResponse, ClippyError>>, + pub stdout_rx: mpsc::Receiver<String>, + pub stderr_rx: mpsc::Receiver<String>, +} + +impl fmt::Debug for ActiveClippy { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ActiveClippy") + .field("task", &"<future>") + .field("stdout_rx", &self.stdout_rx) + .field("stderr_rx", &self.stderr_rx) + .finish() + } +} + +#[derive(Debug, Snafu)] +#[snafu(module)] +pub enum ClippyError { + #[snafu(display("Could not start the container"))] + CouldNotStartContainer { source: Error }, + + #[snafu(display("Could not modify Cargo.toml"))] + CouldNotModifyCargoToml { source: ModifyCargoTomlError }, + + #[snafu(display("Could not delete previous source code"))] + CouldNotDeletePreviousCode { source: CommanderError }, + + #[snafu(display("Could not write source code"))] + CouldNotWriteCode { source: CommanderError }, + + #[snafu(display("Could not start Cargo task"))] + CouldNotStartCargo { source: SpawnCargoError }, + + #[snafu(display("The Cargo task panicked"))] + CargoTaskPanicked { source: tokio::task::JoinError }, + + #[snafu(display("Cargo task failed"))] + CargoFailed { source: SpawnCargoError }, +} + struct SpawnCargo { task: JoinHandle<Result<ExecuteCommandResponse, SpawnCargoError>>, stdin_tx: mpsc::Sender<String>, @@ -2652,6 +2840,76 @@ mod tests { Ok(()) } + const ARBITRARY_CLIPPY_REQUEST: ClippyRequest = ClippyRequest { + channel: Channel::Stable, + crate_type: CrateType::Library(LibraryType::Rlib), + edition: Edition::Rust2021, + code: String::new(), + }; + + #[tokio::test] + #[snafu::report] + async fn clippy() -> Result<()> { + let coordinator = new_coordinator().await; + + let req = ClippyRequest { + code: r#" + fn main() { + let a = 0.0 / 0.0; + println!("NaN is {}", a); + } + "# + .into(), + ..ARBITRARY_CLIPPY_REQUEST + }; + + let response = coordinator.clippy(req).with_timeout().await.unwrap(); + + assert!(!response.success, "stderr: {}", response.stderr); + assert_contains!(response.stderr, "deny(clippy::eq_op)"); + assert_contains!(response.stderr, "warn(clippy::zero_divided_by_zero)"); + + Ok(()) + } + + #[tokio::test] + #[snafu::report] + async fn clippy_edition() -> Result<()> { + let cases = [( + "#![deny(clippy::single_element_loop)] + fn a() { for i in [1] { dbg!(i); } }", + [true, true, false, false], + )]; + + let tests = cases.into_iter().flat_map(|(code, expected_to_be_clean)| { + Edition::ALL.into_iter().zip(expected_to_be_clean).map( + move |(edition, expected_to_be_clean)| async move { + let coordinator = new_coordinator().await; + + let req = ClippyRequest { + edition, + code: code.into(), + ..ARBITRARY_CLIPPY_REQUEST + }; + + let response = coordinator.clippy(req).with_timeout().await.unwrap(); + + assert_eq!( + response.success, expected_to_be_clean, + "{code:?} in {edition:?}, {}", + response.stderr + ); + + Ok(()) + }, + ) + }); + + try_join_all(tests).with_timeout().await?; + + Ok(()) + } + // The next set of tests are broader than the functionality of a // single operation. diff --git a/ui/src/main.rs b/ui/src/main.rs index 5816f29c5..27bc3116e 100644 --- a/ui/src/main.rs +++ b/ui/src/main.rs @@ -165,8 +165,6 @@ impl MetricsToken { enum Error { #[snafu(display("Sandbox creation failed: {}", source))] SandboxCreation { source: sandbox::Error }, - #[snafu(display("Linting operation failed: {}", source))] - Linting { source: sandbox::Error }, #[snafu(display("Expansion operation failed: {}", source))] Expansion { source: sandbox::Error }, #[snafu(display("Interpreting operation failed: {}", source))] @@ -204,11 +202,14 @@ enum Error { source: server_axum::api_orchestrator_integration_impls::ParseFormatRequestError, }, + #[snafu(context(false))] + ClippyRequest { + source: server_axum::api_orchestrator_integration_impls::ParseClippyRequestError, + }, + // Remove at a later point. From here ... #[snafu(display("The value {:?} is not a valid edition", value))] InvalidEdition { value: String }, - #[snafu(display("The value {:?} is not a valid crate type", value))] - InvalidCrateType { value: String }, // ... to here #[snafu(display("No request was provided"))] RequestMissing, @@ -242,6 +243,11 @@ enum Error { source: orchestrator::coordinator::FormatError, }, + #[snafu(display("Unable to convert the Clippy request"))] + Clippy { + source: orchestrator::coordinator::ClippyError, + }, + #[snafu(display("The operation timed out"))] Timeout { source: tokio::time::error::Elapsed }, @@ -356,6 +362,7 @@ struct ClippyRequest { #[derive(Debug, Clone, Serialize)] struct ClippyResponse { success: bool, + exit_detail: String, stdout: String, stderr: String, } @@ -436,28 +443,6 @@ struct EvaluateResponse { error: Option<String>, } -impl TryFrom<ClippyRequest> for sandbox::ClippyRequest { - type Error = Error; - - fn try_from(me: ClippyRequest) -> Result<Self> { - Ok(sandbox::ClippyRequest { - code: me.code, - crate_type: parse_crate_type(&me.crate_type)?, - edition: parse_edition(&me.edition)?, - }) - } -} - -impl From<sandbox::ClippyResponse> for ClippyResponse { - fn from(me: sandbox::ClippyResponse) -> Self { - ClippyResponse { - success: me.success, - stdout: me.stdout, - stderr: me.stderr, - } - } -} - impl TryFrom<MiriRequest> for sandbox::MiriRequest { type Error = Error; @@ -546,20 +531,6 @@ fn parse_edition(s: &str) -> Result<Option<sandbox::Edition>> { }) } -fn parse_crate_type(s: &str) -> Result<sandbox::CrateType> { - use crate::sandbox::{CrateType::*, LibraryType::*}; - Ok(match s { - "bin" => Binary, - "lib" => Library(Lib), - "dylib" => Library(Dylib), - "rlib" => Library(Rlib), - "staticlib" => Library(Staticlib), - "cdylib" => Library(Cdylib), - "proc-macro" => Library(ProcMacro), - value => InvalidCrateTypeSnafu { value }.fail()?, - }) -} - fn default_crate_type() -> String { "bin".into() } diff --git a/ui/src/metrics.rs b/ui/src/metrics.rs index 24dc11f66..af8657f0d 100644 --- a/ui/src/metrics.rs +++ b/ui/src/metrics.rs @@ -197,29 +197,6 @@ where } } -impl GenerateLabels for sandbox::ClippyRequest { - fn generate_labels(&self, outcome: Outcome) -> Labels { - let Self { - code: _, - edition, - crate_type, - } = *self; - - Labels { - endpoint: Endpoint::Clippy, - outcome, - - target: None, - channel: None, - mode: None, - edition: Some(edition), - crate_type: Some(crate_type), - tests: None, - backtrace: None, - } - } -} - impl GenerateLabels for sandbox::MiriRequest { fn generate_labels(&self, outcome: Outcome) -> Labels { let Self { code: _, edition } = *self; @@ -294,12 +271,6 @@ fn common_success_details(success: bool, stderr: &str) -> Outcome { } } -impl SuccessDetails for sandbox::ClippyResponse { - fn success_details(&self) -> Outcome { - common_success_details(self.success, &self.stderr) - } -} - impl SuccessDetails for sandbox::MiriResponse { fn success_details(&self) -> Outcome { common_success_details(self.success, &self.stderr) @@ -465,6 +436,27 @@ impl HasLabelsCore for coordinator::FormatRequest { } } +impl HasLabelsCore for coordinator::ClippyRequest { + fn labels_core(&self) -> LabelsCore { + let Self { + channel, + crate_type, + edition, + code: _, + } = *self; + + LabelsCore { + target: None, + channel: Some(channel.into()), + mode: None, + edition: Some(Some(edition.into())), + crate_type: Some(crate_type.into()), + tests: None, + backtrace: None, + } + } +} + pub(crate) fn record_metric( endpoint: Endpoint, labels_core: LabelsCore, diff --git a/ui/src/sandbox.rs b/ui/src/sandbox.rs index ed9d99242..dc97beb5c 100644 --- a/ui/src/sandbox.rs +++ b/ui/src/sandbox.rs @@ -178,19 +178,6 @@ impl Sandbox { }) } - pub async fn clippy(&self, req: &ClippyRequest) -> Result<ClippyResponse> { - self.write_source_code(&req.code).await?; - let command = self.clippy_command(req); - - let output = run_command_with_timeout(command).await?; - - Ok(ClippyResponse { - success: output.status.success(), - stdout: vec_to_str(output.stdout)?, - stderr: vec_to_str(output.stderr)?, - }) - } - pub async fn miri(&self, req: &MiriRequest) -> Result<MiriResponse> { self.write_source_code(&req.code).await?; let command = self.miri_command(req); @@ -321,19 +308,6 @@ impl Sandbox { Ok(()) } - fn clippy_command(&self, req: impl CrateTypeRequest + EditionRequest) -> Command { - let mut cmd = self.docker_command(Some(req.crate_type())); - - cmd.apply_crate_type(&req); - cmd.apply_edition(&req); - - cmd.arg("clippy").args(&["cargo", "clippy"]); - - debug!("Clippy command is {:?}", cmd); - - cmd - } - fn miri_command(&self, req: impl EditionRequest) -> Command { let mut cmd = self.docker_command(None); cmd.apply_edition(req); @@ -646,32 +620,6 @@ impl<R: BacktraceRequest> BacktraceRequest for &'_ R { } } -#[derive(Debug, Clone)] -pub struct ClippyRequest { - pub code: String, - pub edition: Option<Edition>, - pub crate_type: CrateType, -} - -impl CrateTypeRequest for ClippyRequest { - fn crate_type(&self) -> CrateType { - self.crate_type - } -} - -impl EditionRequest for ClippyRequest { - fn edition(&self) -> Option<Edition> { - self.edition - } -} - -#[derive(Debug, Clone)] -pub struct ClippyResponse { - pub success: bool, - pub stdout: String, - pub stderr: String, -} - #[derive(Debug, Clone)] pub struct MiriRequest { pub code: String, @@ -833,63 +781,6 @@ mod test { } "#; - impl Default for ClippyRequest { - fn default() -> Self { - ClippyRequest { - code: HELLO_WORLD_CODE.to_string(), - crate_type: CrateType::Binary, - edition: None, - } - } - } - - #[tokio::test] - async fn linting_code() { - let _singleton = one_test_at_a_time(); - let code = r#" - fn main() { - let a = 0.0 / 0.0; - println!("NaN is {}", a); - } - "#; - - let req = ClippyRequest { - code: code.to_string(), - ..ClippyRequest::default() - }; - - let sb = Sandbox::new().await.expect("Unable to create sandbox"); - let resp = sb.clippy(&req).await.expect("Unable to lint code"); - - assert!(resp.stderr.contains("deny(clippy::eq_op)")); - assert!(resp.stderr.contains("warn(clippy::zero_divided_by_zero)")); - } - - #[tokio::test] - async fn linting_code_options() { - let _singleton = one_test_at_a_time(); - let code = r#" - use itertools::Itertools; // Edition 2018 feature - - fn example() { - let a = 0.0 / 0.0; - println!("NaN is {}", a); - } - "#; - - let req = ClippyRequest { - code: code.to_string(), - crate_type: CrateType::Library(LibraryType::Rlib), - edition: Some(Edition::Rust2018), - }; - - let sb = Sandbox::new().await.expect("Unable to create sandbox"); - let resp = sb.clippy(&req).await.expect("Unable to lint code"); - - assert!(resp.stderr.contains("deny(clippy::eq_op)")); - assert!(resp.stderr.contains("warn(clippy::zero_divided_by_zero)")); - } - #[tokio::test] async fn interpreting_code() -> Result<()> { let _singleton = one_test_at_a_time(); diff --git a/ui/src/server_axum.rs b/ui/src/server_axum.rs index 6294f3882..14293cf62 100644 --- a/ui/src/server_axum.rs +++ b/ui/src/server_axum.rs @@ -5,10 +5,10 @@ use crate::{ HasLabelsCore, Outcome, SuccessDetails, UNAVAILABLE_WS, }, sandbox::{self, Channel, Sandbox, DOCKER_PROCESS_TIMEOUT_SOFT}, - CachingSnafu, ClippyRequest, ClippyResponse, CompileRequest, CompileResponse, CompileSnafu, - Config, Error, ErrorJson, EvaluateRequest, EvaluateResponse, EvaluateSnafu, ExecuteRequest, - ExecuteResponse, ExecuteSnafu, ExpansionSnafu, FormatRequest, FormatResponse, FormatSnafu, - GhToken, GistCreationSnafu, GistLoadingSnafu, InterpretingSnafu, LintingSnafu, + CachingSnafu, ClippyRequest, ClippyResponse, ClippySnafu, CompileRequest, CompileResponse, + CompileSnafu, Config, Error, ErrorJson, EvaluateRequest, EvaluateResponse, EvaluateSnafu, + ExecuteRequest, ExecuteResponse, ExecuteSnafu, ExpansionSnafu, FormatRequest, FormatResponse, + FormatSnafu, GhToken, GistCreationSnafu, GistLoadingSnafu, InterpretingSnafu, MacroExpansionRequest, MacroExpansionResponse, MetaCratesResponse, MetaGistCreateRequest, MetaGistResponse, MetaVersionResponse, MetricsToken, MiriRequest, MiriResponse, Result, SandboxCreationSnafu, ShutdownCoordinatorSnafu, TimeoutSnafu, @@ -170,13 +170,9 @@ async fn format(Json(req): Json<FormatRequest>) -> Result<Json<FormatResponse>> } async fn clippy(Json(req): Json<ClippyRequest>) -> Result<Json<ClippyResponse>> { - with_sandbox( - req, - |sb, req| async move { sb.clippy(req).await }.boxed(), - LintingSnafu, - ) - .await - .map(Json) + with_coordinator(req, |c, req| c.clippy(req).context(ClippySnafu).boxed()) + .await + .map(Json) } async fn miri(Json(req): Json<MiriRequest>) -> Result<Json<MiriResponse>> { @@ -237,6 +233,10 @@ impl HasEndpoint for FormatRequest { const ENDPOINT: Endpoint = Endpoint::Format; } +impl HasEndpoint for ClippyRequest { + const ENDPOINT: Endpoint = Endpoint::Clippy; +} + trait IsSuccess { fn is_success(&self) -> bool; } @@ -277,6 +277,12 @@ impl IsSuccess for coordinator::FormatResponse { } } +impl IsSuccess for coordinator::ClippyResponse { + fn is_success(&self) -> bool { + self.success + } +} + impl Outcome { fn from_success(other: impl IsSuccess) -> Self { if other.is_success() { @@ -1032,6 +1038,55 @@ pub(crate) mod api_orchestrator_integration_impls { } } + impl TryFrom<crate::ClippyRequest> for ClippyRequest { + type Error = ParseClippyRequestError; + + fn try_from(other: crate::ClippyRequest) -> std::result::Result<Self, Self::Error> { + let crate::ClippyRequest { + code, + edition, + crate_type, + } = other; + + Ok(ClippyRequest { + channel: Channel::Nightly, // TODO: use what user has submitted + crate_type: parse_crate_type(&crate_type)?, + edition: parse_edition(&edition)?, + code, + }) + } + } + + #[derive(Debug, Snafu)] + pub(crate) enum ParseClippyRequestError { + #[snafu(context(false))] + Edition { source: ParseEditionError }, + + #[snafu(context(false))] + CrateType { source: ParseCrateTypeError }, + } + + impl From<WithOutput<ClippyResponse>> for crate::ClippyResponse { + fn from(other: WithOutput<ClippyResponse>) -> Self { + let WithOutput { + response, + stdout, + stderr, + } = other; + let ClippyResponse { + success, + exit_detail, + } = response; + + Self { + success, + exit_detail, + stdout, + stderr, + } + } + } + fn parse_target( target: &str, assembly_flavor: Option<&str>,