Remove duplication around is_trivia functions (#11956)

## Summary

This PR removes the duplication around `is_trivia` functions.

There are two of them in the codebase:
1. In `pycodestyle`, it's for newline, indent, dedent, non-logical
newline and comment
2. In the parser, it's for non-logical newline and comment

The `TokenKind::is_trivia` method used (1) but that's not correct in
that context. So, this PR introduces a new `is_non_logical_token` helper
method for the `pycodestyle` crate and updates the
`TokenKind::is_trivia` implementation with (2).

This also means we can remove `Token::is_trivia` method and the
standalone `token_source::is_trivia` function and use the one on
`TokenKind`.

## Test Plan

`cargo insta test`
This commit is contained in:
Dhruv Manilawala 2024-06-21 15:32:40 +05:30 committed by GitHub
parent 690e94f4fb
commit 4667d8697c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 41 additions and 59 deletions

View file

@ -1,4 +1,18 @@
use ruff_python_parser::TokenKind;
/// Returns `true` if the name should be considered "ambiguous". /// Returns `true` if the name should be considered "ambiguous".
pub(super) fn is_ambiguous_name(name: &str) -> bool { pub(super) fn is_ambiguous_name(name: &str) -> bool {
name == "l" || name == "I" || name == "O" name == "l" || name == "I" || name == "O"
} }
/// Returns `true` if the given `token` is a non-logical token.
///
/// Unlike [`TokenKind::is_trivia`], this function also considers the indent, dedent and newline
/// tokens.
pub(super) const fn is_non_logical_token(token: TokenKind) -> bool {
token.is_trivia()
|| matches!(
token,
TokenKind::Newline | TokenKind::Indent | TokenKind::Dedent
)
}

View file

@ -15,13 +15,14 @@ use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::PySourceType; use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist; use ruff_python_codegen::Stylist;
use ruff_python_parser::TokenKind; use ruff_python_parser::TokenKind;
use ruff_python_trivia::PythonWhitespace;
use ruff_source_file::{Locator, UniversalNewlines}; use ruff_source_file::{Locator, UniversalNewlines};
use ruff_text_size::TextRange; use ruff_text_size::TextRange;
use ruff_text_size::TextSize; use ruff_text_size::TextSize;
use crate::checkers::logical_lines::expand_indent; use crate::checkers::logical_lines::expand_indent;
use crate::line_width::IndentWidth; use crate::line_width::IndentWidth;
use ruff_python_trivia::PythonWhitespace; use crate::rules::pycodestyle::helpers::is_non_logical_token;
/// Number of blank lines around top level classes and functions. /// Number of blank lines around top level classes and functions.
const BLANK_LINES_TOP_LEVEL: u32 = 2; const BLANK_LINES_TOP_LEVEL: u32 = 2;
@ -489,13 +490,13 @@ impl<'a> Iterator for LinePreprocessor<'a> {
(logical_line_kind, range) (logical_line_kind, range)
}; };
if !kind.is_trivia() { if !is_non_logical_token(kind) {
line_is_comment_only = false; line_is_comment_only = false;
} }
// A docstring line is composed only of the docstring (TokenKind::String) and trivia tokens. // A docstring line is composed only of the docstring (TokenKind::String) and trivia tokens.
// (If a comment follows a docstring, we still count the line as a docstring) // (If a comment follows a docstring, we still count the line as a docstring)
if kind != TokenKind::String && !kind.is_trivia() { if kind != TokenKind::String && !is_non_logical_token(kind) {
is_docstring = false; is_docstring = false;
} }
@ -545,7 +546,7 @@ impl<'a> Iterator for LinePreprocessor<'a> {
_ => {} _ => {}
} }
if !kind.is_trivia() { if !is_non_logical_token(kind) {
last_token = kind; last_token = kind;
} }
} }

View file

