From 99877b5cc53ac1969bece1e4a6e668c433c1723a Mon Sep 17 00:00:00 2001 From: Eduardo Almeida Date: Wed, 6 Sep 2023 23:07:51 +0100 Subject: [PATCH] check-style: Refactor check-style-clang-format.py to reduce duplicate code --- utils/check-style-clang-format.py | 432 ++++++++++-------------------- 1 file changed, 142 insertions(+), 290 deletions(-) diff --git a/utils/check-style-clang-format.py b/utils/check-style-clang-format.py index 0fefe5dfb..7616223c8 100755 --- a/utils/check-style-clang-format.py +++ b/utils/check-style-clang-format.py @@ -45,7 +45,7 @@ import shutil import subprocess import sys -from typing import Dict, List, Tuple +from typing import Callable, Dict, List, Tuple ########################################################### # PARAMETERS @@ -131,73 +131,42 @@ TAB_SIZE = 4 ########################################################### # AUXILIARY FUNCTIONS ########################################################### -def skip_directory(dirpath: str) -> bool: +def should_analyze_directory(dirpath: str) -> bool: """ - Check if a directory should be skipped. + Check whether a directory should be analyzed. @param dirpath Directory path. - @return Whether the directory should be skipped or not. + @return Whether the directory should be analyzed. """ _, directory = os.path.split(dirpath) - return (directory in DIRECTORIES_TO_SKIP or - (directory.startswith('.') and directory != '.')) + return not (directory in DIRECTORIES_TO_SKIP or + (directory.startswith('.') and directory != '.')) -def skip_file_formatting(path: str) -> bool: +def should_analyze_file(path: str, + files_to_check: List[str], + file_extensions_to_check: List[str], + ) -> bool: """ - Check if a file should be skipped from formatting analysis. + Check whether a file should be analyzed. @param path Path to the file. - @return Whether the file should be skipped or not. + @param files_to_check List of files that shall be checked. + @param file_extensions_to_check List of file extensions that shall be checked. + @return Whether the file should be analyzed. """ filename = os.path.split(path)[1] if filename in FILES_TO_SKIP: - return True - - _, extension = os.path.splitext(filename) - - return extension not in FILE_EXTENSIONS_TO_CHECK_FORMATTING - - -def skip_file_whitespace(path: str) -> bool: - """ - Check if a file should be skipped from trailing whitespace analysis. - - @param path Path to the file. - @return Whether the file should be skipped or not. - """ - - filename = os.path.split(path)[1] - - if filename in FILES_TO_SKIP: - return True + return False basename, extension = os.path.splitext(filename) - return (basename not in FILES_TO_CHECK_WHITESPACE and - extension not in FILE_EXTENSIONS_TO_CHECK_WHITESPACE) - - -def skip_file_tabs(path: str) -> bool: - """ - Check if a file should be skipped from tabs analysis. - - @param path Path to the file. - @return Whether the file should be skipped or not. - """ - - filename = os.path.split(path)[1] - - if filename in FILES_TO_SKIP: - return True - - _, extension = os.path.splitext(filename) - - return extension not in FILE_EXTENSIONS_TO_CHECK_TABS + return (basename in files_to_check or + extension in file_extensions_to_check) def find_files_to_check_style(path: str) -> Tuple[List[str], List[str], List[str]]: @@ -210,44 +179,38 @@ def find_files_to_check_style(path: str) -> Tuple[List[str], List[str], List[str List of files to check tabs]. """ - files_to_check_formatting: List[str] = [] - files_to_check_whitespace: List[str] = [] - files_to_check_tabs: List[str] = [] - - abs_path = os.path.normpath(os.path.abspath(os.path.expanduser(path))) + files_to_check: List[str] = [] + abs_path = os.path.abspath(os.path.expanduser(path)) if os.path.isfile(abs_path): - if not skip_file_formatting(path): - files_to_check_formatting.append(path) - - if not skip_file_whitespace(path): - files_to_check_whitespace.append(path) - - if not skip_file_tabs(path): - files_to_check_tabs.append(path) + files_to_check = [path] elif os.path.isdir(abs_path): for dirpath, dirnames, filenames in os.walk(path, topdown=True): - if skip_directory(dirpath): + if not should_analyze_directory(dirpath): # Remove directory and its subdirectories dirnames[:] = [] continue - filenames = [os.path.join(dirpath, f) for f in filenames] - - for f in filenames: - if not skip_file_formatting(f): - files_to_check_formatting.append(f) - - if not skip_file_whitespace(f): - files_to_check_whitespace.append(f) - - if not skip_file_tabs(f): - files_to_check_tabs.append(f) + files_to_check.extend([os.path.join(dirpath, f) for f in filenames]) else: raise ValueError(f'Error: {path} is not a file nor a directory') + files_to_check_formatting: List[str] = [] + files_to_check_whitespace: List[str] = [] + files_to_check_tabs: List[str] = [] + + for f in files_to_check: + if should_analyze_file(f, [], FILE_EXTENSIONS_TO_CHECK_FORMATTING): + files_to_check_formatting.append(f) + + if should_analyze_file(f, FILES_TO_CHECK_WHITESPACE, FILE_EXTENSIONS_TO_CHECK_WHITESPACE): + files_to_check_whitespace.append(f) + + if should_analyze_file(f, [], FILE_EXTENSIONS_TO_CHECK_TABS): + files_to_check_tabs.append(f) + return ( files_to_check_formatting, files_to_check_whitespace, @@ -295,27 +258,25 @@ def find_clang_format_path() -> str: ########################################################### -# CHECK STYLE +# CHECK STYLE MAIN FUNCTIONS ########################################################### -def check_style(path: str, - enable_check_formatting: bool, - enable_check_whitespace: bool, - enable_check_tabs: bool, - fix: bool, - verbose: bool, - n_jobs: int = 1, - ) -> None: +def check_style_clang_format(path: str, + enable_check_formatting: bool, + enable_check_whitespace: bool, + enable_check_tabs: bool, + fix: bool, + verbose: bool, + n_jobs: int = 1, + ) -> None: """ - Check / fix the coding style of a list of files, including formatting and - trailing whitespace. + Check / fix the coding style of a list of files. @param path Path to the files. - @param fix Whether to fix the style of the file (True) or - just check if the file is well-formatted (False). @param enable_check_formatting Whether to enable code formatting checking. @param enable_check_whitespace Whether to enable trailing whitespace checking. @param enable_check_tabs Whether to enable tabs checking. - @param verbose Show the lines that are not well-formatted. + @param fix Whether to fix (True) or just check (False) the file. + @param verbose Show the lines that are not compliant with the style. @param n_jobs Number of parallel jobs. """ @@ -328,18 +289,23 @@ def check_style(path: str, check_tabs_successful = True if enable_check_formatting: - check_formatting_successful = check_formatting( + check_formatting_successful = check_style_file( files_to_check_formatting, + check_formatting_file, + 'bad code formatting', fix, verbose, n_jobs, + clang_format_path=find_clang_format_path(), ) print('') if enable_check_whitespace: - check_whitespace_successful = check_trailing_whitespace( + check_whitespace_successful = check_style_file( files_to_check_whitespace, + check_trailing_whitespace_file, + 'trailing whitespace', fix, verbose, n_jobs, @@ -348,8 +314,10 @@ def check_style(path: str, print('') if enable_check_tabs: - check_tabs_successful = check_tabs( + check_tabs_successful = check_style_file( files_to_check_tabs, + check_tabs_file, + 'tabs', fix, verbose, n_jobs, @@ -365,84 +333,88 @@ def check_style(path: str, sys.exit(1) -########################################################### -# CHECK FORMATTING -########################################################### -def check_formatting(filenames: List[str], +def check_style_file(filenames: List[str], + check_style_file_function: Callable, + style_check_str: str, fix: bool, verbose: bool, n_jobs: int, + **kwargs, ) -> bool: """ - Check / fix the coding style of a list of files with clang-format. + Check / fix style of a list of files. - @param filenames List of filenames to be checked. - @param fix Whether to fix the formatting of the file (True) or - just check if the file is well-formatted (False). - @param verbose Show the lines that are not well-formatted. + @param filename Name of the file to be checked. + @param check_style_file_function Function used to check the file. + @param style_check_str Description of the check to be performed. + @param fix Whether to fix (True) or just check (False) the file (True). + @param verbose Show the lines that are not compliant with the style. @param n_jobs Number of parallel jobs. - @return True if all files are well formatted after the check process. - False if there are non-formatted files after the check process. + @param kwargs Additional keyword arguments to the check_style_file_function. + @return Whether all files are compliant with the style. """ # Check files - clang_format_path = find_clang_format_path() - files_not_formatted: List[str] = [] + non_compliant_files: List[str] = [] files_verbose_infos: Dict[str, List[str]] = {} with concurrent.futures.ProcessPoolExecutor(n_jobs) as executor: - files_not_formatted_results = executor.map( - check_formatting_file, + non_compliant_files_results = executor.map( + check_style_file_function, filenames, - itertools.repeat(clang_format_path), itertools.repeat(fix), itertools.repeat(verbose), + *[arg if isinstance(arg, list) else itertools.repeat(arg) for arg in kwargs.values()], ) - for (filename, formatted, verbose_infos) in files_not_formatted_results: - if not formatted: - files_not_formatted.append(filename) + for (filename, is_file_compliant, verbose_infos) in non_compliant_files_results: + if not is_file_compliant: + non_compliant_files.append(filename) if verbose: files_verbose_infos[filename] = verbose_infos # Output results - if not files_not_formatted: - print('- All files are well formatted') + if not non_compliant_files: + print(f'- No files detected with {style_check_str}') return True else: - n_non_formatted_files = len(files_not_formatted) + n_non_compliant_files = len(non_compliant_files) if fix: - print(f'- Fixed formatting of the files ({n_non_formatted_files}):') + print(f'- Fixed {style_check_str} in the files ({n_non_compliant_files}):') else: - print(f'- Detected bad formatting in the files ({n_non_formatted_files}):') + print(f'- Detected {style_check_str} in the files ({n_non_compliant_files}):') - for f in files_not_formatted: + for f in non_compliant_files: if verbose: print(*[f' {l}' for l in files_verbose_infos[f]], sep='\n') else: print(f' - {f}') - # Return True if all files were fixed + # If all files were fixed, there are no more non-compliant files return fix +########################################################### +# CHECK STYLE FUNCTIONS +########################################################### def check_formatting_file(filename: str, - clang_format_path: str, fix: bool, verbose: bool, + clang_format_path: str, ) -> Tuple[str, bool, List[str]]: """ Check / fix the coding style of a file with clang-format. @param filename Name of the file to be checked. + @param fix Whether to fix (True) or just check (False) the style of the file (True). + @param verbose Show the lines that are not compliant with the style. @param clang_format_path Path to clang-format. - @param fix Whether to fix the style of the file (True) or - just check if the file is well-formatted (False). - @param verbose Show the lines that are not well-formatted. - @return Tuple [Filename, Whether the file is well-formatted, Verbose information]. + @return Tuple [Filename, + Whether the file is compliant with the style (before the check), + Verbose information]. """ verbose_infos: List[str] = [] @@ -455,7 +427,7 @@ def check_formatting_file(filename: str, '-style=file', '--dry-run', '--Werror', - # Optimization: In non-verbose mode, only 1 error is needed to check that the file is not formatted + # Optimization: In non-verbose mode, only one error is needed to check that the file is not compliant f'--ferror-limit={0 if verbose else 1}', ], check=False, @@ -463,13 +435,13 @@ def check_formatting_file(filename: str, text=True, ) - file_formatted = (process.returncode == 0) + is_file_compliant = (process.returncode == 0) if verbose: verbose_infos = process.stderr.splitlines() # Fix file - if fix and not file_formatted: + if fix and not is_file_compliant: process = subprocess.run( [ clang_format_path, @@ -482,71 +454,7 @@ def check_formatting_file(filename: str, stderr=subprocess.DEVNULL, ) - return (filename, file_formatted, verbose_infos) - - -########################################################### -# CHECK TRAILING WHITESPACE -########################################################### -def check_trailing_whitespace(filenames: List[str], - fix: bool, - verbose: bool, - n_jobs: int, - ) -> bool: - """ - Check / fix trailing whitespace in a list of files. - - @param filename Name of the file to be checked. - @param fix Whether to fix the file (True) or - just check if it has trailing whitespace (False). - @param verbose Show the lines that are not well-formatted. - @param n_jobs Number of parallel jobs. - @return True if no files have trailing whitespace after the check process. - False if there are trailing whitespace after the check process. - """ - - # Check files - files_with_whitespace: List[str] = [] - files_verbose_infos: Dict[str, List[str]] = {} - - with concurrent.futures.ProcessPoolExecutor(n_jobs) as executor: - files_with_whitespace_results = executor.map( - check_trailing_whitespace_file, - filenames, - itertools.repeat(fix), - itertools.repeat(verbose), - ) - - for (filename, has_whitespace, verbose_infos) in files_with_whitespace_results: - if has_whitespace: - files_with_whitespace.append(filename) - - if verbose: - files_verbose_infos[filename] = verbose_infos - - # Output results - if not files_with_whitespace: - print('- No files detected with trailing whitespace') - return True - - else: - n_files_with_whitespace = len(files_with_whitespace) - - if fix: - print( - f'- Fixed trailing whitespace in the files ({n_files_with_whitespace}):') - else: - print( - f'- Detected trailing whitespace in the files ({n_files_with_whitespace}):') - - for f in files_with_whitespace: - if verbose: - print(*[f' {l}' for l in files_verbose_infos[f]], sep='\n') - else: - print(f' - {f}') - - # If all files were fixed, there are no more trailing whitespace - return fix + return (filename, is_file_compliant, verbose_infos) def check_trailing_whitespace_file(filename: str, @@ -557,13 +465,14 @@ def check_trailing_whitespace_file(filename: str, Check / fix trailing whitespace in a file. @param filename Name of the file to be checked. - @param fix Whether to fix the file (True) or - just check if it has trailing whitespace (False). - @param verbose Show the lines that are not well-formatted. - @return Tuple [Filename, Whether the file has trailing whitespace, Verbose information]. + @param fix Whether to fix (True) or just check (False) the style of the file (True). + @param verbose Show the lines that are not compliant with the style. + @return Tuple [Filename, + Whether the file is compliant with the style (before the check), + Verbose information]. """ - has_trailing_whitespace = False + is_file_compliant = True verbose_infos: List[str] = [] with open(filename, 'r', encoding='utf-8') as f: @@ -573,90 +482,29 @@ def check_trailing_whitespace_file(filename: str, for (i, line) in enumerate(file_lines): line_fixed = line.rstrip() + '\n' - if line_fixed != line: - has_trailing_whitespace = True - file_lines[i] = line_fixed + if line_fixed == line: + continue - if verbose: - verbose_infos.extend([ - f'{filename}:{i + 1}: error: Trailing whitespace detected', - f' {line_fixed.rstrip()}', - f' {"":{len(line_fixed) - 1}}^', - ]) + is_file_compliant = False + file_lines[i] = line_fixed - # Optimization: If running in non-verbose check mode, only one error is needed to check that the file is not formatted - if not fix and not verbose: - break + if verbose: + verbose_infos.extend([ + f'{filename}:{i + 1}: error: Trailing whitespace detected', + f' {line_fixed.rstrip()}', + f' {"":{len(line_fixed) - 1}}^', + ]) + + # Optimization: If running in non-verbose check mode, only one error is needed to check that the file is not compliant + if not fix and not verbose: + break # Update file with the fixed lines - if fix and has_trailing_whitespace: + if fix and not is_file_compliant: with open(filename, 'w', encoding='utf-8') as f: f.writelines(file_lines) - return (filename, has_trailing_whitespace, verbose_infos) - - -########################################################### -# CHECK TABS -########################################################### -def check_tabs(filenames: List[str], - fix: bool, - verbose: bool, - n_jobs: int, - ) -> bool: - """ - Check / fix tabs in a list of files. - - @param filename Name of the file to be checked. - @param fix Whether to fix the file (True) or just check if it has tabs (False). - @param verbose Show the lines that are not well-formatted. - @param n_jobs Number of parallel jobs. - @return True if no files have tabs after the check process. - False if there are tabs after the check process. - """ - - # Check files - files_with_tabs: List[str] = [] - files_verbose_infos: Dict[str, List[str]] = {} - - with concurrent.futures.ProcessPoolExecutor(n_jobs) as executor: - files_with_tabs_results = executor.map( - check_tabs_file, - filenames, - itertools.repeat(fix), - itertools.repeat(verbose), - ) - - for (filename, has_tabs, verbose_infos) in files_with_tabs_results: - if has_tabs: - files_with_tabs.append(filename) - - if verbose: - files_verbose_infos[filename] = verbose_infos - - # Output results - if not files_with_tabs: - print('- No files detected with tabs') - return True - - else: - n_files_with_tabs = len(files_with_tabs) - - if fix: - print( - f'- Fixed tabs in the files ({n_files_with_tabs}):') - else: - print( - f'- Detected tabs in the files ({n_files_with_tabs}):') - - for f in files_with_tabs: - if verbose: - print(*[f' {l}' for l in files_verbose_infos[f]], sep='\n') - else: - print(f' - {f}') - - # If all files were fixed, there are no more trailing whitespace - return fix + return (filename, is_file_compliant, verbose_infos) def check_tabs_file(filename: str, @@ -667,12 +515,14 @@ def check_tabs_file(filename: str, Check / fix tabs in a file. @param filename Name of the file to be checked. - @param fix Whether to fix the file (True) or just check if it has tabs (False). - @param verbose Show the lines that are not well-formatted. - @return Tuple [Filename, Whether the file has tabs, Verbose information]. + @param fix Whether to fix (True) or just check (False) the style of the file (True). + @param verbose Show the lines that are not compliant with the style. + @return Tuple [Filename, + Whether the file is compliant with the style (before the check), + Verbose information]. """ - has_tabs = False + is_file_compliant = True clang_format_enabled = True verbose_infos: List[str] = [] @@ -697,27 +547,29 @@ def check_tabs_file(filename: str, # Check if there are tabs and fix them tab_index = line.find('\t') - if tab_index != -1: - has_tabs = True - file_lines[i] = line.expandtabs(TAB_SIZE) + if tab_index == -1: + continue - if verbose: - verbose_infos.extend([ - f'{filename}:{i + 1}:{tab_index + 1}: error: Tab detected', - f' {line.rstrip()}', - f' {"":{tab_index}}^', - ]) + is_file_compliant = False + file_lines[i] = line.expandtabs(TAB_SIZE) - # Optimization: If running in non-verbose check mode, only one error is needed to check that the file is not formatted - if not fix and not verbose: - break + if verbose: + verbose_infos.extend([ + f'{filename}:{i + 1}:{tab_index + 1}: error: Tab detected', + f' {line.rstrip()}', + f' {"":{tab_index}}^', + ]) + + # Optimization: If running in non-verbose check mode, only one error is needed to check that the file is not compliant + if not fix and not verbose: + break # Update file with the fixed lines - if fix and has_tabs: + if fix and not is_file_compliant: with open(filename, 'w', encoding='utf-8') as f: f.writelines(file_lines) - return (filename, has_tabs, verbose_infos) + return (filename, is_file_compliant, verbose_infos) ########################################################### @@ -759,7 +611,7 @@ if __name__ == '__main__': args = parser.parse_args() try: - check_style( + check_style_clang_format( path=args.path, enable_check_formatting=(not args.no_formatting), enable_check_whitespace=(not args.no_whitespace),