Skip to content

pyright: init#504

Open
otavio wants to merge 1 commit into
numtide:mainfrom
otavio:pyright-init
Open

pyright: init#504
otavio wants to merge 1 commit into
numtide:mainfrom
otavio:pyright-init

Conversation

@otavio
Copy link
Copy Markdown

@otavio otavio commented May 8, 2026

Summary

  • Add programs.pyright module for the Pyright Python static type checker.
  • Mirrors the mypy module's per-directory invocation pattern, since pyright also needs whole-module context to resolve imports rather than running per changed file.
  • Drops mypy's extraPythonPackages/extraPythonPaths because pyright does not consult PYTHONPATH — the option description steers users to extraPaths in pyrightconfig.json / [tool.pyright] instead.

Test plan

  • nix build .#checks.x86_64-linux.formatter-pyright succeeds and produces a valid treefmt config.
  • nix build .#checks.x86_64-linux.examples succeeds (pyright is skipExample, since the wrapper bakes store paths).
  • Multi-directory config evaluated end-to-end: includes resolve to <dir>/<module>/*.py and *.pyi, and the wrapper invokes pyright <options> <modules> from the configured directory.

otavio added a commit to OSSystems/ai-plugin-vendor-tool that referenced this pull request May 8, 2026
Drops the hand-rolled `settings.formatter.pyright` block in favour of the
upcoming `programs.pyright` module from numtide/treefmt-nix#504. The
`treefmt-nix` flake input is temporarily pinned to the PR branch
(`otavio/treefmt-nix/pyright-init`); switch back to `numtide/treefmt-nix`
once the PR lands upstream.

The `--pythonpath` workaround for pytest import resolution is preserved
via `directories."".options`.
Comment thread programs/pyright.nix Outdated
Comment thread programs/pyright.nix Outdated
Comment thread programs/pyright.nix
Comment thread programs/pyright.nix
@otavio
Copy link
Copy Markdown
Author

otavio commented May 8, 2026

Self-review pass on the /review output, with each item resolved. Fixes are folded into the existing pyright: init commit (amended).

1. Default modules = [ "" ] passes empty positional arg to pyrightfixed. cfg.modules is now filtered with lib.filter (m: m != "") before escapeShellArgs, and the positional-arg fragment is omitted entirely when the filtered list is empty. The default invocation is now pyright (no trailing ''), so pyright auto-discovers from cwd / pyrightconfig.json cleanly. This intentionally diverges from programs/mypy.nix, which has the same pattern and arguably the same latent issue.

2. *.py only matches top-level fileswithdrawn. I was wrong: treefmt globs are recursive by default, and every other Python formatter in the repo (black, ruff-format, ruff-check, yapf, isort) uses bare *.py/*.pyi the same way. No change needed.

3. Trailing dash in formatter name (pyright-)fixed. The default key now produces [formatter.pyright]; non-empty keys still produce pyright-<escaped-key> (e.g. pyright-src-foo). Implemented with lib.optionalString (name != "") "-${escapePath name}".

4. cd "${cfg.directory}" not shell-escapedfixed. Now uses lib.escapeShellArg cfg.directory, hardening against directories containing quotes, $, or whitespace.

5. Test plan describes config-eval as "end-to-end"clarified, no code change. The formatter-pyright check generates a TOML config and validates evaluation; this matches every other formatter in the repo and is the established convention. Adding runtime invocation tests would set a precedent for ~140 formatters and is out of scope here.

6. meta.maintainers = [ "otavio" ] — acknowledged, no action.

Bonus follow-up (not in this PR): programs/mypy.nix only emits *.py in includes, missing *.pyi. Worth a separate small PR — mypy reads stub files too.

Verification of the amended commit:

  • nix build .#checks.x86_64-linux.formatter-pyright — passes; emitted TOML shows [formatter.pyright] with includes = ["*.py", "*.pyi"] and no empty positional arg.
  • nix build .#checks.x86_64-linux.examples — passes (pyright remains skipExample).
  • nix build .#checks.x86_64-linux.self-formatting — passes after applying nixfmt's split of the long interpolation line.
  • Multi-directory smoke (directories."src/foo" = { modules = [ "mymod" ]; };) evaluates to formatter name pyright-src-foo, cd src/foo, and includes = ["src/foo/mymod/*.py", "src/foo/mymod/*.pyi"].

@otavio otavio force-pushed the pyright-init branch 2 times, most recently from 3efcf30 to ed5dc86 Compare May 8, 2026 14:59
Copy link
Copy Markdown
Author

@otavio otavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review pass mapping each original change request to its inline location in the amended diff. See per-line comments below.

Comment thread programs/pyright.nix
Comment thread programs/pyright.nix
Comment thread programs/pyright.nix
Comment thread programs/pyright.nix
Comment thread programs/pyright.nix
Comment thread programs/pyright.nix Outdated
Comment thread programs/pyright.nix
@otavio otavio requested a review from jfly May 8, 2026 16:25
@otavio
Copy link
Copy Markdown
Author

otavio commented May 8, 2026

Update on top of the earlier summary — all review threads are now resolved. Additional changes folded into the same pyright: init commit (current HEAD 101eda1):

  • directory option description — expanded to document that the empty string (and the default attrset key "") means the project root, with no cd performed. Addressing @jfly's request for clarification.
  • exec before pyright invocation — independently caught in a /simplify pass before seeing @jfly's nit. Bash now hands off rather than staying alive holding fds during type-checking.
  • let-bound cdLine / modulesArgs — pre-binding pulled out of the bash heredoc so the multi-line string reads top-to-bottom without nested ${lib.optionalString …} interpolation.
  • Wrapper-ignores-args comment — added a 6-line comment above options documenting that pyright is always invoked on the configured modules (or the whole directory), not on the matched files treefmt appends as positional args, and noting the implication of treefmt's batch-size limit (multiple invocations per directory possible on large file sets, each operating on the same modules). Addressing @jfly's request on the includes thread.
  • meta.skipExample = true rationale — explained on the original thread that the embedded lib.getExe store path inside options is what defeats the example sed-stripper; @jfly accepted, no code change needed.

CI is green (formatter-pyright, examples, self-formatting all pass). Ready for another look.

Comment thread programs/pyright.nix Outdated
Comment thread programs/pyright.nix Outdated
Comment thread programs/pyright.nix Outdated
Comment thread programs/pyright.nix Outdated
@otavio otavio force-pushed the pyright-init branch 3 times, most recently from d3613c9 to 1747944 Compare May 11, 2026 13:45
Add support for the Pyright Python static type checker, mirroring the
mypy module's per-directory invocation pattern: pyright needs whole-module
context to resolve imports, so it runs once per configured directory rather
than per changed file.

Unlike mypy, pyright does not honor PYTHONPATH for import resolution, so
the extraPythonPackages/extraPythonPaths knobs were intentionally omitted.
The option description points users at extraPaths in pyrightconfig.json or
[tool.pyright] instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants