igorshubovych / markdownlint-cli

MarkdownLint Command Line Interface
MIT License
851 stars 86 forks source link

json parser tries to parse nested toml #469

Closed dennis-wey closed 5 months ago

dennis-wey commented 5 months ago

I think this is a bug from https://github.com/igorshubovych/markdownlint-cli/issues/113

I'm having trouble getting the new config options to work with pre-commit:

pre-commit-config.yaml

repos:
  - repo: "https://github.com/igorshubovych/markdownlint-cli"
    rev: "v0.40.0"
    hooks:
      - id: "markdownlint-fix"
        args: [--config, pyproject.toml, --configPointer, "/tool/markdownlint"]
        verbose: true

pyproject.toml

[project]
name = "my-package"
description = "description"

[tool.markdownlint]
default = true
MD007.indent = 4

I get the following error:

pre-commit run -a
markdownlint-fix.........................................................Failed
- hook id: markdownlint-fix
- duration: 0.13s
- exit code: 1

Cannot read or parse config file 'pyproject.toml': Unable to parse 'pyproject.toml'; Parser 0: Unable to parse JSON(C) content, InvalidSymbol (offset 1, length 7), InvalidSymbol (offset 10, length 4), InvalidSymbol (offset 15, length 1), EndOfFileExpected (offset 17, length 12); Parser 1: Expected "=", [ \t] or [A-Za-z0-9_\-] but "." found.; Parser 2: end of the stream or a document separator is expected (2:1)

 1 | [project]
 2 | name = "my-package"
-----^
 3 | description = "description"
 4 | 

Process finished with exit code 1

Seems like it tries to apply the JSON Parser first to the toml file. After that the right configuration is found with the toml parser and the actual formatting is taking place but the prior error is causing the action to exit with status code 1.

This is especially problematic in ci workflows which rely on actions exit with status code 0 to pass

DavidAnson commented 5 months ago

When attempting to parse a configuration file, all three parsers are tried and the result of the first successful is used. In this case, all three are failing as shown in the error message above. The TOML parser is second in the array (index 1), so the following output is relevant:

Parser 1: Expected "=", [ \t] or [A-Za-z0-9_-] but "." found.

No line/column information is provided, but I think it is referring to the following key name as being invalid because it includes a dot: MD007.indent.

dennis-wey commented 5 months ago

Hi @DavidAnson thanks for your answer. I tried a bit around with the setup and made some observations:

That said, the file example stated above seems to me like a valid toml format: https://toml.io/en/v1.0.0#table While rewriting the config in the minimal example above would be no problem, the pyproject.toml for most users might be much bigger and there are use cases where avoiding this syntax is more annoying. My other hooks (mypy, ruff) also reads the pyproject.toml and don't have problems with this kind of syntax.

So it seems to me there is either a bug in the used toml parser or the parser doesn't accept the whole toml spec.

DavidAnson commented 5 months ago

This is the TOML parser that is used now: https://www.npmjs.com/package/toml

It says it supports version 0.4.0 of the specification which I confirm prohibits dots in key names: https://toml.io/en/v0.4.0#table

If you know of another well-used, dependency-free TOML parser for Node that supports a more recent version, I can have a look at switching to that.

dennis-wey commented 5 months ago

Hi @DavidAnson, I'm not a node/js developer so I also have no experience in that regard.

Searching through npm this one seems the most promising: https://www.npmjs.com/package/smol-toml

Edit: Seems like you came to the same conclusion a bit earlier 😄