bug#50244: 28.0.50; Support project-wide diagnostics reports in flymake.el

[Dmitry Gutov]

On 14.09.2021 11:43, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>
> > On 13.09.2021 23:53, João Távora wrote:
> >
> > > > Anyway, I asked about this because because the use of global variable
> > > > (flymake-list-only-diagnostics) seems like it would make supporting
> > > > multiple projects at the same time more difficult.
> > > I can't guess what you are hinting at, sorry.  You can call M-x
> > > flymake-show-project-diagnostics in two or more projects, of course, and
> > > you'll get separate listings.  I don't know if that counts as
> > > "supporting multiple projects at the same time".
> >
> > If a backend reports diagnostics through
> > flymake-list-only-diagnostics, woudln't one of those values be
> > overridden when two backends offer reports?
>
> No.  The variable is an alist (could be another type of map), so
> backends only add and remove entries about the files they know about.
> The "and remove" bit is still under study.  For now I've chosen to leave
> the "remove" responsibility to backends, but I've experimented with
> schemes in which flymake.el does the removal strategically.  I'll
> re-study that.  Also, perhaps you're being confused by the variable's
> docstring, which is not as good as its entry in the manual.  I'll try to
> clarify later.

It's a judgment call, but that seems fairly involved for a public API.

That's why I was thinking of a somewhat different shape: where a backend 
reports a full list (corresponding to the default-directory it's called 
from), and then Flymake can either show the list, or discard it at some 
later point.

Then you won't need project.el to filter the full global list, 
delegating the job of compiling the list to the backend. That seems to 
be the most flexible scheme: some authors will prefer Projectile or 
whatever, but it some cases the backend will simply have a different 
view into what a project it (which the user's config might or might not 
reflect). Subtly losing diagnostics in such scenarios seems like a bad 
outcome.

> > > Again, I don't understand your suggestion, but if you can propose it in
> > > the form of code it would be much better, since there would be no
> > > ambiguity.  A word of caution though: making these things work correctly
> > > in tandem with domestic diagnostics, new file visits and buffer killings
> > > was relatively hard.  I tried many different approaches and settled on
> > > these two ("foreign" and "list-only").  Of course if you can clearly
> > > describe a use case where they are unsuitable, a third style may be
> > > invented.  But I would first exhaust the possibilities in these two.
> >
> > I was thinking of a way to get by with only two types: "domestic" and
> > "foreign", without adding a third one.
>
> Those two already exist.

Maybe I could say that we could have two types of diagnostics (maybe 
even just one: with file-based locations), but they would be 
differentiated by the source them came from. Some would be for the 
current buffer, and some would be for all buffers.

> "foreign" diagnostics belong to a given source buffer's flymake-mode but
> don't target the source, rather other buffers or unvisited files.  Like
> "domestic" ones they are systematically updated as the source is
> updated.
>
> They are well suited for flymake-cc and strongly typed languages where a
> compilation unit is created from the strongly coupled aggregation of
> many files and checked as a whole.  As you know if you've ever done
> C/C++ the locus of a syntactic mistake is usually spread out through one
> .cpp file and multiple .h files.

The situation where we have a backend which returns errors only for some 
of the "other" files but not all, is not something I considered, but if 
that is indeed an important use case, no argument from me.

UI-wise, though, they could probably work as the "global" source of 
errors anyway, right? Unless the same projects usually also have a 
project-wide list of errors coming from another source.

But in the former case the syntax check would be triggered by e.g. 
editing and saving the current file -- and you should be able to both 
see the list of errors in the current file, and then the list of all 
remaining errors.

'M-x flymake-show-diagnostics-buffer' could have a slightly different 
view in this case: first show the errors for the current buffer, and 
then the errors in other files.

> LSP servers don't systematically provide this info -- as far as I have
> witnessed, so Eglot can't do much with them.  If I'm wrong, then Eglot
> will also use this functionality.  The LSP protocol doesn't forbid it.
> Maybe I'll approach clangd developers with the idea.

IIUC LSP protocol provides some kind of feed where the client can 
subscribe to the updates, and then it sends diagnostics with some 
regularity, where latency probably varies by language server.

But in any case such syntax checks much be triggered by edits and saving 
of project files.

It would be nice if 'M-x flymake-show-diagnostics-buffer' would provide 
basically the same view into that data: it would update with higher 
delays, but otherwise show the same data. Not sure if LSP can send 
reports like "syntax checking in progress" -- if so, perhaps the view 
could also reflect that.

I suppose some users would prefer to be able to show/hide info not 
pertaining to the current buffer, but that's a matter of basic filtering.

> Currently, what _can_ be seen in the logs is an LSP server shooting out
> a once-only batch of diagnostics for the whole project of unvisited
> files when the server is started.  That is better suited to "list-only"
> diagnostics.  Why "list only"?  Because once the actual file is visited,
> it is assumed that flymake-mode will kick in there proper and be able to
> request fresh, domestic, highlightable, diagnostics to override the
> initial "list only" that came in the starting batch.

I suppose the assumption is that the "list only" diagnostics cannot be 
highlightable?

> > No code, sorry.
>
> If you want a new abnormal hook (as you seemed to propose), you have to
> say, at least in some kind of pseudo-code, _when_ that hook is going to
> be activated.  Maybe by doing that I could start to see the problem that
> it is solving (I also don't see that).

There are options. If the list-only diagnostics will only be displayed 
in the show-diagnostics buffer (and not in the source code buffers), the 
hook could run inside the *Flymake diagnostics* buffer the first time it 
is initiated. Or flymake-global-diagnostic-functions (let's call it 
that) could be called right away when a file is visited.

And maybe it would be called again and again often: whenever the current 
buffer changes, or every second after that. With the assumption that 
calling it too often would not be a problem, would not break/restart 
syntax checking processes (which is the case for LSP, for instance), it 
would just notify each global diagnostics backend whether 
default-directory has changed, and simply ask for updates (with the 
possibility of optimization that the backend would return lists which 
are 'eq' to each other if the list hasn't changed yet).

Alternatively, we could go further than the approach of hooks and make 
the global sources of diagnostics more like subscriptions. In that case 
they would be passed callbacks which are supposed to be called many 
time: every time the diagnostics change. The extra questions are how to 
"unsubscribe" and "relocate" (change default-directory), and whether to 
always "relocate" by "unsubscribing" + "subscribing". Anyway, these 
functions can still reside in a hook, with the general shape of each 
functions like

  (lambda (action &optional multi-callback))

Where ACTION is one of `subscribe', `unsubscribe' or `relocate'. And 
DEFAULT-DIRECTORY serving as an implicit argument, though it can be made 
explicit just as well.

I'm not saying this feature is easy or anything, but it would be nice to 
avoid having multiple sources editing the same alist at once (that logic 
should be part of the framework IMHO, based on the data the sources 
provide), and to let the sources themselves decide which files are part 
of their "project view". They can still use project.el underneath, if 
they find that convenient.