Skip to content

Commit

Permalink
refactor(transformer)!: return String as error instead of OxcDiagno…
Browse files Browse the repository at this point in the history
…stic
  • Loading branch information
Boshen committed Nov 22, 2024
1 parent 52784d2 commit bb91f87
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 45 deletions.
10 changes: 6 additions & 4 deletions crates/oxc/src/napi/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,19 @@ pub struct TransformOptions {
pub inject: Option<FxHashMap<String, Either<String, Vec<String>>>>,
}

impl From<TransformOptions> for oxc_transformer::TransformOptions {
fn from(options: TransformOptions) -> Self {
Self {
impl TryFrom<TransformOptions> for oxc_transformer::TransformOptions {
type Error = String;

fn try_from(options: TransformOptions) -> Result<Self, Self::Error> {
Ok(Self {
cwd: options.cwd.map(PathBuf::from).unwrap_or_default(),
typescript: options
.typescript
.map(oxc_transformer::TypeScriptOptions::from)
.unwrap_or_default(),
jsx: options.jsx.map(Into::into).unwrap_or_default(),
..Self::default()
}
})
}
}

Expand Down
12 changes: 4 additions & 8 deletions crates/oxc_transformer/src/options/babel/env/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use std::str::FromStr;
use rustc_hash::FxHashMap;
use serde::Deserialize;

use oxc_diagnostics::Error;

pub use browserslist::Version;

use crate::options::{BrowserslistQuery, Engine, EngineTargets};
Expand Down Expand Up @@ -35,7 +33,7 @@ pub enum BabelTargetsValue {
}

impl TryFrom<BabelTargets> for EngineTargets {
type Error = Error;
type Error = String;
fn try_from(value: BabelTargets) -> Result<Self, Self::Error> {
match value {
BabelTargets::String(s) => BrowserslistQuery::Single(s).exec(),
Expand All @@ -52,7 +50,7 @@ impl TryFrom<BabelTargets> for EngineTargets {
continue;
};
let BabelTargetsValue::String(v) = value else {
return Err(Error::msg(format!("{value:?} is not a string for {key}.")));
return Err(format!("{value:?} is not a string for {key}."));
};
// TODO: Implement this target.
if key == "node" && v == "current" {
Expand All @@ -66,16 +64,14 @@ impl TryFrom<BabelTargets> for EngineTargets {
// <https://babel.dev/docs/options#targets>:
// Supported environments: android, chrome, deno, edge, electron, firefox, ie, ios, node, opera, rhino, safari, samsung.
let Ok(engine) = Engine::from_str(&key) else {
return Err(Error::msg(format!("engine '{key}' is not supported.")));
return Err(format!("engine '{key}' is not supported."));
};
match Version::parse(&v) {
Ok(version) => {
engine_targets.insert(engine, version);
}
Err(err) => {
return Err(oxc_diagnostics::Error::msg(format!(
"Failed to parse `{v}` for `{key}`\n{err:?}"
)))
return Err(format!("Failed to parse `{v}` for `{key}`\n{err:?}"))
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions crates/oxc_transformer/src/options/browserslist_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use std::sync::OnceLock;
use dashmap::DashMap;
use serde::Deserialize;

use oxc_diagnostics::{Error, OxcDiagnostic};

use super::EngineTargets;

#[derive(Debug, Clone, Deserialize, Eq, PartialEq, PartialOrd, Ord, Hash)]
Expand All @@ -20,7 +18,7 @@ fn cache() -> &'static DashMap<BrowserslistQuery, EngineTargets> {
}

impl BrowserslistQuery {
pub fn exec(&self) -> Result<EngineTargets, Error> {
pub fn exec(&self) -> Result<EngineTargets, String> {
if let Some(v) = cache().get(self) {
return Ok(v.clone());
}
Expand Down Expand Up @@ -50,9 +48,7 @@ impl BrowserslistQuery {
.collect::<Vec<_>>();
EngineTargets::parse_versions(versions)
}
Err(err) => {
return Err(OxcDiagnostic::error(format!("failed to resolve query: {err}")).into())
}
Err(err) => return Err(format!("failed to resolve query: {err}")),
};

cache().insert(self.clone(), result.clone());
Expand Down
11 changes: 5 additions & 6 deletions crates/oxc_transformer/src/options/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::{str::FromStr, sync::OnceLock};

use browserslist::Version;
use cow_utils::CowUtils;
use oxc_diagnostics::Error;
use rustc_hash::FxHashMap;
use serde::Deserialize;

Expand Down Expand Up @@ -38,17 +37,17 @@ impl Engine {
///
/// * No matching target
/// * Invalid version
pub fn parse_name_and_version(s: &str) -> Result<(Engine, Version), Error> {
pub fn parse_name_and_version(s: &str) -> Result<(Engine, Version), String> {
let s = s.cow_to_ascii_lowercase();
for (name, engine) in engines() {
if let Some(v) = s.strip_prefix(name) {
return Version::from_str(v).map(|version| (*engine,version))
.map_err(|_| Error::msg(
r#"All version numbers must be in the format "X", "X.Y", or "X.Y.Z" where X, Y, and Z are non-negative integers."#,
));
.map_err(|_|
String::from(r#"All version numbers must be in the format "X", "X.Y", or "X.Y.Z" where X, Y, and Z are non-negative integers."#),
);
}
}
Err(Error::msg(format!("Invalid target '{s}'.")))
Err(format!("Invalid target '{s}'."))
}
}

Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_transformer/src/options/engine_targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ use browserslist::Version;
use rustc_hash::FxHashMap;
use serde::Deserialize;

use oxc_diagnostics::Error;

use super::{
babel::BabelTargets,
engine::Engine,
Expand Down Expand Up @@ -42,7 +40,7 @@ impl EngineTargets {
/// # Errors
///
/// * Query is invalid.
pub fn try_from_query(query: &str) -> Result<Self, Error> {
pub fn try_from_query(query: &str) -> Result<Self, String> {
BrowserslistQuery::Single(query.to_string()).exec()
}

Expand Down
17 changes: 11 additions & 6 deletions crates/oxc_transformer/src/options/env.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::str::FromStr;

use oxc_diagnostics::Error;
use serde::Deserialize;

use crate::{
Expand Down Expand Up @@ -101,19 +100,25 @@ impl EnvOptions {
/// * When the query failed to parse.
///
/// [browserslist]: <https://github.com/browserslist/browserslist>
pub fn from_browserslist_query(query: &str) -> Result<Self, Error> {
pub fn from_browserslist_query(query: &str) -> Result<Self, String> {
EngineTargets::try_from_query(query).map(Self::from)
}

pub(crate) fn from_target(s: &str) -> Result<Self, Error> {
/// # Errors
///
/// * When the query failed to parse.
pub fn from_target(s: &str) -> Result<Self, String> {
if s.contains(',') {
Self::from_target_list(&s.split(',').collect::<Vec<_>>())
} else {
Self::from_target_list(&[s])
}
}

pub(crate) fn from_target_list<S: AsRef<str>>(list: &[S]) -> Result<Self, Error> {
/// # Errors
///
/// * When the query failed to parse.
pub fn from_target_list<S: AsRef<str>>(list: &[S]) -> Result<Self, String> {
let mut es_target = None;
let mut engine_targets = EngineTargets::default();

Expand All @@ -122,14 +127,14 @@ impl EnvOptions {
// Parse `esXXXX`.
if let Ok(target) = ESTarget::from_str(s) {
if let Some(target) = es_target {
return Err(Error::msg(format!("'{target}' is already specified.")));
return Err(format!("'{target}' is already specified."));
}
es_target = Some(target);
} else {
// Parse `chromeXX`, `edgeXX` etc.
let (engine, version) = Engine::parse_name_and_version(s)?;
if engine_targets.insert(engine, version).is_some() {
return Err(Error::msg(format!("'{s}' is already specified.")));
return Err(format!("'{s}' is already specified."));
}
}
}
Expand Down
14 changes: 6 additions & 8 deletions crates/oxc_transformer/src/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ mod module;

use std::path::PathBuf;

use oxc_diagnostics::Error;

use crate::{
common::helper_loader::{HelperLoaderMode, HelperLoaderOptions},
compiler_assumptions::CompilerAssumptions,
Expand Down Expand Up @@ -97,7 +95,7 @@ impl TransformOptions {
/// * Same targets specified multiple times.
/// * No matching target.
/// * Invalid version.
pub fn from_target(s: &str) -> Result<Self, Error> {
pub fn from_target(s: &str) -> Result<Self, String> {
EnvOptions::from_target(s).map(|env| Self { env, ..Self::default() })
}

Expand All @@ -115,7 +113,7 @@ impl TransformOptions {
/// * Same targets specified multiple times.
/// * No matching target.
/// * Invalid version.
pub fn from_target_list<S: AsRef<str>>(list: &[S]) -> Result<Self, Error> {
pub fn from_target_list<S: AsRef<str>>(list: &[S]) -> Result<Self, String> {
EnvOptions::from_target_list(list).map(|env| Self { env, ..Self::default() })
}
}
Expand All @@ -129,13 +127,13 @@ impl From<ESTarget> for TransformOptions {
}

impl TryFrom<&BabelOptions> for TransformOptions {
type Error = Vec<Error>;
type Error = Vec<String>;

/// If the `options` contains any unknown fields, they will be returned as a list of errors.
fn try_from(options: &BabelOptions) -> Result<Self, Self::Error> {
let mut errors = Vec::<Error>::new();
errors.extend(options.plugins.errors.iter().map(|err| Error::msg(err.clone())));
errors.extend(options.presets.errors.iter().map(|err| Error::msg(err.clone())));
let mut errors = Vec::<String>::new();
errors.extend(options.plugins.errors.iter().map(Clone::clone));
errors.extend(options.presets.errors.iter().map(Clone::clone));

let typescript = options
.presets
Expand Down
1 change: 1 addition & 0 deletions napi/transform/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ export interface TransformOptions {
typescript?: TypeScriptOptions
/** Configure how TSX and JSX are transformed. */
jsx?: JsxOptions
target?: string | Array<string>
/** Define Plugin */
define?: Record<string, string>
/** Inject Plugin */
Expand Down
7 changes: 5 additions & 2 deletions napi/transform/src/transformer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,11 @@ impl Compiler {
.transpose()?
.map(InjectGlobalVariablesConfig::new);

let transform_options =
options.map(oxc::transformer::TransformOptions::from).unwrap_or_default();
let transform_options = match options {
Some(options) => oxc::transformer::TransformOptions::try_from(options)
.map_err(|err| vec![OxcDiagnostic::error(err)])?,
None => oxc::transformer::TransformOptions::default(),
};

Ok(Self {
transform_options,
Expand Down
4 changes: 2 additions & 2 deletions tasks/transform_conformance/src/test_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use oxc::parser::ParseOptions;
use oxc::{
allocator::Allocator,
codegen::{CodeGenerator, CodegenOptions},
diagnostics::{Error, NamedSource, OxcDiagnostic},
diagnostics::{NamedSource, OxcDiagnostic},
parser::Parser,
span::{SourceType, VALID_EXTENSIONS},
transformer::{BabelOptions, HelperLoaderMode, TransformOptions},
Expand All @@ -27,7 +27,7 @@ pub struct TestCase {
pub path: PathBuf,
options: BabelOptions,
source_type: SourceType,
transform_options: Result<TransformOptions, Vec<Error>>,
transform_options: Result<TransformOptions, Vec<String>>,
pub errors: Vec<OxcDiagnostic>,
}

Expand Down

0 comments on commit bb91f87

Please sign in to comment.