Report unbalanced log calls once, honour only bare NOLINT, handle raw strings and comments before branches

This commit is contained in:
J. Nick Koston
2026-08-30 22:26:43 -05:00
parent 68bac9f315
commit 524fc453c1
2 changed files with 60 additions and 12 deletions
+20 -10
View File
@@ -974,13 +974,21 @@ def lint_no_std_bind(fname, match):
LOG_CALL_START_RE = re.compile(r"ESP_LOG\w+\s*\(")
# Comments, string literals and single char literals are consumed whole so ; ( ) ? : inside them
# are never seen. A char literal is exactly one (escaped) char so a digit separator like 1'000'000
# cannot open one.
CPP_SKIP_RE = r'//[^\n]*|/\*.*?\*/|"(?:[^"\\]|\\.)*"|\'(?:[^\'\\\n]|\\.)\''
# Comments, raw/plain string literals and single char literals are consumed whole so ; ( ) ? :
# inside them are never seen. A char literal is exactly one (escaped) char so a digit separator
# like 1'000'000 cannot open one.
CPP_COMMENT_RE = r"//[^\n]*|/\*.*?\*/"
CPP_SKIP_RE = (
CPP_COMMENT_RE + r'|R"([^(\s]*)\(.*?\)\1"|"(?:[^"\\]|\\.)*"|\'(?:[^\'\\\n]|\\.)\''
)
LOG_CALL_TOKEN_RE = re.compile(CPP_SKIP_RE + r"|[()]", re.DOTALL)
# The last alternative matches a ? or : whose next non-space char opens a literal, a ternary branch.
LOG_TERNARY_LITERAL_RE = re.compile(CPP_SKIP_RE + r'|[?:]\s*(?=")', re.DOTALL)
# The last alternative matches a ? or : followed (after spaces or comments) by an opening quote,
# i.e. a string literal used as a ternary branch.
LOG_TERNARY_LITERAL_RE = re.compile(
CPP_SKIP_RE + r"|[?:](?:\s|" + CPP_COMMENT_RE + r')*(?=")', re.DOTALL
)
# A bare NOLINT; a clang-tidy NOLINT(check-name) is aimed at a different tool.
NOLINT_RE = re.compile(r"\bNOLINT\b(?!\()")
def _line_col(content: str, pos: int) -> tuple[int, int]:
@@ -1090,8 +1098,9 @@ LOG_LITERAL_LINT_EXCLUDE = [
"esphome/components/usb_host/*",
"esphome/components/zigbee/*",
"esphome/components/lvgl/*",
# Test fixtures - not production embedded code
# Test fixtures and host only unit tests - not production embedded code
"tests/integration/fixtures/*",
"tests/components/*",
]
@@ -1102,12 +1111,13 @@ def lint_log_no_bare_literal_ternary(
errs = []
for log_start, log_text in _iter_log_calls(content):
if log_text is None:
errs.append(_unbalanced_log_call_error(content, log_start))
continue
continue # reported by lint_log_multiline_continuation, which sees every file
# A NOLINT anywhere on the lines the call spans silences every branch in it
first_line = content.rfind("\n", 0, log_start) + 1
last_line = content.find("\n", log_start + len(log_text))
if "NOLINT" in content[first_line : last_line if last_line != -1 else None]:
if NOLINT_RE.search(
content[first_line : last_line if last_line != -1 else None]
):
continue
for offset, literal in _find_ternary_literals(log_text):
lineno, col = _line_col(content, log_start + offset)
+40 -2
View File
@@ -35,6 +35,8 @@ def _ternary_errors(content: str) -> list[tuple[int, int]]:
"ESP_LOGD(TAG, \"%d\", 1'000'000)",
'ESP_LOGD(TAG, // it\'s a comment with ) and (\n "x")',
'ESP_LOGD(TAG, /* :) */ "x")',
'ESP_LOGD(TAG, "%s", R"(say "hi" :) )")',
'ESP_LOGD(TAG, "%s", R"x(a)"b)x")',
],
)
def test_iter_log_calls_spans_whole_call(content: str) -> None:
@@ -42,13 +44,44 @@ def test_iter_log_calls_spans_whole_call(content: str) -> None:
assert calls == [content]
def test_iter_log_calls_reports_unbalanced_call() -> None:
def test_iter_log_calls_reports_unbalanced_call_once() -> None:
content = 'ESP_LOGD(TAG, "x";\nvoid f();'
assert _calls(content) == [None]
errs = ci_custom.lint_log_no_bare_literal_ternary(Path("x.cpp"), content)
errs = ci_custom.lint_log_multiline_continuation(Path("x.cpp"), content)
assert len(errs) == 1
assert errs[0][:2] == (1, 1)
assert "no matching closing parenthesis" in errs[0][2]
assert _ternary_errors(content) == []
@pytest.mark.parametrize(
("content", "expected"),
[
# A ; inside the format string no longer cuts the call short
('ESP_LOGD(TAG, "a; b\\nc %s", x);', [(1, 20)]),
# A \n%s continuation is exempt since %s may expand to leading whitespace
('ESP_LOGD(TAG, "a\\n%s", x);', []),
('ESP_LOGD(TAG, "a\\n b");', []),
],
)
def test_multiline_continuation_detection(
content: str, expected: list[tuple[int, int]]
) -> None:
errs = ci_custom.lint_log_multiline_continuation(Path("x.cpp"), content)
assert [(line, col) for line, col, _ in errs] == expected
def test_exclusion_list_only_names_components_without_esp8266_tests() -> None:
root = Path(__file__).parent / ".." / ".."
for pattern in ci_custom.LOG_LITERAL_LINT_EXCLUDE:
if not pattern.startswith("esphome/components/"):
continue
prefix = pattern.removeprefix("esphome/components/").split("/")[0]
for comp in (root / "esphome" / "components").glob(prefix):
test = root / "tests" / "components" / comp.name / "test.esp8266-ard.yaml"
assert not test.exists(), (
f"{comp.name} builds for ESP8266, drop {pattern!r}"
)
@pytest.mark.parametrize(
@@ -70,6 +103,11 @@ def test_iter_log_calls_reports_unbalanced_call() -> None:
('ESP_LOGD(TAG, "x:" "y %s", p);', []),
('ESP_LOGD(TAG, "%s", x ? "on" : "off"); // NOLINT', []),
('ESP_LOGD(TAG, "%s",\n x ? "yes"\n : "no"); // NOLINT', []),
('ESP_LOGD(TAG, "%s", x ? /* c */ "on" : "off");', [(1, 33), (1, 40)]),
(
'ESP_LOGD(TAG, "%s",\n x ? "on" // NOLINT(some-clang-check)\n : "off");',
[(2, 14), (3, 14)],
),
],
)
def test_ternary_literal_detection(