@ -4,6 +4,7 @@ use ruff_python_parser::TokenKind;
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::logical_lines::LogicalLinesContext; use crate::checkers::logical_lines::LogicalLinesContext;
use crate::rules::pycodestyle::helpers::is_non_logical_token;
use crate::rules::pycodestyle::rules::logical_lines::LogicalLine; use crate::rules::pycodestyle::rules::logical_lines::LogicalLine;
/// ## What it does /// ## What it does
@ -146,7 +147,9 @@ pub(crate) fn missing_whitespace_around_operator(
context: &mut LogicalLinesContext, context: &mut LogicalLinesContext,
) { ) {
let mut tokens = line.tokens().iter().peekable(); let mut tokens = line.tokens().iter().peekable();
let first_token = tokens.by_ref().find(|token| !token.kind().is_trivia()); let first_token = tokens
.by_ref()
.find(|token| !is_non_logical_token(token.kind()));
let Some(mut prev_token) = first_token else { let Some(mut prev_token) = first_token else {
return; return;
}; };
@ -159,7 +162,7 @@ pub(crate) fn missing_whitespace_around_operator(
while let Some(token) = tokens.next() { while let Some(token) = tokens.next() {
let kind = token.kind(); let kind = token.kind();
if kind.is_trivia() { if is_non_logical_token(kind) {
continue; continue;
} }
@ -234,10 +237,10 @@ pub(crate) fn missing_whitespace_around_operator(
if needs_space != NeedsSpace::No { if needs_space != NeedsSpace::No {
let has_leading_trivia = let has_leading_trivia =
prev_token.end() < token.start() || prev_token.kind().is_trivia(); prev_token.end() < token.start() || is_non_logical_token(prev_token.kind());
let has_trailing_trivia = tokens.peek().map_or(true, |next| { let has_trailing_trivia = tokens.peek().map_or(true, |next| {
token.end() < next.start() || next.kind().is_trivia() token.end() < next.start() || is_non_logical_token(next.kind())
}); });
match (has_leading_trivia, has_trailing_trivia) { match (has_leading_trivia, has_trailing_trivia) {

View file

@ -20,6 +20,8 @@ use ruff_python_parser::{TokenKind, Tokens};
use ruff_python_trivia::is_python_whitespace; use ruff_python_trivia::is_python_whitespace;
use ruff_source_file::Locator; use ruff_source_file::Locator;
use crate::rules::pycodestyle::helpers::is_non_logical_token;
mod extraneous_whitespace; mod extraneous_whitespace;
mod indentation; mod indentation;
mod missing_whitespace; mod missing_whitespace;
@ -167,32 +169,14 @@ impl<'a> LogicalLine<'a> {
let start = tokens let start = tokens
.iter() .iter()
.position(|t| { .position(|t| !is_non_logical_token(t.kind()))
!matches!(
t.kind(),
TokenKind::Newline
| TokenKind::NonLogicalNewline
| TokenKind::Indent
| TokenKind::Dedent
| TokenKind::Comment,
)
})
.unwrap_or(tokens.len()); .unwrap_or(tokens.len());
let tokens = &tokens[start..]; let tokens = &tokens[start..];
let end = tokens let end = tokens
.iter() .iter()
.rposition(|t| { .rposition(|t| !is_non_logical_token(t.kind()))
!matches!(
t.kind(),
TokenKind::Newline
| TokenKind::NonLogicalNewline
| TokenKind::Indent
| TokenKind::Dedent
| TokenKind::Comment,
)
})
.map_or(0, |pos| pos + 1); .map_or(0, |pos| pos + 1);
&tokens[..end] &tokens[..end]
@ -447,14 +431,7 @@ impl LogicalLinesBuilder {
line.flags.insert(TokenFlags::KEYWORD); line.flags.insert(TokenFlags::KEYWORD);
} }
if !matches!( if !is_non_logical_token(kind) {
kind,
TokenKind::Comment
| TokenKind::Newline
| TokenKind::NonLogicalNewline
| TokenKind::Dedent
| TokenKind::Indent
) {
line.flags.insert(TokenFlags::NON_TRIVIA); line.flags.insert(TokenFlags::NON_TRIVIA);
} }
@ -468,7 +445,7 @@ impl LogicalLinesBuilder {
if self.current_line.tokens_start < end { if self.current_line.tokens_start < end {
let is_empty = self.tokens[self.current_line.tokens_start as usize..end as usize] let is_empty = self.tokens[self.current_line.tokens_start as usize..end as usize]
.iter() .iter()
.all(|token| token.kind.is_newline()); .all(|token| token.kind.is_any_newline());
if !is_empty { if !is_empty {
self.lines.push(Line { self.lines.push(Line {
flags: self.current_line.flags, flags: self.current_line.flags,

View file

@ -146,7 +146,7 @@ fn locate_cmp_ops(expr: &Expr, tokens: &Tokens) -> Vec<LocatedCmpOp> {
let mut tok_iter = tokens let mut tok_iter = tokens
.in_range(expr.range()) .in_range(expr.range())
.iter() .iter()
.filter(|token| !token.is_trivia()) .filter(|token| !token.kind().is_trivia())
.peekable(); .peekable();
let mut ops: Vec<LocatedCmpOp> = vec![]; let mut ops: Vec<LocatedCmpOp> = vec![];

View file

@ -1626,12 +1626,6 @@ impl Token {
(self.kind, self.range) (self.kind, self.range)
} }
/// Returns `true` if this is a trivia token.
#[inline]
pub const fn is_trivia(self) -> bool {
matches!(self.kind, TokenKind::Comment | TokenKind::NonLogicalNewline)
}
/// Returns `true` if this is any kind of string token. /// Returns `true` if this is any kind of string token.
const fn is_any_string(self) -> bool { const fn is_any_string(self) -> bool {
matches!( matches!(

View file

@ -192,13 +192,15 @@ pub enum TokenKind {
} }
impl TokenKind { impl TokenKind {
/// Returns `true` if this is an end of file token.
#[inline] #[inline]
pub const fn is_eof(self) -> bool { pub const fn is_eof(self) -> bool {
matches!(self, TokenKind::EndOfFile) matches!(self, TokenKind::EndOfFile)
} }
/// Returns `true` if this is either a newline or non-logical newline token.
#[inline] #[inline]
pub const fn is_newline(self) -> bool { pub const fn is_any_newline(self) -> bool {
matches!(self, TokenKind::Newline | TokenKind::NonLogicalNewline) matches!(self, TokenKind::Newline | TokenKind::NonLogicalNewline)
} }
@ -294,21 +296,16 @@ impl TokenKind {
) )
} }
/// Returns `true` if this is a singleton token i.e., `True`, `False`, or `None`.
#[inline] #[inline]
pub const fn is_singleton(self) -> bool { pub const fn is_singleton(self) -> bool {
matches!(self, TokenKind::False | TokenKind::True | TokenKind::None) matches!(self, TokenKind::False | TokenKind::True | TokenKind::None)
} }
/// Returns `true` if this is a trivia token i.e., a comment or a non-logical newline.
#[inline] #[inline]
pub const fn is_trivia(&self) -> bool { pub const fn is_trivia(&self) -> bool {
matches!( matches!(self, TokenKind::Comment | TokenKind::NonLogicalNewline)
self,
TokenKind::Newline
| TokenKind::Indent
| TokenKind::Dedent
| TokenKind::NonLogicalNewline
| TokenKind::Comment
)
} }
#[inline] #[inline]

View file

@ -114,7 +114,7 @@ impl<'src> TokenSource<'src> {
fn do_bump(&mut self) { fn do_bump(&mut self) {
loop { loop {
let kind = self.lexer.next_token(); let kind = self.lexer.next_token();
if is_trivia(kind) { if kind.is_trivia() {
self.tokens self.tokens
.push(Token::new(kind, self.current_range(), self.current_flags())); .push(Token::new(kind, self.current_range(), self.current_flags()));
continue; continue;
@ -127,7 +127,7 @@ impl<'src> TokenSource<'src> {
fn next_non_trivia_token(&mut self) -> TokenKind { fn next_non_trivia_token(&mut self) -> TokenKind {
loop { loop {
let kind = self.lexer.next_token(); let kind = self.lexer.next_token();
if is_trivia(kind) { if kind.is_trivia() {
continue; continue;
} }
break kind; break kind;
@ -187,7 +187,3 @@ fn allocate_tokens_vec(contents: &str) -> Vec<Token> {
let lower_bound = contents.len().saturating_mul(15) / 100; let lower_bound = contents.len().saturating_mul(15) / 100;
Vec::with_capacity(lower_bound) Vec::with_capacity(lower_bound)
} }
fn is_trivia(token: TokenKind) -> bool {
matches!(token, TokenKind::Comment | TokenKind::NonLogicalNewline)
}