Skip to content

Default pyright settings so LSP doesn't give errors by default#15521

Open
mccakit wants to merge 1 commit intomesonbuild:masterfrom
mccakit:lsp-update
Open

Default pyright settings so LSP doesn't give errors by default#15521
mccakit wants to merge 1 commit intomesonbuild:masterfrom
mccakit:lsp-update

Conversation

@mccakit
Copy link

@mccakit mccakit commented Feb 4, 2026

This commit disables some pyright features so devs can edit meson source without tweaking the lsp. Currently pyright gives 270 errors on zed editor without any changes to lsp settings.

@mccakit mccakit requested a review from jpakkane as a code owner February 4, 2026 11:33
@eli-schwartz
Copy link
Member

I don't know what any of that means -- how do these settings correspond to our current mypy settings?

Copy link
Member

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

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

Also, I unconditionally oppose adding this if pyright doesn't support starting the config filename with a dot. See how we have a .mypy.ini rather than a mypy.ini

This is important because optional linter settings shouldn't clutter up the main view of the repository by default, when I navigate to the repository in my local development environment, and dotfiles are automatically excluded from said clutter. Most linters do support this, we have a bunch of dotfiles already.

@mccakit
Copy link
Author

mccakit commented Feb 4, 2026

Hmm, doc mentions adding configuration settings to pyproject.toml.
Do you believe that is a better alternative?
Since doc doesn't mention anything like .pyrightconfig.json

https://github.com/microsoft/pyright/blob/main/docs/configuration.md#sample-config-file

@eli-schwartz
Copy link
Member

Hmm, doc mentions adding configuration settings to pyproject.toml.
Do you believe that is a better alternative?

Not really. :( I am extremely opposed to the idea that that file should be used for anything other than configuring a PEP 517 build backend. What ends up happening is that people use that file to store information about 15 different programs and then it should really have been named everything.toml rather than pyproject.toml.

As a practical issue, the problem with an everything.toml is that it is impossible to find anything inside it.

Many of the dotfiles we have, could in theory be "migrated" into pyproject.toml, because the upstream tools in question added support for it, I guess on the theory that lots of people these days prefer using an everything.toml. Meson hasn't switched, though. :)

...

I wonder if this would be a good feature request for pyright?

@mccakit
Copy link
Author

mccakit commented Feb 4, 2026

Well, the problem still needs to be solved at the end of the day. Pyright is the default lsp for many editors, and meson having 270 errors without any modifications is a problem.

I recommend just merging this PR, since it is just a config file, overriding is easy and everything can be easily reverted.

@dcbaker
Copy link
Member

dcbaker commented Feb 4, 2026

I'm okay with making pyright and mypy's behavior match, but we shouldn't turn off warnings that mypy has on. My reasoning is that we have some code that is not yet type safe, but the majority of the code is type safe (with the caveat that we are lacking in nullable type safety).

@mccakit
Copy link
Author

mccakit commented Feb 4, 2026

My changes only disables errors, it doesn't touch warnings.
I also don't think mypy get's affected from pyright settings but I haven't tested that

@eli-schwartz
Copy link
Member

Well, the problem still needs to be solved at the end of the day. Pyright is the default lsp for many editors, and meson having 270 errors without any modifications is a problem.

This is the same argument that many people use to demand that projects add a vscode directory to ~every repo on GitHub.

I don't like this argument at all. Everyone's idea of a "default XXX for most people" is different, making the whole notion of "the default XXX" ring awfully hollow.

If your editor is unable to gracefully detect that the repository doesn't include any configuration for pyright, and react accordingly by refraining from emitting "errors" by default when those errors are clearly subjective enough to be worth a configuration option to disable them, then it is a badly designed editor and you should submit a bug report to the editor.

An example title for such a bug report is "$EDITOR provides a bad / scary experience by default when opening a GitHub repository whose maintainers don't happen to coincidentally also use $EDITOR".

It doesn't feel right to me to penalize every single open source python project in the world because one editor has bad defaults and therefore loads of projects have to override the defaults on the off chance that contributors will be using that editor.

I recommend just merging this PR, since it is just a config file, overriding is easy and everything can be easily reverted.

None of this answers the issue of "the mere presence of a filename entry in the repository root decreases the experience of other contributors to the repository".

@mccakit
Copy link
Author

mccakit commented Feb 5, 2026

It is the LSP, and other text editor would have the same errors. It is not an editor thing, but a LSP thing. Pyright just works with defaults without the config.

I already made my arguments, you can close the PR if you wish.

@bonzini
Copy link
Collaborator

bonzini commented Feb 5, 2026

To understand better the scope of the changes: the problem with meson is that not all files are typed; but for those that are, you don't want to remove errors/warnings (other than Optional). Am I right that this patch configures out some checks unconditionally, or did I misunderstand it completely?

If that is correct, is there a way to configure the LSP to skip some files? Maybe we could reuse that configuration from run_mypy.py too (and even, switch from opt-in to opt-out).

@mccakit
Copy link
Author

mccakit commented Feb 5, 2026

Yes it does unconditionally, there is probably some way to whiteflag/black flag files, but I don't think it would be worth it.

@bonzini
Copy link
Collaborator

bonzini commented Feb 6, 2026

Then this patch is incorrect because the warnings are actually useful for the vast majority of files. Please only limit to warnings that trigger in typed files, for example mesonbuild/compilers/detect.py, mesonbuild/coredata.py or mesonbuild/options.py.

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.

4 participants