Fix TOML parse error locations#4098
Conversation
This comment has been minimized.
This comment has been minimized.
NathanTempest
left a comment
There was a problem hiding this comment.
Thanks @goutamadwant for this clean fix. Nice that it fails safe (falls back to the original span when a better one can't be derived) and that both the config-crate and playground tests pin the corrected location.
One follow-up worth considering (non-blocking): the span adjustment recovers the key/value by parsing toml_edit's error message text (in key.path, invalid type: string "…"), which is coupled to toml_edit's wording and could silently stop adjusting on a version bump. It degrades safely, so it's fine to land, but longer-term a toml::Spanned/serde_spanned-based approach (or reconstructing a toml_edit error with the adjusted span rather than the hand-rolled renderer) would be more robust.
|
Thanks @NathanTempest. I pushed a follow-up to address your comments. let me know if you have more suggestions. thanks! |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
NathanTempest
left a comment
There was a problem hiding this comment.
This is a big improvement and thanks for reworking it. Tests for both pyrefly.toml and [tool.pyrefly] plus the playground case are great. Looks import ready to me and thank you for this!
|
@NathanTempest has imported this pull request. If you are a Meta employee, you can view this in D112130825. |
grievejia
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
Summary
Fixes #4060
This fixes TOML parse diagnostics so invalid config values point at the offending value instead of the start of the file.
The config parser now keeps usable TOML span information and adjusts broad serde validation spans to the matching TOML value span when possible. The playground now uses the same config parser and reports the derived range instead of hardcoding line 1, column 1.
Test Plan
cargo test -p pyrefly_configcargo test -p pyrefly test_config_toml_parse_error_range -- --nocapturecargo fmt --all -- --check