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.

Sign up for GitHub

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

Add hidden --preview / --no-preview options to ruff check #7009

Merged
merged 7 commits into from
Aug 31, 2023
Merged

Add hidden --preview / --no-preview options to ruff check #7009

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0e44707
Add `--preview` / `--no-preview` to `ruff check`
zanieb Aug 30, 2023
a16e3f5
Hide `--preview` for now
zanieb Aug 30, 2023
fecfcac
Update schema
zanieb Aug 30, 2023
475769d
Rename to `preview`; extend CLI help string
zanieb Aug 30, 2023
29be01a
Add `PreviewMode` enum
zanieb Aug 30, 2023
432f09b
Lint
zanieb Aug 30, 2023
05810f2
Touchup to `PreviewMode`
zanieb Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/ruff/src/settings/defaults.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use regex::Regex;
use rustc_hash::FxHashSet;
use std::collections::HashSet;

use super::types::{FilePattern, PythonVersion};
use super::types::{FilePattern, PreviewMode, PythonVersion};
use super::Settings;
use crate::codes::{self, RuleCodePrefix};
use crate::line_width::{LineLength, TabSize};
Expand Down Expand Up @@ -84,6 +84,7 @@ impl Default for Settings {
line_length: LineLength::default(),
logger_objects: vec![],
namespace_packages: vec![],
preview: PreviewMode::default(),
per_file_ignores: vec![],
project_root: path_dedot::CWD.clone(),
respect_gitignore: true,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/settings/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::settings::types::{FilePatternSet, PerFileIgnore, PythonVersion, Seria
use super::line_width::{LineLength, TabSize};

use self::rule_table::RuleTable;
use self::types::PreviewMode;

pub mod defaults;
pub mod flags;
Expand Down Expand Up @@ -55,6 +56,7 @@ pub struct Settings {
pub per_file_ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>,

pub target_version: PythonVersion,
pub preview: PreviewMode,

// Resolver settings
pub exclude: FilePatternSet,
Expand Down
16 changes: 16 additions & 0 deletions crates/ruff/src/settings/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,22 @@ impl PythonVersion {
}
}

#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Default, CacheKey, is_macro::Is)]
zanieb marked this conversation as resolved.
Show resolved Hide resolved
pub enum PreviewMode {
Enabled,
#[default]
Disabled,
zanieb marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Member

Choose a reason for hiding this comment

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

This type being inside of the ruff crate means that we can't use it in the formatter, or any other crate (e.g. consider we need to gate some semantic model change)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm it seems wrong to put it elsewhere right now since there's a dedicated settings/types.rs module. This is a good callout though, we should probably have a ruff_settings crate so we can share with the formatter?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can just move the PreviewMode because the type itself isn't specific to settings.

A ruff_settings crate isn't straightforward because the settings depend on the rules. So you would need to make the formatter depend on ruff.

I consider (at least most of what is in settings) settings as the linter-specific settings. The formatter has its own resolved settings that only contain the formatter settings. Most of this already exists with PyFormatOptions, although we probably want to add a few more settings.

What I have in mind is:

  • each tool has its own resolved settings object
  • Configuration is the unified configuration from which the workspace derives the tool Settings

Copy link
Member Author

Choose a reason for hiding this comment

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

Where would you expect it to go?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know 😅

It would need to be some rather low-level crate that other crates can depend on without pulling in too many dependencies.

The alternative is to duplicate the enum in project where it is needed. I don't really mind that (e.g. the Linter has LineLength and the formatter has a very similar LineWidth option type)

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright let's consider this TBD when preview support is added to the formatter

Copy link
Member

@MichaReiser MichaReiser Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Another way we could take is that PreviewMode remains in the configuration only and setting it to true or false changes individual settings in the tool-specific settings. Each tool can then decide whether it uses feature-specific flags or a single preview flag. For example, the formatter can either:

  • Have a single preview setting that is set to enabled/disabled based on the preview mode
  • Or an experimental-string-handling and potentially other settings that configure a specific preview feature. Setting preview=true would enable/disable all of them at once.

The benefit of feature specific flags is that individual features can be moved out of preview mode (and makes it easier to find all places where the check now needs to be removed)


impl From<bool> for PreviewMode {
fn from(version: bool) -> Self {
match version {
true => PreviewMode::Enabled,
false => PreviewMode::Disabled,
}
}
}

#[derive(Debug, Clone, CacheKey, PartialEq, PartialOrd, Eq, Ord)]
pub enum FilePattern {
Builtin(&'static str),
Expand Down
12 changes: 11 additions & 1 deletion crates/ruff_cli/src/args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_hash::FxHashMap;
use ruff::logging::LogLevel;
use ruff::registry::Rule;
use ruff::settings::types::{
FilePattern, PatternPrefixPair, PerFileIgnore, PythonVersion, SerializationFormat,
FilePattern, PatternPrefixPair, PerFileIgnore, PreviewMode, PythonVersion, SerializationFormat,
};
use ruff::RuleSelector;
use ruff_workspace::configuration::{Configuration, RuleSelection};
Expand Down Expand Up @@ -115,6 +115,11 @@ pub struct CheckCommand {
/// The minimum Python version that should be supported.
#[arg(long, value_enum)]
pub target_version: Option<PythonVersion>,
/// Enable preview mode; checks will include unstable rules and fixes.
#[arg(long, overrides_with("no_preview"), hide = true)]
preview: bool,
#[clap(long, overrides_with("preview"), hide = true)]
no_preview: bool,
/// Path to the `pyproject.toml` or `ruff.toml` file to use for
/// configuration.
#[arg(long, conflicts_with = "isolated")]
Expand Down Expand Up @@ -458,6 +463,7 @@ impl CheckCommand {
ignore: self.ignore,
line_length: self.line_length,
per_file_ignores: self.per_file_ignores,
preview: resolve_bool_arg(self.preview, self.no_preview).map(PreviewMode::from),
respect_gitignore: resolve_bool_arg(
self.respect_gitignore,
self.no_respect_gitignore,
Expand Down Expand Up @@ -569,6 +575,7 @@ pub struct Overrides {
pub ignore: Option<Vec<RuleSelector>>,
pub line_length: Option<LineLength>,
pub per_file_ignores: Option<Vec<PatternPrefixPair>>,
pub preview: Option<PreviewMode>,
pub respect_gitignore: Option<bool>,
pub select: Option<Vec<RuleSelector>>,
pub show_source: Option<bool>,
Expand Down Expand Up @@ -632,6 +639,9 @@ impl ConfigProcessor for Overrides {
if let Some(line_length) = &self.line_length {
config.line_length = Some(*line_length);
}
if let Some(preview) = &self.preview {
config.preview = Some(*preview);
}
if let Some(per_file_ignores) = &self.per_file_ignores {
config.per_file_ignores = Some(collect_per_file_ignores(per_file_ignores.clone()));
}
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_wasm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use ruff::directives;
use ruff::line_width::{LineLength, TabSize};
use ruff::linter::{check_path, LinterResult};
use ruff::registry::AsRule;
use ruff::settings::types::PythonVersion;
use ruff::settings::types::{PythonVersion};
use ruff::settings::{defaults, flags, Settings};
use ruff_formatter::{FormatResult, Formatted};
use ruff_python_ast::{Mod, PySourceType};
Expand Down Expand Up @@ -128,6 +128,7 @@ impl Workspace {
external: Some(Vec::default()),
ignore: Some(Vec::default()),
line_length: Some(LineLength::default()),
preview: Some(false),
select: Some(defaults::PREFIXES.to_vec()),
tab_size: Some(TabSize::default()),
target_version: Some(PythonVersion::default()),
Expand Down
7 changes: 6 additions & 1 deletion crates/ruff_workspace/src/configuration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ use ruff::registry::{Rule, RuleSet, INCOMPATIBLE_CODES};
use ruff::rule_selector::Specificity;
use ruff::settings::rule_table::RuleTable;
use ruff::settings::types::{
FilePattern, FilePatternSet, PerFileIgnore, PythonVersion, SerializationFormat, Version,
FilePattern, FilePatternSet, PerFileIgnore, PreviewMode, PythonVersion, SerializationFormat,
Version,
};
use ruff::settings::{defaults, resolve_per_file_ignores, AllSettings, CliSettings, Settings};
use ruff::{fs, warn_user_once_by_id, RuleSelector, RUFF_PKG_VERSION};
Expand Down Expand Up @@ -67,6 +68,7 @@ pub struct Configuration {
pub line_length: Option<LineLength>,
pub logger_objects: Option<Vec<String>>,
pub namespace_packages: Option<Vec<PathBuf>>,
pub preview: Option<PreviewMode>,
pub required_version: Option<Version>,
pub respect_gitignore: Option<bool>,
pub show_fixes: Option<bool>,
Expand Down Expand Up @@ -174,6 +176,7 @@ impl Configuration {
.collect()
}),
logger_objects: self.logger_objects.unwrap_or_default(),
preview: self.preview.unwrap_or_default(),
typing_modules: self.typing_modules.unwrap_or_default(),
// Plugins
flake8_annotations: self
Expand Down Expand Up @@ -387,6 +390,7 @@ impl Configuration {
.namespace_packages
.map(|namespace_package| resolve_src(&namespace_package, project_root))
.transpose()?,
preview: options.preview.map(PreviewMode::from),
per_file_ignores: options.per_file_ignores.map(|per_file_ignores| {
per_file_ignores
.into_iter()
Expand Down Expand Up @@ -676,6 +680,7 @@ impl Configuration {
show_fixes: self.show_fixes.or(config.show_fixes),
src: self.src.or(config.src),
target_version: self.target_version.or(config.target_version),
preview: self.preview.or(config.preview),
task_tags: self.task_tags.or(config.task_tags),
typing_modules: self.typing_modules.or(config.typing_modules),
// Plugins
Expand Down
11 changes: 11 additions & 0 deletions crates/ruff_workspace/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,17 @@ pub struct Options {
/// field (e.g., `requires-python = ">=3.8"`). If Ruff is configured via
/// `ruff.toml` or `.ruff.toml`, no such inference will be performed.
pub target_version: Option<PythonVersion>,
#[option(
default = "false",
value_type = "bool",
example = r#"
# Enable preview features
preview = true
"#
)]
/// Whether to enable preview mode. When preview mode is enabled, Ruff will
/// use unstable rules and fixes.
pub preview: Option<bool>,
Copy link
Member Author

Choose a reason for hiding this comment

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

So.. I think we could make this Option<PreviewMode> too but I'm not entirely sure how to make that play nicely with the JsonSchema. I'm also not sure if we want preview = enabled | disabled or preview = true | false for users.

Copy link
Member

Choose a reason for hiding this comment

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

I quickly looked into this but don't see a good solution for it other than implementing Serialize, Deserialize and JsonSchema manually on PreviewMode if we want preview = true | false (preview = enabled | disabled) should work out of the box.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I'm happy to leave it as-is then

#[option(
default = r#"["TODO", "FIXME", "XXX"]"#,
value_type = "list[str]",
Expand Down
7 changes: 7 additions & 0 deletions ruff.schema.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.