mirror of
https://github.com/servo/servo.git
synced 2025-08-02 20:20:14 +01:00
Removed unused function and minor cleanup
This commit is contained in:
parent
033786cd0d
commit
dfe32b0ada
2 changed files with 64 additions and 77 deletions
|
@ -19,19 +19,21 @@ import subprocess
|
||||||
import sys
|
import sys
|
||||||
from licenseck import licenses
|
from licenseck import licenses
|
||||||
|
|
||||||
|
# License and header checks
|
||||||
|
EMACS_HEADER = "/* -*- Mode:"
|
||||||
|
VIM_HEADER = "/* vim:"
|
||||||
|
MAX_LICENSE_LINESPAN = max(len(license.splitlines()) for license in licenses)
|
||||||
|
|
||||||
# File patterns to include in the non-WPT tidy check.
|
# File patterns to include in the non-WPT tidy check.
|
||||||
file_patterns_to_check = ["*.rs", "*.rc", "*.cpp", "*.c",
|
FILE_PATTERNS_TO_CHECK = ["*.rs", "*.rc", "*.cpp", "*.c",
|
||||||
"*.h", "Cargo.lock", "*.py",
|
"*.h", "Cargo.lock", "*.py",
|
||||||
"*.toml", "*.webidl", "*.json"]
|
"*.toml", "*.webidl", "*.json"]
|
||||||
|
|
||||||
# File patterns that are ignored for all tidy and lint checks.
|
# File patterns that are ignored for all tidy and lint checks.
|
||||||
file_patterns_to_ignore = [
|
FILE_PATTERNS_TO_IGNORE = ["*.#*", "*.pyc"]
|
||||||
"*.#*",
|
|
||||||
"*.pyc",
|
|
||||||
]
|
|
||||||
|
|
||||||
# Files that are ignored for all tidy and lint checks.
|
# Files that are ignored for all tidy and lint checks.
|
||||||
ignored_files = [
|
IGNORED_FILES = [
|
||||||
# Generated and upstream code combined with our own. Could use cleanup
|
# Generated and upstream code combined with our own. Could use cleanup
|
||||||
os.path.join(".", "ports", "geckolib", "gecko_bindings", "bindings.rs"),
|
os.path.join(".", "ports", "geckolib", "gecko_bindings", "bindings.rs"),
|
||||||
os.path.join(".", "ports", "geckolib", "gecko_bindings", "structs_debug.rs"),
|
os.path.join(".", "ports", "geckolib", "gecko_bindings", "structs_debug.rs"),
|
||||||
|
@ -49,7 +51,7 @@ ignored_files = [
|
||||||
]
|
]
|
||||||
|
|
||||||
# Directories that are ignored for the non-WPT tidy check.
|
# Directories that are ignored for the non-WPT tidy check.
|
||||||
ignored_dirs = [
|
IGNORED_DIRS = [
|
||||||
# Upstream
|
# Upstream
|
||||||
os.path.join(".", "support", "android", "apk"),
|
os.path.join(".", "support", "android", "apk"),
|
||||||
os.path.join(".", "support", "rust-task_info"),
|
os.path.join(".", "support", "rust-task_info"),
|
||||||
|
@ -74,7 +76,31 @@ ignored_dirs = [
|
||||||
os.path.join(".", "."),
|
os.path.join(".", "."),
|
||||||
]
|
]
|
||||||
|
|
||||||
spec_base_path = "components/script/dom/"
|
SPEC_BASE_PATH = "components/script/dom/"
|
||||||
|
|
||||||
|
WEBIDL_STANDARDS = [
|
||||||
|
"//www.khronos.org/registry/webgl/specs",
|
||||||
|
"//developer.mozilla.org/en-US/docs/Web/API",
|
||||||
|
"//dev.w3.org/2006/webapi",
|
||||||
|
"//dev.w3.org/csswg",
|
||||||
|
"//dev.w3.org/fxtf",
|
||||||
|
"//dvcs.w3.org/hg",
|
||||||
|
"//dom.spec.whatwg.org",
|
||||||
|
"//domparsing.spec.whatwg.org",
|
||||||
|
"//drafts.csswg.org/cssom",
|
||||||
|
"//drafts.fxtf.org",
|
||||||
|
"//encoding.spec.whatwg.org",
|
||||||
|
"//html.spec.whatwg.org",
|
||||||
|
"//url.spec.whatwg.org",
|
||||||
|
"//xhr.spec.whatwg.org",
|
||||||
|
"//w3c.github.io",
|
||||||
|
"//heycam.github.io/webidl",
|
||||||
|
"//webbluetoothcg.github.io/web-bluetooth/",
|
||||||
|
"//slightlyoff.github.io/ServiceWorker/spec/service_worker/",
|
||||||
|
# Not a URL
|
||||||
|
"// This interface is entirely internal to Servo, and should not be" +
|
||||||
|
" accessible to\n// web pages."
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
def is_iter_empty(iterator):
|
def is_iter_empty(iterator):
|
||||||
|
@ -97,16 +123,16 @@ def progress_wrapper(iterator):
|
||||||
|
|
||||||
|
|
||||||
def filter_file(file_name):
|
def filter_file(file_name):
|
||||||
if any(file_name.startswith(ignored_file) for ignored_file in ignored_files):
|
if any(file_name.startswith(ignored_file) for ignored_file in IGNORED_FILES):
|
||||||
return False
|
return False
|
||||||
base_name = os.path.basename(file_name)
|
base_name = os.path.basename(file_name)
|
||||||
if any(fnmatch.fnmatch(base_name, pattern) for pattern in file_patterns_to_ignore):
|
if any(fnmatch.fnmatch(base_name, pattern) for pattern in FILE_PATTERNS_TO_IGNORE):
|
||||||
return False
|
return False
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
def filter_files(start_dir, only_changed_files, progress):
|
def filter_files(start_dir, only_changed_files, progress):
|
||||||
file_iter = get_file_list(start_dir, only_changed_files, ignored_dirs)
|
file_iter = get_file_list(start_dir, only_changed_files, IGNORED_DIRS)
|
||||||
(has_element, file_iter) = is_iter_empty(file_iter)
|
(has_element, file_iter) = is_iter_empty(file_iter)
|
||||||
if not has_element:
|
if not has_element:
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
|
@ -114,18 +140,13 @@ def filter_files(start_dir, only_changed_files, progress):
|
||||||
file_iter = progress_wrapper(file_iter)
|
file_iter = progress_wrapper(file_iter)
|
||||||
for file_name in file_iter:
|
for file_name in file_iter:
|
||||||
base_name = os.path.basename(file_name)
|
base_name = os.path.basename(file_name)
|
||||||
if not any(fnmatch.fnmatch(base_name, pattern) for pattern in file_patterns_to_check):
|
if not any(fnmatch.fnmatch(base_name, pattern) for pattern in FILE_PATTERNS_TO_CHECK):
|
||||||
continue
|
continue
|
||||||
if not filter_file(file_name):
|
if not filter_file(file_name):
|
||||||
continue
|
continue
|
||||||
yield file_name
|
yield file_name
|
||||||
|
|
||||||
|
|
||||||
EMACS_HEADER = "/* -*- Mode:"
|
|
||||||
VIM_HEADER = "/* vim:"
|
|
||||||
MAX_LICENSE_LINESPAN = max(len(license.splitlines()) for license in licenses)
|
|
||||||
|
|
||||||
|
|
||||||
def check_license(file_name, lines):
|
def check_license(file_name, lines):
|
||||||
if any(file_name.endswith(ext) for ext in (".toml", ".lock", ".json")):
|
if any(file_name.endswith(ext) for ext in (".toml", ".lock", ".json")):
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
|
@ -273,13 +294,6 @@ duplicate versions for package "{package}"
|
||||||
yield (1, message)
|
yield (1, message)
|
||||||
|
|
||||||
|
|
||||||
def maybe_int(value):
|
|
||||||
try:
|
|
||||||
return int(value)
|
|
||||||
except ValueError:
|
|
||||||
return value
|
|
||||||
|
|
||||||
|
|
||||||
def check_toml(file_name, lines):
|
def check_toml(file_name, lines):
|
||||||
if not file_name.endswith(".toml"):
|
if not file_name.endswith(".toml"):
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
|
@ -295,9 +309,9 @@ def check_rust(file_name, lines):
|
||||||
file_name.endswith(os.path.join("geckolib", "build.rs")) or \
|
file_name.endswith(os.path.join("geckolib", "build.rs")) or \
|
||||||
file_name.endswith(os.path.join("unit", "style", "stylesheets.rs")):
|
file_name.endswith(os.path.join("unit", "style", "stylesheets.rs")):
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
|
|
||||||
comment_depth = 0
|
comment_depth = 0
|
||||||
merged_lines = ''
|
merged_lines = ''
|
||||||
|
|
||||||
import_block = False
|
import_block = False
|
||||||
whitespace = False
|
whitespace = False
|
||||||
|
|
||||||
|
@ -314,6 +328,8 @@ def check_rust(file_name, lines):
|
||||||
for idx, original_line in enumerate(lines):
|
for idx, original_line in enumerate(lines):
|
||||||
# simplify the analysis
|
# simplify the analysis
|
||||||
line = original_line.strip()
|
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.
|
# Simple heuristic to avoid common case of no comments.
|
||||||
if '/' in line:
|
if '/' in line:
|
||||||
|
@ -331,17 +347,17 @@ def check_rust(file_name, lines):
|
||||||
merged_lines = ''
|
merged_lines = ''
|
||||||
|
|
||||||
# Keep track of whitespace to enable checking for a merged import block
|
# Keep track of whitespace to enable checking for a merged import block
|
||||||
#
|
|
||||||
# Ignore attributes, comments, and imports
|
# Ignore attributes, comments, and imports
|
||||||
if import_block:
|
if import_block:
|
||||||
if not (line_is_comment(line) or line_is_attribute(line) or line.startswith("use ")):
|
if not (is_comment or is_attribute or line.startswith("use ")):
|
||||||
whitespace = line == ""
|
whitespace = line == ""
|
||||||
|
|
||||||
if not whitespace:
|
if not whitespace:
|
||||||
import_block = False
|
import_block = False
|
||||||
|
|
||||||
# get rid of strings and chars because cases like regex expression, keep attributes
|
# get rid of strings and chars because cases like regex expression, keep attributes
|
||||||
if not line_is_attribute(line):
|
if not is_attribute:
|
||||||
line = re.sub(r'"(\\.|[^\\"])*?"', '""', line)
|
line = re.sub(r'"(\\.|[^\\"])*?"', '""', line)
|
||||||
line = re.sub(r"'(\\.|[^\\'])*?'", "''", line)
|
line = re.sub(r"'(\\.|[^\\'])*?'", "''", line)
|
||||||
|
|
||||||
|
@ -355,21 +371,22 @@ def check_rust(file_name, lines):
|
||||||
# tuple format: (pattern, format_message, filter_function(match, line))
|
# tuple format: (pattern, format_message, filter_function(match, line))
|
||||||
no_filter = lambda match, line: True
|
no_filter = lambda match, line: True
|
||||||
regex_rules = [
|
regex_rules = [
|
||||||
(r",[^\s]", "missing space after ,", lambda match, line: '$' not in line),
|
(r",[^\s]", "missing space after ,",
|
||||||
|
lambda match, line: '$' not in line and not is_attribute),
|
||||||
(r"[A-Za-z0-9\"]=", "missing space before =",
|
(r"[A-Za-z0-9\"]=", "missing space before =",
|
||||||
lambda match, line: line_is_attribute(line)),
|
lambda match, line: is_attribute),
|
||||||
(r"=[A-Za-z0-9\"]", "missing space after =",
|
(r"=[A-Za-z0-9\"]", "missing space after =",
|
||||||
lambda match, line: line_is_attribute(line)),
|
lambda match, line: is_attribute),
|
||||||
# ignore scientific notation patterns like 1e-6
|
# ignore scientific notation patterns like 1e-6
|
||||||
(r"[A-DF-Za-df-z0-9]-", "missing space before -",
|
(r"[A-DF-Za-df-z0-9]-", "missing space before -",
|
||||||
lambda match, line: not line_is_attribute(line)),
|
lambda match, line: not is_attribute),
|
||||||
(r"[A-Za-z0-9]([\+/\*%=])", "missing space before {0}",
|
(r"[A-Za-z0-9]([\+/\*%=])", "missing space before {0}",
|
||||||
lambda match, line: (not line_is_attribute(line) and
|
lambda match, line: (not is_attribute and
|
||||||
not is_associated_type(match, line))),
|
not is_associated_type(match, line))),
|
||||||
# * not included because of dereferencing and casting
|
# * not included because of dereferencing and casting
|
||||||
# - not included because of unary negation
|
# - not included because of unary negation
|
||||||
(r'([\+/\%=])[A-Za-z0-9"]', "missing space after {0}",
|
(r'([\+/\%=])[A-Za-z0-9"]', "missing space after {0}",
|
||||||
lambda match, line: (not line_is_attribute(line) and
|
lambda match, line: (not is_attribute and
|
||||||
not is_associated_type(match, line))),
|
not is_associated_type(match, line))),
|
||||||
(r"\)->", "missing space before ->", no_filter),
|
(r"\)->", "missing space before ->", no_filter),
|
||||||
(r"->[A-Za-z]", "missing space after ->", no_filter),
|
(r"->[A-Za-z]", "missing space after ->", no_filter),
|
||||||
|
@ -402,10 +419,8 @@ def check_rust(file_name, lines):
|
||||||
|
|
||||||
for pattern, message, filter_func in regex_rules:
|
for pattern, message, filter_func in regex_rules:
|
||||||
for match in re.finditer(pattern, line):
|
for match in re.finditer(pattern, line):
|
||||||
if not filter_func(match, line):
|
if filter_func(match, line):
|
||||||
continue
|
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:
|
if prev_open_brace and not line:
|
||||||
yield (idx + 1, "found an empty line following a {")
|
yield (idx + 1, "found an empty line following a {")
|
||||||
|
@ -481,14 +496,6 @@ def is_associated_type(match, line):
|
||||||
return generic_open and generic_close
|
return generic_open and generic_close
|
||||||
|
|
||||||
|
|
||||||
def line_is_attribute(line):
|
|
||||||
return re.search(r"#\[.*\]", line)
|
|
||||||
|
|
||||||
|
|
||||||
def line_is_comment(line):
|
|
||||||
return re.search(r"^//|^/\*|^\*", line)
|
|
||||||
|
|
||||||
|
|
||||||
def check_webidl_spec(file_name, contents):
|
def check_webidl_spec(file_name, contents):
|
||||||
# Sorted by this function (in pseudo-Rust). The idea is to group the same
|
# Sorted by this function (in pseudo-Rust). The idea is to group the same
|
||||||
# organization together.
|
# organization together.
|
||||||
|
@ -508,35 +515,14 @@ def check_webidl_spec(file_name, contents):
|
||||||
# }
|
# }
|
||||||
# a_domain.path().cmp(b_domain.path())
|
# a_domain.path().cmp(b_domain.path())
|
||||||
# }
|
# }
|
||||||
|
|
||||||
if not file_name.endswith(".webidl"):
|
if not file_name.endswith(".webidl"):
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
standards = [
|
|
||||||
"//www.khronos.org/registry/webgl/specs",
|
for i in WEBIDL_STANDARDS:
|
||||||
"//developer.mozilla.org/en-US/docs/Web/API",
|
|
||||||
"//dev.w3.org/2006/webapi",
|
|
||||||
"//dev.w3.org/csswg",
|
|
||||||
"//dev.w3.org/fxtf",
|
|
||||||
"//dvcs.w3.org/hg",
|
|
||||||
"//dom.spec.whatwg.org",
|
|
||||||
"//domparsing.spec.whatwg.org",
|
|
||||||
"//drafts.csswg.org/cssom",
|
|
||||||
"//drafts.fxtf.org",
|
|
||||||
"//encoding.spec.whatwg.org",
|
|
||||||
"//html.spec.whatwg.org",
|
|
||||||
"//url.spec.whatwg.org",
|
|
||||||
"//xhr.spec.whatwg.org",
|
|
||||||
"//w3c.github.io",
|
|
||||||
"//heycam.github.io/webidl",
|
|
||||||
"//webbluetoothcg.github.io/web-bluetooth/",
|
|
||||||
"//slightlyoff.github.io/ServiceWorker/spec/service_worker/",
|
|
||||||
# Not a URL
|
|
||||||
"// This interface is entirely internal to Servo, and should not be" +
|
|
||||||
" accessible to\n// web pages."
|
|
||||||
]
|
|
||||||
for i in standards:
|
|
||||||
if contents.find(i) != -1:
|
if contents.find(i) != -1:
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
yield 0, "No specification link found."
|
yield (0, "No specification link found.")
|
||||||
|
|
||||||
|
|
||||||
def check_json(filename, contents):
|
def check_json(filename, contents):
|
||||||
|
@ -552,9 +538,9 @@ def check_json(filename, contents):
|
||||||
|
|
||||||
|
|
||||||
def check_spec(file_name, lines):
|
def check_spec(file_name, lines):
|
||||||
if spec_base_path not in file_name:
|
if SPEC_BASE_PATH not in file_name:
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
file_name = os.path.relpath(os.path.splitext(file_name)[0], spec_base_path)
|
file_name = os.path.relpath(os.path.splitext(file_name)[0], SPEC_BASE_PATH)
|
||||||
patt = re.compile("^\s*\/\/.+")
|
patt = re.compile("^\s*\/\/.+")
|
||||||
|
|
||||||
# Pattern representing a line with a macro
|
# Pattern representing a line with a macro
|
||||||
|
@ -566,9 +552,10 @@ def check_spec(file_name, lines):
|
||||||
# Pattern representing a line with comment
|
# Pattern representing a line with comment
|
||||||
comment_patt = re.compile("^\s*///?.+$")
|
comment_patt = re.compile("^\s*///?.+$")
|
||||||
|
|
||||||
pattern = "impl {}Methods for {} {{".format(file_name, file_name)
|
|
||||||
brace_count = 0
|
brace_count = 0
|
||||||
in_impl = False
|
in_impl = False
|
||||||
|
pattern = "impl {}Methods for {} {{".format(file_name, file_name)
|
||||||
|
|
||||||
for idx, line in enumerate(lines):
|
for idx, line in enumerate(lines):
|
||||||
if "// check-tidy: no specs after this line" in line:
|
if "// check-tidy: no specs after this line" in line:
|
||||||
break
|
break
|
||||||
|
@ -599,10 +586,10 @@ def collect_errors_for_files(files_to_check, checking_functions, line_checking_f
|
||||||
raise StopIteration
|
raise StopIteration
|
||||||
if print_text:
|
if print_text:
|
||||||
print '\rChecking files for tidiness...'
|
print '\rChecking files for tidiness...'
|
||||||
|
|
||||||
for filename in files_to_check:
|
for filename in files_to_check:
|
||||||
if not os.path.exists(filename):
|
if not os.path.exists(filename):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
with open(filename, "r") as f:
|
with open(filename, "r") as f:
|
||||||
contents = f.read()
|
contents = f.read()
|
||||||
for check in checking_functions:
|
for check in checking_functions:
|
||||||
|
@ -650,7 +637,7 @@ def get_file_list(directory, only_changed_files=False, exclude_dirs=[]):
|
||||||
args = ["git", "ls-files", "--others", "--exclude-standard", directory]
|
args = ["git", "ls-files", "--others", "--exclude-standard", directory]
|
||||||
file_list += subprocess.check_output(args)
|
file_list += subprocess.check_output(args)
|
||||||
for f in file_list.splitlines():
|
for f in file_list.splitlines():
|
||||||
if os.path.join('.', os.path.dirname(f)) not in ignored_dirs:
|
if os.path.join('.', os.path.dirname(f)) not in exclude_dirs:
|
||||||
yield os.path.join('.', f)
|
yield os.path.join('.', f)
|
||||||
elif exclude_dirs:
|
elif exclude_dirs:
|
||||||
for root, dirs, files in os.walk(directory, topdown=True):
|
for root, dirs, files in os.walk(directory, topdown=True):
|
||||||
|
|
|
@ -79,7 +79,7 @@ class CheckTidiness(unittest.TestCase):
|
||||||
self.assertNoMoreErrors(errors)
|
self.assertNoMoreErrors(errors)
|
||||||
|
|
||||||
def test_spec_link(self):
|
def test_spec_link(self):
|
||||||
tidy.spec_base_path = base_path
|
tidy.SPEC_BASE_PATH = base_path
|
||||||
errors = tidy.collect_errors_for_files(iterFile('speclink.rs'), [], [tidy.check_spec], print_text=False)
|
errors = tidy.collect_errors_for_files(iterFile('speclink.rs'), [], [tidy.check_spec], print_text=False)
|
||||||
self.assertEqual('method declared in webidl is missing a comment with a specification link', errors.next()[2])
|
self.assertEqual('method declared in webidl is missing a comment with a specification link', errors.next()[2])
|
||||||
self.assertNoMoreErrors(errors)
|
self.assertNoMoreErrors(errors)
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue