toml-rs / toml

Rust TOML Parser
https://docs.rs/toml
Apache License 2.0
689 stars 103 forks source link

`toml_edit`: value is not written if table is constructed beforehand #762

Open tingerrr opened 1 month ago

tingerrr commented 1 month ago

The following code simply discards the last write to the tests sub-key.

use toml_edit::{DocumentMut, value};

fn main() {
    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    // doc["tool"] = toml_edit::table();
    // doc["tool"]["typst-test"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());
}

Produces the following output:

tool = { typst-test = { tests = "tests" } }

Uncommenting the second line produces:

tool = {}

Uncommenting the second and first line produces:

[tool]

[tool.typst-test]
tests = "tests"

When inspecting the table tool.typst-test, the value exists, it's simply not written.

epage commented 1 month ago

I've updated the reproduction case to make it easier to follow

#!/usr/bin/env nargo
---
[dependencies]
toml_edit = "0.22"
---

use toml_edit::{DocumentMut, value};

fn main() {
    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());

    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());

    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"]["typst-test"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());

    let toml = "";
    let mut doc = toml.parse::<DocumentMut>().expect("invalid doc");
    doc["tool"] = toml_edit::table();
    doc["tool"]["typst-test"] = toml_edit::table();
    doc["tool"]["typst-test"]["tests"] = value("tests");
    println!("doc: \n{}\n", doc.to_string());
}
$ ./toml_edit-762.rs
warning: `package.edition` is unspecified, defaulting to `2021`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.05s
     Running `/home/epage/.cargo/target/84/93c6f0d3e9e9a2/debug/toml_edit-762`
doc:
tool = { typst-test = { tests = "tests" } }

doc:
[tool]
typst-test = { tests = "tests" }

doc:
tool = {}

doc:
[tool]

[tool.typst-test]
tests = "tests"
epage commented 1 month ago

To understand whats going on, I'm going to show the debug repr

[/home/epage/.cargo/target/84/93c6f0d3e9e9a2/toml_edit-762.rs:25:5] &doc = DocumentMut {
    root: Table(
        Table {
            decor: Decor {
                prefix: "default",
                suffix: "default",
            },
            implicit: false,
            dotted: false,
            doc_position: None,
            span: None,
            items: {
                "tool": TableKeyValue {
                    key: Key {
                        key: "tool",
                        repr: None,
                        leaf_decor: Decor {
                            prefix: "default",
                            suffix: "default",
                        },
                        dotted_decor: Decor {
                            prefix: "default",
                            suffix: "default",
                        },
                    },
                    value: Value(
                        InlineTable(
                            InlineTable {
                                preamble: empty,
                                implicit: false,
                                decor: Decor {
                                    prefix: "default",
                                    suffix: "default",
                                },
                                span: None,
                                dotted: false,
                                items: {
                                    "typst-test": TableKeyValue {
                                        key: Key {
                                            key: "typst-test",
                                            repr: None,
                                            leaf_decor: Decor {
                                                prefix: "default",
                                                suffix: "default",
                                            },
                                            dotted_decor: Decor {
                                                prefix: "default",
                                                suffix: "default",
                                            },
                                        },
                                        value: Table(
                                            Table {
                                                decor: Decor {
                                                    prefix: "default",
                                                    suffix: "default",
                                                },
                                                implicit: false,
                                                dotted: false,
                                                doc_position: None,
                                                span: None,
                                                items: {
                                                    "tests": TableKeyValue {
                                                        key: Key {
                                                            key: "tests",
                                                            repr: None,
                                                            leaf_decor: Decor {
                                                                prefix: "default",
                                                                suffix: "default",
                                                            },
                                                            dotted_decor: Decor {
                                                                prefix: "default",
                                                                suffix: "default",
                                                            },
                                                        },
                                                        value: Value(
                                                            String(
                                                                Formatted {
                                                                    value: "tests",
                                                                    repr: "default",
                                                                    decor: Decor {
                                                                        prefix: "default",
                                                                        suffix: "default",
                                                                    },
                                                                },
                                                            ),
                                                        ),
                                                    },
                                                },
                                            },
                                        ),
                                    },
                                },
                            },
                        ),
                    ),
                },
            },
        },
    ),
    trailing: empty,
}

InlineTables are implicitly being created. When we go to render the values in an InlineTable but we come across a Table, we ignore it.

epage commented 1 month ago

Unsure how well this is documented (I wanted to do the above before confirming) but

doc["tool"]["typst-test"]["tests"] = value("tests");

always produces inline tables for the intermediate tables and its left to the caller to put in standard tables, correctly, if they want that.

The question is what we should do moving forward

tingerrr commented 1 month ago

always produces inline tables for the intermediate tables and its left to the caller to put in standard tables, correctly, if they want that.

I think that's the part that confused me when I initially used it (it makes somewhat sense in hindsight). An assertion seems like the best option to me, implicit conversion could cause problems elsewhere and be even more confusing, and leaving it as is, just seems wrong. It should definitely not swallow whatever I put in there without warning.

epage commented 1 month ago

Created a test case


#[test]
fn table_under_inline() {
    let mut doc = DocumentMut::new();
    doc["tool"]["typst-test"] = table();
    doc["tool"]["typst-test"]["tests"] = value("tests");

    assert_data_eq!(doc.to_string(), str![[r#"
tool = {}

"#]]);
}

Another approach is we switch from creating inline tables implicitly to we create Tables implicitly unless we are in a Value and then we create an InlineTable. While it doesn't solve all problems, it will make it so there is less of a chance of people using the API incorrectly.