test: importer spec (#2)

Author: ahabhgk

Cargo.toml

@@ -12,5 +12,9 @@ dashmap = "5"
 crossbeam-channel = "0.5"
 rustc-hash = "1"
 
+[dev-dependencies]
+serde_json = "1"
+tempfile = "3"
+
 [build-dependencies]
 prost-build = "0.10"

src/dispatcher.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 use crate::{
   compiler::Compiler,
   connection::{Connected, ConnectedGuard, Connection, Unconnected},
-  host::{Host},
+  host::Host,
   protocol::{outbound_message, InboundMessage, OutboundMessage},
 };
 
@@ -51,7 +51,7 @@ impl Dispatcher {
   }
 
   pub fn unsubscribe(&self, id: &u32) {
-    self.observers.remove(&id);
+    self.observers.remove(id);
   }
 
   pub fn send_message(&self, inbound_message: InboundMessage) {
@@ -67,10 +67,8 @@ impl Dispatcher {
           for ob in self.observers.iter() {
             ob.error(e.clone());
           }
-        } else {
-          if let Some(ob) = self.observers.get(&e.id) {
-            ob.error(e);
-          }
+        } else if let Some(ob) = self.observers.get(&e.id) {
+          ob.error(e);
         }
       }
       outbound_message::Message::CompileResponse(e) => {

src/embedded.rs

@@ -44,12 +44,16 @@ impl Embedded {
         options.load_paths.unwrap_or_default(),
       )
       .collect();
-    options.logger.map(|l| logger_registry.register(l));
+    if let Some(l) = options.logger {
+      logger_registry.register(l);
+    }
 
     let request = CompileRequest {
       style: options.style as i32,
       source_map: options.source_map,
-      alert_color: options.alert_color.unwrap_or(atty::is(Stream::Stdout)),
+      alert_color: options
+        .alert_color
+        .unwrap_or_else(|| atty::is(Stream::Stdout)),
       alert_ascii: options.alert_ascii,
       verbose: options.verbose,
       quiet_deps: options.quiet_deps,
@@ -65,7 +69,7 @@ impl Embedded {
     let host = Host::new(importer_registry, logger_registry);
     let conn = self.channel.connect(host);
     let response = conn.compile_request(request)?;
-    Ok(CompileResult::try_from(response)?)
+    CompileResult::try_from(response)
   }
 
   pub fn compile_string(
@@ -81,15 +85,17 @@ impl Embedded {
         options.common.load_paths.unwrap_or_default(),
       )
       .collect();
-    options.common.logger.map(|l| logger_registry.register(l));
+    if let Some(l) = options.common.logger {
+      logger_registry.register(l);
+    }
 
     let request = CompileRequest {
       style: options.common.style as i32,
       source_map: options.common.source_map,
       alert_color: options
         .common
         .alert_color
-        .unwrap_or(atty::is(Stream::Stdout)),
+        .unwrap_or_else(|| atty::is(Stream::Stdout)),
       alert_ascii: options.common.alert_ascii,
       verbose: options.common.verbose,
       quiet_deps: options.common.quiet_deps,
@@ -110,7 +116,7 @@ impl Embedded {
     let host = Host::new(importer_registry, logger_registry);
     let conn = self.channel.connect(host);
     let response = conn.compile_request(request)?;
-    Ok(CompileResult::try_from(response)?)
+    CompileResult::try_from(response)
   }
 
   pub fn info(&mut self) -> Result<String> {

src/error.rs

@@ -59,9 +59,9 @@ impl From<ProtocolError> for Exception {
 }
 
 impl Exception {
-  pub fn new(message: String) -> Self {
+  pub fn new(message: impl Into<String>) -> Self {
     Self {
-      message,
+      message: message.into(),
       sass_message: None,
       sass_stack: None,
       span: None,

src/host/importer_registry.rs

@@ -87,11 +87,8 @@ impl ImporterRegistry {
     ) {
       Ok(url) => CanonicalizeResponse {
         id: request.id,
-        result: if let Some(url) = url {
-          Some(canonicalize_response::Result::Url(url.to_string()))
-        } else {
-          None
-        },
+        result: url
+          .map(|url| canonicalize_response::Result::Url(url.to_string())),
       },
       Err(e) => CanonicalizeResponse {
         id: request.id,

src/lib.rs

@@ -12,23 +12,23 @@ mod varint;
 pub use embedded::{CompileResult, Embedded, Embedded as Sass};
 pub use error::{Exception, Result};
 pub use options::{
-  FileImporter, Importer, Logger, LoggerDebugOptions, LoggerWarnOptions,
-  Options, OptionsBuilder, SassLogger, StringOptions, StringOptionsBuilder,
+  FileImporter, Importer, ImporterOptions, ImporterResult, Logger,
+  LoggerDebugOptions, LoggerWarnOptions, Options, OptionsBuilder, SassLogger,
+  StringOptions, StringOptionsBuilder,
 };
 pub use protocol::{OutputStyle, SourceSpan, Syntax};
 pub use url::{self, Url};
 
-#[cfg(test)]
-pub fn exe_path() -> std::path::PathBuf {
-  std::path::PathBuf::from(concat!(env!("CARGO_MANIFEST_DIR")))
-    .join("sass_embedded")
-    .join("dart-sass-embedded")
-}
-
 #[cfg(test)]
 mod tests {
   use super::*;
 
+  pub fn exe_path() -> std::path::PathBuf {
+    std::path::PathBuf::from(concat!(env!("CARGO_MANIFEST_DIR")))
+      .join("sass_embedded")
+      .join("dart-sass-embedded")
+  }
+
   #[test]
   fn version_smoke() {
     let mut sass = Sass::new(exe_path());

src/options.rs

@@ -76,18 +76,18 @@ impl OptionsBuilder {
     self
   }
 
-  pub fn load_paths(mut self, arg: Vec<String>) -> Self {
-    self.options.load_paths = Some(arg);
+  pub fn load_paths(mut self, arg: impl IntoIterator<Item = String>) -> Self {
+    self.options.load_paths = Some(arg.into_iter().collect());
     self
   }
 
-  pub fn load_path(mut self, arg: &str) -> Self {
+  pub fn load_path(mut self, arg: impl Into<String>) -> Self {
     let mut load_paths = if let Some(load_paths) = self.options.load_paths {
       load_paths
     } else {
       Vec::new()
     };
-    load_paths.push(arg.to_owned());
+    load_paths.push(arg.into());
     self.options.load_paths = Some(load_paths);
     self
   }
@@ -228,8 +228,8 @@ impl StringOptionsBuilder {
     self
   }
 
-  pub fn url(mut self, arg: Url) -> Self {
-    self.url = Some(arg);
+  pub fn url(mut self, arg: impl Into<Url>) -> Self {
+    self.url = Some(arg.into());
     self
   }
 
@@ -243,18 +243,18 @@ impl StringOptionsBuilder {
     self
   }
 
-  pub fn load_paths(mut self, arg: Vec<String>) -> Self {
-    self.options.load_paths = Some(arg);
+  pub fn load_paths(mut self, arg: impl IntoIterator<Item = String>) -> Self {
+    self.options.load_paths = Some(arg.into_iter().collect());
     self
   }
 
-  pub fn load_path(mut self, arg: &str) -> Self {
+  pub fn load_path(mut self, arg: impl Into<String>) -> Self {
     let mut load_paths = if let Some(load_paths) = self.options.load_paths {
       load_paths
     } else {
       Vec::new()
     };
-    load_paths.push(arg.to_owned());
+    load_paths.push(arg.into());
     self.options.load_paths = Some(load_paths);
     self
   }
@@ -363,14 +363,14 @@ pub enum SassImporter {
 /// https://sass-lang.com/documentation/js-api/interfaces/Importer
 pub trait Importer: Debug + Send + Sync {
   /// https://sass-lang.com/documentation/js-api/interfaces/Importer#canonicalize
-  fn canonicalize<'u, 'o>(
+  fn canonicalize(
     &self,
-    url: &'u str,
-    options: &'o ImporterOptions,
+    url: &str,
+    options: &ImporterOptions,
   ) -> Result<Option<Url>>;
 
   /// https://sass-lang.com/documentation/js-api/interfaces/Importer#load
-  fn load<'u>(&self, canonical_url: &'u Url) -> Result<Option<ImporterResult>>;
+  fn load(&self, canonical_url: &Url) -> Result<Option<ImporterResult>>;
 }
 
 pub struct ImporterOptions {
@@ -380,10 +380,10 @@ pub struct ImporterOptions {
 /// https://sass-lang.com/documentation/js-api/interfaces/FileImporter
 pub trait FileImporter: Debug + Send + Sync {
   /// https://sass-lang.com/documentation/js-api/interfaces/FileImporter#findFileUrl
-  fn find_file_url<'u, 'o>(
+  fn find_file_url(
     &self,
-    url: &'u str,
-    options: &'o ImporterOptions,
+    url: &str,
+    options: &ImporterOptions,
   ) -> Result<Option<Url>>;
 }