Skip to content

Commit ee50c14

Browse files
rchen152meta-codesync[bot]
authored andcommitted
Make missing-source respect error configuration
Summary: Previously, the `missing-source` error ignored all error configuration (e.g., requesting `--error=missing-source` did nothing). Now this error respects configuration. Implementation-wise, this means we always log the error and choose whether to display it or not based on the configured severity level, rather than choosing whether to log it or not based on the value of `--ignore-missing-source`. For backwards compatibility, I also preserved the behavior of `--ignore-missing-source=false` (enable `missing-source` with severity Error). This does lead to a bit of weirdness when `missing-source` is configured in multiple ways. What I ended up doing is enabling the error if it is explicitly enabled in any way (even if it is also explicitly disabled in a different way!), with the severity defaulting to Error if unspecified. Maybe we should deprecated `--ignore-missing-source` at some point? (cc connernilsen) But I think the behavior in this diff is good enough for now. For #1406. Reviewed By: connernilsen Differential Revision: D85622655 fbshipit-source-id: 45a67f52306303c84b07ba888a7c185e60b18411
1 parent b190da3 commit ee50c14

File tree

7 files changed

+39
-82
lines changed

7 files changed

+39
-82
lines changed

crates/pyrefly_config/src/config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,7 @@ impl ConfigFile {
676676
self.errors(path),
677677
self.ignore_errors_in_generated_code(path),
678678
self.permissive_ignores(path),
679+
self.ignore_missing_source,
679680
)
680681
}
681682

crates/pyrefly_config/src/error.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,21 @@ pub struct ErrorConfig<'a> {
9191
pub display_config: &'a ErrorDisplayConfig,
9292
pub ignore_errors_in_generated_code: bool,
9393
pub permissive_ignores: bool,
94+
pub ignore_missing_source: bool,
9495
}
9596

9697
impl<'a> ErrorConfig<'a> {
9798
pub fn new(
9899
display_config: &'a ErrorDisplayConfig,
99100
ignore_errors_in_generated_code: bool,
100101
permissive_ignores: bool,
102+
ignore_missing_source: bool,
101103
) -> Self {
102104
Self {
103105
display_config,
104106
ignore_errors_in_generated_code,
105107
permissive_ignores,
108+
ignore_missing_source,
106109
}
107110
}
108111
}

crates/pyrefly_config/src/error_kind.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ impl ErrorKind {
284284
ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore,
285285
ErrorKind::ImplicitAbstractClass => Severity::Ignore,
286286
ErrorKind::ImplicitAny => Severity::Ignore,
287+
ErrorKind::MissingSource => Severity::Ignore,
287288
_ => Severity::Error,
288289
}
289290
}

pyrefly/lib/error/collector.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::fmt::Debug;
99
use std::mem;
1010

1111
use dupe::Dupe;
12+
use pyrefly_config::error_kind::ErrorKind;
1213
use pyrefly_util::lock::Mutex;
1314
use ruff_text_size::Ranged;
1415
use ruff_text_size::TextRange;
@@ -161,7 +162,18 @@ impl ErrorCollector {
161162
if err.is_ignored(error_config.permissive_ignores) {
162163
result.suppressed.push(err.clone());
163164
} else {
164-
match error_config.display_config.severity(err.error_kind()) {
165+
let kind = err.error_kind();
166+
let raw_severity = error_config.display_config.severity(kind);
167+
let severity = match (kind, raw_severity, error_config.ignore_missing_source) {
168+
// If missing-source is set to Ignore (the default), and
169+
// ignore-missing-source is to false (the default is true, so false must
170+
// have been explicitly set by the user), enable missing-source. Note that
171+
// this means that if `missing-source` and `--ignore-missing-source` are in
172+
// conflict, the error is enabled if either setting says it should be.
173+
(ErrorKind::MissingSource, Severity::Ignore, false) => Severity::Error,
174+
_ => raw_severity,
175+
};
176+
match severity {
165177
Severity::Error => result.shown.push(err.with_severity(Severity::Error)),
166178
Severity::Warn => result.shown.push(err.with_severity(Severity::Warn)),
167179
Severity::Info => result.shown.push(err.with_severity(Severity::Info)),
@@ -245,7 +257,8 @@ mod tests {
245257
.collect(&ErrorConfig::new(
246258
&ErrorDisplayConfig::default(),
247259
false,
248-
false
260+
false,
261+
true,
249262
))
250263
.shown
251264
.map(|x| x.msg()),
@@ -297,7 +310,7 @@ mod tests {
297310
(ErrorKind::BadAssignment, Severity::Ignore),
298311
(ErrorKind::NotIterable, Severity::Ignore),
299312
]));
300-
let config = ErrorConfig::new(&display_config, false, false);
313+
let config = ErrorConfig::new(&display_config, false, false, true);
301314

302315
assert_eq!(
303316
errors.collect(&config).shown.map(|x| x.msg()),
@@ -321,10 +334,10 @@ mod tests {
321334
);
322335

323336
let display_config = ErrorDisplayConfig::default();
324-
let config0 = ErrorConfig::new(&display_config, false, false);
337+
let config0 = ErrorConfig::new(&display_config, false, false, true);
325338
assert_eq!(errors.collect(&config0).shown.map(|x| x.msg()), vec!["a"]);
326339

327-
let config1 = ErrorConfig::new(&display_config, true, false);
340+
let config1 = ErrorConfig::new(&display_config, true, false, true);
328341
assert!(errors.collect(&config1).shown.map(|x| x.msg()).is_empty());
329342
}
330343

@@ -353,7 +366,8 @@ mod tests {
353366
.collect(&ErrorConfig::new(
354367
&ErrorDisplayConfig::default(),
355368
false,
356-
false
369+
false,
370+
true,
357371
))
358372
.shown
359373
.map(|x| x.msg()),

0 commit comments

Comments
 (0)