Skip to content

Ruff rules SIM for simplification #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aaronliu0130 merged 1 commit into from
Mar 15, 2025
Merged

Ruff rules SIM for simplification #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 51 additions & 68 deletions cpplint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,11 +1326,10 @@ def IsInAlphabeticalOrder(self, clean_lines, linenum, header_path):
#
# If previous line was a blank line, assume that the headers are
# intentionally sorted the way they are.
if self._last_header > header_path and re.match(
r"^\s*#\s*include\b", clean_lines.elided[linenum - 1]
):
return False
return True
return not (
self._last_header > header_path
and re.match(r"^\s*#\s*include\b", clean_lines.elided[linenum - 1])
)

def CheckNextIncludeOrder(self, header_type):
"""Returns a non-empty error message if the next header is out of order.
Expand Down Expand Up @@ -1844,10 +1843,7 @@ def _ShouldPrintError(category, confidence, filename, linenum):
# should have been checked for in SetFilter.
msg = f"Invalid filter: {one_filter}"
raise ValueError(msg)
if is_filtered:
return False

return True
return not is_filtered


def Error(filename, linenum, category, confidence, message):
Expand Down Expand Up @@ -2262,11 +2258,10 @@ def FindEndOfExpressionInLine(line, startpos, stack):

# Pop the stack if there is a matching '<'. Otherwise, ignore
# this '>' since it must be an operator.
if stack:
if stack[-1] == "<":
stack.pop()
if not stack:
return (i + 1, None)
if stack and stack[-1] == "<":
stack.pop()
if not stack:
return (i + 1, None)
elif char == ";":
# Found something that look like end of statements. If we are currently
# expecting a '>', the matching '<' must have been an operator, since
Expand Down Expand Up @@ -2963,10 +2958,7 @@ def IsMacroDefinition(clean_lines, linenum):
if re.search(r"^#define", clean_lines[linenum]):
return True

if linenum > 0 and re.search(r"\\$", clean_lines[linenum - 1]):
return True

return False
return bool(linenum > 0 and re.search(r"\\$", clean_lines[linenum - 1]))


def IsForwardClassDeclaration(clean_lines, linenum):
Expand Down Expand Up @@ -4352,20 +4344,19 @@ def CheckParenthesisSpacing(filename, clean_lines, linenum, error):
line,
)
if match:
if len(match.group(2)) != len(match.group(4)):
if not (
match.group(3) == ";"
and len(match.group(2)) == 1 + len(match.group(4))
or not match.group(2)
and re.search(r"\bfor\s*\(.*; \)", line)
):
error(
filename,
linenum,
"whitespace/parens",
5,
f"Mismatching spaces inside () in {match.group(1)}",
)
if len(match.group(2)) != len(match.group(4)) and not (
match.group(3) == ";"
and len(match.group(2)) == 1 + len(match.group(4))
or not match.group(2)
and re.search(r"\bfor\s*\(.*; \)", line)
):
error(
filename,
linenum,
"whitespace/parens",
5,
f"Mismatching spaces inside () in {match.group(1)}",
)
if len(match.group(2)) not in [0, 1]:
error(
filename,
Expand Down Expand Up @@ -4586,9 +4577,7 @@ def IsDecltype(clean_lines, linenum, column):
(text, _, start_col) = ReverseCloseExpression(clean_lines, linenum, column)
if start_col < 0:
return False
if re.search(r"\bdecltype\s*$", text[0:start_col]):
return True
return False
return bool(re.search(r"\bdecltype\s*$", text[0:start_col]))


def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error):
Expand Down Expand Up @@ -4761,23 +4750,18 @@ def CheckBraces(filename, clean_lines, linenum, error):
# No control clauses with braces should have its contents on the same line
# Exclude } which will be covered by empty-block detect
# Exclude ; which may be used by while in a do-while
if keyword := re.search(
r"\b(else if|if|while|for|switch)" # These have parens
r"\s*\(.*\)\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\};]",
line,
):
error(
filename,
linenum,
"whitespace/newline",
5,
f"Controlled statements inside brackets of {keyword.group(1)} clause"
" should be on a separate line",
if (
keyword := re.search(
r"\b(else if|if|while|for|switch)" # These have parens
r"\s*\(.*\)\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\};]",
line,
)
) or (
keyword := re.search(
r"\b(else|do|try)" # These don't have parens
r"\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\}]",
line,
Comment on lines -4764 to +4763
Copy link
Member

@aaronliu0130 aaronliu0130 Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why it put everything in brackets...

Copy link
Member Author

@cclauss cclauss Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assignment Operator may require parentheses. Also there is a RUF rule that adds parens to clarify precedence between and, or, not.

)
elif keyword := re.search(
r"\b(else|do|try)" # These don't have parens
r"\s*(?:\[\[(?:un)?likely\]\]\s*)?{\s*[^\s\\}]",
line,
):
error(
filename,
Expand Down Expand Up @@ -5430,10 +5414,9 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, er
)

# Check if the line is a header guard.
is_header_guard = False
if IsHeaderExtension(file_extension):
if line.startswith((f"#ifndef {cppvar}", f"#define {cppvar}", f"#endif // {cppvar}")):
is_header_guard = True
is_header_guard = IsHeaderExtension(file_extension) and line.startswith(
(f"#ifndef {cppvar}", f"#define {cppvar}", f"#endif // {cppvar}")
)
# #include lines and header guards can be long, since there's no clean way to
# split them.
#
Expand Down Expand Up @@ -5648,17 +5631,18 @@ def CheckIncludeLine(filename, clean_lines, linenum, include_state, error):
# We also make an exception for Lua headers, which follow google
# naming convention but not the include convention.
match = re.match(r'#include\s*"([^/]+\.(.*))"', line)
if match:
if IsHeaderExtension(match.group(2)) and not _THIRD_PARTY_HEADERS_PATTERN.match(
match.group(1)
):
error(
filename,
linenum,
"build/include_subdir",
4,
"Include the directory when naming header files",
)
if (
match
and IsHeaderExtension(match.group(2))
and not _THIRD_PARTY_HEADERS_PATTERN.match(match.group(1))
):
error(
filename,
linenum,
"build/include_subdir",
4,
"Include the directory when naming header files",
)

# we shouldn't include a file more than once. actually, there are a
# handful of instances where doing so is okay, but in general it's
Expand Down Expand Up @@ -6326,8 +6310,7 @@ def CheckForNonConstReference(filename, clean_lines, linenum, nesting_state, err
# function body, including one that was just introduced by a trailing '{'.
# TODO(unknown): Doesn't account for 'catch(Exception& e)' [rare].
if nesting_state.previous_stack_top and not (
isinstance(nesting_state.previous_stack_top, _ClassInfo)
or isinstance(nesting_state.previous_stack_top, _NamespaceInfo)
isinstance(nesting_state.previous_stack_top, (_ClassInfo, _NamespaceInfo))
):
# Not at toplevel, not within a class, and not within a namespace
return
Expand Down
7 changes: 3 additions & 4 deletions cpplint_clitest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

"""Command Line interface integration test for cpplint.py."""

import contextlib
import glob
import os
import shutil
Expand All @@ -54,7 +55,7 @@ def run_shell_command(cmd: str, args: str, cwd="."):
cwd: from which folder to run.
"""
cmd, args = cmd.split(), args.split()
proc = subprocess.run(cmd + args, cwd=cwd, capture_output=True)
proc = subprocess.run(cmd + args, cwd=cwd, capture_output=True, check=False)
out, err = proc.stdout, proc.stderr

