nextmajor false positives

[jwikman]

When using AL Language extension from nextmajor (v9.0.582246) BC.LinterCop starts to flood me with false positives.

These are the ones I seen so far:

• [ ] LC0002 - Comments is not detected
• [ ] LC0003 - Complains on an Option parameter
• [ ] LC0005 - Says that "Option" has wrong casing, it should be "Option" 😅
• [ ] LC0010 - Maintainability index is 0 for all functions
• [ ] LC0012 - Complains on RecRef.Open(MyRecord.MyFieldNo)
• [ ] LC0014 - Says that PermissionSet names with 19 characters are longer than 30 chars

It's not urgent yet, but it might be something to start looking into. :)

[StefanMaron]

Usually that should be resolved be a recompile against the latest dependencies of bc20.

But thanks for reporting!

[jwikman]

When do you plan to do such a recompile? And when that are done, will the dll be work for AL Language extension v8? Or would it be an idea to have parallell versions for some time before the public AL Language extension gets the new major?

[StefanMaron]

I dont have access to the insider builds. So I will have a look at this as soon as the public preview is available.

in best case, I will just need to recompile the linter against the v8 compiler and wont need to change any code. Could also be that I will need to adjust the code to make things work again.

In either case I will most probably have a separate branch for the upcoming version. And I will have a pipeline compile this. I think I wont do releases for that version, but You can then manually download the compiled linter dll from the pipeline and place it somewhere on your disk. In the al analyzers setting you can specify a full path to that dll to use it in your projects where needed.

[jwikman]

Ok, I understand. That sounds as way forward.

It would've been cool if the linter extension could download the right dll, depending on version of the AL Language extension. Maybe I can assist with something around that for another major release.

But now we're looking forward to the public preview then! 👍

[StefanMaron]

lets do some brainstorming on this one sometime. Maybe we will find a good solution.

[jwikman]

@StefanMaron first post is updated with the false positives I found so far.

Let me know if you need anything more. :)

[StefanMaron]

BusinessCentral.LinterCopBC20.zip

Here is a version compiled against the new version of the compiler. Please try this and report back if any bugs are still present.

[jwikman]

This seems to fix all issues listed above. 👍

But this release suffers from the false positives of LC0016 described in #166, so I first thought it didn't work because of the number of warnings. 😅

Do you have any idea on how to automate the generation of a dll for next major version going forward? It would be really useful a month or two before the next major release. I will have a struggle convincing my colleagues that we should use this on all our AppSource apps if it does not work a couple of month each year, when we are preparing our apps for the upcoming next major... 😕

[NKarolak]

Hi Stefan, as far as I understood from Twitter and Yammer, you now do have access to the insider builds ...?

I'd like to add LC0000:

There was an Error in Rule "Rule0003" of type "unknown"

For Option data type parameters:

Do not use an Object ID for properties or variables declaration. Use Option instead.AL[LC0003]

procedure MyProcedure(StatusValue: Option): Text
    begin
    end;

[StefanMaron]

This should be fixed now with version v0.25.1

[NKarolak]

Perfect timing :D https://twitter.com/KarolakNatalie/status/1509801668581691392 Works for me.

[jwikman]

Thanks @StefanMaron!

Works fine!

[jwikman]

@StefanMaron I'm sorry if this is already solved, and I missed it...

But have you implemented some way to build and distribute BC LinterCop for both current version and the next major?

At least now when the public preview is available, you should be able to pull the vsix from that bc artifact

Get-BCArtifactUrl -type Sandbox -country W1 -version 21 -storageAccount BcPublicPreview

[StefanMaron]

Its not, and thanks for the reminder!

I have thought about this and how to solve this best.

My approach will be to release all new rules as infos to the master branch directly. This way the linter wont break pipelines set to fail on warning. This gives me the possibility to use the prereleases for the Next Major build.

To also get around the need of identifying the version of the AL compiler in use, a the exisiting setting of preleases y/n can just be set to decide which version should be loaded

What do you think?

[jwikman]

I no fan of the approach to let the prerelease setting identify if I am running with the pre-release version of ALC or not... I just know how many times I've been switching between AL Language extension v9 and v10 last months... I might remember to change that prerelease setting every time I install the other version of AL, since I'm really involved in this discussion. But I'm pretty sure that not everyone will find this setting, just cursing about "false positives"...

But with that said, using the latest "prerelease" from your GitHub Releases might not be that bad. But the downside is that it conflicts with the prerelease setting already in use. Adding new rules as "info" would make any introduced issues with them less harmful. But not having beta-testing in the wild, as the prerelease setting allows today, might result in regression of existing rules to become messy for many users... Or what do you think?

I'd prefer if the current behavior of the release feed remains, with the prerelease setting working just as today.

What we, IMHO, would need is

1. A new feed for BusinessCentral.LinterCop.dll build for the pre-release of ALC, with the same prerelease setting logic as today.
2. A way for the BusinessCentral.LinterCop VSCode extension to identify which feed to download from.

One approach for first point could be to create a new GitHub repo, that only is used for the release feed functionality for BusinessCentral.LinterCop.dll compiled against pre-release of ALC. In that way, the versioning and everything could remain as today (even though it might be a good idea to add something to the "Product name" when compiling the dll, just to be able to troubleshoot if wrong version is downloaded 🙂).

The second point is trickier to do, without having to publish information about which ALC version maps to which feed somewhere. Or do you have any good idea here?

[StefanMaron]

IMHO a linter should not be harmful at all. Its a tool for those who want to use it, and I think it still functions as this kind of tool even when rules are introduced as info. I am not sure yet, but maybe those rules will become warnings in the future.

For the Next Major version: What about if the vs code extension loads both versions of the dll with different name? Then you could use the Analyzer setting per project to define the version to be in use.

[jwikman]

Maybe "harmful" was the wrong word for this, what I meant is that if (or when? 😜) a bug is introduced in an existing rule, I know that it becomes quite a lot of unnecessary time spent when we got a lot of developers getting the same issue at the same time and they all tries to fix it... If we, a bunch of "super users" that has deeper knowledge about this, had a chance to catch those bugs through a pre-release channel (as it is today), we can save quite some man hours and bad will for the tool.

How to introduce new rules might be another thread? :)

Loading both versions would still need manual toggling of settings. I think it should be possible to solve this in a more elegant (automated) way , don't you think?

What if you, in each release, published these artifacts:

• BusinessCentral.LinterCop.dll - the same dll as today
• BusinessCentral.LinterCop_v[Major of ALC of BC NextMajor].dll, currently BusinessCentral.LinterCop_v10.dll, after october 1st (or when you decide to support next major) it will become BusinessCentral.LinterCop_v11.dll - this will always be built on the pre-release/nextmajor version of ALC

Then the VSCode extension could try to download BusinessCentral.LinterCop_v[Major of ALC currently in use].dll, with a fallback to BusinessCentral.LinterCop.dll.

In this way we could have the pre-release functionality just as today and as "automagical" as possible.

Thoughts?

[rvanbekkum]

@StefanMaron The 10.0 compiler is now the current, which is what we are using in all our build pipelines to compile the apps. The LinterCop throws false positives since the release of BC 21.0. For now, could you make a new release to resolve the issue?

[StefanMaron]

I have an automation in place which runs each morning to check if there is a new version of the compiler. Due to the release on the Afternoon today I did not get notified yet.

But Thanks for letting me know, the new version is now released

[GOB-Andrea]

Hi @StefanMaron , LinterCop has the same problem as last year with the next major. VS Code shows a lot of unnecessary (false positive) LinterCop Warnings.

For Example: LC0001: FlowFields should not be editable. LC0003: Do not use an Object ID for properties or variables declaration. Use Matches temporary instead. LC0004: Property "LookupPageID" and "DrilldownPageID" must be filled in table LC0005: Wrong casing detected! Use Matches temporary instead.

It would be very helpful if you could provide an updated version. Thank you very much!

[StefanMaron]

@jwikman @rvanbekkum It seems like we have pre releases of the AL Language extension now. I will look into adding next major support with this.

My idea would be to always build current and next, and include both dlls into the releases and to download both automatically.

The name of the current dll wont change. And the next major dll will have a suffix to the name.

This will enable everyone to decide which dll to use in the current repo. And also the extension foes not need to download the correct dll every time you switch repositories. Since I can not overwrite the dll if the language server already is started, I expect less problems if both dlls are present at the same time.

