Non-obvious interaction @if &analyze and @load in deactivated context

[awelzel]

Looking more at #3093, one surprising behavior of @if &analyze is that a @load is recognized and processed in a non-activated context. A later @load for the same file in an activated context is, however, is then skipped and the code never activated.

This seems non-obvious and gotcha behavior and probably just a bug. For the time being I'd make a @load not happening in a non-activated context at the cost of missing out on the coverage. The more proper fix might be to re-parse the file in activated mode instead.

The following test fails with:

 line 13: unknown identifier MyModule::version, at or near "MyModule::version"

It's obvious in the test case, but can imagine it being quite difficult to trace back for users in more complex examples and common utility scripts. Concrete behavior was the docs generation producing an empty trim-trace-file.rst after switching to @if &analyze.

# @TEST-EXEC: zeek -b %INPUT > out
# @TEST-EXEC: btest-diff out

@TEST-START-FILE mod.zeek
module MyModule;

export {
        global version: function(): string;
}

function version(): string {
        return "1.23";
}
@TEST-END-FILE

@if ( F ) &analyze
@load ./mod.zeek
@endif

# this is a noop
@load ./mod.zeek

event zeek_init()
        {
        print MyModule::version();
        }

[awelzel]

I've tried to fix this by tracking "activated" state in the ScannedFile and re-loading if previously only loaded de-activated. This mostly seems to work, but then a baseline diff in the test-suite brings up yet another question: If an @load happens in a non-activated context, should we invoke the HookLoadFile() plugin hook? I think yes, because what if a plugin had acted on the @load ? But then, how do plugins know it was a "deactivated load".

In #3097, it's proposed that for now in deactivated state @load is simply not followed to avoid that, but that obviously decreases coverage compared to loading the file.

@vpax / @rsmmr - cc in case you have a good intuition.

[awelzel]

Not an issue in current master and mentioned it on the discussion so we have a bread-crumb somewhere.