Skip to content

Commit

Permalink
implement e122
Browse files Browse the repository at this point in the history
  • Loading branch information
augustelalande committed May 9, 2024
1 parent dfe4291 commit dd4acf0
Show file tree
Hide file tree
Showing 11 changed files with 449 additions and 4 deletions.
143 changes: 143 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E122.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
#: E122
print("E122", (
"str"))

# OK
print("E122", (
"str"))

#: E122:6:5 E122:7:5 E122:8:1
print(dedent(
'''
mkdir -p ./{build}/
mv ./build/ ./{build}/%(revision)s/
'''.format(
build='build',
# more stuff
)
))

# OK
print(dedent(
'''
mkdir -p ./{build}/
mv ./build/ ./{build}/%(revision)s/
'''.format(
build='build',
# more stuff
)
))

#: E122
if True:
result = some_function_that_takes_arguments(
'a', 'b', 'c',
'd', 'e', 'f',
)

# OK
if True:
result = some_function_that_takes_arguments(
'a', 'b', 'c',
'd', 'e', 'f',
)

#: E122
if some_very_very_very_long_variable_name or var \
or another_very_long_variable_name:
raise Exception()

# OK
if some_very_very_very_long_variable_name or var \
or another_very_long_variable_name:
raise Exception()

#: E122
if some_very_very_very_long_variable_name or var[0] \
or another_very_long_variable_name:
raise Exception()

# OK
if some_very_very_very_long_variable_name or var[0] \
or another_very_long_variable_name:
raise Exception()

#: E122
if True:
if some_very_very_very_long_variable_name or var \
or another_very_long_variable_name:
raise Exception()

# OK
if True:
if some_very_very_very_long_variable_name or var \
or another_very_long_variable_name:
raise Exception()

#: E122
if True:
if some_very_very_very_long_variable_name or var[0] \
or another_very_long_variable_name:
raise Exception()

#: OK
if True:
if some_very_very_very_long_variable_name or var[0] \
or another_very_long_variable_name:
raise Exception()

#: E122
dictionary = {
"is": {
"nested": yes(),
},
}

# OK
dictionary = {
"is": {
"nested": yes(),
},
}

#: E122
setup('',
scripts=[''],
classifiers=[
'Development Status :: 4 - Beta',
'Environment :: Console',
'Intended Audience :: Developers',
])

# OK
setup('',
scripts=[''],
classifiers=[
'Development Status :: 4 - Beta',
'Environment :: Console',
'Intended Audience :: Developers',
])

#: E122:2:1
if True:\
print(True)

# OK
if True:\
print(True)

# E122
def f():
x = ((
(
)
)
+ 2)

# OK
def f():
x = ((
(
)
)
+ 2)
20 changes: 16 additions & 4 deletions crates/ruff_linter/src/checkers/logical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::line_width::IndentWidth;
use crate::registry::Rule;
use ruff_diagnostics::Diagnostic;
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
Expand All @@ -9,10 +10,11 @@ use ruff_text_size::{Ranged, TextRange};

use crate::registry::AsRule;
use crate::rules::pycodestyle::rules::logical_lines::{
extraneous_whitespace, indentation, missing_whitespace, missing_whitespace_after_keyword,
missing_whitespace_around_operator, redundant_backslash, space_after_comma,
space_around_operator, whitespace_around_keywords, whitespace_around_named_parameter_equals,
whitespace_before_comment, whitespace_before_parameters, LogicalLines, TokenFlags,
extraneous_whitespace, indentation, missing_or_outdented_indentation, missing_whitespace,
missing_whitespace_after_keyword, missing_whitespace_around_operator, redundant_backslash,
space_after_comma, space_around_operator, whitespace_around_keywords,
whitespace_around_named_parameter_equals, whitespace_before_comment,
whitespace_before_parameters, LogicalLines, TokenFlags,
};
use crate::settings::LinterSettings;

Expand Down Expand Up @@ -106,6 +108,16 @@ pub(crate) fn check_logical_lines(
}
}

if settings.rules.enabled(Rule::MissingOrOutdentedIndentation) {
missing_or_outdented_indentation(
&line,
indent_level,
settings.tab_size,
locator,
&mut context,
);
}

if !line.is_comment_only() {
prev_line = Some(line);
prev_indent_level = Some(indent_level);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pycodestyle, "E116") => (RuleGroup::Nursery, rules::pycodestyle::rules::logical_lines::UnexpectedIndentationComment),
#[allow(deprecated)]
(Pycodestyle, "E117") => (RuleGroup::Nursery, rules::pycodestyle::rules::logical_lines::OverIndented),
(Pycodestyle, "E122") => (RuleGroup::Preview, rules::pycodestyle::rules::logical_lines::MissingOrOutdentedIndentation),
#[allow(deprecated)]
(Pycodestyle, "E201") => (RuleGroup::Nursery, rules::pycodestyle::rules::logical_lines::WhitespaceAfterOpenBracket),
#[allow(deprecated)]
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ impl Rule {
Rule::ImplicitNamespacePackage | Rule::InvalidModuleName => LintSource::Filesystem,
Rule::IndentationWithInvalidMultiple
| Rule::IndentationWithInvalidMultipleComment
| Rule::MissingOrOutdentedIndentation
| Rule::MissingWhitespace
| Rule::MissingWhitespaceAfterKeyword
| Rule::MissingWhitespaceAroundArithmeticOperator
Expand Down
19 changes: 19 additions & 0 deletions crates/ruff_linter/src/rules/pycodestyle/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,23 @@
use crate::line_width::IndentWidth;

/// Returns `true` if the name should be considered "ambiguous".
pub(super) fn is_ambiguous_name(name: &str) -> bool {
name == "l" || name == "I" || name == "O"
}

/// Return the amount of indentation, expanding tabs to the next multiple of the settings' tab size.
pub(crate) fn expand_indent(line: &str, indent_width: IndentWidth) -> usize {
let line = line.trim_end_matches(['\n', '\r']);

let mut indent = 0;
let tab_size = indent_width.as_usize();
for c in line.bytes() {
match c {
b'\t' => indent += (indent / tab_size) * tab_size + tab_size,
b' ' => indent += 1,
_ => break,
}
}

indent
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ mod tests {
#[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_2.py"))]
#[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_3.py"))]
#[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_4.py"))]
#[test_case(Rule::MissingOrOutdentedIndentation, Path::new("E122.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
use super::{LogicalLine, LogicalLineToken};
use crate::checkers::logical_lines::LogicalLinesContext;
use crate::line_width::IndentWidth;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_parser::TokenKind;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

use crate::rules::pycodestyle::helpers::expand_indent;

/// ## What it does
/// Checks for continuation lines not indented as far as they should be or indented too far.
///
/// ## Why is this bad?
/// This makes distinguishing continuation line harder.
///
/// ## Example
/// ```python
/// print("Python", (
/// "Rules"))
/// ```
///
/// Use instead:
/// ```python
/// print("Python", (
/// "Rules"))
/// ```
///
/// [PEP 8]: https://www.python.org/dev/peps/pep-0008/#indentation
#[violation]
pub struct MissingOrOutdentedIndentation;

impl Violation for MissingOrOutdentedIndentation {
#[derive_message_formats]
fn message(&self) -> String {
format!("Continuation line missing indentation or outdented.")
}
}

/// E122
pub(crate) fn missing_or_outdented_indentation(
line: &LogicalLine,
indent_level: usize,
indent_width: IndentWidth,
locator: &Locator,
context: &mut LogicalLinesContext,
) {
if line.tokens().len() <= 1 {
return;
}

let first_token = line.first_token().unwrap();
let mut line_end = locator.full_line_end(first_token.start());

let tab_size = indent_width.as_usize();
let mut indentation = indent_level;
// Start by increasing indent on any continuation line
let mut desired_indentation = indentation + tab_size;
let mut indent_increased = true;
let mut indentation_stack: std::vec::Vec<usize> = Vec::new();

let mut iter = line.tokens().iter().peekable();
while let Some(token) = iter.next() {
// If continuation line
if token.start() >= line_end {
// Reset and calculate current indentation
indent_increased = false;
indentation = expand_indent(locator.line(token.start()), indent_width);

// Calculate correct indentation
let correct_indentation = if first_token_is_closing_bracket(token, iter.peek().copied())
{
// If first non-indent token is a closing bracket
// then the correct indentation is the one on top of the stack
// unless we are back at the starting indentation in which case
// the initial indentation is correct.
if desired_indentation == indent_level + tab_size {
indent_level
} else {
*indentation_stack
.last()
.expect("Closing brackets should always be preceded by opening brackets")
}
} else {
desired_indentation
};

if indentation < correct_indentation {
let diagnostic = Diagnostic::new(
MissingOrOutdentedIndentation,
TextRange::new(locator.line_start(token.start()), token.start()),
);
context.push_diagnostic(diagnostic);
}

line_end = locator.full_line_end(token.start());
}

match token.kind() {
TokenKind::Lpar | TokenKind::Lsqb | TokenKind::Lbrace => {
// Store indent to return to once bracket closes
indentation_stack.push(desired_indentation);
// Only increase the indent once per continuation line
if !indent_increased {
desired_indentation += tab_size;
indent_increased = true;
}
}
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace => {
// Return to previous indent
desired_indentation = indentation_stack
.pop()
.expect("Closing brackets should always be preceded by opening brackets");
}
_ => {}
}
}
}

fn first_token_is_closing_bracket(
first_token: &LogicalLineToken,
second_token: Option<&LogicalLineToken>,
) -> bool {
match first_token.kind {
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace => true,
TokenKind::Indent => {
second_token.is_some()
&& matches!(
second_token.unwrap().kind,
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace
)
}
_ => false,
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub(crate) use extraneous_whitespace::*;
pub(crate) use indentation::*;
pub(crate) use missing_or_outdented_indentation::*;
pub(crate) use missing_whitespace::*;
pub(crate) use missing_whitespace_after_keyword::*;
pub(crate) use missing_whitespace_around_operator::*;
Expand All @@ -23,6 +24,7 @@ use ruff_source_file::Locator;

mod extraneous_whitespace;
mod indentation;
mod missing_or_outdented_indentation;
mod missing_whitespace;
mod missing_whitespace_after_keyword;
mod missing_whitespace_around_operator;
Expand Down
Loading

0 comments on commit dd4acf0

Please sign in to comment.