mirror of
https://github.com/servo/servo.git
synced 2025-08-16 19:05:33 +01:00
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 <mrobinson@igalia.com>
This commit is contained in:
parent
2ea251117a
commit
6ff5c4ef99
2 changed files with 10 additions and 104 deletions
|
@ -117,13 +117,8 @@ class CheckTidiness(unittest.TestCase):
|
||||||
|
|
||||||
def test_rust(self):
|
def test_rust(self):
|
||||||
errors = tidy.collect_errors_for_files(iterFile("rust_tidy.rs"), [], [tidy.check_rust], print_text=False)
|
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<T>", next(errors)[2])
|
|
||||||
self.assertEqual("use &str instead of &String", next(errors)[2])
|
|
||||||
self.assertEqual("use &T instead of &Root<T>", next(errors)[2])
|
self.assertEqual("use &T instead of &Root<T>", next(errors)[2])
|
||||||
self.assertEqual("use &T instead of &DomRoot<T>", next(errors)[2])
|
self.assertEqual("use &T instead of &DomRoot<T>", 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)
|
self.assertNoMoreErrors(errors)
|
||||||
|
|
||||||
feature_errors = tidy.collect_errors_for_files(iterFile("lib.rs"), [], [tidy.check_rust], print_text=False)
|
feature_errors = tidy.collect_errors_for_files(iterFile("lib.rs"), [], [tidy.check_rust], print_text=False)
|
||||||
|
|
|
@ -552,112 +552,23 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]:
|
||||||
):
|
):
|
||||||
return
|
return
|
||||||
|
|
||||||
comment_depth = 0
|
for idx, line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
|
||||||
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
|
|
||||||
line = re.sub(r"//.*?$|/\*.*?$|^\*.*?$", "//", line)
|
line = re.sub(r"//.*?$|/\*.*?$|^\*.*?$", "//", line)
|
||||||
|
rules = [
|
||||||
# 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<T>", no_filter),
|
|
||||||
# No benefit over using &str
|
|
||||||
(r": &String", "use &str instead of &String", no_filter),
|
|
||||||
# There should be any use of banned types:
|
# There should be any use of banned types:
|
||||||
# Cell<JSVal>, Cell<Dom<T>>, DomRefCell<Dom<T>>, DomRefCell<HEAP<T>>
|
# Cell<JSVal>, Cell<Dom<T>>, DomRefCell<Dom<T>>, DomRefCell<HEAP<T>>
|
||||||
(r"(\s|:)+Cell<JSVal>", "Banned type Cell<JSVal> detected. Use MutDom<JSVal> instead", no_filter),
|
(r"(\s|:)+Cell<JSVal>", "Banned type Cell<JSVal> detected. Use MutDom<JSVal> instead"),
|
||||||
(r"(\s|:)+Cell<Dom<.+>>", "Banned type Cell<Dom<T>> detected. Use MutDom<T> instead", no_filter),
|
(r"(\s|:)+Cell<Dom<.+>>", "Banned type Cell<Dom<T>> detected. Use MutDom<T> instead"),
|
||||||
(r"DomRefCell<Dom<.+>>", "Banned type DomRefCell<Dom<T>> detected. Use MutDom<T> instead", no_filter),
|
(r"DomRefCell<Dom<.+>>", "Banned type DomRefCell<Dom<T>> detected. Use MutDom<T> instead"),
|
||||||
(r"DomRefCell<Heap<.+>>", "Banned type DomRefCell<Heap<T>> detected. Use MutDom<T> instead", no_filter),
|
(r"DomRefCell<Heap<.+>>", "Banned type DomRefCell<Heap<T>> detected. Use MutDom<T> instead"),
|
||||||
# No benefit to using &Root<T>
|
# No benefit to using &Root<T>
|
||||||
(r": &Root<", "use &T instead of &Root<T>", no_filter),
|
(r": &Root<", "use &T instead of &Root<T>"),
|
||||||
(r": &DomRoot<", "use &T instead of &DomRoot<T>", no_filter),
|
(r": &DomRoot<", "use &T instead of &DomRoot<T>"),
|
||||||
(r"^&&", "operators should go at the end of the first line", no_filter),
|
|
||||||
# -> () is unnecessary
|
|
||||||
(r"-> \(\)", "encountered function signature with -> ()", no_filter),
|
|
||||||
]
|
]
|
||||||
|
|
||||||
for pattern, message, filter_func in regex_rules:
|
for pattern, message in rules:
|
||||||
for match in re.finditer(pattern, line):
|
for match in re.finditer(pattern, line):
|
||||||
if filter_func(match, line):
|
yield (idx + 1, message.format(*match.groups(), **match.groupdict()))
|
||||||
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 <Item=Foo> 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
|
|
||||||
|
|
||||||
|
|
||||||
def check_webidl_spec(file_name: str, contents: bytes) -> Iterator[tuple[int, str]]:
|
def check_webidl_spec(file_name: str, contents: bytes) -> Iterator[tuple[int, str]]:
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue