`gdlint` should check for two blank lines between functions

[MikeSchulze]

According to official Godot code style, surround functions and class definitions with two blank lines

https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_styleguide.html#blank-lines

[Scony]

Yes, gdformat formats code this way - what is the problem you're reporting?

[MikeSchulze]

The issue is about gdlint and not gdformat, I would expect it detects too many or less empty lines between functions

[Scony]

I see, yes - this may make sense, even tho it's covered by gdformat.

[Scony]

I've been thinking about it again and I think it's not necessary to duplicate functionality of gdformat in gdlint. Please youse gdformat -c to perform this kind of validation.

[MikeSchulze]

Sorry, but glint and gdformat are different tasks right? The linter should validate the code independent of the used formatter.

So I'm not happy about your decision ;(

[Scony]

Both gdformat and gdlint are part of a single toolkit and therefore I'd like to avoid as much duplication as possible. If those would be a separate projects then it would make sense to have some more overlap I guess. But as it's not the case, each overlapping functionality means double the maintenance and testing effort.

[MikeSchulze]

What is the problem? They are two tools and therefore need to be considered separately. Inserting a test for two blank lines should not really be a problem.

import re
from functools import partial
from types import MappingProxyType
from typing import Callable, List, Tuple

from dataclasses import dataclass

@dataclass
class Problem:
    name: str
    description: str
    line: int
    column: int

TEST_CLASS = """
## An Assertion Tool to verify integer values
class_name GdUnitIntAssert
extends GdUnitAssert

## Verifies that the current value is equal to expected one.
@warning_ignore("unused_parameter")
func is_equal(expected :int) -> GdUnitIntAssert:
    return self

## Verifies that the current value is not equal to expected one.
@warning_ignore("unused_parameter")
func is_not_equal(expected :int) -> GdUnitIntAssert:
    return self
"""

def count_blank_lines_before_func(code: str, expected_blank_lines) -> List[Problem]:
    # Pattern to match blank lines before 'func', excluding lines starting with @warning or #
    pattern = r'((^\n\s*|^(@warning|#)[^\n]*\n)*)func'
    matches = re.finditer(pattern, code, re.MULTILINE)
    problems = []

    for match in matches:
        blank_lines = len(re.findall(r'^\n', match.group(1), re.MULTILINE))
        if blank_lines != expected_blank_lines:
            func_start_line = match.end()
            func_line_end = code.find('\n', func_start_line)
            func_line = code[func_start_line:func_line_end]
            match_line = code[:func_start_line].count('\n')

            problems.append(Problem(
                name="incorrect-blank-lines",
                description=f"Incorrect number of blank lines before 'func{func_line}'. Expected: {expected_blank_lines}, found: {blank_lines}",
                line=match_line,
                column=0  # Assuming the function starts at the beginning of the line
            ))

    return problems

expected_blank_lines = 2

problems = count_blank_lines_before_func(TEST_CLASS, expected_blank_lines)
if problems:
    for problem in problems:
        print(problem.description)
        print("Line number:", problem.line)
else:
    print("No problems found.")

The output

Incorrect number of blank lines before 'func is_equal(expected :int) -> GdUnitIntAssert:'. Expected: 2, found: 3
Line number: 9
Incorrect number of blank lines before 'func is_not_equal(expected :int) -> GdUnitIntAssert:'. Expected: 2, found: 1
Line number: 14

[Scony]

The code is not a problem. The problem is maintenance cost - all the above code is basically irrelevant if gdformat is used. Therefore avoiding having the above is a good thing. Please note that code is one thing, there's also a matter of tests that should cover that. That's even more maintenance cost.