Use Flags::intersects rather than Flags::contains (#6007)

## Summary

This is equivalent for a single flag, but I think it's more likely to be
correct when the bitflags are modified -- the primary reason being that
we sometimes define flags as the union of other flags, e.g.:

```rust
const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_ANNOTATION.bits();
```

In this case, `flags.contains(Flag::ANNOTATION)` requires that _both_
flags in the union are set, whereas `flags.intersects(Flag::ANNOTATION)`
requires that _at least one_ flag is set.
This commit is contained in:
Charlie Marsh 2023-07-22 22:59:31 -04:00 committed by GitHub
parent 0bb175f7f6
commit 057faabcdd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 31 additions and 31 deletions

View file

@ -4513,10 +4513,10 @@ impl<'a> Checker<'a> {
extract_all_names(parent, |name| self.semantic.is_builtin(name)); extract_all_names(parent, |name| self.semantic.is_builtin(name));
let mut flags = BindingFlags::empty(); let mut flags = BindingFlags::empty();
if all_flags.contains(DunderAllFlags::INVALID_OBJECT) { if all_flags.intersects(DunderAllFlags::INVALID_OBJECT) {
flags |= BindingFlags::INVALID_ALL_OBJECT; flags |= BindingFlags::INVALID_ALL_OBJECT;
} }
if all_flags.contains(DunderAllFlags::INVALID_FORMAT) { if all_flags.intersects(DunderAllFlags::INVALID_FORMAT) {
flags |= BindingFlags::INVALID_ALL_FORMAT; flags |= BindingFlags::INVALID_ALL_FORMAT;
} }

View file

@ -71,12 +71,12 @@ pub fn extract_directives(
indexer: &Indexer, indexer: &Indexer,
) -> Directives { ) -> Directives {
Directives { Directives {
noqa_line_for: if flags.contains(Flags::NOQA) { noqa_line_for: if flags.intersects(Flags::NOQA) {
extract_noqa_line_for(lxr, locator, indexer) extract_noqa_line_for(lxr, locator, indexer)
} else { } else {
NoqaMapping::default() NoqaMapping::default()
}, },
isort: if flags.contains(Flags::ISORT) { isort: if flags.intersects(Flags::ISORT) {
extract_isort_directives(lxr, locator) extract_isort_directives(lxr, locator)
} else { } else {
IsortDirectives::default() IsortDirectives::default()

View file

@ -109,11 +109,11 @@ impl Emitter for TextEmitter {
sep = ":".cyan(), sep = ":".cyan(),
code_and_body = RuleCodeAndBody { code_and_body = RuleCodeAndBody {
message, message,
show_fix_status: self.flags.contains(EmitterFlags::SHOW_FIX_STATUS) show_fix_status: self.flags.intersects(EmitterFlags::SHOW_FIX_STATUS)
} }
)?; )?;
if self.flags.contains(EmitterFlags::SHOW_SOURCE) { if self.flags.intersects(EmitterFlags::SHOW_SOURCE) {
writeln!( writeln!(
writer, writer,
"{}", "{}",
@ -124,7 +124,7 @@ impl Emitter for TextEmitter {
)?; )?;
} }
if self.flags.contains(EmitterFlags::SHOW_FIX_DIFF) { if self.flags.intersects(EmitterFlags::SHOW_FIX_DIFF) {
if let Some(diff) = Diff::from_message(message) { if let Some(diff) = Diff::from_message(message) {
writeln!(writer, "{diff}")?; writeln!(writer, "{diff}")?;
} }

View file

@ -56,22 +56,22 @@ impl AlwaysAutofixableViolation for PrintfStringFormatting {
fn simplify_conversion_flag(flags: CConversionFlags) -> String { fn simplify_conversion_flag(flags: CConversionFlags) -> String {
let mut flag_string = String::new(); let mut flag_string = String::new();
if flags.contains(CConversionFlags::LEFT_ADJUST) { if flags.intersects(CConversionFlags::LEFT_ADJUST) {
flag_string.push('<'); flag_string.push('<');
} }
if flags.contains(CConversionFlags::SIGN_CHAR) { if flags.intersects(CConversionFlags::SIGN_CHAR) {
flag_string.push('+'); flag_string.push('+');
} }
if flags.contains(CConversionFlags::ALTERNATE_FORM) { if flags.intersects(CConversionFlags::ALTERNATE_FORM) {
flag_string.push('#'); flag_string.push('#');
} }
if flags.contains(CConversionFlags::BLANK_SIGN) { if flags.intersects(CConversionFlags::BLANK_SIGN) {
if !flags.contains(CConversionFlags::SIGN_CHAR) { if !flags.intersects(CConversionFlags::SIGN_CHAR) {
flag_string.push(' '); flag_string.push(' ');
} }
} }
if flags.contains(CConversionFlags::ZERO_PAD) { if flags.intersects(CConversionFlags::ZERO_PAD) {
if !flags.contains(CConversionFlags::LEFT_ADJUST) { if !flags.intersects(CConversionFlags::LEFT_ADJUST) {
flag_string.push('0'); flag_string.push('0');
} }
} }

View file

@ -99,7 +99,7 @@ impl Printer {
fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> { fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> {
if self.log_level >= LogLevel::Default { if self.log_level >= LogLevel::Default {
if self.flags.contains(Flags::SHOW_VIOLATIONS) { if self.flags.intersects(Flags::SHOW_VIOLATIONS) {
let fixed = diagnostics let fixed = diagnostics
.fixed .fixed
.values() .values()
@ -160,12 +160,12 @@ impl Printer {
return Ok(()); return Ok(());
} }
if !self.flags.contains(Flags::SHOW_VIOLATIONS) { if !self.flags.intersects(Flags::SHOW_VIOLATIONS) {
if matches!( if matches!(
self.format, self.format,
SerializationFormat::Text | SerializationFormat::Grouped SerializationFormat::Text | SerializationFormat::Grouped
) { ) {
if self.flags.contains(Flags::SHOW_FIX_SUMMARY) { if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
if !diagnostics.fixed.is_empty() { if !diagnostics.fixed.is_empty() {
writeln!(writer)?; writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed)?; print_fix_summary(writer, &diagnostics.fixed)?;
@ -192,11 +192,11 @@ impl Printer {
SerializationFormat::Text => { SerializationFormat::Text => {
TextEmitter::default() TextEmitter::default()
.with_show_fix_status(show_fix_status(self.autofix_level)) .with_show_fix_status(show_fix_status(self.autofix_level))
.with_show_fix_diff(self.flags.contains(Flags::SHOW_FIX_DIFF)) .with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF))
.with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.emit(writer, &diagnostics.messages, &context)?; .emit(writer, &diagnostics.messages, &context)?;
if self.flags.contains(Flags::SHOW_FIX_SUMMARY) { if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
if !diagnostics.fixed.is_empty() { if !diagnostics.fixed.is_empty() {
writeln!(writer)?; writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed)?; print_fix_summary(writer, &diagnostics.fixed)?;
@ -208,11 +208,11 @@ impl Printer {
} }
SerializationFormat::Grouped => { SerializationFormat::Grouped => {
GroupedEmitter::default() GroupedEmitter::default()
.with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.with_show_fix_status(show_fix_status(self.autofix_level)) .with_show_fix_status(show_fix_status(self.autofix_level))
.emit(writer, &diagnostics.messages, &context)?; .emit(writer, &diagnostics.messages, &context)?;
if self.flags.contains(Flags::SHOW_FIX_SUMMARY) { if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
if !diagnostics.fixed.is_empty() { if !diagnostics.fixed.is_empty() {
writeln!(writer)?; writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed)?; print_fix_summary(writer, &diagnostics.fixed)?;
@ -367,7 +367,7 @@ impl Printer {
let context = EmitterContext::new(&diagnostics.source_kind); let context = EmitterContext::new(&diagnostics.source_kind);
TextEmitter::default() TextEmitter::default()
.with_show_fix_status(show_fix_status(self.autofix_level)) .with_show_fix_status(show_fix_status(self.autofix_level))
.with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.emit(writer, &diagnostics.messages, &context)?; .emit(writer, &diagnostics.messages, &context)?;
} }
writer.flush()?; writer.flush()?;

View file

@ -45,44 +45,44 @@ impl<'a> Binding<'a> {
/// Return `true` if this [`Binding`] represents an explicit re-export /// Return `true` if this [`Binding`] represents an explicit re-export
/// (e.g., `FastAPI` in `from fastapi import FastAPI as FastAPI`). /// (e.g., `FastAPI` in `from fastapi import FastAPI as FastAPI`).
pub const fn is_explicit_export(&self) -> bool { pub const fn is_explicit_export(&self) -> bool {
self.flags.contains(BindingFlags::EXPLICIT_EXPORT) self.flags.intersects(BindingFlags::EXPLICIT_EXPORT)
} }
/// Return `true` if this [`Binding`] represents an external symbol /// Return `true` if this [`Binding`] represents an external symbol
/// (e.g., `FastAPI` in `from fastapi import FastAPI`). /// (e.g., `FastAPI` in `from fastapi import FastAPI`).
pub const fn is_external(&self) -> bool { pub const fn is_external(&self) -> bool {
self.flags.contains(BindingFlags::EXTERNAL) self.flags.intersects(BindingFlags::EXTERNAL)
} }
/// Return `true` if this [`Binding`] represents an aliased symbol /// Return `true` if this [`Binding`] represents an aliased symbol
/// (e.g., `app` in `from fastapi import FastAPI as app`). /// (e.g., `app` in `from fastapi import FastAPI as app`).
pub const fn is_alias(&self) -> bool { pub const fn is_alias(&self) -> bool {
self.flags.contains(BindingFlags::ALIAS) self.flags.intersects(BindingFlags::ALIAS)
} }
/// Return `true` if this [`Binding`] represents a `nonlocal`. A [`Binding`] is a `nonlocal` /// Return `true` if this [`Binding`] represents a `nonlocal`. A [`Binding`] is a `nonlocal`
/// if it's declared by a `nonlocal` statement, or shadows a [`Binding`] declared by a /// if it's declared by a `nonlocal` statement, or shadows a [`Binding`] declared by a
/// `nonlocal` statement. /// `nonlocal` statement.
pub const fn is_nonlocal(&self) -> bool { pub const fn is_nonlocal(&self) -> bool {
self.flags.contains(BindingFlags::NONLOCAL) self.flags.intersects(BindingFlags::NONLOCAL)
} }
/// Return `true` if this [`Binding`] represents a `global`. A [`Binding`] is a `global` if it's /// Return `true` if this [`Binding`] represents a `global`. A [`Binding`] is a `global` if it's
/// declared by a `global` statement, or shadows a [`Binding`] declared by a `global` statement. /// declared by a `global` statement, or shadows a [`Binding`] declared by a `global` statement.
pub const fn is_global(&self) -> bool { pub const fn is_global(&self) -> bool {
self.flags.contains(BindingFlags::GLOBAL) self.flags.intersects(BindingFlags::GLOBAL)
} }
/// Return `true` if this [`Binding`] represents an assignment to `__all__` with an invalid /// Return `true` if this [`Binding`] represents an assignment to `__all__` with an invalid
/// value (e.g., `__all__ = "Foo"`). /// value (e.g., `__all__ = "Foo"`).
pub const fn is_invalid_all_format(&self) -> bool { pub const fn is_invalid_all_format(&self) -> bool {
self.flags.contains(BindingFlags::INVALID_ALL_FORMAT) self.flags.intersects(BindingFlags::INVALID_ALL_FORMAT)
} }
/// Return `true` if this [`Binding`] represents an assignment to `__all__` that includes an /// Return `true` if this [`Binding`] represents an assignment to `__all__` that includes an
/// invalid member (e.g., `__all__ = ["Foo", 1]`). /// invalid member (e.g., `__all__ = ["Foo", 1]`).
pub const fn is_invalid_all_object(&self) -> bool { pub const fn is_invalid_all_object(&self) -> bool {
self.flags.contains(BindingFlags::INVALID_ALL_OBJECT) self.flags.intersects(BindingFlags::INVALID_ALL_OBJECT)
} }
/// Return `true` if this [`Binding`] represents an unbound variable /// Return `true` if this [`Binding`] represents an unbound variable

View file

@ -163,7 +163,7 @@ impl<'a> Scope<'a> {
/// Returns `true` if this scope uses locals (e.g., `locals()`). /// Returns `true` if this scope uses locals (e.g., `locals()`).
pub const fn uses_locals(&self) -> bool { pub const fn uses_locals(&self) -> bool {
self.flags.contains(ScopeFlags::USES_LOCALS) self.flags.intersects(ScopeFlags::USES_LOCALS)
} }
} }