Add the English-comments check: corpus, ADR-0012, detector, report and reusable workflow #211
@@ -50,7 +50,7 @@ The two English comments `langid` flagged without a threshold are lines that are
|
||||
|
||||
A comment is judged **not English** when `langid` 1.1.6, restricted to `en da no sv de fr ms id nl`, with normalised probabilities, ranks a language other than English first with a probability of at least **0.6**, after this pre-filter, and only when the filter leaves **at least three words**:
|
||||
|
||||
- remove URLs and tokens that contain `/`, `\`, `::`, `_` or a digit, and camelCase tokens; split the rest on `-`;
|
||||
- drop every token that is not a word: a token with a code character (`/ \ { } [ ] " ` = < > | $ @ # % & + ^ ~ *`), an underscore, a digit or `::` (a path, a URL, a JSON key, an identifier); a token that starts with `-` (an option), contains `.` (a file name), is written in capitals (an acronym or a constant) or is camelCase; a leading label such as `Usage:` or `Note:`; and the words of the allowed-words file. Split what is left on `-`;
|
||||
- keep a token only if it is letters, with at most one inner apostrophe (letters include `æ ø å ä ö ü` and the other letters of Latin scripts);
|
||||
- a comment with fewer than three words left is **not judged** (it passes), and so is a comment that holds only identifiers, URLs or numbers.
|
||||
|
||||
@@ -58,6 +58,8 @@ On the corpus this gives 0 false positives in 300 English comments, Danish 44/45
|
||||
|
||||
The versions are pinned (`langid==1.1.6`, `numpy` at a pinned version) and installed in the job's virtual environment like the other tools ([ADR-0005]). Nothing is downloaded at run time and nothing is sent to a service.
|
||||
|
||||
The rules of the pre-filter came from the build, not from the first measurement. A scan of this repository with the first version of the filter found two false positives that the corpus did not contain: a docstring with a JSON example (`{"files": 3, "lines": 120, ...}`, judged French with probability 1.0) and `Usage:` lines with options and a file name (judged French with 0.6 and 0.88). The code-character, option, file-name, acronym and label rules above remove them; the corpus rates stayed the same, and the repository (31 source files) now gives no finding. Tests of the check keep both cases (the JSON docstring and the `Usage:` lines), so a change to the filter is measured against them.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
@@ -3,3 +3,5 @@ PyYAML>=6,<7
|
||||
coverage>=7,<8
|
||||
shellcheck-py==0.10.0.1
|
||||
shfmt-py==3.11.0.2
|
||||
langid==1.1.6
|
||||
numpy==2.1.3
|
||||
|
||||
@@ -0,0 +1,274 @@
|
||||
"""Offline tests for tools/english_comments.py (MIL-013, ADR-0012).
|
||||
|
||||
The detector is measured on the corpus in tests/fixtures/comments/; the rates
|
||||
of criterion 1 of MIL-013 are tests, so a change that lowers them fails.
|
||||
"""
|
||||
|
||||
import csv
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from workflow_source import REPO_ROOT, load_script
|
||||
|
||||
ec = load_script(REPO_ROOT / "tools" / "english_comments.py", "english_comments")
|
||||
CORPUS = REPO_ROOT / "tests" / "fixtures" / "comments"
|
||||
GIT = shutil.which("git")
|
||||
|
||||
|
||||
def corpus() -> list[tuple[str, str]]:
|
||||
rows: list[tuple[str, str]] = []
|
||||
for name in ("english.tsv", "foreign.tsv"):
|
||||
with (CORPUS / name).open(encoding="utf-8", newline="") as handle:
|
||||
reader = csv.DictReader(handle, delimiter="\t", quoting=csv.QUOTE_NONE)
|
||||
rows += [(row["lang"], row["text"]) for row in reader]
|
||||
return rows
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def results():
|
||||
found: dict[str, list[int]] = {}
|
||||
for language, text in corpus():
|
||||
counts = found.setdefault(language, [0, 0])
|
||||
counts[1] += 1
|
||||
counts[0] += ec.judge(text) is not None
|
||||
return found
|
||||
|
||||
|
||||
class TestCorpusRates:
|
||||
"""Criterion 1 of MIL-013: recall of at least 90 % for Danish and Malay, and at most
|
||||
one false positive per 200 English comments."""
|
||||
|
||||
def test_the_corpus_has_the_size_the_milestone_asks_for(self, results):
|
||||
assert results["en"][1] >= 200
|
||||
assert results["da"][1] >= 40
|
||||
assert results["ms"][1] >= 40
|
||||
assert sum(v[1] for k, v in results.items() if k not in ("en", "da", "ms")) >= 40
|
||||
|
||||
def test_english_comments_are_not_flagged(self, results):
|
||||
flagged, total = results["en"]
|
||||
assert flagged <= total // 200
|
||||
|
||||
@pytest.mark.parametrize("language", ["da", "ms"])
|
||||
def test_danish_and_malay_are_found(self, results, language):
|
||||
flagged, total = results[language]
|
||||
assert flagged / total >= 0.90
|
||||
|
||||
def test_other_languages_are_found(self, results):
|
||||
for language in ("de", "sv", "no", "fr"):
|
||||
flagged, total = results[language]
|
||||
assert flagged / total >= 0.80
|
||||
|
||||
|
||||
class TestWords:
|
||||
def test_paths_options_identifiers_and_numbers_are_not_words(self):
|
||||
text = "Run tools/badges.py --metrics code-metrics.json with snake_case camelCase v2 DIR"
|
||||
assert ec.words(text) == ["Run", "with"]
|
||||
|
||||
def test_json_and_code_characters_are_not_words(self):
|
||||
assert ec.words('{"files": 3, "lines": 120} x = 1 <a|b>') == []
|
||||
|
||||
def test_a_label_and_an_acronym_are_dropped(self):
|
||||
assert ec.words("Usage: lint the JSON files") == ["lint", "the", "files"]
|
||||
|
||||
def test_a_hyphenated_word_is_split_and_allowed_words_are_removed(self):
|
||||
assert ec.words("read-only access", frozenset({"access"})) == ["read", "only"]
|
||||
|
||||
def test_a_hidden_file_is_not_a_word_but_an_ellipsis_is_kept(self):
|
||||
assert ec.words("needs a .clang-format file...") == ["needs", "file"]
|
||||
|
||||
def test_quotes_inside_a_word_are_kept_once(self):
|
||||
assert ec.words("don't can't won''t") == ["don't", "can't"]
|
||||
|
||||
|
||||
class TestExtract:
|
||||
def test_python_comments_and_docstrings(self):
|
||||
source = '"""Module line one.\n\nSecond paragraph."""\n\n# a comment\nx = 1 # trailing\n\ndef f():\n """Doc of f."""\n'
|
||||
found = dict(ec.extract("a.py", source))
|
||||
assert found[1] == "Module line one."
|
||||
assert found[5] == "a comment"
|
||||
assert found[6] == "trailing"
|
||||
assert found[9] == "Doc of f."
|
||||
|
||||
def test_python_that_does_not_parse_falls_back_to_the_scanner(self):
|
||||
assert dict(ec.extract("a.py", "def (:\n# still a comment\n")) == {2: "still a comment"}
|
||||
|
||||
def test_shell_and_sql_line_comments(self):
|
||||
assert ec.extract("a.sh", "#!/bin/sh\n# real comment\necho '# not'\n") == [(2, "real comment")]
|
||||
assert ec.extract("a.sql", "SELECT 1; -- one row\n") == [(1, "one row")]
|
||||
|
||||
def test_c_block_and_line_comments_lose_their_decoration(self):
|
||||
source = "/**\n * First line\n * second line\n */\nint x; // trailing\n/// doc\n"
|
||||
assert ec.extract("a.c", source) == [(2, "First line"), (3, "second line"), (5, "trailing"), (6, "doc")]
|
||||
|
||||
def test_a_comment_inside_a_string_is_not_a_comment(self):
|
||||
assert ec.extract("a.c", 'char *s = "// not a comment";\n') == []
|
||||
|
||||
def test_tool_pragmas_are_not_prose(self):
|
||||
source = "x = 1 # noqa: E501\n# type: ignore\n# pylint: disable=x\n# a real comment\n"
|
||||
assert ec.extract("a.py", source) == [(4, "a real comment")]
|
||||
|
||||
def test_a_file_of_an_unknown_language_has_no_comments(self):
|
||||
assert ec.extract("notes.md", "# Heading\n") == []
|
||||
|
||||
def test_consecutive_lines_are_one_block(self):
|
||||
lines = [(1, "a"), (2, "b"), (4, "c"), (5, "d"), (5, "e")]
|
||||
assert ec.blocks(lines) == [(1, 2, "a b"), (4, 5, "c d e")]
|
||||
|
||||
|
||||
class TestCheckText:
|
||||
DANISH = "# Denne funktion beregner totalen for kurven og gemmer resultatet\n"
|
||||
|
||||
def test_a_danish_comment_is_a_finding_with_its_line(self):
|
||||
findings = ec.check_text("a.py", "x = 1\n" + self.DANISH)
|
||||
assert [(f.path, f.line, f.language) for f in findings] == [("a.py", 2, "da")]
|
||||
|
||||
def test_an_english_comment_is_not(self):
|
||||
assert ec.check_text("a.py", "# This function adds the totals of the basket together\n") == []
|
||||
|
||||
def test_only_the_blocks_on_the_given_lines_are_checked(self):
|
||||
text = self.DANISH + "x = 1\n" + self.DANISH
|
||||
assert [f.line for f in ec.check_text("a.py", text, {3})] == [3]
|
||||
assert ec.check_text("a.py", text, {2}) == []
|
||||
|
||||
def test_the_marker_skips_the_block(self):
|
||||
text = self.DANISH.rstrip() + " # english-comments: ignore\n"
|
||||
assert ec.check_text("a.py", text) == []
|
||||
|
||||
def test_allowed_words_are_not_judged(self):
|
||||
allowed = frozenset(self.DANISH.lower().replace("#", "").replace(",", "").split())
|
||||
assert ec.check_text("a.py", self.DANISH, allowed=allowed) == []
|
||||
|
||||
def test_a_short_comment_is_not_judged(self):
|
||||
assert ec.check_text("a.py", "# ret fejlen\n") == []
|
||||
|
||||
def test_json_in_a_docstring_is_not_french(self):
|
||||
source = '"""Count.\n\n {"files": 3, "lines": 120, "comment": 14, "blank": 22,\n "languages": {"Python": {"files": 2, "lines": 100}}}\n"""\n'
|
||||
assert ec.check_text("a.py", source) == []
|
||||
|
||||
def test_usage_lines_with_options_are_not_judged(self):
|
||||
text = "# Usage: lint-action-scripts.sh [<directory>] (default: actions)\n"
|
||||
assert ec.check_text("a.sh", text) == []
|
||||
|
||||
def test_describe_names_the_language_and_cuts_a_long_text(self):
|
||||
finding = ec.Finding("a.py", 1, 1, "da", 0.93, "x " * 100)
|
||||
message = ec.describe(finding)
|
||||
assert message.startswith('looks Danish (0.93): "') and message.endswith('..."')
|
||||
|
||||
|
||||
class TestAllowedWords:
|
||||
def test_words_one_per_line_with_comments(self, tmp_path):
|
||||
file = tmp_path / "allowed.txt"
|
||||
file.write_text("Kubernetes\n# a comment\nSkat # a Danish tax name\n\n", encoding="utf-8")
|
||||
assert ec.read_allowed_words(str(file)) == {"kubernetes", "skat"}
|
||||
|
||||
def test_no_file_named_is_no_words(self):
|
||||
assert ec.read_allowed_words(None) == frozenset()
|
||||
|
||||
def test_an_unreadable_file_is_an_error(self, tmp_path):
|
||||
with pytest.raises(ec.CheckError):
|
||||
ec.read_allowed_words(str(tmp_path / "missing.txt"))
|
||||
|
||||
|
||||
def run_git(directory: Path, *args: str) -> None:
|
||||
subprocess.run(["git", *args], cwd=directory, check=True, capture_output=True)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def repo(tmp_path):
|
||||
"""A repository with a base commit on main and a branch that adds comments."""
|
||||
if GIT is None:
|
||||
pytest.skip("git is needed")
|
||||
run_git(tmp_path, "init", "-q", "-b", "main")
|
||||
run_git(tmp_path, "config", "user.email", "t@example.org")
|
||||
run_git(tmp_path, "config", "user.name", "t")
|
||||
run_git(tmp_path, "config", "commit.gpgsign", "false")
|
||||
(tmp_path / "old.py").write_bytes(b"# Denne gamle funktion beregner totalen for kurven\nx = 1\n")
|
||||
run_git(tmp_path, "add", ".")
|
||||
run_git(tmp_path, "commit", "-q", "-m", "base")
|
||||
run_git(tmp_path, "checkout", "-q", "-b", "feature")
|
||||
return tmp_path
|
||||
|
||||
|
||||
def commit_file(repo: Path, name: str, content: bytes) -> None:
|
||||
(repo / name).write_bytes(content)
|
||||
run_git(repo, "add", ".")
|
||||
run_git(repo, "commit", "-q", "-m", name)
|
||||
|
||||
|
||||
class TestRepository:
|
||||
def test_only_the_comments_a_branch_adds_are_checked(self, repo, capsys):
|
||||
commit_file(repo, "new.py", b"# Denne funktion beregner totalen for kurven\n")
|
||||
code = ec.main(["check", "--directory", str(repo), "--base", "main", "--output", str(repo / "f.json")])
|
||||
out = capsys.readouterr().out
|
||||
assert code == 1
|
||||
assert "::error file=new.py,line=1::[english-comments] looks Danish" in out
|
||||
assert "old.py" not in out # the old Danish comment is not this branch's
|
||||
saved = json.loads((repo / "f.json").read_text(encoding="utf-8"))
|
||||
assert [(f["path"], f["line"], f["language"]) for f in saved] == [("new.py", 1, "da")]
|
||||
|
||||
def test_an_added_line_in_an_old_file_is_checked_and_the_old_ones_are_not(self, repo):
|
||||
commit_file(repo, "old.py", b"# Denne gamle funktion beregner totalen for kurven\nx = 1\n# Hold the lock while the file is written\n")
|
||||
findings, files = ec.check_repository(repo, "main", False, [], frozenset(), 0.6, 3)
|
||||
assert findings == [] and files == 1
|
||||
|
||||
def test_a_clean_branch_exits_zero(self, repo, capsys):
|
||||
commit_file(repo, "new.py", b"# This function adds the totals of the basket together\n")
|
||||
assert ec.main(["check", "--directory", str(repo), "--base", "main"]) == 0
|
||||
assert "All comments are in English: 1 file(s) checked." in capsys.readouterr().out
|
||||
|
||||
def test_scope_all_checks_every_tracked_file(self, repo, capsys):
|
||||
assert ec.main(["check", "--directory", str(repo), "--scope", "all"]) == 1
|
||||
assert "old.py:1" in capsys.readouterr().out
|
||||
|
||||
def test_excluded_binary_unknown_and_large_files_are_skipped(self, repo, monkeypatch):
|
||||
danish = b"# Denne funktion beregner totalen for kurven og gemmer resultatet\n"
|
||||
(repo / "vendor").mkdir()
|
||||
commit_file(repo, "vendor/skip.py", danish)
|
||||
commit_file(repo, "skipme.py", danish)
|
||||
commit_file(repo, "bin.py", danish + b"\x00")
|
||||
commit_file(repo, "notes.md", b"# Denne funktion beregner totalen for kurven\n")
|
||||
monkeypatch.setattr(ec, "MAX_FILE_BYTES", 10)
|
||||
commit_file(repo, "big.py", danish)
|
||||
findings, files = ec.check_repository(repo, "main", False, ["skipme.py"], frozenset(), 0.6, 3)
|
||||
assert (findings, files) == ([], 0)
|
||||
|
||||
def test_a_deleted_file_in_the_selection_is_skipped(self, repo):
|
||||
commit_file(repo, "gone.py", b"x = 1\n")
|
||||
(repo / "gone.py").unlink()
|
||||
findings, files = ec.check_repository(repo, "main", False, [], frozenset(), 0.6, 3)
|
||||
assert findings == []
|
||||
|
||||
def test_changed_lines_reads_every_hunk(self, repo):
|
||||
commit_file(repo, "h.py", b"a\nb\nc\nd\n")
|
||||
commit_file(repo, "h.py", b"a\nB\nc\nD\nE\n")
|
||||
assert ec.changed_lines(repo, "main")["h.py"] == {1, 2, 3, 4, 5}
|
||||
|
||||
|
||||
class TestMain:
|
||||
def test_a_missing_base_is_a_configuration_error(self, repo, capsys):
|
||||
assert ec.main(["check", "--directory", str(repo)]) == 2
|
||||
assert "::error::[configuration] give --base" in capsys.readouterr().out
|
||||
|
||||
def test_an_unknown_base_is_a_configuration_error(self, repo, capsys):
|
||||
assert ec.main(["check", "--directory", str(repo), "--base", "nope"]) == 2
|
||||
assert "failed: fatal" in capsys.readouterr().out
|
||||
|
||||
def test_a_missing_directory_is_a_configuration_error(self, tmp_path, capsys):
|
||||
assert ec.main(["check", "--directory", str(tmp_path / "x"), "--scope", "all"]) == 2
|
||||
assert "does not exist" in capsys.readouterr().out
|
||||
|
||||
@pytest.mark.parametrize("flags", [["--threshold", "0"], ["--threshold", "1.5"], ["--min-words", "0"]])
|
||||
def test_bad_numbers_are_a_configuration_error(self, tmp_path, capsys, flags):
|
||||
assert ec.main(["check", "--directory", str(tmp_path), "--scope", "all", *flags]) == 2
|
||||
assert "threshold" in capsys.readouterr().out
|
||||
|
||||
def test_langid_missing_is_a_configuration_error(self, repo, monkeypatch, capsys):
|
||||
commit_file(repo, "new.py", b"# Denne funktion beregner totalen for kurven og gemmer resultatet\n")
|
||||
monkeypatch.setattr(ec, "_identifier", None)
|
||||
monkeypatch.setitem(__import__("sys").modules, "langid.langid", None)
|
||||
assert ec.main(["check", "--directory", str(repo), "--base", "main"]) == 2
|
||||
assert "langid is not installed" in capsys.readouterr().out
|
||||
+16
-2
@@ -161,8 +161,17 @@ def _skip_string(line: str, start: int, quote: str, escapes: bool) -> int:
|
||||
return len(line)
|
||||
|
||||
|
||||
def _scan_line(line: str, language: Language, state: _State) -> tuple[bool, bool]:
|
||||
"""Scan one line; return (has_code, has_comment) and update the state."""
|
||||
def _scan_line(
|
||||
line: str,
|
||||
language: Language,
|
||||
state: _State,
|
||||
comments: list[str] | None = None,
|
||||
) -> tuple[bool, bool]:
|
||||
"""Scan one line; return (has_code, has_comment) and update the state.
|
||||
|
||||
When ``comments`` is given, the text of every comment on the line is
|
||||
appended to it (the English-comments check, ADR-0012, reads that text).
|
||||
"""
|
||||
has_code = False
|
||||
has_comment = False
|
||||
i = 0
|
||||
@@ -174,6 +183,9 @@ def _scan_line(line: str, language: Language, state: _State) -> tuple[bool, bool
|
||||
has_comment = True
|
||||
end = line.find(block[1], i)
|
||||
nested = line.find(block[0], i) if language.nested_block else -1
|
||||
if comments is not None:
|
||||
stop = nested if nested != -1 and (end == -1 or nested < end) else end
|
||||
comments.append(line[i : size if stop == -1 else stop])
|
||||
if nested != -1 and (end == -1 or nested < end):
|
||||
state.block_depth += 1
|
||||
i = nested + len(block[0])
|
||||
@@ -212,6 +224,8 @@ def _scan_line(line: str, language: Language, state: _State) -> tuple[bool, bool
|
||||
if comment is not None and line.startswith(comment, i):
|
||||
word_start = i == 0 or line[i - 1] in _WORD_START
|
||||
if word_start or not language.hash_at_word_start:
|
||||
if comments is not None:
|
||||
comments.append(line[i + len(comment) :])
|
||||
return has_code, True
|
||||
if language.triple_quotes and (
|
||||
line.startswith('"""', i) or line.startswith("'''", i)
|
||||
|
||||
@@ -0,0 +1,387 @@
|
||||
"""Find comments in source code that are not written in English (MIL-013, ADR-0012).
|
||||
|
||||
Run by .gitea/workflows/english-comments.yml. It reads the comments and docstrings
|
||||
of the source files of the languages of ADR-0010, joins the lines of one comment
|
||||
block, and judges the block with ``langid`` (restricted to a few candidate
|
||||
languages) after removing everything that is not a word: paths, options,
|
||||
identifiers, numbers and the words of the allowed-words file. A block with fewer
|
||||
than ``--min-words`` words left is not judged.
|
||||
|
||||
Usage:
|
||||
python tools/english_comments.py check [--directory DIR] [--scope changed|all]
|
||||
[--base REF] [--exclude PATTERNS] [--allowed-words FILE]
|
||||
[--threshold P] [--min-words N] [--output FILE]
|
||||
|
||||
With ``--scope changed`` (the default) only the comments on lines that the
|
||||
current branch added since ``--base`` (a git ref, for example ``origin/main``)
|
||||
are checked, so a pull request is not blamed for a comment it did not write;
|
||||
``--scope all`` checks every tracked file. A comment block that contains ``english-comments: ignore`` is
|
||||
skipped (a quotation, a name).
|
||||
|
||||
Every finding is printed as ``::error file=PATH,line=N::...`` and as ``PATH:N:
|
||||
looks Danish (0.93): "text"``; ``--output`` writes them as JSON for the
|
||||
reporting step. Exit codes: 0 no finding, 1 at least one finding, 2 a setting
|
||||
or a missing tool (printed as ``::error::[configuration] ...``).
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import ast
|
||||
import importlib.util
|
||||
import io
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tokenize
|
||||
from dataclasses import asdict, dataclass
|
||||
from pathlib import Path
|
||||
|
||||
DEFAULT_THRESHOLD = 0.6
|
||||
DEFAULT_MIN_WORDS = 3
|
||||
MAX_FILE_BYTES = 1_000_000
|
||||
CANDIDATES = ["en", "da", "no", "sv", "de", "fr", "ms", "id", "nl"]
|
||||
LANGUAGE_NAMES = {
|
||||
"da": "Danish",
|
||||
"no": "Norwegian",
|
||||
"sv": "Swedish",
|
||||
"de": "German",
|
||||
"fr": "French",
|
||||
"ms": "Malay",
|
||||
"id": "Indonesian",
|
||||
"nl": "Dutch",
|
||||
}
|
||||
IGNORE_MARKER = re.compile(r"english-comments:\s*ignore", re.IGNORECASE)
|
||||
# comments that configure a tool are not prose
|
||||
_PRAGMA = re.compile(
|
||||
r"^(noqa|type:|pragma|shellcheck|fmt:|pylint|ruff:|mypy:|pyright:|eslint|prettier|"
|
||||
r"-\*-|vim:|coding[:=]|nosec|isort:|nolint|clang-format|cppcheck|lint:)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
_WORD = re.compile(r"^[^\W\d_]+(?:'[^\W\d_]+)?$")
|
||||
_NOT_WORD_CHARS = re.compile(r"[\d_/\{}\[\]\"`=<>|$@#%&+^~*]")
|
||||
_HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
|
||||
|
||||
|
||||
class CheckError(RuntimeError):
|
||||
"""A setting or tool problem (exit code 2)."""
|
||||
|
||||
|
||||
def _load_sibling(name: str):
|
||||
path = Path(__file__).resolve().parent / f"{name}.py"
|
||||
spec = importlib.util.spec_from_file_location(name, path)
|
||||
assert spec is not None and spec.loader is not None
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
count_loc = _load_sibling("count_loc")
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class Finding:
|
||||
"""A comment block that is probably not English."""
|
||||
|
||||
path: str
|
||||
line: int
|
||||
end_line: int
|
||||
language: str
|
||||
probability: float
|
||||
text: str
|
||||
|
||||
|
||||
def _clean(text: str) -> str:
|
||||
"""Remove comment decoration (``*``, ``/``, ``#``, ``-``, ``=``) from both ends."""
|
||||
text = re.sub(r"^[/*!#<>=\-\s]+", "", text.strip())
|
||||
return re.sub(r"[*/\s]+$", "", text).strip()
|
||||
|
||||
|
||||
def _python_comments(source: str) -> list[tuple[int, str]]:
|
||||
"""Comments (tokenizer) and docstring lines (ast) of a Python file."""
|
||||
found: list[tuple[int, str]] = []
|
||||
for token in tokenize.generate_tokens(io.StringIO(source).readline):
|
||||
if token.type == tokenize.COMMENT:
|
||||
found.append((token.start[0], _clean(token.string)))
|
||||
for node in ast.walk(ast.parse(source)):
|
||||
if isinstance(node, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)):
|
||||
first = node.body[0] if node.body else None
|
||||
if (
|
||||
isinstance(first, ast.Expr)
|
||||
and isinstance(first.value, ast.Constant)
|
||||
and isinstance(first.value.value, str)
|
||||
):
|
||||
for offset, line in enumerate(first.value.value.splitlines()):
|
||||
found.append((first.value.lineno + offset, _clean(line)))
|
||||
return found
|
||||
|
||||
|
||||
def _scanned_comments(text: str, language) -> list[tuple[int, str]]:
|
||||
"""Comments of any other language, from the scanner of the code counter."""
|
||||
state = count_loc._State()
|
||||
found: list[tuple[int, str]] = []
|
||||
lines = text.replace("\r\n", "\n").replace("\r", "\n").split("\n")
|
||||
for number, line in enumerate(lines, start=1):
|
||||
pieces: list[str] = []
|
||||
count_loc._scan_line(line, language, state, pieces)
|
||||
if number == 1 and pieces and pieces[0].startswith("!"):
|
||||
pieces = pieces[1:] # a shebang line is not a comment
|
||||
joined = " ".join(_clean(piece) for piece in pieces).strip()
|
||||
if joined:
|
||||
found.append((number, joined))
|
||||
return found
|
||||
|
||||
|
||||
def extract(path: str, text: str) -> list[tuple[int, str]]:
|
||||
"""Return (line, text) for every comment line of a source file, sorted by line.
|
||||
|
||||
A file of a language the code counter does not know has no comments here.
|
||||
"""
|
||||
language = count_loc.language_of(path)
|
||||
if language is None:
|
||||
return []
|
||||
if language.name == "Python":
|
||||
try:
|
||||
found = _python_comments(text)
|
||||
except (SyntaxError, tokenize.TokenError):
|
||||
found = _scanned_comments(text, language)
|
||||
else:
|
||||
found = _scanned_comments(text, language)
|
||||
return sorted((n, t) for n, t in found if t and not _PRAGMA.match(t))
|
||||
|
||||
|
||||
def blocks(lines: list[tuple[int, str]]) -> list[tuple[int, int, str]]:
|
||||
"""Join comment lines on consecutive line numbers: (first line, last line, text)."""
|
||||
joined: list[tuple[int, int, str]] = []
|
||||
for number, text in lines:
|
||||
if joined and joined[-1][1] in (number, number - 1):
|
||||
first, _, previous = joined[-1]
|
||||
joined[-1] = (first, number, f"{previous} {text}")
|
||||
else:
|
||||
joined.append((number, number, text))
|
||||
return joined
|
||||
|
||||
|
||||
def words(text: str, allowed: frozenset[str] = frozenset()) -> list[str]:
|
||||
"""The words of a comment: no URLs, paths, options, identifiers or allowed words."""
|
||||
out: list[str] = []
|
||||
tokens = text.split()
|
||||
if tokens and tokens[0].endswith(":") and tokens[0][:-1].isalpha():
|
||||
tokens = tokens[1:] # a label such as "Usage:" or "Note:" is not prose
|
||||
for token in tokens:
|
||||
if len(token) > 1 and token[0] == "." and token[1].isalpha():
|
||||
continue # a hidden file such as .clang-format
|
||||
token = token.strip("().,;:!?")
|
||||
# a token with code characters (quotes, braces, `=`, `/`, `_`, digits) is not a word
|
||||
if not token or _NOT_WORD_CHARS.search(token) or "::" in token:
|
||||
continue
|
||||
if token.startswith("-") or "." in token or (len(token) > 1 and token.isupper()):
|
||||
continue # an option, a file name, an acronym or a constant
|
||||
if re.search(r"[a-z][A-Z]", token):
|
||||
continue
|
||||
for part in token.split("-"):
|
||||
if len(part) > 1 and _WORD.match(part) and part.lower() not in allowed:
|
||||
out.append(part)
|
||||
return out
|
||||
|
||||
|
||||
_identifier = None
|
||||
|
||||
|
||||
def _identifier_model():
|
||||
global _identifier
|
||||
if _identifier is None:
|
||||
try:
|
||||
from langid.langid import LanguageIdentifier, model
|
||||
except ImportError:
|
||||
raise CheckError("langid is not installed (pip install langid==1.1.6).") from None
|
||||
_identifier = LanguageIdentifier.from_modelstring(model, norm_probs=True)
|
||||
_identifier.set_languages(CANDIDATES)
|
||||
return _identifier
|
||||
|
||||
|
||||
def judge(
|
||||
text: str,
|
||||
allowed: frozenset[str] = frozenset(),
|
||||
threshold: float = DEFAULT_THRESHOLD,
|
||||
min_words: int = DEFAULT_MIN_WORDS,
|
||||
) -> tuple[str, float] | None:
|
||||
"""Return (language code, probability) when the text is not English, else None."""
|
||||
kept = words(text, allowed)
|
||||
if len(kept) < min_words:
|
||||
return None
|
||||
code, probability = _identifier_model().classify(" ".join(kept))
|
||||
if code != "en" and probability >= threshold:
|
||||
return code, float(probability)
|
||||
return None
|
||||
|
||||
|
||||
def check_text(
|
||||
path: str,
|
||||
text: str,
|
||||
lines: set[int] | None = None,
|
||||
allowed: frozenset[str] = frozenset(),
|
||||
threshold: float = DEFAULT_THRESHOLD,
|
||||
min_words: int = DEFAULT_MIN_WORDS,
|
||||
) -> list[Finding]:
|
||||
"""Findings of one file; with ``lines`` only the blocks that touch those line numbers."""
|
||||
findings: list[Finding] = []
|
||||
for first, last, body in blocks(extract(path, text)):
|
||||
if IGNORE_MARKER.search(body):
|
||||
continue
|
||||
if lines is not None and not any(first <= n <= last for n in lines):
|
||||
continue
|
||||
verdict = judge(body, allowed, threshold, min_words)
|
||||
if verdict:
|
||||
findings.append(Finding(path, first, last, verdict[0], round(verdict[1], 2), body))
|
||||
return findings
|
||||
|
||||
|
||||
def read_allowed_words(path: str | None) -> frozenset[str]:
|
||||
"""Words, one per line, ``#`` starts a comment; the file may be missing only when not named."""
|
||||
if not path:
|
||||
return frozenset()
|
||||
try:
|
||||
content = Path(path).read_text(encoding="utf-8")
|
||||
except OSError:
|
||||
raise CheckError(f"allowed-words file '{path}' cannot be read.") from None
|
||||
return frozenset(
|
||||
line.split("#", 1)[0].strip().lower()
|
||||
for line in content.splitlines()
|
||||
if line.split("#", 1)[0].strip()
|
||||
)
|
||||
|
||||
|
||||
def _git(directory: Path, *args: str) -> str:
|
||||
result = subprocess.run(
|
||||
["git", "-c", "core.quotepath=off", *args],
|
||||
cwd=directory,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
raise CheckError(f"git {' '.join(args[:2])} failed: {result.stderr.strip()[:200]}")
|
||||
return result.stdout
|
||||
|
||||
|
||||
def changed_lines(directory: Path, base: str) -> dict[str, set[int]]:
|
||||
"""Added or changed line numbers per file since the merge base with ``base``."""
|
||||
out = _git(directory, "diff", "-U0", "--no-color", "--diff-filter=ACMR", f"{base}...HEAD")
|
||||
result: dict[str, set[int]] = {}
|
||||
current: set[int] | None = None
|
||||
for line in out.splitlines():
|
||||
if line.startswith("+++ "):
|
||||
name = line[4:]
|
||||
current = result.setdefault(name[2:], set()) if name.startswith("b/") else None
|
||||
continue
|
||||
hunk = _HUNK.match(line)
|
||||
if hunk and current is not None:
|
||||
start = int(hunk.group(1))
|
||||
size = int(hunk.group(2)) if hunk.group(2) is not None else 1
|
||||
current.update(range(start, start + size))
|
||||
return result
|
||||
|
||||
|
||||
def tracked_files(directory: Path) -> list[str]:
|
||||
return [name for name in _git(directory, "ls-files").splitlines() if name]
|
||||
|
||||
|
||||
def _excluded(path: str, excludes) -> bool:
|
||||
parts = path.split("/")
|
||||
if any(part in count_loc.DEFAULT_EXCLUDED_DIRS for part in parts[:-1]):
|
||||
return True
|
||||
return excludes.matches(parts[-1], path)
|
||||
|
||||
|
||||
def check_repository(
|
||||
directory: Path,
|
||||
base: str | None,
|
||||
check_all: bool,
|
||||
patterns: list[str],
|
||||
allowed: frozenset[str],
|
||||
threshold: float,
|
||||
min_words: int,
|
||||
) -> tuple[list[Finding], int]:
|
||||
"""Return (findings, number of files read)."""
|
||||
excludes = count_loc.Excludes(patterns)
|
||||
if check_all:
|
||||
selection: dict[str, set[int] | None] = {name: None for name in tracked_files(directory)}
|
||||
else:
|
||||
if not base:
|
||||
raise CheckError("give --base (a git ref such as origin/main) or --all.")
|
||||
selection = dict(changed_lines(directory, base))
|
||||
findings: list[Finding] = []
|
||||
read = 0
|
||||
for name in sorted(selection):
|
||||
if count_loc.language_of(name) is None or _excluded(name, excludes):
|
||||
continue
|
||||
target = directory / name
|
||||
if not target.is_file() or target.stat().st_size > MAX_FILE_BYTES:
|
||||
continue
|
||||
data = target.read_bytes()
|
||||
if not count_loc._is_text(data):
|
||||
continue
|
||||
read += 1
|
||||
text = data.decode("utf-8", errors="replace")
|
||||
findings += check_text(name, text, selection[name], allowed, threshold, min_words)
|
||||
return findings, read
|
||||
|
||||
|
||||
def describe(finding: Finding) -> str:
|
||||
name = LANGUAGE_NAMES.get(finding.language, finding.language)
|
||||
shown = finding.text if len(finding.text) <= 120 else finding.text[:117] + "..."
|
||||
return f'looks {name} ({finding.probability}): "{shown}"'
|
||||
|
||||
|
||||
def _split(value: str) -> list[str]:
|
||||
return [item.strip() for item in re.split(r"[,\n]", value) if item.strip()]
|
||||
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
parser = argparse.ArgumentParser(description="Check that source code comments are in English.")
|
||||
parser.add_argument("command", choices=["check"])
|
||||
parser.add_argument("--directory", default=".")
|
||||
parser.add_argument("--base", default="")
|
||||
parser.add_argument("--scope", choices=["changed", "all"], default="changed")
|
||||
parser.add_argument("--exclude", default="")
|
||||
parser.add_argument("--allowed-words", default="")
|
||||
parser.add_argument("--threshold", type=float, default=DEFAULT_THRESHOLD)
|
||||
parser.add_argument("--min-words", type=int, default=DEFAULT_MIN_WORDS)
|
||||
parser.add_argument("--output", default="")
|
||||
args = parser.parse_args(argv)
|
||||
try:
|
||||
if not 0.0 < args.threshold <= 1.0 or args.min_words < 1:
|
||||
raise CheckError("threshold must be above 0 and at most 1, and min-words at least 1.")
|
||||
directory = Path(args.directory)
|
||||
if not directory.is_dir():
|
||||
raise CheckError(f"directory '{args.directory}' does not exist.")
|
||||
allowed = read_allowed_words(args.allowed_words or None)
|
||||
findings, files = check_repository(
|
||||
directory,
|
||||
args.base or None,
|
||||
args.scope == "all",
|
||||
_split(args.exclude),
|
||||
allowed,
|
||||
args.threshold,
|
||||
args.min_words,
|
||||
)
|
||||
except CheckError as error:
|
||||
print(f"::error::[configuration] {error}")
|
||||
return 2
|
||||
for finding in findings:
|
||||
message = describe(finding)
|
||||
print(f"::error file={finding.path},line={finding.line}::[english-comments] {message}")
|
||||
print(f"{finding.path}:{finding.line}: {message}")
|
||||
if args.output:
|
||||
Path(args.output).write_text(
|
||||
json.dumps([asdict(f) for f in findings], indent=2, ensure_ascii=False) + "\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
summary = f"{len(findings)} comment(s) not in English in {files} file(s) checked."
|
||||
print(summary if findings else f"All comments are in English: {files} file(s) checked.")
|
||||
return 1 if findings else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in New Issue
Block a user