Object Ninja ignores objects assigned within #if ... sometimes.

[rdebath]

After the fix for the previous issue (thanks) I think I see what rules this uses (eg: how "#if not false and" toggles it) and why I was still getting object number collisions, but perhaps a bit of documentation might be in order? Especially with respect to how it looks like it breaks "replace recorded object ID assignments with actual state".

In addition I suspect some sort of toggle along the lines of this might be nice:

• Ignore code inside "#if" if the condition does not begin with "not"
• Ignore all code inside '#if' regions.
• Ignore only code in '#if false' regions.

Note, it is impossible to "get this right", because it's quite possible for a pipeline to compile different versions (eg: OnPrem and Cloud) at exactly the same time.

[vjekob]

The way Ninja is designed to handle these is to treat all code inside precompiler directives as valid, regardless of whether condition is true or false and what the actual setup would compile or not. If it doesn't behave like this (which it obviously doesn't in your example) then it's a bug.

The reasoning behind this design decision is the same as with git branches: code that only exists in one branche (or conditionally exists as in your case), still exists, because a number seen in one repo in one branch would still conflict with a number assigned in another branch (or under different conditional setup in your case).

I'll take a look, this is possibly a bug that happened when Ninja's parser was migrated from TypeScript to Rust, and somehow slipped through the test scaffolding. Should be an easy fix.

[rdebath]

What does it do if an object is defined in both the #if and the #else portions? (Or a #if and #if not for the same variable)
Does it produce a "duplicate object number" false positive?

[vjekob]

#if PTE
codeunit 50001 Test
#else
codeunit 391039121 "PFX Test"
#endif

This thing marks both 50001 and 391039121 as consumed. That's how it was designed, it's intentional, and nothing else makes sense.

If that's not how it behaves, I'll track it down and fix it.

[rdebath]

No, I would disagree that your way is the only one that makes sense. There are several working methods that have been used in other languages. BUT I will agree that ignoring the preprocessor lines is almost certainly the easiest to use.

The example I was more thinking of was as below where there are two libraries that are similar, but not identical and don't fit the enum model of interfaces.

This can be adjusted for "ignore" mode but does lead to you having to take care not to be too precise in your parsing of AL. That should be fine there's always indenting. I also do suspect it's going to make fixing #58 harder.

The important part is this should be documented.

Current code:

#if not OnPrem
codeunit 50001 Test
{
    trigger OnRun()
    begin
        Error('Full process fails in SaaS, please use only supported components');
    end;
    // ...
}
#endif

#if OnPrem
codeunit 50001 Test
{
    trigger OnRun()
    begin
        ProcessPart1();
        ProcessPart2();
        // ...
    end;
    // ...
}
#endif

Has to be changed to force both variants into one file, but may include better sharing of common code.

codeunit 50001 Test
#if not OnPrem
{
    trigger OnRun()
    begin
        Error('Full process fails in SaaS, please use only supported components');
    end;
    //...
#endif

#if OnPrem
{
    trigger OnRun()
    begin
        ProcessPart1();
        ProcessPart2();
        // ...
    end;
    //...
#endif

    // ...
}

[vjekob]

There are several working methods that have been used in other languages.

Other languages don't have object IDs that need to be uniquely tracked, so whatever they do with precompiler directives has nothing to do with object ID uniqueness problem.

BUT I will agree that ignoring the preprocessor lines is almost certainly the easiest to use.

Ninja isn't ignoring these. It's fully parsing those and simulating each available path, in fact. Ninja's parser is exactly precise, it doesn't only look at object IDs, it goes much deeper, simply because if it wouldn't be, it wouldn't know exactly where objects logically start or end, and wouldn't know when to take into account multiple object definitions in the same file (which is a real possibility). The only way would be to fall back to some sort of pattern matching which is both slower and less accurate. That's the reason why Ninja parses the entire object structure, including AL syntax, the only thing it doesn't do is build the AST from everything because it has no use for it.

Looking at your actual code, I don't see where the problem is - both objects are the same ID so Ninja should pick it?

Can you be more specific about exactly what's going on? I am trying to reconstruct it from bits and pieces, and I am not sure I see the exact picture.

I can see that you have:

• an app pool
• "current code" shown as belonging to one file, which from your further comment I assume actually belongs to two separate files, and also may or may not belong to two different apps in the same pool

If you would specify exactly what your app setup is, what your pool setup is, what exactly you do where, and what problems you get, I could probably understand this.

[rdebath]

Other languages don't have object IDs

True, but there are many reasons to have non-standard parsers for other tools, For example programs like the BC *Cops.

... simulating each available path, ...

Ah, I didn't understand that it was supposed to follow the exponential count of all permutations. That does have implications for the couple of time people have included #if false or #if OLD_NAV to keep NAV variants of code available; it's probably fine.
Also it bumps up the cleanup of the few times we use #if not BC or BC27_OR_LATER and similar tags.

As for what my problem is:

• The current version V3.4.4 is working in unexpected ways WRT #if preprocessor lines when displayed under "App Pool->Lost-Codeunits". For example: a line of #if Bevica_Import hides the following codeunit line (even if the token is defined in app.json) and shows it as lost. Changing it to #if not BadNinja and Bevica_Import allows Object Ninja to see it. This does not change the state as seen by VSCode. BadNinja will never be true.
  This happens with Both APP-Pool registered Apps and standalone.

I now realise that this is distinct from the full scan issue I saw before, a full scan does collect the object number presumably because of the permutation scanning you've described. I now understand that the "lost processing" does not match the full scan, nor does it match VSCode's compile of the current workspace.

I don't suggest you use the full permutation scanning for "lost" processing. The issues I see with using the full permutation scanning for lost objects would be that it will interfere with duplicate scanning (VerA vs VerB conflicts) as some permutations will be forbidden (especially defining the variable false). Also it could get slow with pathological cases when the 2**N permutations start getting big.

So TODO:

Please make sure that #if, #if not and #if ... do what they're supposed to ON THE FAST PATH for "lost" and "duplicate" object processing.
And document it.

Finally:

This, will blow away all the other projects in the App pool. It's probably something you see as allowed, but not supported.

Also "Synchronize Object IDs with Azure Back End" -> "Replace. ..." [Select project]"

This Works fine ...

"Automatically Synchronize Object IDs for Entire Workspace"