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

[red-knot] Resolve Options to Settings #16000

Merged
merged 3 commits into from
Feb 10, 2025
Merged

[red-knot] Resolve Options to Settings #16000

merged 3 commits into from
Feb 10, 2025

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Feb 6, 2025

Summary

This PR generalize the idea that we may want to emit diagnostics for
invalid or incompatible configuration values similar to how we already
do it for rules.

This PR introduces a new Settings struct that is similar to Options but, unlike
Options, are fields have their default values filled in and they use a representation optimized for reads.

The diagnostics created during loading the Settings are stored on the Project so that we can emit them when calling check.

The motivation for this work is that it simplifies adding new settings. That's also why I went ahead and added the terminal.error-on-warning setting to demonstrate how new settings are added.

Test Plan

Existing tests, new CLI test.

@MichaReiser MichaReiser added cli Related to the command-line interface red-knot Multi-file analysis & type inference labels Feb 6, 2025
@MichaReiser MichaReiser marked this pull request as ready for review February 6, 2025 18:22
///
/// Settings that are part of [`ProgramSettings`] are not included here.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Settings {
Copy link
Member

Choose a reason for hiding this comment

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

I believe this will help in converting the mdtest config options into the final resolved settings for the project?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not because the mdtest framework doesn't depend on red_knot_project, unless we decide to change that.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right.

Copy link
Contributor

github-actions bot commented Feb 7, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser merged commit 678b0c2 into main Feb 10, 2025
21 checks passed
@MichaReiser MichaReiser deleted the micha/settings branch February 10, 2025 14:28
dcreager added a commit that referenced this pull request Feb 10, 2025
* main: (991 commits)
  [red-knot] Resolve `Options` to `Settings` (#16000)
  Bump version to 0.9.6 (#16074)
  Revert tailwindcss v4 update (#16075)
  Improve migration document (#16072)
  Fix reference definition labels for backtick-quoted shortcut links (#16035)
  RUF009 should behave similar to B008 and ignore attributes with immutable types (#16048)
  [`pylint`] Also report when the object isn't a literal (`PLE1310`) (#15985)
  Update Rust crate rustc-hash to v2.1.1 (#16060)
  Root exclusions in the server to project root (#16043)
  Directly include `Settings` struct for the server (#16042)
  Update Rust crate clap to v4.5.28 (#16059)
  Update Rust crate strum_macros to 0.27.0 (#16065)
  Update NPM Development dependencies (#16067)
  Update Rust crate uuid to v1.13.1 (#16066)
  Update Rust crate strum to 0.27.0 (#16064)
  Update pre-commit dependencies (#16063)
  Update dependency ruff to v0.9.5 (#16062)
  Update Rust crate toml to v0.8.20 (#16061)
  [`flake8-builtins`] Make strict module name comparison optional (`A005`) (#15951)
  [`ruff`] Indented form feeds (`RUF054`) (#16049)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants