nxstyle (Was Re: Working Effectively (was Point of Order))

[Gregory Nutt <sp...@gmail.com>]

> Greg,  I'll finish the nxstyle & check script these days firstly and then
> take time to handle the two issues you mentioned if allowed. I wish I could
> be competent to take over full responsibility for nxstyle.c.

I think it would be great if you took over primary responsibility for 
nxstyle.  With regard to the additional features I was planning to add, 
I do have a design in mind for the automatic line width detection.

For auto-detecting the line width, I was thinking of adding two 
additional passes through the file (frewind-ing to the beginning of the 
file after each pass).

The first pass would check the width of all block comments. Perhaps a 
function called block_comment_width() could be called for each line.  It 
would return zero if the line is not the first or last line of a block 
comment and the position of the final '*' + 1  if is it is a block comment.

The first pass would call block_comment_width() to find the minimum and 
maximum line width.  If the minimum != maximum, then there is a problem

The second pass would only be necessary if minimum != maximum and would 
just call block_comment_width() again to list the block comments that 
have the inconsistent line width.

I would also re-interpret the -m option.  I would make this be the 
number of bytes that I would permit the line to extend beyond the last 
'*' in the block comment.  So if the line width is 78, the -m 86 with 
the old tool would be equivalent to -m 8 with the modified tool.


We probabaly should talk about how to test nxstyle as well. There are 
two possible errors:  (1) False negative:  When nxstyle failes to detect 
an error in the file. and (2) flase positive: When nystyle complains 
about error when there is no error.

For (1) false negatives, I just create a few simple test files and 
verify that nxstyle detects the error. Pretty easy.

For (2) false positives, it is a little more difficult.  I use 
nuttx/sched to do these because it probably follows the coding standard 
the best.  I use a little script like (let's call it schedtest.sh:

    #!/bin/bash

    FILELIST=`find sched -name "*.c"`

    make -C tools -f Makefile.host nxstyle

    for file in $FILELIST; do
       tools/nxstyle -m 86 $file
    done

The -m 86 is just so I won't be bothered by lines that are a little too 
long.

Then I use the script this way:

    $ git stash
    $ ./schedtest.sh 1>before.log 2>&1
    $ git stash pop
    $ ./schedtest.sh 1>after.log 2>&1

Then compare before.log and after.log.  They should be identical UNLESS 
the new version of nxstyle detects a new coding standard violation.

Greg

Re: nxstyle (Was Re: Working Effectively (was Point of Order))

[Gregory Nutt <sp...@gmail.com>]

> ....  Perhaps a function called block_comment_width() could be called 
> for each line.  It would return zero if the line is not the first or 
> last line of a block comment and the position of the final '*' + 1  if 
> is it is a block comment.
>
The first line of a block comment starts with "/***" (perhaps preceded 
by whitespace) and ends with "***"  (perhaps with trailing whitspace).  
The last line of a block begins with whitespace then "***" and ends with 
"***/"  (perhaps with trailing whitspace).

But there is also a special single line comment that begins with "/* " 
(perhaps preceded by whitespace) and ends with "***/" (perhaps with 
trailing whitspace). for example:

    /* Comment ************/

The final '*' in that comment should also align with the block comments.

Greg

Re: nxstyle (Was Re: Working Effectively (was Point of Order))

[Haitao Liu <li...@gmail.com>]

Yes, nxstyle check script comes at first. After it can check the PR patch,
then I'll jump into the build stage including CI setup, autobuild. As to
apache build, ZhangDuo or David may help in the process since they are
familiar with apache infra and github integration.

Gregory Nutt <sp...@gmail.com> 于2020年1月2日周四 上午10:25写道：

>
> > Thanks your detailed explanation to nxstyle. I'll try my best to update
> > nxstyle step by step.
> You also have other responsibilities with the tooling for the workflow,
> correct?  If you need any help with nxstyle, I can also work with it.
> Let me know if you need anything.
>

Re: nxstyle (Was Re: Working Effectively (was Point of Order))

[Gregory Nutt <sp...@gmail.com>]

> Thanks your detailed explanation to nxstyle. I'll try my best to update
> nxstyle step by step.
You also have other responsibilities with the tooling for the workflow, 
correct?  If you need any help with nxstyle, I can also work with it.  
Let me know if you need anything.

Re: nxstyle (Was Re: Working Effectively (was Point of Order))

[Haitao Liu <li...@gmail.com>]

Thanks your detailed explanation to nxstyle. I'll try my best to update
nxstyle step by step.

Gregory Nutt <sp...@gmail.com> 于2020年1月1日周三 上午4:42写道：

>
> > For (1) false negatives, I just create a few simple test files and
> > verify that nxstyle detects the error. Pretty easy.
> >
> > For (2) false positives, it is a little more difficult.  I use
> > nuttx/sched to do these because it probably follows the coding
> > standard the best.  ...
> >
> There is another test that is needed.  When we add a new test to
> nxstyle.c, we may often break other logic that detects different coding
> standard violations.  The above do not address that.
>
> I think we would need to collect a directory full of code with known
> coding standard violations and run and modified nxstyle against those to
> to assure that we have not broken other detection logic.
>
> The code in nxstyle is not pretty.  It uses many booleans and line
> number varialbes to detect problems through some really rather crude
> heuristics (it knows nothing about C syntax).  Since it is convoluted,
> unexpected side effects and unexpected breakage is really common.
>
> Greg
>
>
>
>

Re: nxstyle (Was Re: Working Effectively (was Point of Order))

[Gregory Nutt <sp...@gmail.com>]

> For (1) false negatives, I just create a few simple test files and 
> verify that nxstyle detects the error. Pretty easy.
>
> For (2) false positives, it is a little more difficult.  I use 
> nuttx/sched to do these because it probably follows the coding 
> standard the best.  ...
>
There is another test that is needed.  When we add a new test to 
nxstyle.c, we may often break other logic that detects different coding 
standard violations.  The above do not address that.

I think we would need to collect a directory full of code with known 
coding standard violations and run and modified nxstyle against those to 
to assure that we have not broken other detection logic.

The code in nxstyle is not pretty.  It uses many booleans and line 
number varialbes to detect problems through some really rather crude 
heuristics (it knows nothing about C syntax).  Since it is convoluted, 
unexpected side effects and unexpected breakage is really common.

Greg