-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fallback to requires-python
in certain cases when target-version
is not found
#16319
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
Changes from 33 commits
7bbda72
186235a
92bc35a
7c2c9b1
812fb20
759c0ba
cc7d0bb
c8c10cd
3c3c812
9fbe52a
4a2d9e1
cd37ed8
7f4403f
3613121
1b52717
e2b30e9
9039594
7e1f777
0d99dba
081518a
3c45426
2a20b69
123006f
513b1c8
5f3ec8c
2d22b76
511129e
ac55239
54acafa
a1a7548
b098255
229e905
6093d74
f5d0203
49a593e
dc26d44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,12 +5,14 @@ use log::debug; | |
use path_absolutize::path_dedot; | ||
|
||
use ruff_workspace::configuration::Configuration; | ||
use ruff_workspace::pyproject; | ||
use ruff_workspace::pyproject::{self, find_fallback_target_version}; | ||
use ruff_workspace::resolver::{ | ||
resolve_root_settings, ConfigurationTransformer, PyprojectConfig, PyprojectDiscoveryStrategy, | ||
Relativity, | ||
resolve_root_settings, ConfigurationOrigin, ConfigurationTransformer, PyprojectConfig, | ||
PyprojectDiscoveryStrategy, | ||
}; | ||
|
||
use ruff_python_ast as ast; | ||
|
||
use crate::args::ConfigArguments; | ||
|
||
/// Resolve the relevant settings strategy and defaults for the current | ||
|
@@ -35,7 +37,11 @@ pub fn resolve( | |
// `pyproject.toml` for _all_ configuration, and resolve paths relative to the | ||
// current working directory. (This matches ESLint's behavior.) | ||
if let Some(pyproject) = config_arguments.config_file() { | ||
let settings = resolve_root_settings(pyproject, Relativity::Cwd, config_arguments)?; | ||
let settings = resolve_root_settings( | ||
pyproject, | ||
config_arguments, | ||
ConfigurationOrigin::UserSpecified, | ||
)?; | ||
debug!( | ||
"Using user-specified configuration file at: {}", | ||
pyproject.display() | ||
|
@@ -61,7 +67,8 @@ pub fn resolve( | |
"Using configuration file (via parent) at: {}", | ||
pyproject.display() | ||
); | ||
let settings = resolve_root_settings(&pyproject, Relativity::Parent, config_arguments)?; | ||
let settings = | ||
resolve_root_settings(&pyproject, config_arguments, ConfigurationOrigin::Ancestor)?; | ||
return Ok(PyprojectConfig::new( | ||
PyprojectDiscoveryStrategy::Hierarchical, | ||
settings, | ||
|
@@ -78,7 +85,11 @@ pub fn resolve( | |
"Using configuration file (via cwd) at: {}", | ||
pyproject.display() | ||
); | ||
let settings = resolve_root_settings(&pyproject, Relativity::Cwd, config_arguments)?; | ||
let settings = resolve_root_settings( | ||
&pyproject, | ||
config_arguments, | ||
ConfigurationOrigin::UserSettings, | ||
)?; | ||
return Ok(PyprojectConfig::new( | ||
PyprojectDiscoveryStrategy::Hierarchical, | ||
settings, | ||
|
@@ -91,7 +102,24 @@ pub fn resolve( | |
// "closest" `pyproject.toml` file for every Python file later on, so these act | ||
// as the "default" settings.) | ||
debug!("Using Ruff default settings"); | ||
let config = config_arguments.transform(Configuration::default()); | ||
let mut config = config_arguments.transform(Configuration::default()); | ||
if config.target_version.is_none() { | ||
MichaReiser marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// If we have arrived here we know that there was no `pyproject.toml` | ||
// containing a `[tool.ruff]` section found in an ancestral directory. | ||
// (This is an implicit requirement in the function | ||
// `pyproject::find_settings_toml`.) | ||
// However, there may be a `pyproject.toml` with a `requires-python` | ||
// specified, and that is what we look for in this step. | ||
let fallback = find_fallback_target_version( | ||
stdin_filename | ||
.as_ref() | ||
.unwrap_or(&path_dedot::CWD.as_path()), | ||
); | ||
if let Some(version) = fallback { | ||
debug!("Derived `target-version` from found `requires-python`: {version:?}"); | ||
} | ||
config.target_version = fallback.map(ast::PythonVersion::from); | ||
} | ||
let settings = config.into_settings(&path_dedot::CWD)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we extract the path into a variable and use it both in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure, it depends on what behavior we want. The way I have it currently set up I'm trying to make the behavior change as little as possible, so I've kept the settings resolution path the same no matter what, but attempted to find a fallback version using the same logic as we did when searching for a user configuration file. That logic would start from the stdin filename directory if it was passed in. In other words, I don't think I can extract a |
||
Ok(PyprojectConfig::new( | ||
PyprojectDiscoveryStrategy::Hierarchical, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems inconsistent that we don't apply the fallback behavior if a user only has the user-level configuration. It means that we won't pick up the right python version if you have:
pyproject.toml
without atool.ruff
section at the project levelI think it would be good to apply the
requires-python
fallback in this case as well.