format: reject raw pointer type aliases#44062
format: reject raw pointer type aliases#44062Retr0-XD wants to merge 2 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Retr0-XD <sakthi.harish@edgeverve.com>
|
Hi @Retr0-XD, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
Added a format validation check for raw pointer type aliases per STYLE.md. Would be great to get this reviewed. |
|
Anyway if fine to me and will defer this to @phlax |
There was a problem hiding this comment.
Pull request overview
Adds a new check_format validation to enforce the STYLE.md rule that raw (non-function) pointer types should not be introduced via using aliases.
Changes:
- Added a regex-based check in
tools/code_format/check_format.pyto flagusing Alias = T*;-style aliases. - Exempts aliases that include
(to avoid flagging function-pointer aliases.
tools/code_format/check_format.py
Outdated
| report_error("Don't use 'using testing::Test;, elaborate the type instead") | ||
| if line.startswith("using testing::TestWithParams;"): | ||
| report_error("Don't use 'using testing::Test;, elaborate the type instead") | ||
| if re.match(r"^\s*using\s+\w+\s*=\s*[^;]*\*[^;]*;", line) and "(" not in line: |
There was a problem hiding this comment.
This pattern is too broad: it matches any using ... = ...*...; line (any * anywhere), which will flag legitimate existing aliases and likely break the format CI (check_format is run repo-wide). For example, the repo already has raw-pointer aliases in iterator trait types (e.g. using element_type = AsyncClient::Stream*; / using pointer = element_type*; in source/common/http/muxdemux.h) and using ReflectableMessage = ::google::protobuf::Message*; in source/common/protobuf/protobuf.h.
Consider narrowing the check to only catch the STYLE.md case of aliasing a top-level raw pointer type (e.g. require the aliased type to end with * before optional const/volatile), and/or restrict to the naming pattern you want to ban (e.g. aliases whose name ends with Ptr). If some raw-pointer aliases are intentionally required, add an allowlist for those paths/patterns.
| if re.match(r"^\s*using\s+\w+\s*=\s*[^;]*\*[^;]*;", line) and "(" not in line: | |
| if re.match(r"^\s*using\s+\w*Ptr\s*=\s*[^;]*\*\s*(?:const|volatile)?\s*;", line) and "(" not in line: |
There was a problem hiding this comment.
this feedback seems correct/reasonable
There was a problem hiding this comment.
added suggestions to feat branch
|
merged suggestions into fix branch |
Changed regex from overly broad pattern that matches any line containing
a '*' character to a narrowed check that only flags aliases ending with
'Ptr' pattern (e.g., using BadPtr = int*;). This avoids false positives
on legitimate existing aliases like element_type and pointer iterators.
The check still exempts function pointers (lines containing '(').
Signed-off-by: Retr0-XD <sakthi.harish@edgeverve.com>
|
/retest |
Summary
Adds a check_format validation for a STYLE.md type-alias rule: regular raw pointers should not be type aliased.
Fixes #9840.
What changed
Validation