lint: Convert lint-include-guards.sh to Python #24902

pull brydinh wants to merge 1 commits into bitcoin:master from brydinh:24783-port-lint-script-to-python changing 2 files +100 −30

brydinh commented at 10:04 PM on April 17, 2022: contributor

This PR addresses issue 24783. Converted lint-include-guards.sh to python.
DrahtBot added the label Tests on Apr 17, 2022

theStack commented at 4:35 PM on April 18, 2022: member

Concept ACK

Note that there is currently a linter error in the CI:

Traceback (most recent call last):
  File "test/lint/lint-include-guards.py", line 88, in <module>
    main()
  File "test/lint/lint-include-guards.py", line 71, in main
    header_file_contents_string = open(header_file, 'r').read()
  File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xce in position 2003: ordinal not in range(128)
^---- failure generated from test/lint/lint-include-guards.py
Python's open(...) seems to be used to open text files without explicitly
specifying encoding="utf8":

test/lint/lint-include-guards.py:        header_file_contents_string = open(header_file, 'r').read()
^---- failure generated from test/lint/lint-python-utf8-encoding.sh
Success: no issues found in 236 source files

in test/lint/lint-include-guards.py:71 in 657d0d654d outdated

  66 | +    for header_file in header_file_lst:
  67 | +        header_id = _get_header_id(header_file)
  68 | +
  69 | +        regex_pattern = f'#(ifndef|define|endif //) {header_id}'
  70 | +
  71 | +        header_file_contents_string = open(header_file, 'r').read()

brunoerg commented at 6:55 PM on April 18, 2022:

I prefer to open files using with, it ensures that the file will be closed at the end of the operation.

brydinh commented at 9:21 PM on April 18, 2022:

Good catch!

KevinMusgrave commented at 2:26 AM on April 20, 2022: contributor
I tested ea3e7abc45730e7a6eb1e840c6fa40780b3dee2f by commenting out the include guard in bench/bench.h and got the following results:

Before (test/lint/lint-include-guards.sh):
```
src/bench/bench.h seems to be missing the expected include guard:
  #ifndef BITCOIN_BENCH_BENCH_H
  #define BITCOIN_BENCH_BENCH_H
  ...
  #endif // BITCOIN_BENCH_BENCH_H
```
After (test/lint/lint-include-guards.py): The test passes

If I delete (rather than comment out) the include guard, then it prints the correct message.

in test/lint/lint-include-guards.py:83 in ea3e7abc45 outdated

  78 | +            header_file_contents_string = f.read()
  79 | +
  80 | +        include_guard_lst = re.findall(
  81 | +            regex_pattern, header_file_contents_string)
  82 | +
  83 | +        if len(include_guard_lst) != 3:

KevinMusgrave commented at 2:49 AM on April 20, 2022:

The following is one way to fix it:

        regex_pattern = f'^#(ifndef|define|endif //) {header_id}'

        with open(header_file, 'r', encoding='utf-8') as f:
            header_file_contents = f.readlines()

        count = 0
        for header_file_contents_string in header_file_contents:
            include_guard_lst = re.findall(
                regex_pattern, header_file_contents_string)
            
            count += len(include_guard_lst)

        if count != 3:

brydinh commented at 1:29 PM on April 20, 2022:

thanks, awesome catch!

Convert lint-include-guards.sh to python

Specify encoding when reading header files, add docstring

Update test/lint/lint-include-guards.py  include guard count logic

Co-authored-by: Kevin Musgrave <tkm45@cornell.edu>

Update test/lint/lint-include-guards.py by removing whitespace

d5fdec5cf8

brydinh force-pushed on Apr 20, 2022

KevinMusgrave commented at 6:43 PM on April 20, 2022: contributor

Tested ACK d5fdec5cf8cffe8ece5460cd8c6e83ea6eebf625

Outputs and exit codes match. Example:

src/attributes.h seems to be missing the expected include guard:
  #ifndef BITCOIN_ATTRIBUTES_H
  #define BITCOIN_ATTRIBUTES_H
  ...
  #endif // BITCOIN_ATTRIBUTES_H

src/banman.h seems to be missing the expected include guard:
  #ifndef BITCOIN_BANMAN_H
  #define BITCOIN_BANMAN_H
  ...
  #endif // BITCOIN_BANMAN_H

src/bech32.h seems to be missing the expected include guard:
  #ifndef BITCOIN_BECH32_H
  #define BITCOIN_BECH32_H
  ...
  #endif // BITCOIN_BECH32_H

laanwj commented at 4:14 PM on April 25, 2022: member

Code review ACK d5fdec5cf8cffe8ece5460cd8c6e83ea6eebf625
laanwj merged this on Apr 25, 2022
laanwj closed this on Apr 25, 2022
sidhujag referenced this in commit 5972adab82 on Apr 26, 2022
DrahtBot locked this on Apr 25, 2023

Contributors

Labels

Linked (view graph)

#24783 Port lint scripts to Python