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

Extract ResolverSettings #7542

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Extract ResolverSettings #7542

merged 1 commit into from
Sep 20, 2023

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 20, 2023

Stack Summary

This stack splits Settings into FormatterSettings and LinterSettings and moves it into ruff_workspace. This change is necessary to add the FormatterSettings to Settings without adding ruff_python_formatter as a dependency to ruff_linter (and the linter should not contain the formatter settings).

A quick overview of our settings struct at play:

  • Options: 1:1 representation of the options in the pyproject.toml or ruff.toml. Used for deserialization.
  • Configuration: Resolved Options, potentially merged from multiple configurations (when using extend). The representation is very close if not identical to the Options.
  • Settings: The resolved configuration that uses a data format optimized for reading. Optional fields are initialized with their default values. Initialized by Configuration::into_settings .

The goal of this stack is to split Settings into tool-specific resolved Settings that are independent of each other. This comes at the advantage that the individual crates don't need to know anything about the other tools. The downside is that information gets duplicated between Settings. Right now the duplication is minimal (line-length, tab-width) but we may need to come up with a solution if more expensive data needs sharing.

This stack focuses on Settings. Splitting Configuration into some smaller structs is something I'll follow up on later.

PR Summary

This PR extracts a ResolverSettings struct that holds all the resolver-relevant fields (uninteresting for the Formatter or Linter). This will allow us to move the ResolverSettings out of ruff_linter further up in the stack.

Test Plan

cargo test

(I'll to more extensive testing at the top of this stack)

@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Sep 20, 2023
@MichaReiser MichaReiser marked this pull request as ready for review September 20, 2023 12:56
@MichaReiser MichaReiser force-pushed the merge-settings-and-all-settings branch from 22d29dd to 530dba0 Compare September 20, 2023 12:58
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

This is great.

crates/ruff_linter/src/settings/mod.rs Outdated Show resolved Hide resolved
@MichaReiser MichaReiser force-pushed the merge-settings-and-all-settings branch from 530dba0 to 49afb83 Compare September 20, 2023 13:46
Base automatically changed from merge-settings-and-all-settings to main September 20, 2023 13:56
@MichaReiser MichaReiser changed the title Extract resolver settings Extract ResolverSettings Sep 20, 2023
@MichaReiser MichaReiser merged commit 8f41eab into main Sep 20, 2023
16 checks passed
@MichaReiser MichaReiser deleted the resolver-settings branch September 20, 2023 14:37
@MichaReiser
Copy link
Member Author

Merge Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants