mirror of
https://github.com/servo/servo.git
synced 2025-08-14 09:55:35 +01:00
mach(test-tidy): Remove alphabetical order and line length rule from tidy (#38538)
As we plan to adopt more rules from Ruff and rustfmt, we would like to retire the following rules: 1. `Line length check`, as this is already handled by Ruff and rustfmt configurations. 2. `Alphabetical order` Testing: `./mach test-tidy --no-progress --all` Fixes: #37121 --------- Signed-off-by: Jerens Lensun <jerensslensun@gmail.com>
This commit is contained in:
parent
4784668fa9
commit
3ab56b16de
4 changed files with 0 additions and 107 deletions
|
@ -73,11 +73,6 @@ class CheckTidiness(unittest.TestCase):
|
||||||
self.assertEqual("file is empty", next(errors)[2])
|
self.assertEqual("file is empty", next(errors)[2])
|
||||||
self.assertNoMoreErrors(errors)
|
self.assertNoMoreErrors(errors)
|
||||||
|
|
||||||
def test_long_line(self):
|
|
||||||
errors = tidy.collect_errors_for_files(iterFile("long_line.rs"), [], [tidy.check_by_line], print_text=False)
|
|
||||||
self.assertEqual("Line is longer than 120 characters", next(errors)[2])
|
|
||||||
self.assertNoMoreErrors(errors)
|
|
||||||
|
|
||||||
def test_whatwg_link(self):
|
def test_whatwg_link(self):
|
||||||
errors = tidy.collect_errors_for_files(iterFile("whatwg_link.rs"), [], [tidy.check_by_line], print_text=False)
|
errors = tidy.collect_errors_for_files(iterFile("whatwg_link.rs"), [], [tidy.check_by_line], print_text=False)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
|
@ -122,9 +117,6 @@ 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.assertTrue("mod declaration is not in alphabetical order" in next(errors)[2])
|
|
||||||
self.assertEqual("mod declaration spans multiple lines", next(errors)[2])
|
|
||||||
self.assertTrue("derivable traits list is not in alphabetical order" in next(errors)[2])
|
|
||||||
self.assertEqual("found an empty line following a {", next(errors)[2])
|
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 &[T] instead of &Vec<T>", next(errors)[2])
|
||||||
self.assertEqual("use &str instead of &String", next(errors)[2])
|
self.assertEqual("use &str instead of &String", next(errors)[2])
|
||||||
|
@ -138,10 +130,6 @@ class CheckTidiness(unittest.TestCase):
|
||||||
|
|
||||||
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)
|
||||||
|
|
||||||
self.assertTrue("feature attribute is not in alphabetical order" in next(feature_errors)[2])
|
|
||||||
self.assertTrue("feature attribute is not in alphabetical order" in next(feature_errors)[2])
|
|
||||||
self.assertTrue("feature attribute is not in alphabetical order" in next(feature_errors)[2])
|
|
||||||
self.assertTrue("feature attribute is not in alphabetical order" in next(feature_errors)[2])
|
|
||||||
self.assertNoMoreErrors(feature_errors)
|
self.assertNoMoreErrors(feature_errors)
|
||||||
|
|
||||||
ban_errors = tidy.collect_errors_for_files(iterFile("ban.rs"), [], [tidy.check_rust], print_text=False)
|
ban_errors = tidy.collect_errors_for_files(iterFile("ban.rs"), [], [tidy.check_rust], print_text=False)
|
||||||
|
|
|
@ -1,7 +1,6 @@
|
||||||
key-outside = ""
|
key-outside = ""
|
||||||
|
|
||||||
[configs]
|
[configs]
|
||||||
skip-check-length = false
|
|
||||||
skip-check-licenses = false
|
skip-check-licenses = false
|
||||||
wrong-key = false
|
wrong-key = false
|
||||||
|
|
||||||
|
|
|
@ -59,9 +59,7 @@ IgnoreConfig = TypedDict(
|
||||||
Config = TypedDict(
|
Config = TypedDict(
|
||||||
"Config",
|
"Config",
|
||||||
{
|
{
|
||||||
"skip-check-length": bool,
|
|
||||||
"skip-check-licenses": bool,
|
"skip-check-licenses": bool,
|
||||||
"check-alphabetical-order": bool,
|
|
||||||
"lint-scripts": list,
|
"lint-scripts": list,
|
||||||
"blocked-packages": dict[str, Any],
|
"blocked-packages": dict[str, Any],
|
||||||
"ignore": IgnoreConfig,
|
"ignore": IgnoreConfig,
|
||||||
|
@ -70,9 +68,7 @@ Config = TypedDict(
|
||||||
)
|
)
|
||||||
|
|
||||||
config: Config = {
|
config: Config = {
|
||||||
"skip-check-length": False,
|
|
||||||
"skip-check-licenses": False,
|
"skip-check-licenses": False,
|
||||||
"check-alphabetical-order": True,
|
|
||||||
"lint-scripts": [],
|
"lint-scripts": [],
|
||||||
"blocked-packages": {},
|
"blocked-packages": {},
|
||||||
"ignore": {
|
"ignore": {
|
||||||
|
@ -314,16 +310,6 @@ def check_modeline(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, st
|
||||||
yield (idx + 1, "emacs file variables present")
|
yield (idx + 1, "emacs file variables present")
|
||||||
|
|
||||||
|
|
||||||
def check_length(file_name: str, idx: int, line: bytes) -> Iterator[tuple[int, str]]:
|
|
||||||
if any(file_name.endswith(ext) for ext in (".lock", ".json", ".html", ".toml")) or config["skip-check-length"]:
|
|
||||||
return
|
|
||||||
|
|
||||||
# Prefer shorter lines when shell scripting.
|
|
||||||
max_length = 80 if file_name.endswith(".sh") else 120
|
|
||||||
if len(line.decode("utf-8").rstrip("\n")) > max_length and not is_unsplittable(file_name, line):
|
|
||||||
yield (idx + 1, "Line is longer than %d characters" % max_length)
|
|
||||||
|
|
||||||
|
|
||||||
def contains_url(line: bytes) -> bool:
|
def contains_url(line: bytes) -> bool:
|
||||||
return bool(URL_REGEX.search(line))
|
return bool(URL_REGEX.search(line))
|
||||||
|
|
||||||
|
@ -391,7 +377,6 @@ def check_for_raw_urls_in_rustdoc(file_name: str, idx: int, line: bytes) -> Iter
|
||||||
def check_by_line(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]:
|
def check_by_line(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]:
|
||||||
for idx, line in enumerate(lines):
|
for idx, line in enumerate(lines):
|
||||||
errors = itertools.chain(
|
errors = itertools.chain(
|
||||||
check_length(file_name, idx, line),
|
|
||||||
check_whitespace(idx, line),
|
check_whitespace(idx, line),
|
||||||
check_whatwg_specific_url(idx, line),
|
check_whatwg_specific_url(idx, line),
|
||||||
check_whatwg_single_page_url(idx, line),
|
check_whatwg_single_page_url(idx, line),
|
||||||
|
@ -572,8 +557,6 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]:
|
||||||
import_block = False
|
import_block = False
|
||||||
whitespace = False
|
whitespace = False
|
||||||
|
|
||||||
is_lib_rs_file = file_name.endswith("lib.rs")
|
|
||||||
|
|
||||||
PANIC_NOT_ALLOWED_PATHS = [
|
PANIC_NOT_ALLOWED_PATHS = [
|
||||||
os.path.join("*", "components", "compositing", "compositor.rs"),
|
os.path.join("*", "components", "compositing", "compositor.rs"),
|
||||||
os.path.join("*", "components", "constellation", "*"),
|
os.path.join("*", "components", "constellation", "*"),
|
||||||
|
@ -586,20 +569,12 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]:
|
||||||
|
|
||||||
prev_open_brace = False
|
prev_open_brace = False
|
||||||
multi_line_string = False
|
multi_line_string = False
|
||||||
prev_mod: dict[int, str] = {}
|
|
||||||
prev_feature_name = ""
|
|
||||||
indent = 0
|
|
||||||
|
|
||||||
check_alphabetical_order = config["check-alphabetical-order"]
|
|
||||||
decl_message = "{} is not in alphabetical order"
|
|
||||||
decl_expected = "\n\t\033[93mexpected: {}\033[0m"
|
|
||||||
decl_found = "\n\t\033[91mfound: {}\033[0m"
|
|
||||||
panic_message = "unwrap() or panic!() found in code which should not panic."
|
panic_message = "unwrap() or panic!() found in code which should not panic."
|
||||||
|
|
||||||
for idx, original_line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
|
for idx, original_line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
|
||||||
# simplify the analysis
|
# simplify the analysis
|
||||||
line = original_line.strip()
|
line = original_line.strip()
|
||||||
indent = len(original_line) - len(line)
|
|
||||||
|
|
||||||
is_attribute = re.search(r"#\[.*\]", line)
|
is_attribute = re.search(r"#\[.*\]", line)
|
||||||
is_comment = re.search(r"^//|^/\*|^\*", line)
|
is_comment = re.search(r"^//|^/\*|^\*", line)
|
||||||
|
@ -685,78 +660,11 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]:
|
||||||
yield (idx + 1, "found an empty line following a {")
|
yield (idx + 1, "found an empty line following a {")
|
||||||
prev_open_brace = line.endswith("{")
|
prev_open_brace = line.endswith("{")
|
||||||
|
|
||||||
# check alphabetical order of feature attributes in lib.rs files
|
|
||||||
if is_lib_rs_file:
|
|
||||||
match = re.search(r"#!\[feature\((.*)\)\]", line)
|
|
||||||
|
|
||||||
if match:
|
|
||||||
features = list(map(lambda w: w.strip(), match.group(1).split(",")))
|
|
||||||
sorted_features = sorted(features)
|
|
||||||
if sorted_features != features and check_alphabetical_order:
|
|
||||||
yield (
|
|
||||||
idx + 1,
|
|
||||||
decl_message.format("feature attribute")
|
|
||||||
+ decl_expected.format(tuple(sorted_features))
|
|
||||||
+ decl_found.format(tuple(features)),
|
|
||||||
)
|
|
||||||
|
|
||||||
if prev_feature_name > sorted_features[0] and check_alphabetical_order:
|
|
||||||
yield (
|
|
||||||
idx + 1,
|
|
||||||
decl_message.format("feature attribute")
|
|
||||||
+ decl_expected.format(prev_feature_name + " after " + sorted_features[0])
|
|
||||||
+ decl_found.format(prev_feature_name + " before " + sorted_features[0]),
|
|
||||||
)
|
|
||||||
|
|
||||||
prev_feature_name = sorted_features[0]
|
|
||||||
else:
|
|
||||||
# not a feature attribute line, so empty previous name
|
|
||||||
prev_feature_name = ""
|
|
||||||
|
|
||||||
if is_panic_not_allowed_rs_file:
|
if is_panic_not_allowed_rs_file:
|
||||||
match = re.search(r"unwrap\(|panic!\(", line)
|
match = re.search(r"unwrap\(|panic!\(", line)
|
||||||
if match:
|
if match:
|
||||||
yield (idx + 1, panic_message)
|
yield (idx + 1, panic_message)
|
||||||
|
|
||||||
# modules must be in the same line and alphabetically sorted
|
|
||||||
if line.startswith("mod ") or line.startswith("pub mod "):
|
|
||||||
# strip /(pub )?mod/ from the left and ";" from the right
|
|
||||||
mod = line[4:-1] if line.startswith("mod ") else line[8:-1]
|
|
||||||
|
|
||||||
if (idx - 1) < 0 or "#[macro_use]" not in lines[idx - 1].decode("utf-8"):
|
|
||||||
match = line.find(" {")
|
|
||||||
if indent not in prev_mod:
|
|
||||||
prev_mod[indent] = ""
|
|
||||||
if match == -1 and not line.endswith(";"):
|
|
||||||
yield (idx + 1, "mod declaration spans multiple lines")
|
|
||||||
if prev_mod[indent] and mod < prev_mod[indent] and check_alphabetical_order:
|
|
||||||
yield (
|
|
||||||
idx + 1,
|
|
||||||
decl_message.format("mod declaration")
|
|
||||||
+ decl_expected.format(prev_mod[indent])
|
|
||||||
+ decl_found.format(mod),
|
|
||||||
)
|
|
||||||
prev_mod[indent] = mod
|
|
||||||
else:
|
|
||||||
# we now erase previous entries
|
|
||||||
prev_mod = {}
|
|
||||||
|
|
||||||
# derivable traits should be alphabetically ordered
|
|
||||||
if is_attribute:
|
|
||||||
# match the derivable traits filtering out macro expansions
|
|
||||||
match = re.search(r"#\[derive\(([a-zA-Z, ]*)", line)
|
|
||||||
if match:
|
|
||||||
derives = list(map(lambda w: w.strip(), match.group(1).split(",")))
|
|
||||||
# sort, compare and report
|
|
||||||
sorted_derives = sorted(derives)
|
|
||||||
if sorted_derives != derives and check_alphabetical_order:
|
|
||||||
yield (
|
|
||||||
idx + 1,
|
|
||||||
decl_message.format("derivable traits list")
|
|
||||||
+ decl_expected.format(", ".join(sorted_derives))
|
|
||||||
+ decl_found.format(", ".join(derives)),
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
# Avoid flagging <Item=Foo> constructs
|
# Avoid flagging <Item=Foo> constructs
|
||||||
def is_associated_type(match, line):
|
def is_associated_type(match, line):
|
||||||
|
|
|
@ -1,7 +1,5 @@
|
||||||
[configs]
|
[configs]
|
||||||
skip-check-length = false
|
|
||||||
skip-check-licenses = false
|
skip-check-licenses = false
|
||||||
check-alphabetical-order = true
|
|
||||||
|
|
||||||
[ignore]
|
[ignore]
|
||||||
# Files that are ignored for all tidy and lint checks.
|
# Files that are ignored for all tidy and lint checks.
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue