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

1. 
   brydinh commented at 10:04 pm on April 17, 2022: contributor

   This PR addresses issue 24783. Converted lint-include-guards.sh to python.
2. DrahtBot added the label Tests on Apr 17, 2022
3. 
   theStack commented at 4:35 pm on April 18, 2022: member

   Concept ACK

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

    0Traceback (most recent call last):
    1  File "test/lint/lint-include-guards.py", line 88, in <module>
    2    main()
    3  File "test/lint/lint-include-guards.py", line 71, in main
    4    header_file_contents_string = open(header_file, 'r').read()
    5  File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
    6    return codecs.ascii_decode(input, self.errors)[0]
    7UnicodeDecodeError: 'ascii' codec can't decode byte 0xce in position 2003: ordinal not in range(128)
    8^---- failure generated from test/lint/lint-include-guards.py
    9Python's open(...) seems to be used to open text files without explicitly
   10specifying encoding="utf8":
   11
   12test/lint/lint-include-guards.py:        header_file_contents_string = open(header_file, 'r').read()
   13^---- failure generated from test/lint/lint-python-utf8-encoding.sh
   14Success: no issues found in 236 source files
   

4. 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!
5. 
   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):

   0src/bench/bench.h seems to be missing the expected include guard:
   1  #ifndef BITCOIN_BENCH_BENCH_H
   2  #define BITCOIN_BENCH_BENCH_H
   3  ...
   4  #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.

6. 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:

    0        regex_pattern = f'^#(ifndef|define|endif //) {header_id}'
    1
    2        with open(header_file, 'r', encoding='utf-8') as f:
    3            header_file_contents = f.readlines()
    4
    5        count = 0
    6        for header_file_contents_string in header_file_contents:
    7            include_guard_lst = re.findall(
    8                regex_pattern, header_file_contents_string)
    9            
   10            count += len(include_guard_lst)
   11
   12        if count != 3:
   

   ---

   

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

   thanks, awesome catch!
7. 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
8. brydinh force-pushed on Apr 20, 2022
9. 
   KevinMusgrave commented at 6:43 pm on April 20, 2022: contributor

   Tested ACK d5fdec5cf8cffe8ece5460cd8c6e83ea6eebf625

   Outputs and exit codes match. Example:

    0src/attributes.h seems to be missing the expected include guard:
    1  #ifndef BITCOIN_ATTRIBUTES_H
    2  #define BITCOIN_ATTRIBUTES_H
    3  ...
    4  #endif // BITCOIN_ATTRIBUTES_H
    5
    6src/banman.h seems to be missing the expected include guard:
    7  #ifndef BITCOIN_BANMAN_H
    8  #define BITCOIN_BANMAN_H
    9  ...
   10  #endif // BITCOIN_BANMAN_H
   11
   12src/bech32.h seems to be missing the expected include guard:
   13  #ifndef BITCOIN_BECH32_H
   14  #define BITCOIN_BECH32_H
   15  ...
   16  #endif // BITCOIN_BECH32_H
   

10. 
    laanwj commented at 4:14 pm on April 25, 2022: member

    Code review ACK d5fdec5cf8cffe8ece5460cd8c6e83ea6eebf625
11. laanwj merged this on Apr 25, 2022
12. laanwj closed this on Apr 25, 2022

    ---

13. sidhujag referenced this in commit 5972adab82 on Apr 26, 2022
14. DrahtBot locked this on Apr 25, 2023

Contributors
brydinh theStack brunoerg KevinMusgrave laanwj

Labels
Tests