Skip to content

Commit

Permalink
Use Flags::intersects rather than Flags::contains (#6007)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
charliermarsh authored Jul 23, 2023
1 parent 0bb175f commit 057faab
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 31 deletions.
4 changes: 2 additions & 2 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4513,10 +4513,10 @@ impl<'a> Checker<'a> {
extract_all_names(parent, |name| self.semantic.is_builtin(name));

let mut flags = BindingFlags::empty();
if all_flags.contains(DunderAllFlags::INVALID_OBJECT) {
if all_flags.intersects(DunderAllFlags::INVALID_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;
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ pub fn extract_directives(
indexer: &Indexer,
) -> 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)
} else {
NoqaMapping::default()
},
isort: if flags.contains(Flags::ISORT) {
isort: if flags.intersects(Flags::ISORT) {
extract_isort_directives(lxr, locator)
} else {
IsortDirectives::default()
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/message/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ impl Emitter for TextEmitter {
sep = ":".cyan(),
code_and_body = RuleCodeAndBody {
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!(
writer,
"{}",
Expand All @@ -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) {
writeln!(writer, "{diff}")?;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,22 +56,22 @@ impl AlwaysAutofixableViolation for PrintfStringFormatting {

fn simplify_conversion_flag(flags: CConversionFlags) -> String {
let mut flag_string = String::new();
if flags.contains(CConversionFlags::LEFT_ADJUST) {
if flags.intersects(CConversionFlags::LEFT_ADJUST) {
flag_string.push('<');
}
if flags.contains(CConversionFlags::SIGN_CHAR) {
if flags.intersects(CConversionFlags::SIGN_CHAR) {
flag_string.push('+');
}
if flags.contains(CConversionFlags::ALTERNATE_FORM) {
if flags.intersects(CConversionFlags::ALTERNATE_FORM) {
flag_string.push('#');
}
if flags.contains(CConversionFlags::BLANK_SIGN) {
if !flags.contains(CConversionFlags::SIGN_CHAR) {
if flags.intersects(CConversionFlags::BLANK_SIGN) {
if !flags.intersects(CConversionFlags::SIGN_CHAR) {
flag_string.push(' ');
}
}
if flags.contains(CConversionFlags::ZERO_PAD) {
if !flags.contains(CConversionFlags::LEFT_ADJUST) {
if flags.intersects(CConversionFlags::ZERO_PAD) {
if !flags.intersects(CConversionFlags::LEFT_ADJUST) {
flag_string.push('0');
}
}
Expand Down
18 changes: 9 additions & 9 deletions crates/ruff_cli/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Printer {

fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> {
if self.log_level >= LogLevel::Default {
if self.flags.contains(Flags::SHOW_VIOLATIONS) {
if self.flags.intersects(Flags::SHOW_VIOLATIONS) {
let fixed = diagnostics
.fixed
.values()
Expand Down Expand Up @@ -160,12 +160,12 @@ impl Printer {
return Ok(());
}

if !self.flags.contains(Flags::SHOW_VIOLATIONS) {
if !self.flags.intersects(Flags::SHOW_VIOLATIONS) {
if matches!(
self.format,
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() {
writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed)?;
Expand All @@ -192,11 +192,11 @@ impl Printer {
SerializationFormat::Text => {
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.autofix_level))
.with_show_fix_diff(self.flags.contains(Flags::SHOW_FIX_DIFF))
.with_show_source(self.flags.contains(Flags::SHOW_SOURCE))
.with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF))
.with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.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() {
writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed)?;
Expand All @@ -208,11 +208,11 @@ impl Printer {
}
SerializationFormat::Grouped => {
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))
.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() {
writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed)?;
Expand Down Expand Up @@ -367,7 +367,7 @@ impl Printer {
let context = EmitterContext::new(&diagnostics.source_kind);
TextEmitter::default()
.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)?;
}
writer.flush()?;
Expand Down
14 changes: 7 additions & 7 deletions crates/ruff_python_semantic/src/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,44 +45,44 @@ impl<'a> Binding<'a> {
/// Return `true` if this [`Binding`] represents an explicit re-export
/// (e.g., `FastAPI` in `from fastapi import FastAPI as FastAPI`).
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
/// (e.g., `FastAPI` in `from fastapi import FastAPI`).
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
/// (e.g., `app` in `from fastapi import FastAPI as app`).
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`
/// if it's declared by a `nonlocal` statement, or shadows a [`Binding`] declared by a
/// `nonlocal` statement.
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
/// declared by a `global` statement, or shadows a [`Binding`] declared by a `global` statement.
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
/// value (e.g., `__all__ = "Foo"`).
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
/// invalid member (e.g., `__all__ = ["Foo", 1]`).
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
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<'a> Scope<'a> {

/// Returns `true` if this scope uses locals (e.g., `locals()`).
pub const fn uses_locals(&self) -> bool {
self.flags.contains(ScopeFlags::USES_LOCALS)
self.flags.intersects(ScopeFlags::USES_LOCALS)
}
}

Expand Down

0 comments on commit 057faab

Please sign in to comment.