[Possible Parser Bug] Key entries with the token ','
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 597
- Forks
- 223
- Avg merge
- 11h 23m
- Merged PRs (30d)
- 3
Description
Hello ruby maintainers of psych-yaml,
I may have found a small bug. Perhaps it is not a "real" bug, but the
error reporting by psych was strange still, so I report it here.
First, so that you can reproduce it, here is the minimum example
to demonstrate the problem:
---
bla: bar
,abc: foo
Edit: Seems github wants markdown parsing, so the line before bla: is
actually three times - which I think is the default header for yaml files.
So the problem is the second entry, called ",abc", in particular the key
',' there. If this ',' token is removed, or if we put this in '' strings
such as:
',abc': foo
Then it works.
If I load this right now, I get:
#<Psych::SyntaxError: (foo.yml): did not find expected key while parsing a block mapping at line 2 column 1>
The problem is that Psych actually reports the wrong line. It reports the
error very early on.
How did I discover this problem?
I have a huge yaml file, about 20.000 lines of code. I was making such
a change with a leading ',' token and forgot the next day where I made
it.
So when I tried to load the .yml file, psych was pointing out that there
was an error. But the message it showed me was not useful - the error
was not on that particular line.
It took me some attempts to isolate the faulty lines, by cutting down
on a copy of that big .yml file.
So what is my proposal to change this?
Well, I assume here that the specification is clear and that leading
',' tokens may not be correct. I don't know whether this is true or
not, but let's first assume this is the case, as otherwise that behaviour
has to change as well.
My gripe here is mostly with psych reporting the wrong line. It seems
to instead report the very first entry found, rather than the line itself.
You can test this yourself. Simply add more correct lines to that .yml
file, before the faulty line, and afterwards, yet psych will still only
always claim that the error happens near the beginning of the yaml
dataset, which I think is not ideal. The real error is only (!) on the
line with the ',' token. At the least I believe this would be more
useful to report this to the end user too.
You may of course say that the current behaviour of psych is correct,
which may be true - but the current behaviour is not really very
helpful to users who try to debug the problem at hand. At the least in
my use case, it would have been better to specifically report which
line was flawed. No clue if there is a good simple way to change this
behaviour, but perhaps in the long run, if anyone thinks about making
psych a bit more user-friendly, similar how the did-you-mean gem
helped improve (some) error messages for ruby, then perhaps something
similar could happen to psych.
I am using psych all the time now, used to use syck instead for
many, many years, but psych seems to work better now - and perhaps
being stricter than syck also was the right decision in the long
run.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
The payload names no source file or test; begin by reproducing the minimal YAML example and inspecting how Psych reports the syntax error location. Done means the invalid key is diagnosed at its actual line, with regression coverage for the example and its surrounding valid entries.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100