What do you think?

[rvanbekkum]

That sounds good and I agree with your reasoning! Maybe @jwikman comes up with something we have not thought about yet?

[jwikman]

Yes, finally we got the Pre-Release! :)

I don't agree to the approach, though..

I do not want to add a setting to the repo (in workspace or folder settings), since the need for this setting will change over time. The day that Microsoft releases the Pre-Release version as the "real" release, I do not want to go and update hundreds of repos.

The dll is built against a specific runtime and is also saved in the extension folder for a specific runtime. I think we could come up with a more elegant solution, where the user do not need to think about what version of BCLinterCop to use - they only need to care about which AL Language version they are running.

We already got the path to the AL Language extension folder (https://github.com/StefanMaron/vsc-lintercop/blob/bf80c5a2545c359acd1dd0cf80ab451404664e87/src/extension.ts#L30) and that contains the version numbers. If the VSCode extension only knew which dll is built against which runtime version, we could download the correct dll. Maybe you could add a json file with that information to the release assets, that can be downloaded first?

Something like this (notice that there are no suffix on file names):

{
    "version":"0.29.1",
    "currentLinterCopUrl": "https://github.com/StefanMaron/BusinessCentral.LinterCop/releases/download/v0.29.1/current/BusinessCentral.LinterCop.dll",
    "versionMap":[
        {
            "alMajorVersion":"11",
            "linterCopUrl":"https://github.com/StefanMaron/BusinessCentral.LinterCop/releases/download/v0.29.1/v11/BusinessCentral.LinterCop.dll"
        },
        {
            "alMajorVersion":"12",
            "linterCopUrl":"https://github.com/StefanMaron/BusinessCentral.LinterCop/releases/download/v0.29.1/v12/BusinessCentral.LinterCop.dll"
        }
    ]
}

Then the downloader can try to match the major versions of the AL Language extension folder with above version map, and use that link if found - with a fallback to currentLinterCopUrl

In that way we would get the v11 version of BCLinterCop in the v11 AL Language extension folder and the v12 version of BCLinterCop in the v12 AL Language extension folder.

[StefanMaron]

Ah, you are right. The ddls will get downloaded to different folders.

I dont think I need a config though. I will look into this. Are you both on discord? Then I would start a new thread there and post updates to not spam this issue here

[jwikman]

One thing that we might be able to solve as well with a new download implementation, with or without json mapping, could be support for the last X versions of AL Language extension. As it is today, there are a couple of scenarios when the versions of LC and AL is out-of-sync:

1. A user has updated to a new version of AL when it is released (as a Release or Pre-Release), and LC is not built yet for that version - this will always be a time slot where we are waiting for a new dll to be built, published and downloaded
2. New version of AL released, user has not updated, but new version of LC is published - now the user get a dll built for a newer version of AL, which I expect to give strange issues

Scenario 2 above happens when you don't have auto-update of extensions enabled, or if you need to downgrade the AL Language extension for some reason (I've been in both those cases, for different reasons).

Since all versions of the AL Language extension is available to download from VSCode Marketplace, the build pipeline could build for the last X releases and last X Pre-Releases (see https://github.com/microsoft/navcontainerhelper/blob/master/ContainerHandling/Get-LatestAlLanguageExtensionUrl.ps1 for download inspiration)

I think I'm too old for discord... 😅 Don't you guys use Teams?

[StefanMaron]

Sorry, but I really dont see the need for the second scenario. An update of the AL Language extension is just a matter of clicking "update".

[jwikman]

It's not very common, but we've hade cases where a new AL version had issues and we stick with the previous version until new version is published (which is done within a few days). Just thought that we could have a solution that solved that scenario as well. But if it is too much work, it might not be worth the effort. :)

[StefanMaron]

The pipeline is updated now. I always build the current and the next .dll

Next will be the update of the vs code extension. I am not sure if I will be able to get this done before easter, but I will keep this issue updated

[jwikman]

Cool 👍

Just curious: How do you plan to identify if the AL Language extension is "current" or "next"?

[StefanMaron]

I hope there is a property "prerelease" I can read. If not I will query the marketplace API the same way the pipeline does to get the exact version of current and next