Skip to content

[Feat] Determine the format automatically for input files on the CLI #2267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cli/tests/integration/inputs/mixed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extra": {
"subfield": "here",
"json": true
}
}
3 changes: 3 additions & 0 deletions cli/tests/integration/inputs/mixed.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
extra.nickel = true,
}
5 changes: 5 additions & 0 deletions cli/tests/integration/inputs/mixed.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
foo = "hello"
bar = 123

[extra]
toml = true
10 changes: 10 additions & 0 deletions cli/tests/integration/inputs/mixed_contract.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
bar | Number,
extra | {
nickel | Bool,
subfield | String,
toml | Bool,
json | Bool,
},
foo | String,
}
56 changes: 56 additions & 0 deletions cli/tests/integration/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{
io::Write,
path::PathBuf,
process::{Command, Stdio},
};

Expand Down Expand Up @@ -73,3 +74,58 @@ fn automatic_color_on_non_tty() {
);
}
}

#[test]
// This test didn't fit the snapshot test specification very well. While it can be encoded as
// snapshot test, it didn't work on the CI (where the working directory doesn't seem to be properly
// set for some reason), so we are using a manual test for now.
fn merge_mixed_formats() {
let mut input_base_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
// We don't push "tests/integration/inputs" at once because that wouldn't work on Windows.
input_base_path.push("tests");
input_base_path.push("integration");
input_base_path.push("inputs");

let nickel_bin = env!("CARGO_BIN_EXE_nickel");

let nickel = Command::new(nickel_bin)
.arg("export")
.args([
input_base_path.join("mixed.ncl").into_os_string(),
input_base_path.join("mixed.json").into_os_string(),
input_base_path.join("mixed.toml").into_os_string(),
])
.arg("--apply-contract")
.arg(input_base_path.join("mixed_contract.ncl").into_os_string())
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.expect("Nickel should be runnable");

let output = nickel
.wait_with_output()
.expect("couldn't retrieve stdout handle to Nickel");

let stderr_output =
String::from_utf8(output.stderr).expect("The result of Nickel should be valid utf8");
let stdout_output =
String::from_utf8(output.stdout).expect("The result of Nickel should be valid utf8");

assert_eq!(stderr_output, "");
assert_eq!(
stdout_output,
"\
{
\"bar\": 123,
\"extra\": {
\"json\": true,
\"nickel\": true,
\"subfield\": \"here\",
\"toml\": true
},
\"foo\": \"hello\"
}
"
);
}
6 changes: 4 additions & 2 deletions core/src/bytecode/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,8 +995,10 @@ impl<'a> Pretty<'a, Allocator> for &Node<'_> {
allocator,
"import",
allocator.space(),
allocator.as_string(path.to_string_lossy()).double_quotes(),
if Some(*format) != InputFormat::from_path(std::path::Path::new(path)) {
allocator
.escaped_string(path.to_string_lossy().as_ref())
.double_quotes(),
if Some(*format) != InputFormat::from_path(path) {
docs![
allocator,
allocator.space(),
Expand Down
4 changes: 2 additions & 2 deletions core/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ pub enum InputFormat {

impl InputFormat {
/// Returns an [InputFormat] based on the file extension of a path.
pub fn from_path(path: &Path) -> Option<InputFormat> {
match path.extension().and_then(OsStr::to_str) {
pub fn from_path(path: impl AsRef<Path>) -> Option<InputFormat> {
match path.as_ref().extension().and_then(OsStr::to_str) {
Some("ncl") => Some(InputFormat::Nickel),
Some("json") => Some(InputFormat::Json),
Some("yaml") | Some("yml") => Some(InputFormat::Yaml),
Expand Down
3 changes: 1 addition & 2 deletions core/src/parser/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ pub fn mk_import_based_on_filename(
_span: RawSpan,
) -> Result<Node<'_>, ParseError> {
let path = OsString::from(path);
let format: Option<InputFormat> =
InputFormat::from_path(std::path::Path::new(path.as_os_str()));
let format: Option<InputFormat> = InputFormat::from_path(&path);

// Fall back to InputFormat::Nickel in case of unknown filename extension for backwards compatiblilty.
let format = format.unwrap_or_default();
Expand Down
8 changes: 4 additions & 4 deletions core/src/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,10 +1157,10 @@ impl<'a> Pretty<'a, Allocator> for &Term {
allocator,
"import",
allocator.space(),
allocator.as_string(path.to_string_lossy()).double_quotes(),
if Some(*format)
!= InputFormat::from_path(std::path::Path::new(path.as_os_str()))
{
allocator
.escaped_string(path.to_string_lossy().as_ref())
.double_quotes(),
if Some(*format) != InputFormat::from_path(path) {
docs![
allocator,
allocator.space(),
Expand Down
24 changes: 19 additions & 5 deletions core/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ impl<EC: EvalCache> Program<EC> {

/// Contructor that abstracts over the Input type (file, string, etc.). Used by
/// the other constructors. Published for those that need abstraction over the kind of Input.
///
/// The format of the input is Nickel by default. However, for [Input::Path]s, the format is
/// determined from the file extension. This is useful to merge Nickel and non-Nickel files, or
/// to apply extra contracts to non-Nickel configurations.
pub fn new_from_input<T, S>(
input: Input<T, S>,
trace: impl Write + 'static,
Expand All @@ -280,7 +284,11 @@ impl<EC: EvalCache> Program<EC> {
let mut cache = CacheHub::new();

let main_id = match input {
Input::Path(path) => cache.sources.add_file(path, InputFormat::Nickel)?,
Input::Path(path) => {
let path = path.into();
let format = InputFormat::from_path(&path).unwrap_or_default();
cache.sources.add_file(path, format)?
}
Input::Source(source, name) => {
let path = PathBuf::from(name.into());
cache
Expand All @@ -302,6 +310,10 @@ impl<EC: EvalCache> Program<EC> {
/// Constructor that abstracts over an iterator of Inputs (file, strings,
/// etc). Published for those that need abstraction over the kind of Input
/// or want to mix multiple different kinds of Input.
///
/// The format of each input is Nickel by default. However, for [Input::Path]s, the format is
/// determined from the file extension. This is useful to merge Nickel and non-Nickel files, or
/// to apply extra contracts to non-Nickel configurations.
pub fn new_from_inputs<I, T, S>(
inputs: I,
trace: impl Write + 'static,
Expand All @@ -318,10 +330,12 @@ impl<EC: EvalCache> Program<EC> {
let merge_term = inputs
.into_iter()
.map(|input| match input {
Input::Path(path) => RichTerm::from(Term::Import(Import::Path {
path: path.into(),
format: InputFormat::Nickel,
})),
Input::Path(path) => {
let path = path.into();
let format = InputFormat::from_path(&path).unwrap_or_default();

RichTerm::from(Term::Import(Import::Path { path, format }))
}
Input::Source(source, name) => {
let path = PathBuf::from(name.into());
cache
Expand Down
8 changes: 6 additions & 2 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,12 @@
txtFilter = mkFilter ".*txt$";
snapFilter = mkFilter ".*snap$";
scmFilter = mkFilter ".*scm$";
mdFilter = mkFilter ".*md$"; # include markdown files for checking snippets in the documentation
# include markdown files for checking snippets in the documentation
mdFilter = mkFilter ".*md$";
cxxFilter = mkFilter ".*(cc|hh)$";
importsFilter = mkFilter ".*/core/tests/integration/inputs/imports/imported/.*$"; # include all files that are imported in tests
# include all files that are used in tests
importsFilter = mkFilter ".*/core/tests/integration/inputs/imports/imported/.*$";
testsInputsFilter = mkFilter ".*/cli/tests/integration/inputs/.*$";
in
pkgs.lib.cleanSourceWith {
src = pkgs.lib.cleanSource ./.;
Expand All @@ -210,6 +213,7 @@
cxxFilter
filterCargoSources
importsFilter
testsInputsFilter
];
};

Expand Down
Loading