Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pyright does not correctly parse toml [[listsOfDictionaries]] #4336

Closed
smackesey opened this issue Dec 14, 2022 · 4 comments
Closed

Pyright does not correctly parse toml [[listsOfDictionaries]] #4336

smackesey opened this issue Dec 14, 2022 · 4 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@smackesey
Copy link

smackesey commented Dec 14, 2022

The following TOML snippets map to identical JSON (can confirm here). See also the TOML spec.

[[executionEnvironments]]
root = "src/sub"
executionEnvironments = [
  { root = "src/sub" },
]

However, pyright does not interpret them identically. Below is a repro zsh script that works on macOS:

#!/usr/bin/env zsh

local projroot=$(mktemp -d /tmp/temp.XXXX)
pushd $projroot >/dev/null

cat > pyproject.toml <<EOF
[tool.pyright]

include = [
  "src"
]

# (syntax 1)
executionEnvironments = [
  { root = "src/sub" },
]
EOF

# sub this in (syntax 2) for (syntax 1) above

# (syntax 2)
# [[executionEnvironments]]
# root = "src/sub"

mkdir src
mkdir src/sub
mkdir src/sub/foo

cat > src/sub/foo/__init__.py <<EOF
import fake
EOF

pyright --verbose src

popd >/dev/null
rm -rf $projroot

Run this once and you should approximately this in the output:

Found 1 source file
Could not import 'fake' in file '/private/tmp/temp.fDZA/src/sub/foo/__init__.py'
  ...
  Looking in root directory of execution environment '/private/tmp/temp.xzO8/src/sub'

Now sub in syntax (2) for syntax (1) and run again, you should see:

Found 1 source file
Could not import 'fake' in file '/private/tmp/temp.fDZA/src/sub/foo/__init__.py'
  ...
  Looking in root directory of execution environment '/private/tmp/temp.xzO8'

The execution environment root directory is not as-specified-- I think the specified [[executionEnvironments]] is being ignored completely.

Looks like the TOML parser used by pyright is old and unmaintained: https://github.com/iarna/iarna-toml. Pyright should switch to a 1.0-compliant parser.

@erictraut
Copy link
Collaborator

erictraut commented Dec 14, 2022

Yeah, I think your diagnosis is spot on. This is because the TOML parser we use is not 1.0-compliant. And you're right that it's no longer maintained. On the other hand, it's by far the most popular TOML parser library in Javascript with 2.9M downloads a week.

The next most popular TOML parser library for Javascript (with 700K+ downloads a week) is also not 1.0 compliant and not maintained. :(

There are two 1.0-complaint TOML parser Javascript libraries listed on the page you linked to above. One of them has 40K weekly downloads but is unfortunately licensed under L-GPL, so we can't use it. The other has no usage (averaging 2 downloads a week) and doesn't appear to be maintained, so I don't think it would be wise to take a dependency on it.

I'll keep looking, but I currently don't see a great solution here. You may need to just work around the limitation for now. Since you're the first person to report an issue with the TOML parser, I don't think this limitation is affecting many pyright users.

@erictraut
Copy link
Collaborator

Good news. It looks like the version of the TOML parser that pyright currently uses has been updated to support the TOML 1.0 spec. This updated version was published as a release candidate rather than a final release, so it wasn't listed as the "most recent version", which is why I didn't notice it initially. The rc appears to be stable enough that we can switch to it. This should address the issue you're seeing.

This will be included in the next release.

@erictraut erictraut added bug Something isn't working addressed in next version Issue is fixed and will appear in next published version labels Dec 14, 2022
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.285, which I just published. It will also be included in a future release of pylance.

@erictraut
Copy link
Collaborator

Note: We needed to revert this change because it introduced a bad regression. See #4367 for details.

erictraut pushed a commit that referenced this issue Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants