Re: [dpdk-dev] [RFC PATCH] devtools: script to check meson indentation of lists

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 22-Apr-21 10:02 AM, Bruce Richardson wrote:
> This is a draft script developed when I was working on the whitespace rework
> changes, since extended a little to attempt to fix some trailing comma issues.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   devtools/dpdk_meson_check.py | 106 +++++++++++++++++++++++++++++++++++
>   1 file changed, 106 insertions(+)
>   create mode 100755 devtools/dpdk_meson_check.py
> 
> diff --git a/devtools/dpdk_meson_check.py b/devtools/dpdk_meson_check.py
> new file mode 100755
> index 000000000..dc4c714ad
> --- /dev/null
> +++ b/devtools/dpdk_meson_check.py
> @@ -0,0 +1,106 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2021 Intel Corporation
> +
> +'''
> +A Python script to run some checks on meson.build files in DPDK
> +'''
> +
> +import sys
> +import os
> +from os.path import relpath, join
> +from argparse import ArgumentParser
> +
> +VERBOSE = False
> +FIX = False
> +
> +def scan_dir(path):
> +    '''return meson.build files found in path'''
> +    for root, dirs, files in os.walk(path):
> +        if 'meson.build' in files:
> +            yield(relpath(join(root, 'meson.build')))
> +
> +
> +def check_indentation(filename, contents):
> +    '''check that a list or files() is correctly indented'''
> +    infiles = False
> +    inlist = False
> +    edit_count = 0
> +    for lineno in range(len(contents)):

for lineno, line in enumerate(contents)

?

> +        line = contents[lineno].rstrip()
> +        if not line:
> +            continue
> +        if line.endswith('files('):
> +            if infiles:
> +                raise(f'Error parsing {filename}:{lineno}, got "files(" when already parsing files list')
> +            if inlist:
> +                print(f'Error parsing {filename}:{lineno}, got "files(" when already parsing array list')
> +            infiles = True
> +            indent = 0
> +            while line[indent] == ' ':
> +                indent += 1

Here and in other places, if this is measuring length of indent, maybe 
do something like:

indent = len(line) - len(line.lstrip(' '))

?

> +            indent += 8  # double indent required
> +        elif line.endswith('= ['):
> +            if infiles:
> +                raise(f'Error parsing {filename}:{lineno}, got start of array when already parsing files list')
> +            if inlist:
> +                print(f'Error parsing {filename}:{lineno}, got start of array when already parsing array list')
> +            inlist = True
> +            indent = 0
> +            while line[indent] == ' ':
> +                indent += 1
> +            indent += 8  # double indent required
> +        elif infiles and (line.endswith(')') or line.strip().startswith(')')):

It's kinda hard to read with all the endswith/startswith, maybe extract 
those into a function? e.g. 'elif infiles and is_file_start(line)'

> +            infiles = False
> +            continue
> +        elif inlist and line.endswith(']') or line.strip().startswith(']'):
> +            inlist = False
> +            continue
> +        elif inlist or infiles:
> +            # skip further subarrays or lists
> +            if '[' in line  or ']' in line:
> +                continue

I guess you could make it recursive instead of giving up? Does this 
happen with any kind of regularity?

> +            if not line.startswith(' ' * indent) or line[indent] == ' ':
> +                print(f'Error: Incorrect indent at {filename}:{lineno + 1}')
> +                contents[lineno] = (' ' * indent) + line.strip() + '\n'
> +                line = contents[lineno].rstrip()
> +                edit_count += 1
> +            if not line.endswith(',') and '#' not in line:
> +                # TODO: support stripping comment and adding ','
> +                print(f'Error: Missing trailing "," in list at {filename}:{lineno + 1}')
> +                contents[lineno] = line + ',\n'
> +                line = contents[lineno].rstrip()

What is the point of setting `line` here?

> +                edit_count += 1
> +    return edit_count
> +
> +
> +def process_file(filename):
> +    '''run checks on file "filename"'''
> +    if VERBOSE:
> +        print(f'Processing {filename}')
> +    with open(filename) as f:
> +        contents = f.readlines()

I guess meson build files don't get too big so it's OK to read the 
entire file in memory and then work on it, rather than go line by line...

> +
> +    if check_indentation(filename, contents) > 0 and FIX:
> +        print(f"Fixing {filename}")
> +        with open(filename, 'w') as f:
> +            f.writelines(contents)
> +
> +
> +def main():
> +    '''parse arguments and then call other functions to do work'''
> +    global VERBOSE
> +    global FIX

Seems like globals are unnecessary here when you can just pass them into 
process_file?

> +    parser = ArgumentParser(description='Run syntax checks on DPDK meson.build files')
> +    parser.add_argument('-d', metavar='directory', default='.', help='Directory to process')
> +    parser.add_argument('--fix', action='store_true', help='Attempt to fix errors')
> +    parser.add_argument('-v', action='store_true', help='Verbose output')
> +    args = parser.parse_args()
> +
> +    VERBOSE = args.v
> +    FIX = args.fix
> +    for f in scan_dir(args.d):
> +        process_file(f)
> +
> +if __name__ == "__main__":
> +    main()
> --
> 2.27.0
> 


-- 
Thanks,
Anatoly