# Make output system-agnostic, aka support Windows
Expand Down Expand Up @@ -99,10 +100,8 @@ def setUpClass(cls):
shutil.copytree("samples", os.path.join(cls._root, "samples"))
cls.prepare_directory(cls._root)
except Exception:
try:
with contextlib.suppress(Exception):
cls.tearDownClass()
except Exception: # noqa: BLE001
pass
raise

@classmethod
Expand Down
13 changes: 5 additions & 8 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,12 @@ lint.select = [
"PERF", # Perflint
"PGH", # pygrep-hooks
"PIE", # flake8-pie
"PLC", # Pylint conventions
"PLE", # Pylint errors
"PL", # Pylint
"PYI", # flake8-pyi
"Q", # flake8-quotes
"RET", # flake8-return
"RSE", # flake8-raise
"SIM", # flake8-simplify
"SLOT", # flake8-slots
"T10", # flake8-debugger
"TC", # flake8-type-checking
Expand All @@ -114,14 +115,10 @@ lint.select = [
# "DOC", # pydoclint
# "ERA", # eradicate
# "N", # pep8-naming
# "PLR", # Pylint refactor
# "PLW", # Pylint warnings
# "PT", # flake8-pytest-style
# "PTH", # flake8-use-pathlib
# "Q", # flake8-quotes
# "RUF", # Ruff-specific rules
# "S", # flake8-bandit
# "SIM", # flake8-simplify
# "SLF", # flake8-self
# "T20", # flake8-print
# "TD", # flake8-todos
Expand All @@ -132,8 +129,8 @@ lint.ignore = [
"ISC003", # flake8-implicit-str-concat
"PIE790", # Unnecessary `pass` statement
]
lint.per-file-ignores."cpplint.py" = [ "ICN001", "PERF401", "PLR5501", "PLW0603", "PLW2901" ]
lint.per-file-ignores."cpplint_unittest.py" = [ "FLY002", "PLW0604", "UP031" ]
lint.per-file-ignores."cpplint.py" = [ "ICN001", "PERF401", "PLR5501", "PLW0603", "PLW2901", "SIM102", "SIM108" ]
lint.per-file-ignores."cpplint_unittest.py" = [ "FLY002", "PLW0604", "SIM115", "UP031" ]
lint.mccabe.max-complexity = 29
lint.pylint.allow-magic-value-types = [ "int", "str" ]
lint.pylint.max-args = 10 # Default is 5
Expand Down
Loading