tools/nxstyle: fix most of the incompatibilities with the coding standard - #19555
tools/nxstyle: fix most of the incompatibilities with the coding standard#19555raiden00pl wants to merge 8 commits into
Conversation
|
|
@raiden00pl please fix conflict |
75001e4 to
cb2aa42
Compare
|
I'll be adding changes to nxstyle in separate PRs. I'm still unsure whether to fix all the nxstyle upstream issues, or leave it for the future and fix it step by step. |
cb2aa42 to
2e7023c
Compare
Only lines beginning with a C keyword were checked, so an assignment or a call could sit at any column. Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
|
While debugging a CI failure on an unrelated PR (apache/nuttx-apps#3751), I found a false positive in the Root cause: the exemption for a macro-that-takes-a-block (e.g. else if (prevlastcode == ')' && indent == prevcodeindent + 2)
{
}When the macro call wraps its arguments onto a continuation line, Minimal repro (built master's void add_ready(FAR struct action_manager_s *am,
FAR struct action_s *ready)
{
list_for_every_entry(&am->ready_actions, ready, struct action_s,
ready_node)
{
do_something(ready);
}
}This isn't an isolated case — the same false positive currently fires on existing, unmodified code such as I confirmed this branch ( else if (prevlastcode == ')' &&
(indent == prevcodeindent + 2 ||
indent == stmt_lineindent + 2))
{
}Given the scope of this PR is large and it currently has merge conflicts against master, would it be worth splitting out just the nxstyle.c fix (or at least this specific brace-alignment exemption) into its own smaller PR so the regression can be addressed independently? Happy to help if useful. |
cederom
left a comment
There was a problem hiding this comment.
Thank you @raiden00pl looks like this one is really important and welcome as it fixes CI checks in PRs that does not touch the impacted parts :-)
The brace was compared against the line before it rather than the line the statement began on, so a macro broken over two lines was reported. Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
The fixed list of type names holds neither uint64_t nor any NuttX typedef, so declarations using them went unchecked. Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
The standard asks for a blank line between the local declarations at the head of a function and the code. A declaration is recognised by its shape, since no list of type names can be complete. Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
…races Alternatives selected by conditional compilation share the braces that follow, and a branch may hold statements before reaching its condition. Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
fix nxstyle errors for sched Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
fix nxstyle errors for audio Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
fix nxstyle errors for net Signed-off-by: raiden00pl <raiden00@railab.me> Assisted-by: Claude Code
|
@JianyuWang0623 the fix for this should be in 7ead748 I created PR for this commit: #19955 but first #19934 must be merged |
2e7023c to
f50b1ad
Compare
#19934 is now merged :-) |
Summary
This PR fixes a lot of gaps in the nxstyle tool. Unfortunately, the codebase contains many inconsistencies with the coding standard. Fixing everything might mess up git history a bit, so I'm not sure if it's worth it.
For now I only fixed nxstyle.c, sched, audio and net
Impact
better compatibility with nuttx coding style.
This solves some of the issues related to nxstyle, but I didn't check which ones: https://github.com/apache/nuttx/issues?q=is%3Aissue%20state%3Aopen%20nxstyle
Testing
Output from
/tools/nxstyle_sweep.shthat checks all files: