mockersf / hocon.rs

Parse HOCON configuration files in Rust
MIT License
78 stars 16 forks source link

`HoconLoader.load_file` always loads empty object #25

Closed frantisekhanzlikbl closed 4 years ago

frantisekhanzlikbl commented 4 years ago

When loading a file using HoconLoader's load_file, the parsed object is always empty.

When reading the same file using fs::read_to_string and then loading it with HoconLoader.load_str works as expected.

Sample code:

println!(
    "{:?}",
    HoconLoader::new()
        .load_file(&path)
        .unwrap()
        .hocon()
        .unwrap(),
);

Expected output

Parsed contents of the file located at path

Actual output

Hash({}) regardless of the actual contents of the file at path

Repository

https://gitlab.com/frantisekhanzlikbl/hocon.rs-issue-25

Versions:

component version
hocon 0.3.4
cargo cargo 1.45.0-nightly (40ebd5220 2020-06-01)
rustc rustc 1.46.0-nightly (feb3536eb 2020-06-09)
mockersf commented 4 years ago

sorry but I can't reproduce using this file with stable or nightly : https://github.com/mockersf/hocon.rs/blob/master/tests/data/comments.conf 😞

> cat ../../hocon.rs/tests/data/comments.conf
{
    // first comment
    "a": 5, // second comment
    "b": 6.7 # other comment
    "c": [
        1 // comment in array
        2, # other comment in array
        # multi line comment

        // and new comment
        3
    ],
    "d": true,
    "e": "val",
    "f": {
        "g": 12,
        "g": false
    }
}

> cargo run  ../../hocon.rs/tests/data/comments.conf
   Compiling hocon_test v0.1.0 (/Users/francois/Dev/bugs/hocon.rs-issue-25)
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/hocon_test ../../hocon.rs/tests/data/comments.conf`
metadata: Hash({"a": Integer(5), "e": String("val"), "d": Boolean(true), "c": Array([Integer(1), Integer(2), Integer(3)]), "b": Real(6.7), "f": Hash({"g": Boolean(false)})})

> cargo +nightly rustc -- --version
   Compiling hocon_test v0.1.0 (/Users/francois/Dev/bugs/hocon.rs-issue-25)
rustc 1.46.0-nightly (feb3536eb 2020-06-09)
    Finished dev [unoptimized + debuginfo] target(s) in 0.18s

> cargo +nightly run  ../../hocon.rs/tests/data/comments.conf
   Compiling hocon_test v0.1.0 (/Users/francois/Dev/bugs/hocon.rs-issue-25)
    Finished dev [unoptimized + debuginfo] target(s) in 1.21s
     Running `target/debug/hocon_test ../../hocon.rs/tests/data/comments.c`
metadata: Hash({"d": Boolean(true), "e": String("val"), "f": Hash({"g": Boolean(false)}), "c": Array([Integer(1), Integer(2), Integer(3)]), "a": Integer(5), "b": Real(6.7)})

this function uses petty much the same way I use to read a file, could you check how it is working ?

use std::io::prelude::*;
fn read_at_path<P: AsRef<std::path::Path>>(path: P) -> Result<(), Box<dyn std::error::Error>> {
    let mut file_path = path.as_ref().to_path_buf();
    if !file_path.has_root() {
        let mut current_path = std::env::current_dir()?;
        current_path.push(path.as_ref());
        file_path = current_path;
    }
    let mut file = std::fs::File::open(file_path.as_os_str())?;
    let mut contents = String::new();
    file.read_to_string(&mut contents)?;
    println!("{:?}", contents);
    Ok(())
}
frantisekhanzlikbl commented 4 years ago

Found the culprit: All of the files I was trying to load had the extension .hocon, and it seems that this behaviour happens with all files of an unknown extension. .json and .conf work as expected.

I'm not sure whether this is a bug or intended behaviour, but at least a warning could be emitted because the current behaviour is pretty confusing.

I am interested in doing something about this, but I'm not sure whether to error out, issue warning or just parse normally. Your opinion?

frantisekhanzlikbl commented 4 years ago

My understanding is that the problem occurs here: loader_config.rs:193 If an unknown extension is detected at loader_config.rs:61, then attempts to read files with the same base name but known extensions are made, which of course fail.

mockersf commented 4 years ago

It's because I'm using the same path when reading the first file or reading a file included from another, and https://github.com/lightbend/config/blob/master/HOCON.md#include-semantics-file-formats-and-extensions say that it should work even if the are no extensions.

The usual extension for hocon is .conf even though rereading the spec rapidly it is not said explicitly...

I think it should not fail, but rather add a test here (loader_config.rs:193) to check if the file exist with it's specified path and opening it as is before trying to add the extension.

If you're still interested, a PR would be welcomed, otherwise I'll fix it before end of July.

Sorry for taking so long to respond and thank you for your patience and your investigation

frantisekhanzlikbl commented 4 years ago

That makes sense. I didn't know that the spec had that requirement on imports.

I will give it a try; I'm not sure when, though, as I'm currently a little short on time. I'll try to make it within this week and will keep you posted.

Thanks for the information and reply!

frantisekhanzlikbl commented 4 years ago

So sorry that I haven't given any info!

I did make a little work on it, but work got in the way, and I don't think I'll be able to complete it any time soon. :slightly_frowning_face:

As this doesn't seem to bother many users, I think it's that important for you to work on it any time soon. If it would take too much of your time, I guess I could attempt to fix it sometime later in this month myself.

Again, sorry that I haven't informed you any sooner, and thank you for the patience.

mockersf commented 4 years ago

No problem it was holidays for me!

I fixed it and publish version 0.3.5 with the fix.

Thank you!