microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
744 stars 245 forks source link

VS Code does not display all warnings #6198

Closed NKarolak closed 4 years ago

NKarolak commented 4 years ago

Describe the bug VS Code is not able to display all warnings, although there would be only a few to find. (I am aware, that per file there is a limit of 100 - but this is not touched.)

I was correcting warning AA0008 (missing parenthesis after a method). VS Code displayed a lot occurrencies, but apperently not all, like the one in the following screenshot: image. I've tried multiple times to restart VS Code / Reload Window. Some code lines will simply never be warned about.

This is not an issue of the code analyzer rule itself - my pipline running the same settings does throw all warnings correctly on app compile. I am not using pragmas in the affected files.

To Reproduce I cannot provide simple demo code; just my whole app on request.

My current settings.json contains

    "al.enableCodeAnalysis": true,
    "al.backgroundCodeAnalysis": true,
    "al.codeAnalyzers": [
        "${AppSourceCop}",
        "${CodeCop}",
        "${UICop}"
    ],

5. Versions:

kalberes commented 4 years ago

What if you build your project with Ctrl Shift B. Does it appear then?

NKarolak commented 4 years ago

Good point, let me check tomorrow.

NKarolak commented 4 years ago

Yes, when building the package, all warnings are shown as Output, but still not under Problems / in the code. What does that mean?

kalberes commented 4 years ago

We have one more settings(I guess undocumented, but important). Similar to the 100 diagnostics per file (that is also undocumented as far as I know). If you have a project larger than 50 files than our diagnostics will only emit something we call partial diagnostics. Partial diagnostics will ignore code analysis based diagnostics, since they are very expensive in our current (and original) design.

We have a slice hanging around for quite some time now to enhance how our code analysis framework works. We still have not had time to get to it. But I hope, we will soon. I know, we should document these important settings. I will ask around see if we could. Maybe we could also expose them, that you can change them, similar as we have exposed the delayAfterLastDocumentChange and similar.

NKarolak commented 4 years ago

Thanks Kálmán.

  1. So what would be your current suggestion for me? To always rely on the Output window, until the setting is exposed by you?
  2. Can you please confirm (again) that it is really depending on the number of files, not on the total file size? I'd like to twitter about it.

Edit:

  1. is the label "input needed" still up-to-date? ;-)
  2. Partial diagnostics

    Partial diagnostics will ignore code analysis based diagnostics

Then I don't understand what I saw. Rule AA0008 needed to be applied across one large file. The Problems window showed most of them, also spread across the file. The few ones that were still missing afterwards were still randomly (?) spread across this file. I really see no rule which line was warned about, and which not.

kalberes commented 4 years ago
  1. For now yes. That is the current design if you have large projects.

  2. I will list here all our diagnostics creation options. a. There are a maximum 100 diagnostics shown per file b. If a project contains more than 50 documents then we will not show code analysis diagnostics based on background compilation tracking (in other words almost as you type). You have to provoke a full diagnostics and that is Ctrl Shift B to see code analysis issues. c. There are two public options to control how often the diagnostic queue is processed. i. DelayAfterLastDocumentChange: Specifies the number of milliseconds to wait after the last buffer changes before getting document diagnostics ii. DelayAfterLastProjectChange:Specifies the number of milliseconds to wait after the last buffer changes before getting complete diagnostics

  3. No, it is by design.

  4. Now what happens is that there may be tracked and not cancelled compilations that were actually treating code analysis diagnostics until you have reached the 50 files per project and these diagnostics made it back to the language client (vscode). Then diagnostics on code analysis has stopped after more then 50 files were loaded, but the language client has not refreshed the state (I call them dancing squiggles). If this was the issue agree this sounds a bit buggy. They should probably be controllable with the settings in c.

NKarolak commented 4 years ago

Thanks a lot for your detailed explanation!

NKarolak commented 4 years ago

Oh, I forgot to mention something relevant for point 4: After a fresh clone of my repository, my colleague removed a () from my already corrected file, and the warning AA0008 was not shown (as visible in the screenshot of my initial post, btw). Is that still the bug you were relating to? I could still share my repository with you.

kalberes commented 4 years ago

If you have removed the () then a new tracked compilation kicks in. But since you probably have more then 50 files diagnostics from code analysis will not be shown. This is why the warning will be gone. It was there because there was a compilation started that could not cancel in time when the project has loaded and there were less then 50 files parsed and bound. The rule of thumb that if you should not see code analysis diagnostics for large projects. But that is a promise the current compiler apparently cannot keep. It can be treated as a bug, but I am sure it will not hit the bar to consider it for now. We have a slice as I have mentioned where code analysis diagnostics should be part of tracked compilations. Unfortunately we have not started on that yet. Ctrl Shift B should always do full diagnostics.

NKarolak commented 4 years ago

Then why, when I opened the project before the first correction, why DID the compiler show lots of other AA0008 warnings, even within the same file? As I understood you, none should have been shown from the very beginning.

So you say that's a bug, but it is not important for now to consider? Just to summarize it for someone who just fell out of bed 😉

kalberes commented 4 years ago

The compiler will show code analysis errors until its internal buffer handling for a workspace reaches 50 files. Then it will stop. How the diagnostic handling is working is highly nondeterministic, thus some of these may be shown some some may not not since the number 50 has been reached. This all depends how fast these old compilation tasks get cancelled on the thread pool.