diff --git a/AGENTS.md b/AGENTS.md index 113cfe1b2a..30ce6dfe9a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -737,7 +737,9 @@ file does, and it is the authority when they disagree. The most useful starting 6. **Avoid `std::deque`:** It allocates in 512-byte blocks regardless of element size, guaranteeing at least 512 bytes of RAM usage immediately. This is a major source of crashes on memory-constrained devices. - 7. **Detection:** Look for these patterns in compiler output: + 7. **Never use `new (std::nothrow)`:** On ESP-IDF exceptions are disabled, so a failed nothrow allocation aborts instead of returning `nullptr`. Use `RAMAllocator` from `esphome/core/helpers.h`; CI rejects `std::nothrow`. + + 8. **Detection:** Look for these patterns in compiler output: - Large code sections with STL symbols (vector, map, set) - `alloc`, `realloc`, `dealloc` in symbol names - `_M_realloc_insert`, `_M_default_append` (vector reallocation) diff --git a/script/ci-custom.py b/script/ci-custom.py index bdf7750ce8..aaf177c941 100755 --- a/script/ci-custom.py +++ b/script/ci-custom.py @@ -164,7 +164,21 @@ def lint_post_check(func): return func -def lint_re_check(regex, **kwargs): +def _nolint_in_match(content, haystack, match, mask): + """With masking, only a trailing comment counts: the raw span still holds string contents, and + the masked text is blank exactly where comments and strings were, so NOLINT must sit after the + last real code character of the span.""" + raw = content[match.start() : match.end()] + if not mask: + return "NOLINT" in raw + masked = haystack[match.start() : match.end()].rstrip() + return "NOLINT" in raw[len(masked) :] + + +def lint_re_check(regex, mask=False, prefilter=None, **kwargs): + """mask=True blanks comments and string literals first so prose about the pattern is not reported; + the masked text keeps its length, so match offsets still index the original content. + prefilter is a literal every match must contain, checked before the costlier masking.""" flags = kwargs.pop("flags", re.MULTILINE) prog = re.compile(regex, flags) decor = lint_content_check(**kwargs) @@ -173,8 +187,11 @@ def lint_re_check(regex, **kwargs): @functools.wraps(func) def new_func(fname, content): errs = [] - for match in prog.finditer(content): - if "NOLINT" in match.group(0): + if prefilter is not None and prefilter not in content: + return errs + haystack = _mask_cpp_comments_strings(content) if mask else content + for match in prog.finditer(haystack): + if _nolint_in_match(content, haystack, match, mask): continue lineno = content.count("\n", 0, match.start()) + 1 substr = content[: match.start()] @@ -1199,6 +1216,25 @@ def lint_no_std_bind(fname, match): ) +@lint_re_check( + r"[^\w]std\s*::\s*nothrow\b" + CPP_RE_EOL, + mask=True, + prefilter="nothrow", + include=cpp_include, +) +def lint_no_std_nothrow(fname, match): + return ( + f"{highlight('new (std::nothrow)')} aborts on ESP-IDF when the allocation fails, exceptions are disabled " + f"there, so it never returns nullptr.\n" + f"Please use {highlight('RAMAllocator')} from esphome/core/helpers.h, which does.\n" + f" Before: {highlight('auto *buf = new (std::nothrow) uint8_t[n];')}\n" + f" After: {highlight('auto buf = RAMAllocator().make_unique_array_for_overwrite(n);')}\n" + f"For one object use {highlight('RAMAllocator().make_unique(args...)')}; both return empty on failure.\n" + f"Default flags prefer PSRAM; pass RAMAllocator::PREFER_INTERNAL to keep it where new put it.\n" + f"(If strictly necessary, add `// NOLINT` to the end of the line)" + ) + + LOG_CALL_START_RE = re.compile(r"ESP_LOG\w+\s*\(") # 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 diff --git a/tests/script/test_ci_custom.py b/tests/script/test_ci_custom.py index be2c98051f..5a943cda08 100644 --- a/tests/script/test_ci_custom.py +++ b/tests/script/test_ci_custom.py @@ -1,4 +1,6 @@ -"""Unit tests for the ESP_LOG-needs-braces lint rule in script/ci-custom.py. +"""Unit tests for the ESP_LOG-needs-braces and std::nothrow lint rules in script/ci-custom.py. + +The nothrow rule is a masked lint_re_check, so its tests also pin the decorator's mask option. The rule flags an if/else/for/while whose only body is an unbraced ESP_LOG*() call (which becomes an empty statement -- and a -Wempty-body warning -- once the log level compiles the macro out). These @@ -151,6 +153,43 @@ def test_nolint_on_control_line_suppresses() -> None: assert not _lint("if (x) // NOLINT\n ESP_LOGD(t);\n") +# --- std::nothrow --- + + +def _lint_nothrow(content: str) -> list: + return ci_custom.lint_no_std_nothrow("test.cpp", content) + + +def test_nothrow_is_reported_at_its_line_and_column_and_points_at_ramallocator() -> ( + None +): + errors = _lint_nothrow( + "int a;\nint b;\n auto *p = new (std::nothrow) uint8_t[n];\n" + ) + assert [(line, col) for line, col, _msg in errors] == [(3, 18)] + assert "RAMAllocator" in errors[0][2] + + +def test_nothrow_spacing_and_the_nothrow_t_type() -> None: + assert len(_lint_nothrow("auto *p = new (std :: nothrow) Foo;\n")) == 1 + assert not _lint_nothrow( + "void *operator new(size_t n, const std::nothrow_t &) noexcept;\n" + ) + + +def test_nothrow_in_comments_and_strings_is_masked() -> None: + assert not _lint_nothrow("// new (std::nothrow) aborts on ESP-IDF\n") + assert not _lint_nothrow('ESP_LOGD(TAG, "std::nothrow");\n') + + +def test_nothrow_nolint_suppresses() -> None: + assert not _lint_nothrow("auto *p = new (std::nothrow) Foo; // NOLINT\n") + + +def test_nothrow_nolint_inside_a_string_does_not_suppress() -> None: + assert len(_lint_nothrow('auto *p = new (std::nothrow) Foo; log("NOLINT");\n')) == 1 + + # --- rule: UNIT_ constants must not be redefined (mirror of the CONF_ check) --- # Real UNIT_ constants that live in each canonical home.