Skip to content

Commit 8b66278

Browse files
jeffcharlesjeffcharles
authored
Fix JS runtime option help (#1158)
* Fix JS runtime option help * Add test for javy build -C help
1 parent d1ef6ed commit 8b66278

3 files changed

Lines changed: 106 additions & 80 deletions

File tree

‎crates/cli/src/commands.rs‎

Lines changed: 63 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use crate::{
2-
CliPlugin, WitOptions,
2+
CliPlugin, Plugin, PluginKind, WitOptions,
33
js_config::{ConfigSchema, JsConfig},
44
option::OptionMeta,
55
option_group,
6+
plugin::PLUGIN_MODULE,
67
};
78
use anyhow::{Result, anyhow, bail};
89
use clap::{
@@ -65,7 +66,7 @@ pub struct BuildCommandOpts {
6566
#[arg(short = RUNTIME_CONFIG_ARG_SHORT, long = RUNTIME_CONFIG_ARG_LONG)]
6667
/// JavaScript runtime options.
6768
/// Use `-J help` for more details.
68-
pub js: Vec<JsGroupValue>,
69+
pub js: Vec<JsGroupOption>,
6970
}
7071

7172
#[derive(Debug, Parser)]
@@ -281,13 +282,6 @@ impl TryFrom<Vec<GroupOption<CodegenOption>>> for CodegenOptionGroup {
281282
}
282283
}
283284

284-
/// A runtime config group value.
285-
#[derive(Debug, Clone)]
286-
pub(super) enum JsGroupValue {
287-
Option(JsGroupOption),
288-
Help,
289-
}
290-
291285
/// A runtime config group option.
292286
#[derive(Debug, Clone)]
293287
pub(super) struct JsGroupOption {
@@ -300,7 +294,7 @@ pub(super) struct JsGroupOption {
300294
#[derive(Debug, Clone)]
301295
pub(super) struct JsGroupOptionParser;
302296

303-
impl ValueParserFactory for JsGroupValue {
297+
impl ValueParserFactory for JsGroupOption {
304298
type Parser = JsGroupOptionParser;
305299

306300
fn value_parser() -> Self::Parser {
@@ -309,7 +303,7 @@ impl ValueParserFactory for JsGroupValue {
309303
}
310304

311305
impl TypedValueParser for JsGroupOptionParser {
312-
type Value = JsGroupValue;
306+
type Value = JsGroupOption;
313307

314308
fn parse_ref(
315309
&self,
@@ -320,7 +314,26 @@ impl TypedValueParser for JsGroupOptionParser {
320314
let val = StringValueParser::new().parse_ref(cmd, arg, value)?;
321315

322316
if val == "help" {
323-
return Ok(JsGroupValue::Help);
317+
let cli_plugin = CliPlugin::new(
318+
Plugin::new(PLUGIN_MODULE.into()).expect("default plugin to be valid"),
319+
PluginKind::Default,
320+
);
321+
let supported_properties = ConfigSchema::from_cli_plugin(&cli_plugin)
322+
.expect("config schema to be retrievable")
323+
.map_or(Vec::new(), |schema| schema.supported_properties);
324+
fmt_help(
325+
RUNTIME_CONFIG_ARG_LONG,
326+
&RUNTIME_CONFIG_ARG_SHORT.to_string(),
327+
&supported_properties
328+
.into_iter()
329+
.map(|prop| OptionMeta {
330+
name: prop.name,
331+
help: "[=y|n]".to_string(),
332+
doc: prop.doc,
333+
})
334+
.collect::<Vec<_>>(),
335+
);
336+
std::process::exit(0);
324337
}
325338

326339
let mut splits = val.splitn(2, '=');
@@ -331,18 +344,18 @@ impl TypedValueParser for JsGroupOptionParser {
331344
None => true,
332345
_ => return Err(clap::Error::new(clap::error::ErrorKind::InvalidValue)),
333346
};
334-
Ok(JsGroupValue::Option(JsGroupOption {
347+
Ok(JsGroupOption {
335348
name: key.to_string(),
336349
enabled: value,
337-
}))
350+
})
338351
}
339352
}
340353

341354
impl JsConfig {
342355
/// Build a JS runtime config from valid runtime config values.
343356
pub(super) fn from_group_values(
344357
cli_plugin: &CliPlugin,
345-
group_values: Vec<JsGroupValue>,
358+
group_values: Vec<JsGroupOption>,
346359
) -> Result<JsConfig> {
347360
// Always attempt to fetch the supported properties from a plugin.
348361
let supported_properties = ConfigSchema::from_cli_plugin(cli_plugin)?
@@ -354,40 +367,19 @@ impl JsConfig {
354367
}
355368

356369
let mut config = HashMap::new();
357-
for value in group_values {
358-
match value {
359-
JsGroupValue::Help => {
360-
fmt_help(
361-
RUNTIME_CONFIG_ARG_LONG,
362-
&RUNTIME_CONFIG_ARG_SHORT.to_string(),
363-
&supported_properties
364-
.into_iter()
365-
.map(|prop| OptionMeta {
366-
name: prop.name,
367-
help: "[=y|n]".to_string(),
368-
doc: prop.doc,
369-
})
370-
.collect::<Vec<_>>(),
371-
);
372-
std::process::exit(0);
373-
}
374-
JsGroupValue::Option(JsGroupOption { name, enabled }) => {
375-
if supported_names.contains(name.as_str()) {
376-
if config.contains_key(&name) {
377-
bail!("{name} can only be specified once");
378-
}
379-
config.insert(name, enabled);
380-
} else {
381-
Cli::command()
382-
.error(
383-
ErrorKind::InvalidValue,
384-
format!(
385-
"Property {name} is not supported for runtime configuration",
386-
),
387-
)
388-
.exit();
389-
}
370+
for JsGroupOption { name, enabled } in group_values {
371+
if supported_names.contains(name.as_str()) {
372+
if config.contains_key(&name) {
373+
bail!("{name} can only be specified once");
390374
}
375+
config.insert(name, enabled);
376+
} else {
377+
let msg = if matches!(cli_plugin.kind, PluginKind::User) {
378+
"JavaScript runtime options (-J) are not supported when using a plugin (-C plugin=...)".into()
379+
} else {
380+
format!("Property {name} is not supported for runtime configuration",)
381+
};
382+
Cli::command().error(ErrorKind::InvalidValue, msg).exit();
391383
}
392384
}
393385
Ok(JsConfig::from_hash(config))
@@ -400,7 +392,7 @@ mod tests {
400392

401393
use crate::{
402394
CliPlugin, Plugin, PluginKind,
403-
commands::{JsGroupOption, JsGroupValue, Source},
395+
commands::{JsGroupOption, Source},
404396
js_config::JsConfig,
405397
plugin::PLUGIN_MODULE,
406398
};
@@ -419,73 +411,73 @@ mod tests {
419411

420412
let group = JsConfig::from_group_values(
421413
&plugin,
422-
vec![JsGroupValue::Option(JsGroupOption {
414+
vec![JsGroupOption {
423415
name: "javy-stream-io".to_string(),
424416
enabled: false,
425-
})],
417+
}],
426418
)?;
427419
assert_eq!(group.get("javy-stream-io"), Some(false));
428420

429421
let group = JsConfig::from_group_values(
430422
&plugin,
431-
vec![JsGroupValue::Option(JsGroupOption {
423+
vec![JsGroupOption {
432424
name: "javy-stream-io".to_string(),
433425
enabled: true,
434-
})],
426+
}],
435427
)?;
436428
assert_eq!(group.get("javy-stream-io"), Some(true));
437429

438430
let group = JsConfig::from_group_values(
439431
&plugin,
440-
vec![JsGroupValue::Option(JsGroupOption {
432+
vec![JsGroupOption {
441433
name: "simd-json-builtins".to_string(),
442434
enabled: false,
443-
})],
435+
}],
444436
)?;
445437
assert_eq!(group.get("simd-json-builtins"), Some(false));
446438

447439
let group = JsConfig::from_group_values(
448440
&plugin,
449-
vec![JsGroupValue::Option(JsGroupOption {
441+
vec![JsGroupOption {
450442
name: "simd-json-builtins".to_string(),
451443
enabled: true,
452-
})],
444+
}],
453445
)?;
454446
assert_eq!(group.get("simd-json-builtins"), Some(true));
455447

456448
let group = JsConfig::from_group_values(
457449
&plugin,
458-
vec![JsGroupValue::Option(JsGroupOption {
450+
vec![JsGroupOption {
459451
name: "text-encoding".to_string(),
460452
enabled: false,
461-
})],
453+
}],
462454
)?;
463455
assert_eq!(group.get("text-encoding"), Some(false));
464456

465457
let group = JsConfig::from_group_values(
466458
&plugin,
467-
vec![JsGroupValue::Option(JsGroupOption {
459+
vec![JsGroupOption {
468460
name: "text-encoding".to_string(),
469461
enabled: true,
470-
})],
462+
}],
471463
)?;
472464
assert_eq!(group.get("text-encoding"), Some(true));
473465

474466
let group = JsConfig::from_group_values(
475467
&plugin,
476468
vec![
477-
JsGroupValue::Option(JsGroupOption {
469+
JsGroupOption {
478470
name: "javy-stream-io".to_string(),
479471
enabled: false,
480-
}),
481-
JsGroupValue::Option(JsGroupOption {
472+
},
473+
JsGroupOption {
482474
name: "simd-json-builtins".to_string(),
483475
enabled: false,
484-
}),
485-
JsGroupValue::Option(JsGroupOption {
476+
},
477+
JsGroupOption {
486478
name: "text-encoding".to_string(),
487479
enabled: false,
488-
}),
480+
},
489481
],
490482
)?;
491483
assert_eq!(group.get("javy-stream-io"), Some(false));
@@ -603,14 +595,14 @@ mod tests {
603595
let result = JsConfig::from_group_values(
604596
&plugin,
605597
vec![
606-
JsGroupValue::Option(JsGroupOption {
598+
JsGroupOption {
607599
name: "javy-stream-io".to_string(),
608600
enabled: false,
609-
}),
610-
JsGroupValue::Option(JsGroupOption {
601+
},
602+
JsGroupOption {
611603
name: "javy-stream-io".to_string(),
612604
enabled: true,
613-
}),
605+
},
614606
],
615607
);
616608
assert_eq!(

‎crates/cli/tests/dynamic_linking_test.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ fn test_using_runtime_flag_with_dynamic_triggers_error(builder: &mut Builder) ->
8282
let build_result = builder.input("console.js").text_encoding(false).build();
8383
assert!(build_result.is_err_and(|e| {
8484
e.to_string()
85-
.contains("error: Property text-encoding is not supported for runtime configuration")
85+
.contains("error: JavaScript runtime options (-J) are not supported when using a plugin (-C plugin=...)")
8686
}));
8787
Ok(())
8888
}

‎crates/cli/tests/integration_test.rs‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,9 @@ fn test_using_wasip1_plugin_with_static_build_fails_with_runtime_config(
150150
.simd_json_builtins(true)
151151
.build();
152152
let err = result.err().unwrap();
153-
assert!(
154-
err.to_string()
155-
.contains("Property simd-json-builtins is not supported for runtime configuration")
156-
);
153+
assert!(err.to_string().contains(
154+
"JavaScript runtime options (-J) are not supported when using a plugin (-C plugin=...)"
155+
));
157156

158157
Ok(())
159158
}
@@ -167,10 +166,9 @@ fn test_using_wasip2_plugin_with_static_build_fails_with_runtime_config(
167166
.simd_json_builtins(true)
168167
.build();
169168
let err = result.err().unwrap();
170-
assert!(
171-
err.to_string()
172-
.contains("Property simd-json-builtins is not supported for runtime configuration")
173-
);
169+
assert!(err.to_string().contains(
170+
"JavaScript runtime options (-J) are not supported when using a plugin (-C plugin=...)"
171+
));
174172

175173
Ok(())
176174
}
@@ -414,6 +412,42 @@ fn test_source_code_omitted(builder: &mut Builder) -> Result<()> {
414412
Ok(())
415413
}
416414

415+
#[test]
416+
fn test_js_help() -> Result<()> {
417+
let output = Command::new(env!("CARGO_BIN_EXE_javy"))
418+
.args(["build", "-J", "help"])
419+
.output()?;
420+
assert!(
421+
output.status.success(),
422+
"build -J help failed: {}",
423+
str::from_utf8(&output.stderr)?
424+
);
425+
let stdout = str::from_utf8(&output.stdout)?;
426+
assert!(
427+
stdout.contains("Available options for javascript"),
428+
"unexpected help output: {stdout}"
429+
);
430+
Ok(())
431+
}
432+
433+
#[test]
434+
fn test_codegen_help() -> Result<()> {
435+
let output = Command::new(env!("CARGO_BIN_EXE_javy"))
436+
.args(["build", "-C", "help"])
437+
.output()?;
438+
assert!(
439+
output.status.success(),
440+
"build -C help failed: {}",
441+
str::from_utf8(&output.stderr)?
442+
);
443+
let stdout = str::from_utf8(&output.stdout)?;
444+
assert!(
445+
stdout.contains("Available options for codegen"),
446+
"unexpected help output: {stdout}"
447+
);
448+
Ok(())
449+
}
450+
417451
#[test]
418452
fn test_init_plugin() -> Result<()> {
419453
let engine = Engine::default();

0 commit comments

Comments
 (0)