From 6ff5c4ef99d749c16d7077b01a6e8f6a5738449d Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 12 Aug 2025 19:57:35 +0200 Subject: [PATCH] tidy: Remove dead and redundant code from Servo Rust tidy check (#38632) - Remove all handling of comments and attributes. This was not affecting the checks in any way. - Remove `is_associated_type()` as it was unused. - Remove the check for `&String`, `&Vec`, operators at the end of the line, and the unit return type as clippy already checks these (and handles the mutable variants). Testing: This is covered by tidy script tests. Signed-off-by: Martin Robinson --- python/tidy/test.py | 5 -- python/tidy/tidy.py | 109 ++++---------------------------------------- 2 files changed, 10 insertions(+), 104 deletions(-) diff --git a/python/tidy/test.py b/python/tidy/test.py index 40ce3ca9495..1b4207ca683 100644 --- a/python/tidy/test.py +++ b/python/tidy/test.py @@ -117,13 +117,8 @@ class CheckTidiness(unittest.TestCase): def test_rust(self): errors = tidy.collect_errors_for_files(iterFile("rust_tidy.rs"), [], [tidy.check_rust], print_text=False) - self.assertEqual("found an empty line following a {", next(errors)[2]) - self.assertEqual("use &[T] instead of &Vec", next(errors)[2]) - self.assertEqual("use &str instead of &String", next(errors)[2]) self.assertEqual("use &T instead of &Root", next(errors)[2]) self.assertEqual("use &T instead of &DomRoot", next(errors)[2]) - self.assertEqual("encountered function signature with -> ()", next(errors)[2]) - self.assertEqual("operators should go at the end of the first line", next(errors)[2]) self.assertNoMoreErrors(errors) feature_errors = tidy.collect_errors_for_files(iterFile("lib.rs"), [], [tidy.check_rust], print_text=False) diff --git a/python/tidy/tidy.py b/python/tidy/tidy.py index aef2732f505..5630cea32d4 100644 --- a/python/tidy/tidy.py +++ b/python/tidy/tidy.py @@ -552,112 +552,23 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]: ): return - comment_depth = 0 - merged_lines = "" - import_block = False - whitespace = False - - prev_open_brace = False - multi_line_string = False - - for idx, original_line in enumerate(map(lambda line: line.decode("utf-8"), lines)): - # simplify the analysis - line = original_line.strip() - - is_attribute = re.search(r"#\[.*\]", line) - is_comment = re.search(r"^//|^/\*|^\*", line) - - # Simple heuristic to avoid common case of no comments. - if "/" in line: - comment_depth += line.count("/*") - comment_depth -= line.count("*/") - - if line.endswith("\\"): - merged_lines += line[:-1] - continue - if comment_depth: - merged_lines += line - continue - if merged_lines: - line = merged_lines + line - merged_lines = "" - - if multi_line_string: - line, count = re.subn(r'^(\\.|[^"\\])*?"', "", line, count=1) - if count == 1: - multi_line_string = False - else: - continue - - # Ignore attributes, comments, and imports - # Keep track of whitespace to enable checking for a merged import block - if import_block: - if not (is_comment or is_attribute or line.startswith("use ")): - whitespace = line == "" - - if not whitespace: - import_block = False - - # get rid of strings and chars because cases like regex expression, keep attributes - if not is_attribute and not is_comment: - line = re.sub(r'"(\\.|[^\\"])*?"', '""', line) - line = re.sub(r"'(\\.|[^\\']|(\\x[0-9a-fA-F]{2})|(\\u{[0-9a-fA-F]{1,6}}))'", "''", line) - # If, after parsing all single-line strings, we still have - # an odd number of double quotes, this line starts a - # multiline string - if line.count('"') % 2 == 1: - line = re.sub(r'"(\\.|[^\\"])*?$', '""', line) - multi_line_string = True - - # get rid of comments + for idx, line in enumerate(map(lambda line: line.decode("utf-8"), lines)): line = re.sub(r"//.*?$|/\*.*?$|^\*.*?$", "//", line) - - # get rid of attributes that do not contain = - line = re.sub(r"^#[A-Za-z0-9\(\)\[\]_]*?$", "#[]", line) - - # flag this line if it matches one of the following regular expressions - # tuple format: (pattern, format_message, filter_function(match, line)) - def no_filter(match, line) -> bool: - return True - - regex_rules = [ - # There should not be any extra pointer dereferencing - (r": &Vec<", "use &[T] instead of &Vec", no_filter), - # No benefit over using &str - (r": &String", "use &str instead of &String", no_filter), + rules = [ # There should be any use of banned types: # Cell, Cell>, DomRefCell>, DomRefCell> - (r"(\s|:)+Cell", "Banned type Cell detected. Use MutDom instead", no_filter), - (r"(\s|:)+Cell>", "Banned type Cell> detected. Use MutDom instead", no_filter), - (r"DomRefCell>", "Banned type DomRefCell> detected. Use MutDom instead", no_filter), - (r"DomRefCell>", "Banned type DomRefCell> detected. Use MutDom instead", no_filter), + (r"(\s|:)+Cell", "Banned type Cell detected. Use MutDom instead"), + (r"(\s|:)+Cell>", "Banned type Cell> detected. Use MutDom instead"), + (r"DomRefCell>", "Banned type DomRefCell> detected. Use MutDom instead"), + (r"DomRefCell>", "Banned type DomRefCell> detected. Use MutDom instead"), # No benefit to using &Root - (r": &Root<", "use &T instead of &Root", no_filter), - (r": &DomRoot<", "use &T instead of &DomRoot", no_filter), - (r"^&&", "operators should go at the end of the first line", no_filter), - # -> () is unnecessary - (r"-> \(\)", "encountered function signature with -> ()", no_filter), + (r": &Root<", "use &T instead of &Root"), + (r": &DomRoot<", "use &T instead of &DomRoot"), ] - for pattern, message, filter_func in regex_rules: + for pattern, message in rules: for match in re.finditer(pattern, line): - if filter_func(match, line): - yield (idx + 1, message.format(*match.groups(), **match.groupdict())) - - if prev_open_brace and not line: - yield (idx + 1, "found an empty line following a {") - prev_open_brace = line.endswith("{") - - -# Avoid flagging constructs -def is_associated_type(match, line): - if match.group(1) != "=": - return False - open_angle = line[0 : match.end()].rfind("<") - close_angle = line[open_angle:].find(">") if open_angle != -1 else -1 - generic_open = open_angle != -1 and open_angle < match.start() - generic_close = close_angle != -1 and close_angle + open_angle >= match.end() - return generic_open and generic_close + yield (idx + 1, message.format(*match.groups(), **match.groupdict())) def check_webidl_spec(file_name: str, contents: bytes) -> Iterator[tuple[int, str]]: