`analysis_server_plugin`: async visitors

[RohitSaily]

environment:
  sdk: '3.10.0-162.1.beta' 
  # Dart SDK version: 3.10.0-162.1.beta (beta) (Mon Sep 1 23:24:50 2025 -0700) on "macos_x64"

dependencies:
  analysis_server_plugin: 0.2.2

when using a visitor that performs async operations, reports do not end up happening. eg with a SimpleASTVisitor<Future<void>>:

@override Future<void> visitMethodDeclaration(MethodDeclaration declaration) async
{  await Future.microtask(()=>rule.reportAtNode(declaration))
}

the report never happens

cc @srawlins

[RohitSaily]

i realized i might need to use a recursive visitor, i'll do that and report back here if it's any different

[FMorschel]

It should report, but it would not await the children's return, I don't think. I'm guessing you do need a recursive visitor.

[RohitSaily]

i tried it and it still didn't work. i will share the code here when i get back to it later

---

update see GeneralizingAstVisitor code in: #61517 (comment)

[FMorschel]

Maybe the report does need to be synchronous... I wonder if that's documented somewhere. I'll look for it.

[RohitSaily]

good idea, i didn't look

i do remember all the methods in package:analysis_server_plugin/src/analysis_server_plugin.dart dealing with analysis using futures and awaits that ultimately, at the top level of the call stack, are called from a channel listener

so it seems like it, from the code, it should support async calls

[bwilkerson]

The AST visitors do not support asynchronous usage. In particular, in the code shown above the returned future will not be awaited, which means that it's possible that it won't run until after the code that invoked the visitor has already accessed the reported diagnostics, which, as you discovered, will cause the diagnostic to not be captured.

If there's async work that needs to happen then you probably want to either do that work before invoking the visitor or, if you need to use the visitor in order to discover what work needs to be done, then split the code into two visitors: one that figures out what async work needs to be done and one that runs after the work has been done.

[RohitSaily]

in the legacy plugin system, i used to be able to define a generalizing visitor that would aggregate all the futures via visitNode ie if i did the following override in a GeneralizingAstVisitor<Future<void>>

@override Future<Void> visitNode(AstNode node)
{  final visits=<Future<void>>[];
    for (final child in node.childEntities.whereType<AstNode>())
    {  final visit=child.accept(this);
       if (visit!=null)
       {  visits.add(visit);
       }
     }
    return Future.wait(visits);
}

and then since the entry point for the (legacy) plugin-analysis of a file supported asynchronous operations i could do

await unit.accept(visitor)

and all async operations within visitations would work as expected since they would ultimately be aggregated into one future
and awaited by whatever ran the legacy plugin

---

i believe it is worth supporting to avoid redundant visitations and to keep the processing code localized instead of fragmenting it across two visitors

[RohitSaily]

the only thing i noticed is accept is not awaited:

currentUnit.unit.accept(
  AnalysisRuleVisitor(nodeRegistry, shouldPropagateExceptions: true),
);

making it a potential Future and adding await did not work

otherwise asynchronous behavior is awaited and propogated all the way to what seems to be the top level call (within the package):

  /// Starts this plugin by listening to the given communication [channel].
  void start(PluginCommunicationChannel channel) {
    _channel = channel;
    _channel.listen(_handleRequestZoned,
        // TODO(srawlins): Implement.
        onDone: () {});
  }

which is here

[srawlins]

What type of work in the visitor needs asynchrony?

[RohitSaily]

for my case i am performing file system operations dependent on the code analysis. this includes looking up information in other files on-demand according to the analyzed code

[srawlins]

I think we do not have existing support for this because we've never done async work in our built-in lint rules. Especially a slow operation like reading a file would (I strongly suspect) be way too slow to execute on every incremental analysis of every file.

[RohitSaily]

that makes sense as to why it isn't currently supported. in my experience when i did this with the legacy plugin system, there was no notable slow down, although, that would probably be a different story for a very large project like the dart SDK or perhaps on less powerful devices. although, it may not be as bad if all the work is done asynchronously so multiple operations can progress while others are blocked

the code seems to be using futures and awaiting when conducting the analysis, i don't understand why it doesn't work even if it is modified so that unit.accept(visitor) is awaited if its a future. it seems like it should already work