Ericsson / CodecheckerVSCodePlugin

VSCode plugin that shows bugs detected by the Clang Static Analyzer and Clang Tidy analyzers using CodeChecker as a backend.
Apache License 2.0
24 stars 7 forks source link

Don't popup the "CodeChecker: analysis in progress..." #61

Closed steakhal closed 2 years ago

steakhal commented 2 years ago

Please don't create an unclosable popup stating "CodeChecker: analysis in progress..." image

This is really annoying. And it's open for about 5 minutes for the largest translation units; since the analysis takes so much time. That being said, I hope we cancel the background analysis if we saved some changes.

IMO we should just remove this popup completely since the user already knows that an analysis is in progress here: image

Alternatively, give an option or something to make it disappear.

csordasmarton commented 2 years ago

@steakhal I totally agree with you that it is really annoying on large projects. I created to patch to remove this pop-up window (#64) and also added an extra tree item to be able to stop the analysis from the GUI: image

steakhal commented 2 years ago

How does the plugin handle the case when I make frequent changes to a single but large file - where an analysis would take about 5 minutes? It would always cancel the analysis it fired, would it? How should the plugin behave in such a scenario? One option could be to manually disable the analysis on save and trigger the analysis manually or alternatively have a debounce time option. If you were storing the last successful analysis time of a given file, you could say that the plugin will only analyze the file if that's idle for 10% of the time required by an estimated successful analysis. For the files we have not analyzed yet we could have concrete seconds or something similar. For the rest, it would scale nicely. WDYT?

csordasmarton commented 2 years ago

As far as I know this use case is handled and it will stop the previous analysis if you start a new analysis (e.g: saving your file). By the way @Discookie knows more about it.

Discookie commented 2 years ago

The way it currently works is that it doesn't kill off the analysis in progress, but instead queues a new one, and waits for the current one to finish. The queue is always prepended with new entries - when you switch to a new file and save, the new file's analysis will be queued first. When you cancel an analysis, it purges the entire queue. AFAIK there's any deduplication on the queued analyses, that's certainly something to improve.

steakhal commented 2 years ago

@Discookie the Q approach won't work generally. Users might have other tools triggered on saves, which they want to use frequently. For example, I have clang-format which prettifies my code whenever I save it. Such tools might advocate users to save more frequently than you would expect. Other than that what's the benefit of queuing these actions? Since we can only assume that the latest state is consistent with the rest of the files such as multiple header files etc. Are the entries in the queue any different than each other (for a given translation unit)? Since I'm not changing the compilation_database, I would expect them to be the exact same invocations.

Discookie commented 2 years ago

The benefit of this approach is when the user switches files, saves, then switches again, all of the files involved will get a re-analyze. This way the user knows the analysis is up-to-date for every file they open (assuming they didn't cancel).

If there's proper deduplication in place, saving frequently won't be an issue, as an analysis will always queued on the freshest version of the file. However, saving frequently wouldn't work when killing off the analysis, because saving too frequently wouldn't let the tools finish running, and the later tools (eg. clang-tidy after clang-sa) wouldn't get to run at all. (Currently only a full-project analysis clears this queue entirely.)

The queued analyses aren't different otherwise, they all follow the pattern of CodeChecker analyze [args] --output <cc_folder> --file <current_file>.

The state of the header files is a good point, I'm not sure if the tools support a file changing mid-analysis. My assumption is that they either support it or fail quickly, and so far I haven't had any issues related to this.