1952 lines
72 KiB
Python
Executable File
1952 lines
72 KiB
Python
Executable File
#!/usr/bin/env python3
|
||
|
||
"""
|
||
Runs custom linting on our code.
|
||
|
||
Adding "NOLINT" to any line makes the linter ignore that line. Adding a pair of "NOLINT_START" and "NOLINT_END" makes
|
||
the linter ignore these lines, as well as all lines in between.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import argparse
|
||
import os
|
||
import re
|
||
import sys
|
||
from pathlib import Path
|
||
from typing import Any
|
||
|
||
import git
|
||
from ci.frontmatter import load_frontmatter
|
||
from gitignore_parser import parse_gitignore
|
||
|
||
# -----------------------------------------------------------------------------
|
||
|
||
# Path prefix from git root to the rerun directory.
|
||
# "./rerun/" in the monorepo (reality), "./" in the standalone rerun repo.
|
||
# Set in main().
|
||
rerun_prefix = "./"
|
||
|
||
debug_format_of_err = re.compile(r"\{\:#?\?\}.*, \w*err")
|
||
error_match_name = re.compile(r"Err\((\w+)\)")
|
||
error_map_err_name = re.compile(r"map_err\(\|(\w+)\|")
|
||
|
||
# Detect log macros with inline sensitive data (URLs, paths) in the format string.
|
||
# Sensitive data should be passed as structured fields instead.
|
||
# Bad: re_log::warn!("Failed to open URL {url}: {err}");
|
||
# Good: re_log::warn!(?url, "Failed to open URL: {err}");
|
||
log_with_inline_sensitive_data = re.compile(
|
||
r're_log::(error|warn)(_once)?!\s*\(\s*"[^"]*\{(url|path|filepath|file_path|uri|file|filename|dir|directory|folder)[^}]*\}'
|
||
)
|
||
|
||
# Detect thiserror #[error(...)] with multiple unnamed tuple fields like {0}, {1}
|
||
# Bad: #[error("Failed to do {0}: {1}")]
|
||
# Good: #[error("Failed to do {path}: {err}")]
|
||
thiserror_multiple_unnamed = re.compile(r"#\[error\(.*\{1")
|
||
wasm_caps = re.compile(r"\bWASM\b")
|
||
nb_prefix = re.compile(r"nb_")
|
||
else_return = re.compile(r"else\s*{\s*return;?\s*};")
|
||
# Looks for: \"{foo}\" (manual quotes), including \"{foo:?}\" (excess quotes).
|
||
explicit_quotes = re.compile(r'[^(]\\"\{[^}]*\}\\"')
|
||
ellipsis = re.compile(r"[^.]\.\.\.([^\-.0-9a-zA-Z]|$)")
|
||
ellipsis_expression = re.compile(r"[\[\]\(\)<>\{\}]?.*\.\.\..*[\[\]\(\)<>\{\}]")
|
||
ellipsis_import = re.compile(r"from \.\.\.")
|
||
ellipsis_reference = re.compile(r"&\.\.\.")
|
||
ellipsis_bare = re.compile(r"^\s*\.\.\.\s*$")
|
||
|
||
anyhow_result = re.compile(r"Result<.*, anyhow::Error>")
|
||
tonic_result = re.compile(r"Result<.*?,\s*tonic::Status\s*,?\s*>", re.DOTALL)
|
||
pyclass_start = re.compile(r"#\[pyclass\(")
|
||
pymethods_start = re.compile(r"#\[pymethods\]")
|
||
|
||
double_the = re.compile(r"\bthe the\b")
|
||
double_word = re.compile(r" ([a-z]+) \1[ \.]")
|
||
|
||
# reStructuredText syntax that we don't want in Python docstrings.
|
||
# Our Python API docs use MkDocs + mkdocstrings (not Sphinx), so rST isn't rendered.
|
||
# See `CLAUDE.md` → "Python docstring formatting".
|
||
rst_role = re.compile(r":(class|meth|func|mod|attr|exc|data|const|obj|ref):`")
|
||
rst_directive = re.compile(
|
||
r"\.\.\s+(warning|note|deprecated|code-block|seealso|versionadded|versionchanged|admonition|caution|danger|hint|important|tip|attention|toctree|automodule|autoclass|autofunction)::"
|
||
)
|
||
# rST inline literal — ``code``. Markdown uses single backticks.
|
||
# Avoid matching triple-backtick code fences.
|
||
rst_double_backtick = re.compile(r"(?<!`)``[^`\n]+``(?!`)")
|
||
|
||
Frontmatter = dict[str, Any]
|
||
|
||
|
||
def get_rerun_root() -> str:
|
||
# Search upward for .RERUN_ROOT sentinel file
|
||
# TODO(RR-3355): Use a shared utility for this
|
||
current = Path(__file__).resolve().parent
|
||
while current != current.parent:
|
||
# Look for sentinel file
|
||
if (current / ".RERUN_ROOT").exists():
|
||
return str(current)
|
||
# Break if we reach a git root
|
||
if (current / ".git").exists():
|
||
break
|
||
current = current.parent
|
||
raise FileNotFoundError(f"Could not find .RERUN_ROOT sentinel file in any parent directory under {current}")
|
||
|
||
|
||
def is_valid_todo_part(part: str) -> bool:
|
||
part = part.strip()
|
||
|
||
if re.match(r"^[\w/-]*#\d+$", part):
|
||
return True # org/repo#42 or #42
|
||
|
||
if part.lower() in ("agent", "claude", "codex", "llm"):
|
||
return False # coding agent, not a person
|
||
|
||
if re.match(r"^[a-z][a-z0-9_]+$", part):
|
||
return True # user-name
|
||
|
||
return bool(re.match(r"^RR-\d+$", part)) # linear issue
|
||
|
||
|
||
def check_string(s: str) -> str | None:
|
||
"""Check that the string has the correct casing."""
|
||
if len(s) == 0:
|
||
return None
|
||
|
||
bad_titles = [
|
||
"Blueprint",
|
||
"Class",
|
||
"Container",
|
||
"Entity",
|
||
"EntityPath",
|
||
"Epoch",
|
||
"Instance",
|
||
"Path",
|
||
"Recording",
|
||
"Result",
|
||
"Space",
|
||
"Store",
|
||
"View",
|
||
"Viewport",
|
||
]
|
||
|
||
if m := re.search(r"[^.] ([A-Z]\w+)", s):
|
||
word = m.group(1)
|
||
if word in bad_titles:
|
||
return f"Do not use title casing ({word}). See https://github.com/rerun-io/rerun/blob/main/DESIGN.md"
|
||
|
||
return None
|
||
|
||
|
||
def lint_url(url: str) -> str | None:
|
||
ALLOW_LIST_URLS = {
|
||
"https://github.com/lycheeverse/lychee/blob/master/lychee.example.toml",
|
||
"https://github.com/rerun-io/documentation/blob/main/src/utils/tokens.ts",
|
||
"https://github.com/rerun-io/landing/blob/main/src/lib/lang.ts", # if this file moves we should check the linked code.
|
||
"https://github.com/rerun-io/platform-operator/blob/main/README-tech-overview.md",
|
||
"https://github.com/rerun-io/rerun/blob/main/ARCHITECTURE.md",
|
||
"https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md",
|
||
"https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md",
|
||
"https://github.com/rerun-io/rerun/blob/main/LICENSE-APACHE",
|
||
"https://github.com/rerun-io/rerun/blob/main/LICENSE-MIT",
|
||
}
|
||
|
||
if url in ALLOW_LIST_URLS:
|
||
return None
|
||
|
||
if m := re.match(r"https://github.com/.*/blob/(\w+)/.*", url):
|
||
branch = m.group(1)
|
||
if branch in ("main", "master", "trunk", "latest"):
|
||
if "#L" in url:
|
||
return f"Do not link directly to a file:line on '{branch}' - it may change! Use a perma-link instead (commit hash or tag). Url: {url}"
|
||
|
||
if "/README.md" in url:
|
||
pass # Probably fine
|
||
elif url.startswith("https://github.com/rerun-io/rerun/blob/"):
|
||
pass # TODO(#6077): figure out how we best link to our own code from our docs
|
||
elif url.startswith("https://github.com/anthropics/claude-code-action"):
|
||
pass
|
||
else:
|
||
return f"Do not link directly to a file on '{branch}' - it may disappear! Use a commit hash or tag instead. Url: {url}"
|
||
|
||
return None
|
||
|
||
|
||
def lint_line(
|
||
line: str,
|
||
prev_line: str | None,
|
||
file_extension: str = "rs",
|
||
is_in_docstring: bool = False,
|
||
is_in_oss_rerun_repo: bool = True,
|
||
) -> str | None:
|
||
if line == "":
|
||
return None
|
||
|
||
if prev_line is None:
|
||
prev_line_stripped = ""
|
||
else:
|
||
prev_line_stripped = prev_line.strip()
|
||
|
||
if line[-1].isspace():
|
||
return "Trailing whitespace"
|
||
|
||
if "NOLINT" in line:
|
||
return None # NOLINT ignores the linter
|
||
|
||
if file_extension not in ("py", "txt", "yaml", "yml"):
|
||
if "Github" in line:
|
||
return "It's 'GitHub', not 'Github'"
|
||
|
||
if " github " in line:
|
||
return "It's 'GitHub', not 'github'"
|
||
|
||
if re.search(r"[.a-zA-Z] [a-zA-Z]", line):
|
||
if r"\n " not in line: # Allow `\n `, which happens e.g. when markdown is embedded in a string
|
||
return "Found double space"
|
||
|
||
if double_the.search(line.lower()):
|
||
return "Found 'the the'"
|
||
|
||
if m := double_word.search(line):
|
||
return f"Found double word: '{m.group(0)}'"
|
||
|
||
if m := re.search(r'https?://[^ )"]+', line):
|
||
url = m.group(0)
|
||
if err := lint_url(url):
|
||
return err
|
||
|
||
if file_extension != "" and is_in_oss_rerun_repo:
|
||
# We lint against writing ellipsis using three dots for the sake of our UI:
|
||
# * We want it consistent
|
||
# * We want it beautiful (`…` looks different from `...`)
|
||
# * We don't want linebreaks in the middle of an ellipsis
|
||
#
|
||
# This lint is therefore most important in user-facing code, such as the UI,
|
||
# but we also care about beautiful docs, so at the moment this lint is quite "inclusive".
|
||
if ellipsis.search(line):
|
||
has_quote = '"' in line or "'" in line
|
||
if (has_quote and "Callable" not in line) or (
|
||
file_extension not in "py"
|
||
and not ellipsis_expression.search(line)
|
||
and not ellipsis_import.search(line)
|
||
and not ellipsis_bare.search(line)
|
||
and not ellipsis_reference.search(line)
|
||
):
|
||
return "Use … instead of ... (on Mac it's option+;)"
|
||
|
||
if "http" not in line:
|
||
# Strip markdown link/image destinations to avoid false positives on file paths (e.g. `/3d-camera.png`)
|
||
line_without_link_targets = re.sub(r"\]\([^)]*\)", "]()", line)
|
||
if re.search(r"\b2d\b", line_without_link_targets):
|
||
return "we prefer '2D' over '2d'"
|
||
if re.search(r"\b3d\b", line_without_link_targets):
|
||
return "we prefer '3D' over '3d'"
|
||
|
||
# Em dash should be spaced (` — `, not `word—word`). See DESIGN.md.
|
||
# The UI placeholder literal `"—"` (em dash inside quotes) is naturally exempt
|
||
# since the regex requires word/paren/asterisk characters on both sides.
|
||
if re.search(r"[\w\)\*]—[\w\(\*]", line):
|
||
return "Use a spaced em dash (' — '), not 'word—word'. See DESIGN.md."
|
||
|
||
# En dash (`–`) is for numeric ranges only; as a sentence dash, use an em dash (` — `).
|
||
# Detection: en dash with spaces, flanked by letters (digit-flanked allows `100 – 200`).
|
||
if re.search(r"[A-Za-z\"'\)]\s–\s[A-Za-z]", line):
|
||
return "Use an em dash (' — '), not an en dash, as a sentence dash. See DESIGN.md."
|
||
|
||
if (
|
||
"recording=rec" in line
|
||
and "rr." not in line
|
||
and "recording=rec.to_native()" not in line
|
||
and "recording=recording.to_native()" not in line
|
||
):
|
||
return "you must cast the RecordingStream first: `recording=recording.to_native()"
|
||
|
||
if "FIXME" in line:
|
||
return "we prefer TODO over FIXME"
|
||
|
||
if "HACK" in line:
|
||
return "we prefer TODO over HACK"
|
||
|
||
if "todo:" in line:
|
||
return "write 'TODO:' in upper-case"
|
||
|
||
if "todo!()" in line:
|
||
return 'todo!() should be written as todo!("$details")'
|
||
|
||
if m := re.search(r"TODO\(([^)]*)\)", line):
|
||
parts = m.group(1).split(",")
|
||
if len(parts) == 0 or not all(is_valid_todo_part(p) for p in parts):
|
||
return "TODOs should be formatted as either TODO(name), TODO(#42) or TODO(org/repo#42)"
|
||
|
||
if re.search(r'TODO([^_"(]|$)', line):
|
||
return "TODO:s should be written as `TODO(yourname): what to do`"
|
||
|
||
if re.search(r"\{\w*err:#?\?\}", line) or debug_format_of_err.search(line):
|
||
return "Format errors with re_error::format or using Display - NOT Debug formatting!"
|
||
|
||
if re.search(r"\?\w*err\b", line):
|
||
return "Use `%err` (Display) instead of `?err` (Debug) in tracing macros"
|
||
|
||
if log_with_inline_sensitive_data.search(line):
|
||
return 'URLs and paths should be passed as structured fields, not inline in log messages. Use e.g. `re_log::warn!(?url, "message: {err}")` instead of `re_log::warn!("message {url}: {err}")`'
|
||
|
||
if thiserror_multiple_unnamed.search(line):
|
||
return "Use named fields for complex errors instead of multiple unnamed tuple fields like {0}, {1}"
|
||
|
||
if "from attr import dataclass" in line:
|
||
return "Avoid 'from attr import dataclass'; prefer 'from dataclasses import dataclass'"
|
||
|
||
if anyhow_result.search(line):
|
||
return "Prefer using anyhow::Result<>"
|
||
|
||
if m := re.search(error_map_err_name, line) or re.search(error_match_name, line):
|
||
name = m.group(1)
|
||
# if name not in ("_", "_ignored") and not re.fullmatch(r"_?\w*err", name):
|
||
if name == "e" or name.endswith(("error", "status", "res")):
|
||
return f"Errors should be called `err` or have a `_err` suffix. Found: '{name}'"
|
||
|
||
if m := re.search(else_return, line):
|
||
match = m.group(0)
|
||
if match != "else { return; };":
|
||
# Because cargo fmt doesn't handle let-else
|
||
return f"Use 'else {{ return; }};' instead of '{match}'"
|
||
|
||
if wasm_caps.search(line):
|
||
return "WASM should be written 'Wasm'"
|
||
|
||
if nb_prefix.search(line):
|
||
return "Don't use nb_things - use num_things or thing_count instead"
|
||
|
||
if explicit_quotes.search(line):
|
||
return (
|
||
"Prefer using {:?} over explicit quotes - it will also escape newlines etc. "
|
||
"See: https://github.com/rerun-io/rerun/blob/main/CODE_STYLE.md#misc"
|
||
)
|
||
|
||
if m := re.search(r'"([^"]*)"', line):
|
||
if err := check_string(m.group(1)):
|
||
return err
|
||
|
||
if "rec_stream" in line or "rr_stream" in line:
|
||
return "Instantiated RecordingStreams should be named `rec`"
|
||
|
||
# rST syntax is not rendered by MkDocs/mkdocstrings (our Python API docs).
|
||
# Python docstrings and Rust `///` doc comments (pyo3 exposes them as Python docstrings).
|
||
is_in_rust_doc_comment = file_extension == "rs" and re_docstring.match(line) is not None
|
||
if is_in_docstring or is_in_rust_doc_comment:
|
||
if m := rst_role.search(line):
|
||
role = m.group(1)
|
||
return (
|
||
f"Found rST role `:{role}:` in docstring — use markdown/mkdocstrings cross-references "
|
||
"like `[`Name`][]` or `[`Name`][module.Name]` instead "
|
||
"(our Python API docs are built with MkDocs, not Sphinx)"
|
||
)
|
||
if m := rst_directive.search(line):
|
||
directive = m.group(1)
|
||
if directive == "deprecated":
|
||
return (
|
||
"Found rST `.. deprecated::` directive — use the `@deprecated` decorator instead "
|
||
"(mkdocstrings renders it automatically)"
|
||
)
|
||
if directive == "code-block":
|
||
return "Found rST `.. code-block::` directive — use markdown fenced code blocks (```lang) instead"
|
||
return (
|
||
f"Found rST `.. {directive}::` directive — use a MkDocs admonition "
|
||
f"(`!!! {directive}` with an indented body) instead"
|
||
)
|
||
if rst_double_backtick.search(line):
|
||
return (
|
||
"Found rST double-backtick literal ``…`` — use a single backtick `…` in markdown "
|
||
"(our Python API docs are built with MkDocs, not Sphinx)"
|
||
)
|
||
|
||
if is_in_oss_rerun_repo:
|
||
# Deprecated brand names. Replacement is context-dependent:
|
||
# - 'Rerun Hub' → commercial managed offering
|
||
# - 'catalog server' → generic OSS or managed
|
||
# - or rephrase to avoid naming the product
|
||
# Matched case-insensitively so that lowercase variants (e.g. 'rerun cloud') are also caught.
|
||
deprecated_msg = "is a deprecated name. Use 'Rerun Hub' (commercial), 'catalog server' (generic), or rephrase."
|
||
if re.search(r"\bRerun\s+Cloud\b", line, re.IGNORECASE):
|
||
return f"'Rerun Cloud' {deprecated_msg}"
|
||
if re.search(r"\bRerun\s+Base\b", line, re.IGNORECASE):
|
||
return f"'Rerun Base' {deprecated_msg}"
|
||
if re.search(r"\bRerun\s+Data\s+Platform\b", line, re.IGNORECASE):
|
||
return f"'Rerun Data Platform' {deprecated_msg}"
|
||
if re.search(r"\bData\s+Platform\b", line, re.IGNORECASE):
|
||
return f"'Data Platform' {deprecated_msg}"
|
||
# Skip URL paths (`/dataplatform/`) and python package extras specifiers
|
||
# (`rerun-sdk[dataloader,dataplatform]`) — those reference the feature name, not prose.
|
||
if re.search(r"(?<![/\[,])dataplatform(?![/\],])", line, re.IGNORECASE):
|
||
return f"'dataplatform' {deprecated_msg}"
|
||
|
||
# Enforce 'Rerun Hub' capitalization: flag any case variant that isn't exactly 'Rerun Hub'.
|
||
for m in re.finditer(r"\bRerun\s+Hub\b", line, re.IGNORECASE):
|
||
if m.group(0) != "Rerun Hub":
|
||
return "'Rerun Hub' must be properly capitalized."
|
||
|
||
if not is_in_docstring:
|
||
if m := re.search(
|
||
r'(RecordingStreamBuilder::new|\.init|RecordingStream)\("([^"]*)',
|
||
line,
|
||
) or re.search(
|
||
r'(rr.script_setup)\(args, "(\w*)',
|
||
line,
|
||
):
|
||
app_id = m.group(2)
|
||
if not app_id.startswith("rerun_example_") and app_id != "<your_app_name>":
|
||
return f"All examples should have an app_id starting with 'rerun_example_'. Found '{app_id}'"
|
||
|
||
# Deref impls should be marked #[inline] or #[inline(always)].
|
||
if "fn deref(&self)" in line or "fn deref_mut(&mut self)" in line:
|
||
if prev_line_stripped not in {"#[inline]", "#[inline(always)]"}:
|
||
return "Deref/DerefMut impls should be marked #[inline]"
|
||
|
||
# Deref impls should be marked #[inline] or #[inline(always)].
|
||
if "fn as_ref(&self)" in line or "fn borrow(&self)" in line:
|
||
if prev_line_stripped not in {"#[inline]", "#[inline(always)]"}:
|
||
return "as_ref/borrow implementations should be marked #[inline]"
|
||
|
||
if any(
|
||
s in line
|
||
for s in (
|
||
": &dyn std::any::Any",
|
||
": &mut dyn std::any::Any",
|
||
": &dyn Any",
|
||
": &mut dyn Any",
|
||
)
|
||
):
|
||
return """Functions should never take `&dyn std::any::Any` as argument since `&Box<std::any::Any>`
|
||
itself implements `Any`, making it easy to accidentally pass the wrong object. Expect purpose defined traits instead."""
|
||
|
||
if file_extension == "rs":
|
||
if re.search(r"\.zip\(", line):
|
||
return (
|
||
"Prefer `std::iter::zip(a, b)` (iterators), `itertools::izip!(a, b, …)` (3+ iterators), "
|
||
"or `Option::zip(a, b)` (options) over `a.zip(b)`"
|
||
)
|
||
if re.search(r"\.chain\(", line):
|
||
return "Prefer `std::iter::chain(a, b)` or `itertools::chain!(a, b, …)` over `a.chain(b)`"
|
||
|
||
return None
|
||
|
||
|
||
def test_lint_line() -> None:
|
||
assert lint_line("hello world", None) is None
|
||
|
||
should_pass = [
|
||
"hello world",
|
||
"this is a 2D view",
|
||
"todo lowercase is fine",
|
||
'todo!("Macro is ok with text")',
|
||
"TODO_TOKEN",
|
||
"TODO(bob):",
|
||
"TODO(bob,alice):",
|
||
"TODO(bob, alice):",
|
||
"TODO(#42):",
|
||
"TODO(#42,#43):",
|
||
"TODO(#42, #43):",
|
||
"TODO(n4m3/w1th-numb3r5#42)",
|
||
"TODO(rust-lang/rust#42):",
|
||
"TODO(rust-lang/rust#42,rust-lang/rust#43):",
|
||
"TODO(rust-lang/rust#42, rust-lang/rust#43):",
|
||
'eprintln!("{:?}, {err}", foo)',
|
||
'eprintln!("{:#?}, {err}", foo)',
|
||
'eprintln!("{err}")',
|
||
'eprintln!("{}", err)',
|
||
"if let Err(err) = foo",
|
||
"if let Err(_err) = foo",
|
||
"if let Err(_) = foo",
|
||
"map_err(|err| …)",
|
||
"map_err(|_err| …)",
|
||
"map_err(|_| …)",
|
||
"WASM_FOO env var",
|
||
"Wasm",
|
||
"num_instances",
|
||
"instances_count",
|
||
"let Some(foo) = bar else { return; };",
|
||
"{foo:?}",
|
||
'ui.label("This is fine. Correct casing.")',
|
||
"rec",
|
||
"anyhow::Result<()>",
|
||
"The theme is great",
|
||
"template <typename... Args>",
|
||
'_TFunc = TypeVar("_TFunc", bound=Callable[..., Any])',
|
||
'protoc_prebuilt::init("22.0")',
|
||
'rr.init("rerun_example_app")',
|
||
'rr.script_setup(args, "rerun_example_app")',
|
||
"""
|
||
#[inline]
|
||
fn foo(mut self) -> Self {
|
||
""",
|
||
"""
|
||
#[inline(always)]
|
||
fn foo_always(mut self) -> Self {
|
||
""",
|
||
"""
|
||
#[inline]
|
||
fn deref(&self) -> Self::Target {
|
||
""",
|
||
"""
|
||
#[inline(always)]
|
||
fn deref(&self) -> Self::Target {
|
||
""",
|
||
"""
|
||
#[inline]
|
||
fn deref_mut(&mut self) -> &mut Self::Target {
|
||
""",
|
||
"""
|
||
#[inline(always)]
|
||
fn deref_mut(&mut self) -> &mut Self::Target {
|
||
""",
|
||
"""
|
||
#[inline]
|
||
fn borrow(&self) -> &Self {
|
||
""",
|
||
"""
|
||
#[inline(always)]
|
||
fn borrow(&self) -> &Self {
|
||
""",
|
||
"""
|
||
#[inline]
|
||
fn as_ref(&self) -> &Self {
|
||
""",
|
||
"""
|
||
#[inline(always)]
|
||
fn as_ref(&self) -> &Self {
|
||
""",
|
||
"fn ret_any() -> &dyn std::any::Any",
|
||
"fn ret_any_mut() -> &mut dyn std::any::Any",
|
||
# URL paths and python package extras still reference the feature name.
|
||
"Visit /dataplatform/docs for more info",
|
||
"The https://example.com/dataplatform/api endpoint",
|
||
'dependencies = ["rerun-sdk[dataloader,dataplatform]"]',
|
||
'override-dependencies = ["rerun-sdk[dataplatform]"]',
|
||
'extras = ["dataplatform,extra"]',
|
||
# New approved names.
|
||
"Connect to Rerun Hub for hosted catalogs.",
|
||
"Spin up a catalog server locally.",
|
||
"We use the catalog server in production.",
|
||
# %err (Display) in tracing macros is good
|
||
'tracing::warn!(%err, "something failed");',
|
||
're_log::error!(%err, "something failed");',
|
||
# Structured logging with sensitive data as fields (good pattern)
|
||
're_log::warn!(?url, "Failed to open URL: {err}");',
|
||
're_log::error!(?path, "Failed to read file: {err}");',
|
||
're_log::info!(?filepath, loader = ?exe, "Loading data…");',
|
||
're_log::debug!(url = ?url, "Fetching data");',
|
||
# _once variants with structured fields (good)
|
||
're_log::warn_once!(?url, "Failed to open URL: {err}");',
|
||
're_log::error_once!(?path, "Cannot read file: {err}");',
|
||
# Log messages without sensitive inline data (also fine)
|
||
're_log::info!("Starting server on port {port}");',
|
||
're_log::warn!("Connection failed: {err}");',
|
||
# info! is allowed to have inline paths (user-facing)
|
||
're_log::info!("Saving to {filepath}");',
|
||
're_log::info!("Scanning {dir}");',
|
||
# debug! and trace! are allowed to have inline paths (developer-facing)
|
||
're_log::debug!("Loading {file_path}");',
|
||
're_log::trace!("Connecting to {uri}");',
|
||
're_log::debug!("Entering {directory}");',
|
||
're_log::trace!("Created {folder}");',
|
||
# thiserror with named fields (good)
|
||
'#[error("Failed to open {path}: {err}")]',
|
||
'#[error("Something went wrong: {0}")]', # single unnamed is fine
|
||
'#[error("Simple error message")]',
|
||
# Spaced em dash is the convention.
|
||
"Use a spaced em dash (` — `) for parenthetical breaks.",
|
||
"foo — bar — baz",
|
||
# En dash is fine in numeric/character ranges (no spaces, or digit-flanked).
|
||
"Range: 2020–2025",
|
||
"pp. 10–15",
|
||
"100 KB–10 MB",
|
||
"Chunks 100 – 200", # digit on right side — allowed range with spaces
|
||
"A–Z and a–z and 0–9",
|
||
# Em dash as a UI placeholder literal in a string (not prose).
|
||
'"—".to_owned()',
|
||
'return sha[:8] if sha else "—"',
|
||
# Mathematical/UI display with en dash, no spaces.
|
||
'ui.button("–∞")',
|
||
# Preferred zip/chain alternatives.
|
||
"let it = std::iter::zip(a, b);",
|
||
"let it = std::iter::chain(a, b);",
|
||
"for (x, y, z) in izip!(a, b, c) {",
|
||
"for x in itertools::chain!(a, b, c) {",
|
||
]
|
||
|
||
should_error = [
|
||
"this is a 2d view",
|
||
"FIXME",
|
||
"HACK",
|
||
"TODO",
|
||
"# TODO make",
|
||
"TODO:",
|
||
"TODO(42)",
|
||
"TODO(https://github.com/rerun-io/rerun/issues/42)",
|
||
"TODO(bob/alice)",
|
||
"TODO(bob|alice)",
|
||
"TODO(agent)", # TODOs left for a coding agent
|
||
"TODO(Claude)", # TODOs left for a coding agent
|
||
"TODO(codex)", # TODOs left for a coding agent
|
||
"TODO(llm)", # TODOs left for a coding agent
|
||
"todo!()",
|
||
'eprintln!("{err:?}")',
|
||
'eprintln!("{err:#?}")',
|
||
'eprintln!("{:?}", err)',
|
||
'eprintln!("{:#?}", err)',
|
||
'eprintln!("{js_err:?}")',
|
||
'eprintln!("{js_err:#?}")',
|
||
'eprintln!("{:?}", js_err)',
|
||
'eprintln!("{:#?}", js_err)',
|
||
# ?err (Debug) in tracing macros (bad - use %err for Display):
|
||
'tracing::warn!(?err, "something failed");',
|
||
're_log::error!(?err, "something failed");',
|
||
'tracing::warn!(?js_err, "something failed");',
|
||
're_log::error!(?js_err, "something failed");',
|
||
"if let Err(error) = foo",
|
||
"Ok(Err(status))",
|
||
"map_err(|e| …)",
|
||
"We use WASM in Rerun",
|
||
"nb_instances",
|
||
"inner_nb_instances",
|
||
"let Some(foo) = bar else {return;};",
|
||
"let Some(foo) = bar else {return};",
|
||
"let Some(foo) = bar else { return };",
|
||
r'println!("Problem: \"{}\"", string)',
|
||
r'println!("Problem: \"{0}\"")',
|
||
r'println!("Problem: \"{string}\"")',
|
||
'ui.label("This uses ugly title casing for View.")',
|
||
"trailing whitespace ",
|
||
"rr_stream",
|
||
"rec_stream",
|
||
"Result<(), anyhow::Error>",
|
||
"The the problem with double words",
|
||
"More than meets the eye...",
|
||
're_log::trace!("Performing migrations...");',
|
||
'rr.log("/", rr.TextLog("Logging things..."))',
|
||
'logging.info("Detection finished...")',
|
||
'RecordingStreamBuilder::new("missing_prefix")',
|
||
'args.rerun.init("missing_prefix")',
|
||
'RecordingStream("missing_prefix")',
|
||
'rr.init("missing_prefix")',
|
||
'rr.script_setup(args, "missing_prefix")',
|
||
'rr.script_setup(args, "")',
|
||
"I accidentally wrote the same same word twice",
|
||
"fn deref(&self) -> Self::Target {",
|
||
"fn deref_mut(&mut self) -> &mut Self::Target",
|
||
"fn borrow(&self) -> &Self",
|
||
"fn as_ref(&self) -> &Self",
|
||
"fn take_any(thing: &dyn std::any::Any)",
|
||
"fn take_any_mut(thing: &mut dyn std::any::Any)",
|
||
"fn take_any(thing: &dyn Any)",
|
||
"fn take_any_mut(thing: &mut dyn Any)",
|
||
# Deprecated brand names — must use 'Rerun Hub' or 'catalog server' instead.
|
||
# Matched case-insensitively, so lowercase variants must also error.
|
||
"The dataplatform is powerful",
|
||
"Using dataplatform for analytics",
|
||
"Using DATAPLATFORM in caps",
|
||
"I love the data platform",
|
||
"The Rerun data platform is great",
|
||
"We use the Rerun Data Platform.",
|
||
"We use the RERUN DATA PLATFORM.",
|
||
"Connect via Rerun Cloud today.",
|
||
"Connect via rerun cloud today.",
|
||
"Connect via RERUN CLOUD today.",
|
||
"The Data Platform stores recordings.",
|
||
"The data platform stores recordings.",
|
||
"Rerun Base is the new commercial offering.",
|
||
"rerun base is the new commercial offering.",
|
||
# Wrong 'Rerun Hub' capitalization.
|
||
"Connect to Rerun hub today.",
|
||
"Use rerun Hub for catalogs.",
|
||
"Use rerun hub for catalogs.",
|
||
"USE RERUN HUB FOR CATALOGS.",
|
||
# Inline sensitive data in log messages (bad pattern) - only error/warn are linted
|
||
're_log::warn!("Failed to open URL {url}: {err}");',
|
||
're_log::error!("Failed to read file at {path}: {err}");',
|
||
're_log::warn!("Cannot find {file}");',
|
||
're_log::error!("Missing {filename}");',
|
||
# _once variants should also be linted
|
||
're_log::warn_once!("Failed to open URL {url}: {err}");',
|
||
're_log::error_once!("Cannot read {path}");',
|
||
# thiserror with multiple unnamed fields (bad)
|
||
'#[error("Failed to do {0}: {1}")]',
|
||
'#[error("{0} failed with {1} at {2}")]',
|
||
# Unspaced em dash (should be spaced).
|
||
"the layout—are computed",
|
||
"components—the viewer no longer",
|
||
"*data blueprints*—the entity",
|
||
"(SN)—and the storage node",
|
||
# En dash used as a sentence dash (should be em dash).
|
||
"Foo – the description",
|
||
"[Python](./install-rerun/python.md) – the Python SDK",
|
||
"done – next step",
|
||
# Method `.zip(` / `.chain(` — prefer `std::iter::*` or `itertools::izip!/chain!`.
|
||
"let it = a.iter().zip(b.iter());",
|
||
"let it = a.iter().chain(b.iter());",
|
||
]
|
||
|
||
for test in should_pass:
|
||
prev_line = None
|
||
for line in test.split("\n"):
|
||
err = lint_line(line, prev_line)
|
||
assert err is None, f'expected "{line}" to pass, but got error: "{err}"'
|
||
prev_line = line
|
||
|
||
for test in should_error:
|
||
prev_line = None
|
||
for line in test.split("\n"):
|
||
assert lint_line(line, prev_line) is not None, f'expected "{line}" to fail'
|
||
prev_line = line
|
||
|
||
# rST (reStructuredText) is not rendered by MkDocs/mkdocstrings.
|
||
# Flagged inside Python docstrings and Rust `///` doc comments only.
|
||
rst_should_fail_in_docstring = [
|
||
"A :class:`Foo` object.",
|
||
"See :meth:`Foo.bar` for details.",
|
||
"Use :func:`rerun.init` to start.",
|
||
"Reference :attr:`Foo.x`.",
|
||
".. warning::",
|
||
" .. warning::",
|
||
".. note:: This is important",
|
||
".. deprecated:: 0.1",
|
||
".. code-block:: python",
|
||
".. seealso:: related",
|
||
"Handles ``list<double>`` and ``list<list<double>>``.",
|
||
"Returns ``None`` when empty.",
|
||
]
|
||
for line in rst_should_fail_in_docstring:
|
||
assert lint_line(line, None, "py", is_in_docstring=True) is not None, (
|
||
f'expected "{line}" to fail inside a Python docstring'
|
||
)
|
||
# Same lines should not fire outside docstrings (e.g. in regular code/comments).
|
||
assert lint_line(line, None, "py", is_in_docstring=False) is None, (
|
||
f'expected "{line}" to pass outside a Python docstring, but it was flagged'
|
||
)
|
||
# Rust `///` doc comments (exposed as Python docstrings via pyo3) are checked too.
|
||
assert lint_line(f"/// {line}", None, "rs") is not None, f'expected "/// {line}" to fail in a Rust doc comment'
|
||
# Regular `//` comments are not checked.
|
||
assert lint_line(f"// {line}", None, "rs") is None, f'expected "// {line}" to pass in a regular Rust comment'
|
||
|
||
rst_should_pass_in_docstring = [
|
||
"Use [`Foo`][] instead.",
|
||
"Reference [`Foo.bar`][rerun.Foo.bar].",
|
||
"!!! warning",
|
||
" !!! warning",
|
||
"!!! note",
|
||
"A regular sentence with no rST.",
|
||
"```python",
|
||
"Handles `list<double>` and `list<list<double>>`.",
|
||
# Parameter section headers (numpy style) — these look superficially similar but aren't rST directives.
|
||
"Parameters",
|
||
"----------",
|
||
]
|
||
for line in rst_should_pass_in_docstring:
|
||
err = lint_line(line, None, "py", is_in_docstring=True)
|
||
assert err is None, f'expected "{line}" to pass inside a Python docstring, but got: "{err}"'
|
||
|
||
|
||
# -----------------------------------------------------------------------------
|
||
|
||
re_declaration = re.compile(r"^\s*((pub(\(\w*\))? )?(async )?((impl|fn|struct|enum|union|trait|type)\b))")
|
||
re_attribute = re.compile(r"^\s*\#\[(error|derive|inline)")
|
||
re_docstring = re.compile(r"^\s*///")
|
||
|
||
|
||
def is_missing_blank_line_between(prev_line: str, line: str) -> bool:
|
||
def is_empty(line: str) -> bool:
|
||
return line == "" or line.startswith(("#", "//")) or line.endswith(("{", "(", "\\", 'r"', 'r#"', "]"))
|
||
|
||
"""Only for Rust files."""
|
||
if re_declaration.match(line) or re_attribute.match(line) or re_docstring.match(line):
|
||
line = line.strip()
|
||
prev_line = prev_line.strip()
|
||
|
||
if "template<" in prev_line:
|
||
return False # C++ template inside Rust code that generates C++ code.
|
||
|
||
if is_empty(prev_line) or prev_line.strip().startswith("```"):
|
||
return False
|
||
|
||
if line.startswith("fn ") and line.endswith(";"):
|
||
return False # maybe a trait function
|
||
|
||
if line.startswith("type ") and prev_line.endswith(";"):
|
||
return False # many type declarations in a row is fine
|
||
|
||
if prev_line.endswith(",") and line.startswith("impl"):
|
||
return False
|
||
|
||
if prev_line.endswith("*"):
|
||
return False # maybe in a macro
|
||
|
||
return not prev_line.endswith('r##"') # part of a multi-line string
|
||
|
||
return False
|
||
|
||
|
||
def lint_vertical_spacing(lines_in: list[str]) -> tuple[list[str], list[str]]:
|
||
"""Only for Rust files."""
|
||
prev_line = None
|
||
|
||
errors: list[str] = []
|
||
lines_out: list[str] = []
|
||
|
||
for line_nr, line in enumerate(lines_in):
|
||
line_nr = line_nr + 1
|
||
|
||
if prev_line is not None and is_missing_blank_line_between(prev_line, line):
|
||
errors.append(f"{line_nr}: for readability, add newline before `{line.strip()}`")
|
||
lines_out.append("\n")
|
||
|
||
lines_out.append(line)
|
||
prev_line = line
|
||
|
||
return errors, lines_out
|
||
|
||
|
||
def test_lint_vertical_spacing() -> None:
|
||
assert re_declaration.match("fn foo() {}")
|
||
assert re_declaration.match("async fn foo() {}")
|
||
assert re_declaration.match("pub async fn foo() {}")
|
||
|
||
should_pass = [
|
||
"hello world",
|
||
"""
|
||
/// docstring
|
||
foo
|
||
|
||
/// docstring
|
||
bar
|
||
""",
|
||
"""
|
||
trait Foo {
|
||
fn bar();
|
||
fn baz();
|
||
}
|
||
""",
|
||
# macros:
|
||
"""
|
||
$(#[$meta])*
|
||
#[derive(Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
|
||
""",
|
||
"""
|
||
Item = (
|
||
&PointCloudBatchInfo,
|
||
impl Iterator<Item = &PointCloudVertex>,
|
||
),
|
||
""",
|
||
"""
|
||
type Response = Response<Body>;
|
||
type Error = hyper::Error;
|
||
""",
|
||
"""
|
||
template<typename T>
|
||
struct AsComponents;
|
||
""", # C++ template inside Rust code that generates C++ code.
|
||
]
|
||
|
||
should_fail = [
|
||
"""
|
||
/// docstring
|
||
foo
|
||
/// docstring
|
||
bar
|
||
""",
|
||
"""
|
||
Foo,
|
||
#[error]
|
||
Bar,
|
||
""",
|
||
"""
|
||
slotmap::new_key_type! { pub struct ViewBuilderHandle; }
|
||
type ViewBuilderMap = slotmap::SlotMap<ViewBuilderHandle, ViewBuilder>;
|
||
""",
|
||
"""
|
||
fn foo() {}
|
||
fn bar() {}
|
||
""",
|
||
"""
|
||
async fn foo() {}
|
||
async fn bar() {}
|
||
""",
|
||
]
|
||
|
||
for code in should_pass:
|
||
errors, _ = lint_vertical_spacing(code.split("\n"))
|
||
assert len(errors) == 0, f"expected this to pass:\n{code}\ngot: {errors}"
|
||
|
||
for code in should_fail:
|
||
errors, _ = lint_vertical_spacing(code.split("\n"))
|
||
assert len(errors) > 0, f"expected this to fail:\n{code}"
|
||
|
||
|
||
# -----------------------------------------------------------------------------
|
||
|
||
|
||
workspace_lints = re.compile(r"\[lints\]\nworkspace\s*=\s*true")
|
||
|
||
|
||
def lint_workspace_lints(cargo_file_content: str) -> str | None:
|
||
"""Checks that a non-example cargo file has a lints section that sets workspace to true."""
|
||
|
||
if workspace_lints.search(cargo_file_content):
|
||
return None
|
||
else:
|
||
return "Non-example cargo files should have a [lints] section with workspace = true"
|
||
|
||
|
||
# -----------------------------------------------------------------------------
|
||
|
||
|
||
def lint_pyclass_requirements(lines_in: list[str]) -> tuple[list[str], list[int], list[str]]:
|
||
"""Only for Rust files. Check that #[pyclass(...)] declarations include 'eq' and the correct module."""
|
||
|
||
errors: list[str] = []
|
||
error_linenumbers: list[int] = []
|
||
error_codes: list[str] = []
|
||
|
||
i = 0
|
||
while i < len(lines_in):
|
||
line = lines_in[i]
|
||
line_nr = i + 1
|
||
|
||
# Check if this line starts a pyclass declaration
|
||
if pyclass_start.search(line.strip()):
|
||
# Collect the entire pyclass declaration (it might span multiple lines)
|
||
pyclass_content = line
|
||
original_line_nr = line_nr
|
||
|
||
# Keep reading lines until we find the closing parenthesis
|
||
paren_count = line.count("(") - line.count(")")
|
||
j = i + 1
|
||
|
||
while paren_count > 0 and j < len(lines_in):
|
||
next_line = lines_in[j]
|
||
pyclass_content += next_line
|
||
paren_count += next_line.count("(") - next_line.count(")")
|
||
j += 1
|
||
|
||
# First remove comments to avoid false matches in comments
|
||
pyclass_content_no_comments = re.sub(r"//.*", "", pyclass_content)
|
||
|
||
# Extract class name: prefer 'name = "..."' from pyclass, fallback to struct name
|
||
name_match = re.search(r'name\s*=\s*"([^"]+)"', pyclass_content_no_comments)
|
||
if name_match:
|
||
cls_name = name_match.group(1)
|
||
else:
|
||
# Look at the struct definition that follows
|
||
cls_name = None
|
||
struct_line_idx = j
|
||
while struct_line_idx < len(lines_in):
|
||
sl = lines_in[struct_line_idx].strip()
|
||
if sl.startswith(("pub struct ", "struct ")):
|
||
struct_match = re.search(r"struct\s+(\w+)", sl)
|
||
if struct_match:
|
||
cls_name = struct_match.group(1)
|
||
break
|
||
struct_line_idx += 1
|
||
|
||
if cls_name and cls_name.endswith("Internal"):
|
||
i = j
|
||
continue
|
||
|
||
# Check if 'eq' is present in the pyclass declaration
|
||
# Look for 'eq' as a standalone parameter (not part of another word)
|
||
if not re.search(r"\beq\b", pyclass_content_no_comments):
|
||
errors.append(
|
||
f"{original_line_nr}: #[pyclass(...)] should include 'eq' parameter for Python equality support"
|
||
)
|
||
error_linenumbers.append(original_line_nr)
|
||
error_codes.append("py-cls-eq")
|
||
|
||
# Check if the correct module is specified
|
||
expected_module = 'module = "rerun_bindings.rerun_bindings"'
|
||
if expected_module not in pyclass_content:
|
||
errors.append(
|
||
f"{original_line_nr}: #[pyclass(...)] should include 'module = \"rerun_bindings.rerun_bindings\"' parameter"
|
||
)
|
||
error_linenumbers.append(original_line_nr)
|
||
error_codes.append("py-cls-mod")
|
||
|
||
# Move the index to after the pyclass declaration
|
||
i = j
|
||
else:
|
||
i += 1
|
||
|
||
return errors, error_linenumbers, error_codes
|
||
|
||
|
||
def lint_pymethods_requirements(lines_in: list[str]) -> tuple[list[str], list[int], list[str]]:
|
||
"""Only for Rust files. Check that #[pymethods] blocks have a __str__ method."""
|
||
|
||
errors: list[str] = []
|
||
error_linenumbers: list[int] = []
|
||
error_codes: list[str] = []
|
||
|
||
i = 0
|
||
while i < len(lines_in):
|
||
line = lines_in[i]
|
||
line_nr = i + 1
|
||
|
||
# Check if this line starts a pymethods declaration
|
||
if pymethods_start.search(line.strip()):
|
||
# Find the corresponding impl block
|
||
j = i + 1
|
||
impl_start_line = None
|
||
class_name = None
|
||
|
||
# Look for the impl block that follows
|
||
while j < len(lines_in):
|
||
impl_line = lines_in[j].strip()
|
||
if impl_line.startswith("impl "):
|
||
impl_start_line = j
|
||
# Extract class name from "impl ClassName {"
|
||
match = re.search(r"impl\s+(\w+)\s*\{", impl_line)
|
||
if match:
|
||
class_name = match.group(1)
|
||
break
|
||
elif impl_line and not impl_line.startswith("//"):
|
||
# If we hit any non-comment, non-empty line that's not impl, stop looking
|
||
break
|
||
j += 1
|
||
|
||
if impl_start_line is None or class_name is None:
|
||
i += 1
|
||
continue
|
||
|
||
if class_name.endswith("Internal"):
|
||
i += 1
|
||
continue
|
||
|
||
# Find the end of the impl block by counting braces
|
||
brace_count = 0
|
||
impl_content = ""
|
||
k = impl_start_line
|
||
|
||
while k < len(lines_in):
|
||
current_line = lines_in[k]
|
||
impl_content += current_line
|
||
brace_count += current_line.count("{") - current_line.count("}")
|
||
|
||
if brace_count == 0 and "{" in lines_in[impl_start_line]:
|
||
# We've found the end of the impl block
|
||
break
|
||
k += 1
|
||
|
||
# Check if __str__ or __repr__ is present in the impl content
|
||
# Remove comments to avoid false matches
|
||
impl_content_no_comments = re.sub(r"//.*", "", impl_content)
|
||
|
||
has_str = re.search(r"\b__str__\b", impl_content_no_comments)
|
||
has_repr = re.search(r"\b__repr__\b", impl_content_no_comments)
|
||
|
||
if not has_str and not has_repr:
|
||
errors.append(
|
||
f"{line_nr}: #[pymethods] impl {class_name} should include a '__str__' method (or '__repr__' which serves as fallback)"
|
||
)
|
||
error_linenumbers.append(line_nr)
|
||
error_codes.append("py-mthd-str")
|
||
|
||
# Move the index to after the impl block
|
||
i = k + 1
|
||
else:
|
||
i += 1
|
||
|
||
return errors, error_linenumbers, error_codes
|
||
|
||
|
||
def test_lint_pymethods_requirements() -> None:
|
||
"""Test the lint_pymethods_requirements function with various pymethods declarations."""
|
||
|
||
should_pass = [
|
||
# pymethods with __str__
|
||
"""#[pymethods]
|
||
impl MyClass {
|
||
pub fn __str__(&self) -> String {
|
||
"test".to_string()
|
||
}
|
||
}""",
|
||
# pymethods with __repr__ (serves as __str__ fallback)
|
||
"""#[pymethods]
|
||
impl MyClass {
|
||
fn __repr__(&self) -> String {
|
||
"test".to_string()
|
||
}
|
||
}""",
|
||
# pymethods with both __str__ and __repr__
|
||
"""#[pymethods]
|
||
impl MyClass {
|
||
pub fn __str__(&self) -> String {
|
||
"str".to_string()
|
||
}
|
||
fn __repr__(&self) -> String {
|
||
"repr".to_string()
|
||
}
|
||
}""",
|
||
# pymethods with other methods and __str__
|
||
"""#[pymethods]
|
||
impl MyClass {
|
||
#[new]
|
||
pub fn new() -> Self {
|
||
Self {}
|
||
}
|
||
pub fn __str__(&self) -> String {
|
||
"test".to_string()
|
||
}
|
||
}""",
|
||
# Internal class without __str__ should be auto-skipped
|
||
"""#[pymethods]
|
||
impl PyFooInternal {
|
||
#[new]
|
||
pub fn new() -> Self {
|
||
Self {}
|
||
}
|
||
}""",
|
||
]
|
||
|
||
should_error = [
|
||
# pymethods without __str__ or __repr__
|
||
"""#[pymethods]
|
||
impl MyClass {
|
||
#[new]
|
||
pub fn new() -> Self {
|
||
Self {}
|
||
}
|
||
}""",
|
||
# pymethods with other methods but no __str__ or __repr__
|
||
"""#[pymethods]
|
||
impl MyClass {
|
||
pub fn other_method(&self) -> i32 {
|
||
42
|
||
}
|
||
pub fn another_method(&self) -> bool {
|
||
true
|
||
}
|
||
}""",
|
||
]
|
||
|
||
# Test cases that should pass (no errors)
|
||
for test_case in should_pass:
|
||
lines = test_case.split("\n")
|
||
errors, _, _ = lint_pymethods_requirements(lines)
|
||
assert len(errors) == 0, f'expected "{test_case}" to pass, but got errors: {errors}'
|
||
|
||
# Test cases that should fail (produce errors)
|
||
for test_case in should_error:
|
||
lines = test_case.split("\n")
|
||
errors, _, _ = lint_pymethods_requirements(lines)
|
||
assert len(errors) > 0, f'expected "{test_case}" to fail, but got no errors'
|
||
|
||
|
||
def test_lint_pyclass_requirements() -> None:
|
||
"""Test the lint_pyclass_requirements function with various pyclass declarations."""
|
||
|
||
should_pass = [
|
||
# Simple pyclass with eq and module
|
||
'#[pyclass(eq, module = "rerun_bindings.rerun_bindings")]',
|
||
# Multiple parameters including eq and module
|
||
'#[pyclass(frozen, eq, hash, module = "rerun_bindings.rerun_bindings")]',
|
||
# eq in different position
|
||
'#[pyclass(eq, frozen, module = "rerun_bindings.rerun_bindings")]',
|
||
# Multi-line pyclass with eq and module
|
||
'#[pyclass(\n frozen,\n eq,\n hash,\n module = "rerun_bindings.rerun_bindings"\n)]',
|
||
# eq at the end
|
||
'#[pyclass(frozen, hash, eq, module = "rerun_bindings.rerun_bindings")]',
|
||
# With module specification and eq
|
||
'#[pyclass(eq, module = "rerun_bindings.rerun_bindings")]',
|
||
# Complex real-world example
|
||
"""#[pyclass(
|
||
frozen,
|
||
eq,
|
||
hash,
|
||
name = "IndexColumnDescriptor",
|
||
module = "rerun_bindings.rerun_bindings"
|
||
)]""",
|
||
# With name parameter
|
||
'#[pyclass(eq, name = "MyClass", module = "rerun_bindings.rerun_bindings")]',
|
||
# Internal class (via name param) without eq should be auto-skipped
|
||
'#[pyclass(name = "FooInternal", module = "rerun_bindings.rerun_bindings")]\npub struct PyFooInternal {}',
|
||
# Internal class (via struct name fallback) without eq should be auto-skipped
|
||
'#[pyclass(module = "rerun_bindings.rerun_bindings")]\npub struct PyFooInternal {}',
|
||
]
|
||
|
||
should_error = [
|
||
# Missing eq parameter
|
||
'#[pyclass(frozen, module = "rerun_bindings.rerun_bindings")]',
|
||
# Multiple parameters but no eq
|
||
'#[pyclass(frozen, hash, module = "rerun_bindings.rerun_bindings")]',
|
||
# With module but no eq
|
||
'#[pyclass(module = "rerun_bindings.rerun_bindings")]',
|
||
# With eq but no module
|
||
"#[pyclass(eq, frozen)]",
|
||
# Missing both eq and module
|
||
"#[pyclass(frozen)]",
|
||
# Multi-line without eq
|
||
'#[pyclass(\n frozen,\n hash,\n module = "rerun_bindings.rerun_bindings"\n)]',
|
||
# Multi-line without module
|
||
"#[pyclass(\n frozen,\n eq,\n hash\n)]",
|
||
# Complex example without eq
|
||
"""#[pyclass(
|
||
frozen,
|
||
hash,
|
||
name = "IndexColumnDescriptor",
|
||
module = "rerun_bindings.rerun_bindings"
|
||
)]""",
|
||
# Complex example without module
|
||
"""#[pyclass(
|
||
frozen,
|
||
eq,
|
||
hash,
|
||
name = "IndexColumnDescriptor"
|
||
)]""",
|
||
# Wrong module name
|
||
'#[pyclass(eq, module = "wrong_module")]',
|
||
]
|
||
|
||
# Test cases that should pass (no errors)
|
||
for test_case in should_pass:
|
||
lines = test_case.split("\n")
|
||
errors, _, _ = lint_pyclass_requirements(lines)
|
||
assert len(errors) == 0, f'expected "{test_case}" to pass, but got errors: {errors}'
|
||
|
||
# Test cases that should fail (produce errors)
|
||
for test_case in should_error:
|
||
lines = test_case.split("\n")
|
||
errors, _, _ = lint_pyclass_requirements(lines)
|
||
assert len(errors) > 0, f'expected "{test_case}" to fail, but got no errors'
|
||
|
||
|
||
# -----------------------------------------------------------------------------
|
||
|
||
force_capitalized = [
|
||
"2D",
|
||
"3D",
|
||
"Apache",
|
||
"API",
|
||
"APIs",
|
||
"April",
|
||
"Bevy",
|
||
"C",
|
||
"C++",
|
||
"C++17,", # easier than coding up a special case
|
||
"CI",
|
||
"Colab",
|
||
"Google",
|
||
"Gradio",
|
||
"gRPC",
|
||
"GUI",
|
||
"GUIs",
|
||
"Intel",
|
||
"July",
|
||
"Jupyter",
|
||
"LeRobot",
|
||
"Linux",
|
||
"Mac",
|
||
"macOS",
|
||
"Macs",
|
||
"ML",
|
||
"Numpy",
|
||
"nuScenes",
|
||
"Pandas",
|
||
"PDF",
|
||
"Pixi",
|
||
"Polars",
|
||
"Python",
|
||
"Q1",
|
||
"Q2",
|
||
"Q3",
|
||
"Q4",
|
||
"Rerun",
|
||
"Rust",
|
||
"SAM",
|
||
"SDK",
|
||
"SDKs",
|
||
"UI",
|
||
"UIs",
|
||
"UX",
|
||
"Wasm",
|
||
"Windows",
|
||
# "Arrow", # Would be nice to capitalize in the right context, but it's a too common word.
|
||
# "Windows", # Consider "multiple plot windows"
|
||
]
|
||
|
||
allow_capitalized = [
|
||
"Viewer",
|
||
# Referring to the Rerun Viewer as just "the Viewer" is fine, but not all mentions of "viewer" are capitalized.
|
||
"Arrow",
|
||
# Referring to the Apache Arrow project as just "Arrow" is fine, but not all mentions of "arrow" are capitalized.
|
||
"Hub",
|
||
# Referring to Rerun Hub as just "Hub" is fine, but "hub" as a common noun isn't capitalized.
|
||
]
|
||
|
||
force_capitalized_as_lower = [word.lower() for word in force_capitalized]
|
||
allow_capitalized_as_lower = [word.lower() for word in allow_capitalized]
|
||
|
||
|
||
def split_words(input_string: str) -> list[str]:
|
||
result = []
|
||
word = ""
|
||
for char in input_string:
|
||
if char.isalpha() or char.isdigit() or char in "/_@`.!?+-()":
|
||
word += char
|
||
else:
|
||
if word:
|
||
result.append(word)
|
||
word = ""
|
||
result.append(char)
|
||
if word:
|
||
result.append(word)
|
||
return result
|
||
|
||
|
||
def is_emoji(s: str) -> bool:
|
||
"""Returns true if the string contains an emoji."""
|
||
# Written by Copilot
|
||
return any(
|
||
0x1F600 <= ord(c) <= 0x1F64F # Emoticons
|
||
or 0x1F300 <= ord(c) <= 0x1F5FF # Miscellaneous Symbols and Pictographs
|
||
or 0x1F680 <= ord(c) <= 0x1F6FF # Transport and Map Symbols
|
||
or 0x2600 <= ord(c) <= 0x26FF # Miscellaneous Symbols
|
||
or 0x2700 <= ord(c) <= 0x27BF # Dingbats
|
||
or 0xFE00 <= ord(c) <= 0xFE0F # Variation Selectors
|
||
or 0x1F900 <= ord(c) <= 0x1F9FF # Supplemental Symbols and Pictographs
|
||
or 0x1FA70 <= ord(c) <= 0x1FAFF # Symbols and Pictographs Extended-A
|
||
for c in s
|
||
)
|
||
|
||
|
||
def test_is_emoji() -> None:
|
||
assert not is_emoji("A")
|
||
assert not is_emoji("Ö")
|
||
assert is_emoji("😀")
|
||
assert is_emoji("⚠️")
|
||
|
||
|
||
def test_split_words() -> None:
|
||
test_cases = [
|
||
("hello world", ["hello", " ", "world"]),
|
||
("hello foo@rerun.io", ["hello", " ", "foo@rerun.io"]),
|
||
("www.rerun.io", ["www.rerun.io"]),
|
||
("`rerun`", ["`rerun`"]),
|
||
]
|
||
|
||
for input, expected in test_cases:
|
||
actual = split_words(input)
|
||
assert actual == expected, f"Expected '{input}' to split into {expected}, got {actual}"
|
||
|
||
|
||
def fix_header_casing(s: str) -> str:
|
||
def is_acronym_or_pascal_case(s: str) -> bool:
|
||
return sum(1 for c in s if c.isupper()) > 1
|
||
|
||
if s.startswith("["):
|
||
return s # We don't handle links in headers, yet
|
||
|
||
new_words: list[str] = []
|
||
last_punctuation = None
|
||
inline_code_block = False
|
||
is_first_word = True
|
||
|
||
words = s.strip().split(" ")
|
||
|
||
for word in words:
|
||
if word == "":
|
||
continue
|
||
|
||
if word == "I":
|
||
new_words.append(word)
|
||
continue
|
||
|
||
if is_emoji(word):
|
||
new_words.append(word)
|
||
continue
|
||
|
||
if word.startswith("`"):
|
||
inline_code_block = True
|
||
|
||
if last_punctuation:
|
||
word = word.capitalize()
|
||
last_punctuation = None
|
||
elif not inline_code_block and not word.startswith("`") and not word.startswith('"'):
|
||
try:
|
||
idx = force_capitalized_as_lower.index(word.lower())
|
||
except ValueError:
|
||
idx = None
|
||
|
||
if word.endswith(("?", "!", ".")):
|
||
last_punctuation = word[-1]
|
||
word = word[:-1]
|
||
elif idx is not None:
|
||
word = force_capitalized[idx]
|
||
elif is_acronym_or_pascal_case(word) or any(c in ("_", "(", ".") for c in word):
|
||
pass # acroym, PascalCase, code, …
|
||
elif word.lower() in allow_capitalized_as_lower:
|
||
pass
|
||
elif is_first_word:
|
||
word = word.capitalize()
|
||
else:
|
||
word = word.lower()
|
||
|
||
if word.endswith("`"):
|
||
inline_code_block = False
|
||
|
||
new_words.append((word + last_punctuation) if last_punctuation else word)
|
||
is_first_word = False
|
||
|
||
return " ".join(new_words)
|
||
|
||
|
||
def fix_enforced_upper_case(s: str) -> str:
|
||
new_words: list[str] = []
|
||
inline_code_block = False
|
||
|
||
for word in split_words(s):
|
||
if word.startswith("`"):
|
||
inline_code_block = True
|
||
if word.endswith("`"):
|
||
inline_code_block = False
|
||
|
||
if word.strip() != "" and not inline_code_block and not word.startswith("`"):
|
||
try:
|
||
idx = force_capitalized_as_lower.index(word.lower())
|
||
word = force_capitalized[idx]
|
||
except ValueError:
|
||
pass
|
||
|
||
new_words.append(word)
|
||
|
||
return "".join(new_words)
|
||
|
||
|
||
def lint_markdown(filepath: str, source: SourceFile) -> tuple[list[str], list[str]]:
|
||
"""Only for .md files."""
|
||
|
||
errors: list[str] = []
|
||
lines_out: list[str] = []
|
||
|
||
in_example_readme = (
|
||
"/examples/python/" in filepath
|
||
and filepath.endswith("README.md")
|
||
and not filepath.endswith("/examples/python/README.md")
|
||
)
|
||
in_code_of_conduct = filepath.endswith("CODE_OF_CONDUCT.md")
|
||
|
||
if in_code_of_conduct:
|
||
return errors, source.lines
|
||
|
||
in_code_block = False
|
||
in_frontmatter = False
|
||
in_metadata = False
|
||
for line_nr, line in enumerate(source.lines):
|
||
line_nr = line_nr + 1
|
||
|
||
if line.strip().startswith("```"):
|
||
in_code_block = not in_code_block
|
||
|
||
if line.startswith("---"):
|
||
in_frontmatter = not in_frontmatter
|
||
if line.startswith("<!--[metadata]"):
|
||
in_metadata = True
|
||
if in_metadata and line.startswith("-->"):
|
||
in_metadata = False
|
||
|
||
if not in_code_block and not source.should_ignore(line_nr) and filepath.startswith(rerun_prefix):
|
||
if not in_metadata:
|
||
# Check the casing on markdown headers
|
||
if m := re.match(r"(\#+ )(.*)", line):
|
||
new_header = fix_header_casing(m.group(2))
|
||
if new_header != m.group(2):
|
||
errors.append(
|
||
f"{line_nr}: Markdown headers should NOT be title cased, except certain words which are always capitalized. This should be '{new_header}'.",
|
||
)
|
||
line = m.group(1) + new_header + "\n"
|
||
|
||
# Check the casing on `title = "…"` frontmatter
|
||
elif m := re.match(r'title\s*\=\s*"(.*)"', line):
|
||
new_title = fix_header_casing(m.group(1))
|
||
if new_title != m.group(1):
|
||
errors.append(
|
||
f"{line_nr}: Titles should NOT be title cased, except certain words which are always capitalized. This should be '{new_title}'.",
|
||
)
|
||
line = f'title = "{new_title}"\n'
|
||
|
||
# Enforce capitalization on certain words in the main text.
|
||
elif not in_frontmatter:
|
||
new_line = fix_enforced_upper_case(line)
|
||
if new_line != line:
|
||
errors.append(f"{line_nr}: Certain words should be capitalized. This should be '{new_line}'.")
|
||
line = new_line
|
||
|
||
if in_example_readme and not in_metadata:
|
||
# Check that <h1> is not used in example READMEs
|
||
if line.startswith("#") and not line.startswith("##"):
|
||
errors.append(
|
||
f"{line_nr}: Do not use top-level headers in example READMEs, they are reserved for page title.",
|
||
)
|
||
|
||
lines_out.append(line)
|
||
|
||
return errors, lines_out
|
||
|
||
|
||
def lint_example_description(filepath: str) -> list[str]:
|
||
# only applies to examples' readme
|
||
|
||
if not filepath.startswith(f"{rerun_prefix}examples/python") or not filepath.endswith("README.md"):
|
||
return []
|
||
|
||
return []
|
||
|
||
|
||
def lint_frontmatter(filepath: str, content: str) -> list[str]:
|
||
"""Only for Markdown files."""
|
||
|
||
errors: list[str] = []
|
||
if not filepath.endswith(".md"):
|
||
return errors
|
||
|
||
try:
|
||
load_frontmatter(content)
|
||
except Exception as e:
|
||
errors.append(f"Error parsing frontmatter: {e}")
|
||
|
||
errors += lint_example_description(filepath)
|
||
|
||
return errors
|
||
|
||
|
||
# -----------------------------------------------------------------------------
|
||
|
||
|
||
def _index_to_line_nr(content: str, index: int) -> int:
|
||
"""Converts a 0-based index into a 0-based line number."""
|
||
return content[:index].count("\n")
|
||
|
||
|
||
class SourceFile:
|
||
"""Wrapper over a source file with some utility functions."""
|
||
|
||
def __init__(self, path: str) -> None:
|
||
self.path = path
|
||
self.ext = path.split(".")[-1]
|
||
with open(path, encoding="utf8") as f:
|
||
self.lines = f.readlines()
|
||
self._update_content()
|
||
|
||
def _update_content(self) -> None:
|
||
"""Sync everything with `self.lines`."""
|
||
self.content = "".join(self.lines)
|
||
|
||
# gather lines with a `NOLINT` marker
|
||
# nolints is a dict from code to set of line numbers
|
||
# None key is used for unqualified NOLINT
|
||
self.nolints: dict[str | None, set[int]] = {}
|
||
is_in_nolint_block = False
|
||
for i, line in enumerate(self.lines):
|
||
if "NOLINT" in line:
|
||
# Check for NOLINT: ignore[<code>] format
|
||
if "NOLINT: ignore[" in line:
|
||
match = re.search(r"NOLINT: ignore\[([^\]]+)\]", line)
|
||
if match:
|
||
code = match.group(1)
|
||
if code not in self.nolints:
|
||
self.nolints[code] = set()
|
||
self.nolints[code].add(i)
|
||
else:
|
||
# Fallback to unqualified NOLINT if parsing fails
|
||
if None not in self.nolints:
|
||
self.nolints[None] = set()
|
||
self.nolints[None].add(i)
|
||
else:
|
||
# Unqualified NOLINT
|
||
if None not in self.nolints:
|
||
self.nolints[None] = set()
|
||
self.nolints[None].add(i)
|
||
|
||
if "NOLINT_START" in line:
|
||
# Check if this is trying to use the ignore[code] format with NOLINT_START
|
||
if "NOLINT_START: ignore[" in line:
|
||
raise NotImplementedError(
|
||
f"NOLINT_START: ignore[<code>] format is not implemented yet. "
|
||
f"Found at line {i + 1}: {line.strip()}"
|
||
)
|
||
is_in_nolint_block = True
|
||
|
||
if is_in_nolint_block:
|
||
# NOLINT_START/END blocks are always unqualified
|
||
if None not in self.nolints:
|
||
self.nolints[None] = set()
|
||
self.nolints[None].add(i)
|
||
if "NOLINT_END" in line:
|
||
is_in_nolint_block = False
|
||
|
||
def rewrite(self, new_lines: list[str]) -> None:
|
||
"""Rewrite the contents of the file."""
|
||
if new_lines != self.lines:
|
||
self.lines = new_lines
|
||
with open(self.path, "w", encoding="utf8") as f:
|
||
f.writelines(new_lines)
|
||
self._update_content()
|
||
print(f"{self.path} fixed.")
|
||
|
||
def should_ignore(self, from_line: int, to_line: int | None = None, code: str | None = None) -> bool:
|
||
"""
|
||
Determines if we should ignore a violation.
|
||
|
||
NOLINT might be on the same line(s) as the violation or the previous line.
|
||
|
||
Args:
|
||
from_line: Starting line number (1-based)
|
||
to_line: Ending line number (1-based), defaults to from_line
|
||
code: Specific error code to check for (e.g., 'py-cls-eq'),
|
||
or None to check for unqualified NOLINT
|
||
|
||
"""
|
||
|
||
if to_line is None:
|
||
to_line = from_line
|
||
|
||
line_range = range(from_line - 1, to_line + 1)
|
||
|
||
# Check for specific code if provided
|
||
if code in self.nolints:
|
||
return any(i in self.nolints[code] for i in line_range)
|
||
return False
|
||
|
||
def should_ignore_index(self, start_idx: int, end_idx: int | None = None, code: str | None = None) -> bool:
|
||
"""Same as `should_ignore` but takes 0-based indices instead of line numbers."""
|
||
return self.should_ignore(
|
||
_index_to_line_nr(self.content, start_idx),
|
||
_index_to_line_nr(self.content, end_idx) if end_idx is not None else None,
|
||
code,
|
||
)
|
||
|
||
def error(self, message: str, *, line_nr: int | None = None, index: int | None = None) -> str:
|
||
"""Construct an error message. If either `line_nr` or `index` is passed, it's used to indicate a line number."""
|
||
if line_nr is None and index is not None:
|
||
line_nr = _index_to_line_nr(self.content, index)
|
||
if line_nr is None:
|
||
return f"{self.path}:{message}"
|
||
else:
|
||
return f"{self.path}:{line_nr + 1}: {message}"
|
||
|
||
|
||
def lint_file(filepath: str, args: Any) -> int:
|
||
source = SourceFile(filepath)
|
||
num_errors = 0
|
||
|
||
error: str | None
|
||
|
||
is_in_docstring = False
|
||
is_in_oss_rerun_repo = filepath.startswith(rerun_prefix)
|
||
|
||
prev_line = None
|
||
for line_nr, line in enumerate(source.lines):
|
||
if source.should_ignore(line_nr):
|
||
continue
|
||
|
||
if line == "" or line[-1] != "\n":
|
||
error = "Missing newline at end of file"
|
||
else:
|
||
line = line[:-1]
|
||
if line.strip() == '"""':
|
||
is_in_docstring = not is_in_docstring
|
||
error = lint_line(line, prev_line, source.ext, is_in_docstring, is_in_oss_rerun_repo)
|
||
prev_line = line
|
||
if error is not None:
|
||
num_errors += 1
|
||
print(source.error(error, line_nr=line_nr))
|
||
|
||
if filepath.endswith(".hpp"):
|
||
if not any(line.startswith("#pragma once") for line in source.lines):
|
||
print(source.error("Missing `#pragma once` in C++ header file"))
|
||
num_errors += 1
|
||
|
||
if filepath.endswith(".rs"):
|
||
for match in tonic_result.finditer(source.content):
|
||
line_nr = _index_to_line_nr(source.content, match.start())
|
||
print(source.error("Prefer using tonic::Result<>", line_nr=line_nr))
|
||
num_errors += 1
|
||
|
||
if filepath.endswith(".proto"):
|
||
for line_nr, line in enumerate(source.lines):
|
||
if source.should_ignore(line_nr):
|
||
continue
|
||
if "/// " in line:
|
||
print(source.error("Use `//` not `///` for comments in .proto files", line_nr=line_nr))
|
||
num_errors += 1
|
||
|
||
if filepath.endswith((".rs", ".fbs")):
|
||
errors, lines_out = lint_vertical_spacing(source.lines)
|
||
for error in errors:
|
||
print(source.error(error))
|
||
num_errors += len(errors)
|
||
|
||
# Check for pyclass requirements (eq and module) in rerun_py Rust files
|
||
if filepath.startswith(f"{rerun_prefix}rerun_py/") and filepath.endswith(".rs"):
|
||
pyclass_errors, error_lines, error_codes = lint_pyclass_requirements(source.lines)
|
||
valid_errors = 0
|
||
for error, line_number, error_code in zip(pyclass_errors, error_lines, error_codes, strict=True):
|
||
if not source.should_ignore(line_number, code=error_code):
|
||
print(
|
||
source.error(error)
|
||
+ f"\n\tUnqualified NOLINT not allowed for pyclass lints. Use `NOLINT: ignore[{error_code}]` instead."
|
||
)
|
||
valid_errors += 1
|
||
num_errors += valid_errors
|
||
|
||
# Check for pymethods requirements (__str__ method) in rerun_py Rust files
|
||
pymethods_errors, pymethods_error_lines, pymethods_error_codes = lint_pymethods_requirements(source.lines)
|
||
valid_pymethods_errors = 0
|
||
for error, line_number, error_code in zip(
|
||
pymethods_errors, pymethods_error_lines, pymethods_error_codes, strict=True
|
||
):
|
||
if not source.should_ignore(line_number, code=error_code):
|
||
print(
|
||
source.error(error)
|
||
+ f"\n\tUnqualified NOLINT not allowed for pymethods lints. Use `NOLINT: ignore[{error_code}]` instead."
|
||
)
|
||
valid_pymethods_errors += 1
|
||
num_errors += valid_pymethods_errors
|
||
|
||
if args.fix:
|
||
source.rewrite(lines_out)
|
||
|
||
if filepath.endswith(".md"):
|
||
errors, lines_out = lint_markdown(filepath, source)
|
||
|
||
for error in errors:
|
||
print(source.error(error))
|
||
num_errors += len(errors)
|
||
|
||
if args.fix:
|
||
source.rewrite(lines_out)
|
||
elif 0 < num_errors:
|
||
print(f"Run with --fix to automatically fix {num_errors} errors.")
|
||
|
||
if filepath.endswith("Cargo.toml") and not filepath.startswith(f"{rerun_prefix}examples/rust"):
|
||
is_workspace = "[workspace]" in source.content
|
||
if not is_workspace:
|
||
error = lint_workspace_lints(source.content)
|
||
|
||
if error is not None:
|
||
print(source.error(error))
|
||
num_errors += 1
|
||
|
||
# Markdown-specific lints
|
||
if filepath.endswith(".md"):
|
||
errors = lint_frontmatter(filepath, source.content)
|
||
|
||
for error in errors:
|
||
print(source.error(error))
|
||
num_errors += len(errors)
|
||
|
||
return num_errors
|
||
|
||
|
||
def lint_crate_docs() -> int:
|
||
"""Make sure ARCHITECTURE.md talks about every single crate we have."""
|
||
|
||
crates_dir = Path(f"{rerun_prefix}crates")
|
||
architecture_md_file = Path(f"{rerun_prefix}ARCHITECTURE.md")
|
||
|
||
architecture_md = architecture_md_file.read_text("utf-8")
|
||
|
||
# extract all crate names ("re_…") from ARCHITECTURE.md to ensure they actually exist
|
||
listed_crates: dict[str, int] = {}
|
||
for i, line in enumerate(architecture_md.split("\n"), start=1):
|
||
for crate_name in re.findall(r"\bre_\w+", line):
|
||
if crate_name not in listed_crates:
|
||
listed_crates[crate_name] = i
|
||
|
||
error_count = 0
|
||
for cargo_toml in crates_dir.glob("**/Cargo.toml"):
|
||
crate = cargo_toml.parent
|
||
crate_name = crate.name
|
||
|
||
listed_crates.pop(crate_name, None)
|
||
|
||
if not re.search(r"\b" + crate_name + r"\b", architecture_md):
|
||
print(f"{architecture_md_file}: missing documentation for crate {crate.name}")
|
||
error_count += 1
|
||
|
||
for crate_name, line_nr in sorted(listed_crates.items(), key=lambda x: x[1]):
|
||
print(f"{architecture_md_file}:{line_nr}: crate name {crate_name} does not exist")
|
||
error_count += 1
|
||
|
||
return error_count
|
||
|
||
|
||
def main() -> None:
|
||
# Make sure we are bug free before we run:
|
||
test_split_words()
|
||
test_lint_line()
|
||
test_lint_vertical_spacing()
|
||
test_lint_pyclass_requirements()
|
||
test_lint_pymethods_requirements()
|
||
test_is_emoji()
|
||
|
||
parser = argparse.ArgumentParser(description="Lint code with custom linter.")
|
||
parser.add_argument(
|
||
"files",
|
||
metavar="file",
|
||
type=str,
|
||
nargs="*",
|
||
help="File paths. Empty = all files, recursively.",
|
||
)
|
||
parser.add_argument(
|
||
"--fix",
|
||
dest="fix",
|
||
action="store_true",
|
||
help="Automatically fix some problems.",
|
||
)
|
||
parser.add_argument(
|
||
"--extra",
|
||
dest="extra",
|
||
action="store_true",
|
||
help="Run some extra checks.",
|
||
)
|
||
|
||
args = parser.parse_args()
|
||
|
||
num_errors = 0
|
||
|
||
# This list of file extensions matches the one in `.github/workflows/documentation.yaml`
|
||
extensions = [
|
||
"c",
|
||
"cpp",
|
||
"fbs",
|
||
"h",
|
||
"hpp",
|
||
"html",
|
||
"js",
|
||
"md",
|
||
"mjs",
|
||
"proto",
|
||
"py",
|
||
"rs",
|
||
"sh",
|
||
"toml",
|
||
"ts",
|
||
"txt",
|
||
"wgsl",
|
||
"yaml",
|
||
"yml",
|
||
]
|
||
|
||
rerun_root = get_rerun_root()
|
||
|
||
# Find the git root. In the monorepo (reality), it's the parent of rerun_root.
|
||
# In the standalone rerun repo, it IS rerun_root.
|
||
repo = git.Repo(rerun_root, search_parent_directories=True)
|
||
assert repo.working_tree_dir is not None, "Expected a non-bare git repository"
|
||
repo_root = repo.working_tree_dir
|
||
os.chdir(repo_root)
|
||
|
||
# Path prefix from git root to the rerun directory.
|
||
# Monorepo: "./rerun/", Standalone: "./"
|
||
global rerun_prefix
|
||
rerun_relative = Path(rerun_root).relative_to(repo_root)
|
||
rerun_prefix = str(rerun_relative).replace("\\", "/")
|
||
if rerun_prefix == ".":
|
||
rerun_prefix = "./"
|
||
else:
|
||
rerun_prefix = "./" + rerun_prefix + "/"
|
||
|
||
# Helper to build a path relative to the git root, inside the rerun directory.
|
||
def rerun(path: str) -> str:
|
||
return f"{rerun_prefix}{path}"
|
||
|
||
exclude_paths = (
|
||
"./dataplatform/crates/redap_protos/Cargo.toml", # intentional [lints.clippy] override (see file header)
|
||
"./dataplatform/crates/redap_protos/src/v1alpha1", # auto-generated
|
||
rerun(".github/workflows/reusable_checks.yml"), # zombie TODO hunting job
|
||
rerun(".nox"),
|
||
rerun(".pytest_cache"),
|
||
rerun("CODE_STYLE.md"),
|
||
rerun("crates/build/re_types_builder/src/reflection.rs"), # auto-generated
|
||
rerun("crates/store/re_protos/proto/schema_snapshot.yaml"), # auto-generated
|
||
rerun("crates/store/re_protos/src/v0"), # auto-generated
|
||
rerun("crates/store/re_protos/src/v1alpha1"), # auto-generated
|
||
rerun("crates/viewer/re_ui/data/Inter-README.txt"), # third-party font readme (Inter)
|
||
rerun("crates/viewer/re_web_viewer_server/web_viewer/re_viewer.js"), # auto-generated by wasm_bindgen
|
||
rerun("docs/content/concepts/app-model.md"), # this really needs custom letter casing
|
||
rerun("docs/content/reference/cli.md"), # auto-generated
|
||
rerun("docs/snippets/all/tutorials/custom-application-id.cpp"), # nuh-uh, I don't want rerun_example_ here
|
||
rerun("docs/snippets/all/tutorials/custom-application-id.py"), # nuh-uh, I don't want rerun_example_ here
|
||
rerun("docs/snippets/all/tutorials/custom-application-id.rs"), # nuh-uh, I don't want rerun_example_ here
|
||
rerun("examples/assets"),
|
||
rerun("examples/python/detect_and_track_objects/cache/version.txt"),
|
||
rerun("examples/python/objectron/objectron/proto/"), # auto-generated
|
||
rerun("examples/rust/objectron/src/objectron.rs"), # auto-generated
|
||
rerun("rerun_cpp/docs/doxygen-awesome/"), # copied from an external repository
|
||
rerun("rerun_cpp/docs/html"),
|
||
rerun("rerun_cpp/src/rerun/c/arrow_c_data_interface.h"), # Not our code
|
||
rerun("rerun_cpp/src/rerun/third_party/cxxopts.hpp"), # vendored
|
||
rerun("rerun_js/docs/"), # auto-generated
|
||
rerun("rerun_js/node_modules"),
|
||
rerun("rerun_js/web-viewer-react/node_modules"),
|
||
rerun("rerun_js/web-viewer/index.js"),
|
||
rerun("rerun_js/web-viewer/inlined.js"),
|
||
rerun("rerun_js/web-viewer/node_modules"),
|
||
rerun("rerun_js/web-viewer/re_viewer_bg.js"), # auto-generated by wasm_bindgen
|
||
rerun("rerun_js/web-viewer/re_viewer.js"),
|
||
rerun("rerun_notebook/node_modules"),
|
||
rerun("rerun_notebook/src/rerun_notebook/static"),
|
||
rerun("rerun_py/.pytest_cache/"),
|
||
rerun("rerun_py/site/"), # is in `.gitignore` which this script doesn't fully respect
|
||
rerun("run_wasm/README.md"), # Has a "2d" lowercase example in a code snippet
|
||
rerun("scripts/lint.py"), # we contain all the patterns we are linting against
|
||
rerun("scripts/zombie_todos.py"),
|
||
rerun("tests/assets/lerobot/apple_storage/README.md"), # not ours
|
||
rerun("tests/python/gil_stress/main.py"),
|
||
rerun("tests/python/release_checklist/main.py"),
|
||
)
|
||
|
||
should_ignore = parse_gitignore(".gitignore") # TODO(#6730): parse all .gitignore files, not just top-level
|
||
|
||
if args.files:
|
||
for filepath in args.files:
|
||
filepath = os.path.join(".", os.path.relpath(filepath, repo_root))
|
||
filepath = str(filepath).replace("\\", "/")
|
||
extension = filepath.split(".")[-1]
|
||
if extension in extensions:
|
||
if should_ignore(filepath) or filepath.startswith(exclude_paths):
|
||
continue
|
||
num_errors += lint_file(filepath, args)
|
||
else:
|
||
tracked_files = [str(item[1].path) for item in repo.index.iter_blobs()]
|
||
for filepath in tracked_files:
|
||
filepath = "./" + filepath
|
||
filepath = filepath.replace("\\", "/")
|
||
|
||
# Only lint files inside the rerun or dataplatform directories.
|
||
# In the standalone rerun repo `rerun_prefix` is "./" so everything matches.
|
||
# In the monorepo (reality) we explicitly include both top-level Rust
|
||
# workspaces (`./rerun/` and `./dataplatform/`) so they share the same
|
||
# custom lints, and skip everything else (`node_modules/`, `landing/`, …).
|
||
allowed_prefixes: tuple[str, ...] = (rerun_prefix,)
|
||
if rerun_prefix != "./":
|
||
allowed_prefixes = allowed_prefixes + ("./dataplatform/",)
|
||
if not filepath.startswith(allowed_prefixes):
|
||
continue
|
||
|
||
extension = filepath.split(".")[-1]
|
||
if extension in extensions:
|
||
if filepath.startswith(exclude_paths):
|
||
continue
|
||
num_errors += lint_file(filepath, args)
|
||
|
||
# Since no files have been specified, we also run the global lints.
|
||
num_errors += lint_crate_docs()
|
||
|
||
if num_errors == 0:
|
||
print(f"{sys.argv[0]} finished without error")
|
||
sys.exit(0)
|
||
else:
|
||
print(f"{sys.argv[0]} found {num_errors} errors.")
|
||
sys.exit(1)
|
||
|
||
|
||
if __name__ == "__main__":
|
||
main()
|