Skip to content

Support multiple glob patterns in sub-config matches #3516

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

Open
kavix wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support multiple glob patterns in sub-config matches #3516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
99 changes: 83 additions & 16 deletions crates/pyrefly_config/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ use pyrefly_util::telemetry::TelemetrySourceDbRebuildInstanceStats;
use pyrefly_util::telemetry::TelemetrySourceDbRebuildStats;
use pyrefly_util::watch_pattern::WatchPattern;
use serde::Deserialize;
use serde::de::SeqAccess;
use serde::de::Visitor;
use serde::Serialize;
use starlark_map::small_map::SmallMap;
use starlark_map::small_set::SmallSet;
Expand Down Expand Up @@ -79,15 +81,67 @@ pub static GENERATED_FILE_CONFIG_OVERRIDE: LazyLock<

#[derive(Debug, PartialEq, Eq, Deserialize, Serialize, Clone)]
pub struct SubConfig {
pub matches: Glob,
#[serde(alias = "match", deserialize_with = "deserialize_sub_config_matches")]
pub matches: Vec<Glob>,
#[serde(flatten)]
pub settings: ConfigBase,
}

impl SubConfig {
fn rewrite_with_path_to_config(&mut self, config_root: &Path) {
self.matches = self.matches.clone().from_root(config_root);
self.matches = self
.matches
.clone()
.into_iter()
.map(|glob| glob.from_root(config_root))
.collect();
}

fn matches_path(&self, path: &Path) -> bool {
self.matches.iter().any(|glob| glob.matches(path))
}
}

fn deserialize_sub_config_matches<'de, D>(deserializer: D) -> Result<Vec<Glob>, D::Error>
where
D: serde::Deserializer<'de>,
{
struct GlobListVisitor;

impl<'de> Visitor<'de> for GlobListVisitor {
type Value = Vec<Glob>;

fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
formatter.write_str("a glob string or an array of glob strings")
}

fn visit_string<E>(self, value: String) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(vec![Glob::new(value).map_err(E::custom)?])
}

fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
self.visit_string(value.to_owned())
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: SeqAccess<'de>,
{
let mut matches = Vec::new();
while let Some(pattern) = seq.next_element::<Glob>()? {
matches.push(pattern);
}
Ok(matches)
}
}

deserializer.deserialize_any(GlobListVisitor)
}

