Working on max-returns lint function

[NathanLovato]

trafficstars

I'll start with a simple function looking at what you did. checking for max returns seems like an easy one.

Do you have any tips to get started? E.g. pretty-printing the parsed syntax tree or something like that to get a sense of how to find the right information.

[Scony]

Well, you can write some dummy GDScript file and use gdparse -p [file] to take a brief look at tree structure. For more details run gdparse -v [file] it will show you a more detailed tree in terms of the lark classes (a mixture of Trees and Tokens). Once you got some insights you can start doing some TDD using pytest

For default config, you can refer to https://pylint.readthedocs.io/en/latest/technical_reference/features.html#design-checker as it's a good starting point probably

[NathanLovato]

Working on the code now, adding functions the way it is, there's a lot of repetition on finding data. E.g. in design_checks.py, you'd end up fetching functions in every check, and on large codebases, it's probably not optimal.

Even though from a CI you can only lint the last commits from a given PR, so you could work around potential performance issues, there's going to be a lot of repetition in the code.

How about gathering all the functions' info once, caching it, and having simpler linting functions? I'd need to experiment to make it as useful as possible but for example:

def get_function_data():
    """
    Walks through all the function definitions in the parse tree and gathers data about each of them for linting.
    Returns a list of Function objects.
    """
    data = []
    functions = parse_tree.find_data("func_def")
    for function in functions:
        function_name = function.children[0]
        has_arguments = (
            isinstance(func_def.children[1], Tree)
            and func_def.children[1].data == "func_args"
        )
        arguments = func_def.children[1].children if has_arguments else []
        returns = parse_tree.find_data("return_stmt")
        local_variables = parse_tree.find_data("func_var_stmt")
        data.append(Function(function_name, arguments, local_variables, returns,))
    return data

You do it once, and then you can pass the functions around. This can also be turned into a generator, taking functions one at a time, passing them to every linting function, also enabling easier multithreading moving forward if it can make a difference performance-wise (with pylint it does make a big difference here).

[Scony]

It's nice you've figured it out 🙂 yes - at the moment bare parse tree is used which leads to performance problems. What you've described is actually part of abstract syntax tree. Eventually it should contain not only functions but also classes which should contain attributes functions etc. So in general yes. Caching data is good way forward.

[Scony]

@NathanLovato I've created the initial version of AST (https://github.com/Scony/godot-gdscript-toolkit/blob/master/gdtoolkit/linter/ast.py) you may extend it for caching functions and their data