This is fine if it's possible to specify only the package, but if you needed to specify all types separately like

class Example(SuiteVisitor[TestSuite, TestCase, Keyword, If, IfBranch, Try, TryBranch, For, While, Break, Continue, Return, Var, Error]):
    ...

this isn't a good idea. In addition to the usage being horrible, there's a problem that on the result side we have ForIteration and WhileIteration that aren't present on the running side. This would also mean that all such usages would need to be updated if new model objects are introduced.

Currently the recommended way to use typing with visitors is to import appropriate type hints:

from robot.result import TestSuite, TestCase

class Example(SuiteVisitor):

    def start_suite(self, suite: TestSuite):
        ...

    def visit_test(self, test: TestCase):
        ...

I consider the above pretty convenient, which means this enhancement doesn't have too high priority.

unfortunately mypy doesn't allow that:

foo.py: note: In class "Example":
foo.py:16:27: error: Argument 1 of "start_suite" is incompatible with supertype "SuiteVisitor"; supertype
defines the argument type as "TestSuite[Untyped, Untyped]"  [override]
        def start_suite(self, suite: TestSuite): ...
                              ^~~~~~~~~~~~~~~~
foo.py:19:26: error: Argument 1 of "visit_test" is incompatible with supertype "SuiteVisitor"; supertype     
defines the argument type as "TestCase[Untyped]"  [override]
        def visit_test(self, test: TestCase): ...
                             ^~~~~~~~~~~~~~
Found 2 errors in 1 file (checked 1 source file)

it's not typesafe because you could theoretically do something like this:

from robot.api import SuiteVisitor
from robot import model, result

class ResultVisitor(SuiteVisitor):

    def start_suite(self, suite: result.TestSuite):
        print(suite.passed) # a model.TestSuite can be passed to this method which would cause this to crash

foo: SuiteVisitor = ResultVisitor()

foo.start_suite(model.TestSuite()) # crashes because `start_suite` was expecting a `running.TestSuite` but received a `model.TestSuite`

on a related note, mypy doesn't even allow you to type-check code from robot without adding a py.typed file, so it would be nice if #4823 could be merged

Do you have ideas how to implement this without making the visitor implementation horribly complicated? Mypy compatibility isn't a goal for us for various reasons, one of them Mypy being annoyingly buggy, and the main reason we are adding type hints is easing IDE usage. The current approach already works well in that regard, so I'm not interested to make the code much more complex just to make Mypy happy.

Do you have ideas how to implement this without making the visitor implementation horribly complicated?

the idea i suggested in the original post doesn't seem too complicated to me. the only issue i can see is the redundancy of having to specify both the TestCase and TestSuite generics. but that can be solved by exporting type aliases for each type of suite visitor instead of the suite visitor class itself:

from typing import TypeAlias

_TS = TypeVar("_TS", bound=model.TestSuite)
_TC = TypeVar("_TS", bound=model.TestCase)

class _SuiteVisitor(Generic[_TS, _TC]):
    def start_suite(self, suite: _TS) ->  bool | None:
        ...
    def visit_test(self, test: _TC) -> None:
        ...

RunningSuiteVisitor: TypeAlias = _SuiteVisitor[running.TestSuite, running.TestCase]
ResultSuiteVisitor: TypeAlias = _SuiteVisitor[result.TestSuite, result.TestCase]

that way, users can easily subtype whichever one they want depending on what their suite visitor is used for:

class ResultVisitor(ResultSuiteVisitor):
    ...

Mypy compatibility isn't a goal for us for various reasons, one of them Mypy being annoyingly buggy

i agree, mypy seems to be very unreliable and in my experience many of the errors it gives me turn out to be false positives. no idea why it ended up being the most popular type checker. but if you push past those issues, its definitely better than not using a type checker at all and saves lots of time in the long run since it prevents runtime errors that could have easily been caught by a type checker.

the main reason we are adding type hints is easing IDE usage

assuming the IDE you're targeting is vscode, i would recommend checking out the pyright type checker as an alternative to mypy. in my experience it's probably the most capable of competing with mypy, and seems far less buggy.

it's also what vscode uses for autocomplete, so if you want to validate that vscode's autocomplete works, it wouldn't hurt to add it to the CI imo

Having separate visitors for the running and result model would certainly be easier for users than needing to list all possible item types with SuiteVisitor[...]. That said, I don't consider it too much better than just importing needed types that works already now:

from robot.model import SuiteVisitor
from robot.running import TestSuite, TestCase


class Example(SuiteVisitor):

    def start_suite(suite: TestSuite):
        ...

    def start_test(test: TestCase):
        ...

If there was a separate RunningSuiteVisitor, you could use the following, and IDEs ought to know the types of suite and test without type hints. I guess that's a benefit, but I would think many users prefer having the type hints there anyway and in that case there's no difference with the above.

from robot.running import RunningSuiteVisitor


class Example(RunningSuiteVisitor):

    def start_suite(suite):
        ...

    def start_test(test):
        ...

I personally don't consider adding separate RunningSuiteVisitor and ResultSuiteVisitor worth the effort, but I'm fine adding them if someone else provides a pull request. I guess they could live in robot.running.visitor and robot.result.visitor modules, respectively, and exposed also via robot.running and robot.result packages.

One problem with ResultSuiteVisitor is that we already have ResultVisitor for visiting the whole result object. I guess the names are clear enough there not to be too much problems, but this difference needs to be documented. The ResultVisitor could probably be then enhanced to extend ResultSuiteVisitor to get proper type hints for the suite structure. That would actually be a pretty nice benefit of this enhancement.

I personally don't consider adding separate RunningSuiteVisitor and ResultSuiteVisitor worth the effort, but I'm fine adding them if someone else provides a pull request. I guess they could live in robot.running.visitor and robot.result.visitor modules, respectively, and exposed also via robot.running and robot.result packages.

thanks. i'll give that a go and open a PR

It's not enough for to make just TestSuite and TestCase generic, that needs to be done with Keyword, If, etc. as well. A problem is that if/when we add more control structures, also they need to be supported and contents of Generic[...] changes.

If the main need is having running and result specific visitors, I believe it's easiest to do that my directly extending the current SuiteVisitor and adding appropriate type hints. That would be less complicated and would ease maintenance.

Do you have ideas how to implement this without making the visitor implementation horribly complicated? Mypy compatibility isn't a goal for us for various reasons, one of them Mypy being annoyingly buggy, and the main reason we are adding type hints is easing IDE usage. The current approach already works well in that regard, so I'm not interested to make the code much more complex just to make Mypy happy.

Hmm...I don't think Mypy is that buggy. Sure, there might be some corner cases. Have you thought about introducing Ruff (and Ruff formatted)?
Here's some analysis of what it looks like today:

# mypy src/robot
Found 1472 errors in 83 files (checked 239 source files)

# ruff check src/robot
Found 2184 errors.
[*] 759 fixable with the `--fix` option (373 hidden fixes can be enabled with the `--unsafe-fixes` option).

Note: Ruff is configured to use the most obvious rules.

Some gradual approach is required to introduce and fix these.