/// Why a `ConfigFile` was synthesized rather than loaded from a real config
Expand Down Expand Up @@ -1007,7 +1061,7 @@ impl ConfigFile {
path: &Path,
) -> Option<T> {
self.sub_configs.iter().find_map(|c| {
if c.matches.matches(path) {
if c.matches_path(path) {
return getter(&c.settings);
}
None
Expand Down Expand Up @@ -1538,9 +1592,13 @@ impl ConfigFile {
for sub_config in &config.sub_configs {
if !sub_config.settings.extras.0.is_empty() {
let extra_keys = sub_config.settings.extras.0.keys().join(", ");
let matches = sub_config
.matches
.iter()
.map(|glob| glob.to_string())
.join(", ");
errors.push(ConfigError::warn(anyhow!(
"Extra keys found in sub config matching {}: {extra_keys}",
sub_config.matches
"Extra keys found in sub config matching {matches}: {extra_keys}"
Comment thread
kavix marked this conversation as resolved.
)));
}
}
Expand Down Expand Up @@ -1729,7 +1787,7 @@ mod tests {
},
source_db: Default::default(),
sub_configs: vec![SubConfig {
matches: Glob::new("sub/project/**".to_owned()).unwrap(),
matches: vec![Glob::new("sub/project/**".to_owned()).unwrap()],
settings: ConfigBase {
extras: Default::default(),
errors: Some(ErrorDisplayConfig::new(HashMap::from_iter([
Expand Down Expand Up @@ -1822,7 +1880,7 @@ mod tests {
python_platform = "windows"

[[sub_config]]
matches = "abcd"
matches = ["abcd", "efgh"]

atliens = 1
"#;
Expand All @@ -1835,6 +1893,7 @@ mod tests {
config.sub_configs[0].settings.extras.0,
Table::from_iter([("atliens".to_owned(), Value::Integer(1))])
);
assert_eq!(config.sub_configs[0].matches.len(), 2);
}

#[test]
Expand Down Expand Up @@ -1996,7 +2055,7 @@ mod tests {
source_db: Default::default(),
build_system: Default::default(),
sub_configs: vec![SubConfig {
matches: Glob::new("sub/project/**".to_owned()).unwrap(),
matches: vec![Glob::new("sub/project/**".to_owned()).unwrap()],
settings: Default::default(),
}],
typeshed_path: Some(PathBuf::from(typeshed)),
Expand Down Expand Up @@ -2026,13 +2085,13 @@ mod tests {
python_environment.site_package_path =
Some(vec![test_path.join("venv/lib/python1.2.3/site-packages")]);

let sub_config_matches = Glob::new(
let sub_config_matches = vec![Glob::new(
test_path
.join("sub/project/**")
.to_string_lossy()
.into_owned(),
)
.unwrap();
.unwrap()];

config.rewrite_with_path_to_config(&test_path);

Expand Down Expand Up @@ -2174,7 +2233,10 @@ output-format = "omit-errors"
},
sub_configs: vec![
SubConfig {
matches: Glob::new("**/highest/**".to_owned()).unwrap(),
matches: vec![
Glob::new("**/highest/**".to_owned()).unwrap(),
Glob::new("**/top/**".to_owned()).unwrap(),
],
settings: ConfigBase {
replace_imports_with_any: Some(vec![
ModuleWildcard::new("highest").unwrap(),
Expand All @@ -2184,7 +2246,7 @@ output-format = "omit-errors"
},
},
SubConfig {
matches: Glob::new("**/priority*".to_owned()).unwrap(),
matches: vec![Glob::new("**/priority*".to_owned()).unwrap()],
settings: ConfigBase {
replace_imports_with_any: Some(vec![
ModuleWildcard::new("second").unwrap(),
Expand All @@ -2203,6 +2265,11 @@ output-format = "omit-errors"
ModuleName::from_str("highest")
));

assert!(config.replace_imports_with_any(
Some(Path::new("this/is/top/priority")),
ModuleName::from_str("highest")
));

// test find fallback match
assert!(config.replace_imports_with_any(
Some(Path::new("this/is/second/priority")),
Expand Down Expand Up @@ -2233,7 +2300,7 @@ output-format = "omit-errors"
..Default::default()
},
sub_configs: vec![SubConfig {
matches: Glob::new("sub/**".to_owned()).unwrap(),
matches: vec![Glob::new("sub/**".to_owned()).unwrap()],
settings: ConfigBase {
errors: Some(ErrorDisplayConfig::new(HashMap::from([(
ErrorKind::BadReturn,
Expand Down Expand Up @@ -2282,7 +2349,7 @@ output-format = "omit-errors"
..Default::default()
},
sub_configs: vec![SubConfig {
matches: Glob::new("strict/**".to_owned()).unwrap(),
matches: vec![Glob::new("strict/**".to_owned()).unwrap()],
settings: ConfigBase {
errors: Some(ErrorDisplayConfig::new(HashMap::from([(
ErrorKind::BadAssignment,
Expand Down Expand Up @@ -2314,7 +2381,7 @@ output-format = "omit-errors"
..Default::default()
},
sub_configs: vec![SubConfig {
matches: Glob::new("sub/**".to_owned()).unwrap(),
matches: vec![Glob::new("sub/**".to_owned()).unwrap()],
settings: ConfigBase {
check_unannotated_defs: Some(false),
..Default::default()
Expand Down Expand Up @@ -2616,7 +2683,7 @@ output-format = "omit-errors"
let mut config = ConfigFile {
preset: Some(Preset::Legacy),
sub_configs: vec![SubConfig {
matches: Glob::new("tests/**".to_owned()).unwrap(),
matches: vec![Glob::new("tests/**".to_owned()).unwrap()],
settings: ConfigBase {
errors: Some(ErrorDisplayConfig::new(HashMap::from([(
ErrorKind::BadOverride,
Expand Down
2 changes: 1 addition & 1 deletion crates/pyrefly_config/src/migration/pyright.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl ExecEnv {
..Default::default()
};
Ok(SubConfig {
matches: Glob::new(self.root)?,
matches: vec![Glob::new(self.root)?],
Copy link
Copy Markdown
Contributor

@kinto0 kinto0 May 21, 2026

Choose a reason for hiding this comment

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

did pyright allow both single glob and vec of glob? can we have tests for what happens in both cases?

settings,
})
}
Expand Down
46 changes: 19 additions & 27 deletions crates/pyrefly_config/src/migration/sub_configs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,21 @@ impl ConfigOptionMigrater for SubConfigs {

let sub_configs_vec = sub_configs
.into_iter()
.flat_map(|(section, errors)| {
.map(|(section, errors)| -> anyhow::Result<SubConfig> {
// Split the section headers into individual modules and pair them with the section's error config.
// mypy uses module wildcards for its per-module sections, but we use globs.
// A simple translation: turn `.` into `/` and `*` into `**`, e.g. `a.*.b` -> `a/**/b`.
section
let matches = section
.split(",")
.map(|x| x.trim())
.filter(|x| !x.is_empty())
.map(|module| Glob::new(module.replace('.', "/").replace('*', "**")))
.collect::<Vec<_>>()
.into_iter()
.zip(std::iter::repeat(Some(errors)))
})
.map(|(matches, errors)| -> anyhow::Result<SubConfig> {
.collect::<Result<Vec<_>, _>>()?;

Ok(SubConfig {
matches: matches?,
matches,
settings: ConfigBase {
errors,
errors: Some(errors),
..Default::default()
},
})
Expand Down Expand Up @@ -140,7 +137,7 @@ mod tests {
assert_eq!(pyrefly_cfg.sub_configs.len(), 1);

let sub_config = &pyrefly_cfg.sub_configs[0];
assert_eq!(sub_config.matches.to_string(), "app/models");
assert_eq!(sub_config.matches[0].to_string(), "app/models");

let errors = sub_config.settings.errors.as_ref().unwrap();
assert_eq!(
Expand Down Expand Up @@ -174,7 +171,7 @@ mod tests {
let models_config = pyrefly_cfg
.sub_configs
.iter()
.find(|c| c.matches.to_string() == "app/models")
.find(|c| c.matches.iter().any(|glob| glob.to_string() == "app/models"))
.unwrap();
let models_errors = models_config.settings.errors.as_ref().unwrap();
assert_eq!(
Expand All @@ -186,7 +183,7 @@ mod tests {
let views_config = pyrefly_cfg
.sub_configs
.iter()
.find(|c| c.matches.to_string() == "app/views")
.find(|c| c.matches.iter().any(|glob| glob.to_string() == "app/views"))
.unwrap();
let views_errors = views_config.settings.errors.as_ref().unwrap();
assert_eq!(
Expand Down Expand Up @@ -223,31 +220,26 @@ mod tests {
let sub_configs = SubConfigs;
let _ = sub_configs.migrate_from_mypy(&mypy_cfg, &mut pyrefly_cfg);

assert_eq!(pyrefly_cfg.sub_configs.len(), 2);
assert_eq!(pyrefly_cfg.sub_configs.len(), 1);

// Check that both modules have the same error config
// Check that both modules share the same error config in one sub-config.
let models_config = pyrefly_cfg
.sub_configs
.iter()
.find(|c| c.matches.to_string() == "app/models")
.find(|c| c.matches.iter().any(|glob| glob.to_string() == "app/models"))
.unwrap();
let views_config = pyrefly_cfg
.sub_configs

assert!(models_config
.matches
.iter()
.find(|c| c.matches.to_string() == "app/views")
.unwrap();
.any(|glob| glob.to_string() == "app/views"));

let models_errors = models_config.settings.errors.as_ref().unwrap();
let views_errors = views_config.settings.errors.as_ref().unwrap();

assert_eq!(
models_errors.severity(ErrorKind::MissingAttribute),
Severity::Ignore
);
assert_eq!(
views_errors.severity(ErrorKind::MissingAttribute),
Severity::Ignore
);
}

#[test]
Expand All @@ -268,7 +260,7 @@ mod tests {

let sub_config = &pyrefly_cfg.sub_configs[0];
// Check that the module wildcard was converted to a glob
assert_eq!(sub_config.matches.to_string(), "app/**/models");
assert_eq!(sub_config.matches[0].to_string(), "app/**/models");

let errors = sub_config.settings.errors.as_ref().unwrap();
assert_eq!(
Expand Down Expand Up @@ -325,14 +317,14 @@ mod tests {
let src_config = pyrefly_cfg
.sub_configs
.iter()
.find(|c| c.matches.to_string() == "src")
.find(|c| c.matches.iter().any(|glob| glob.to_string() == "src"))
.unwrap();

// Find the sub_config for tests
let tests_config = pyrefly_cfg
.sub_configs
.iter()
.find(|c| c.matches.to_string() == "tests")
.find(|c| c.matches.iter().any(|glob| glob.to_string() == "tests"))
.unwrap();

// Verify that the error settings were properly migrated
Expand Down
Loading
Loading