bug#34418: 27.0.50; Flymake adds markup to buffers not specified in `flymake-make-diagnostic'

bug#34418: 27.0.50; Flymake adds markup to buffers not specified in `flymake-make-diagnostic'

[Philipp]

1. Create two scratch buffers *a* and *b* (e.g. C-x b *a* RET).

2. Insert some text into the buffers (e.g. "text a" and "text b",
   respectively)

3. Add a trivial Flymake backend to buffer *b*.  E.g., select *b* and
   run M-: with the following code:
   
   (add-hook 'flymake-diagnostic-functions
             (lambda (report-fn &rest _args)
                    (funcall report-fn (list (with-current-buffer "*a*"
                    (flymake-make-diagnostic (current-buffer)
                                             (point-min) (point-max)
                     :error "message"))))))
                     
   Note that this backend adds a diagnostic for buffer *a*, not *b*.
   
4. Enable Flymake mode in buffer *b*.

Buffer *b* will now show a diagnostic, even though it was reported for
buffer *a*.  This should either add a diagnostic for buffer *a* or
signal an error.

See
https://lists.gnu.org/archive/html/help-gnu-emacs/2018-12/msg00170.html
for more context.


In GNU Emacs 27.0.50 (build 37, x86_64-apple-darwin18.2.0, NS appkit-1671.20 Version 10.14.3 (Build 18D109))
 of 2019-02-10 built on p
Repository revision: 24695e8977c3d048e60d1fe3714b789567c2213c
Repository branch: master
Windowing system distributor 'Apple', version 10.3.1671
System Description:  Mac OS X 10.14.3

Recent messages:
C-+ is undefined
Region has 0 lines, 0 words, and 0 characters.
M-+ is undefined
Use +,-,0 for further adjustment [5 times]
Loading flymake...done
next-line: End of buffer
user-error: End of history; no default available
((lambda (report-fn &rest _args) (funcall report-fn (list ...))) flymake-proc-legacy-flymake)
Flymake mode enabled in current buffer [2 times]
mwheel-scroll: Beginning of buffer [4 times]
mwheel-scroll: Beginning of buffer
Configured using:
 'configure --without-xml2 --with-modules --without-pop --with-mailutils
 --enable-gcc-warnings=warn-only --enable-checking=yes
 --enable-check-lisp-object-type 'CFLAGS=-ggdb3 -O0''

Configured features:
IMAGEMAGICK NOTIFY KQUEUE ACL GNUTLS ZLIB TOOLKIT_SCROLL_BARS NS MODULES
THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: de_DE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Messages

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt bytecomp
byte-compile cconv dired dired-loaddefs format-spec rfc822 mml mml-sec
password-cache epa derived epg epg-config gnus-util rmail rmail-loaddefs
time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils cl-macs gv cl-seq flymake-proc flymake easymenu
compile comint ansi-color ring warnings thingatpt cl-loaddefs cl-lib
face-remap elec-pair tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads kqueue cocoa ns lcms2 multi-tty make-network-process
emacs)

Memory information:
((conses 16 56619 10244)
 (symbols 48 6929 1)
 (strings 32 17753 1870)
 (string-bytes 1 600979)
 (vectors 16 11313)
 (vector-slots 8 139126 16778)
 (floats 8 29 130)
 (intervals 56 213 28)
 (buffers 992 14))

bug#34418: 27.0.50; Flymake adds markup to buffers not specified in `flymake-make-diagnostic'

[João Távora]

Philipp <p.stephani2@gmail.com> writes:

> 1. Create two scratch buffers *a* and *b* (e.g. C-x b *a* RET).
>
> 2. Insert some text into the buffers (e.g. "text a" and "text b",
>    respectively)
>
> 3. Add a trivial Flymake backend to buffer *b*.  E.g., select *b* and
>    run M-: with the following code:
>    
>    (add-hook 'flymake-diagnostic-functions
>              (lambda (report-fn &rest _args)
>                     (funcall report-fn (list (with-current-buffer "*a*"
>                     (flymake-make-diagnostic (current-buffer)
>                                              (point-min) (point-max)
>                      :error "message"))))))
>                      
>    Note that this backend adds a diagnostic for buffer *a*, not *b*.
>    
> 4. Enable Flymake mode in buffer *b*.
>
> Buffer *b* will now show a diagnostic, even though it was reported for
> buffer *a*.  This should either add a diagnostic for buffer *a* or
> signal an error.

Hi Phillip,

Sorry for the very slow turnaround.

The behaviour for handling the mismatch between the (1) the buffer
passed to flymake-make-diagnostic (2) the buffer where the report
function executes is unspecified and it is so by design.

I've yet to come to a good conclusion on what that behaviour should be,
and maybe you can help me.  Let's see some plausible real-world scenarios.

1. Some uses of Flymake, notably Eglot's via LSP (Language Server
   Protocol) can possibly take advantage of a good definition of this
   behaviour, for aggregating the errors reports across a project for
   example.  So `flymake-make-diagnostic` could be specified to take a
   BUFFER-OR-FILE, and we could heuristically decide to add the
   diagnostic to, say, a per-project database.

2. In another simpler scenario, checking .c file might issue errors for
   included .h files, and if that file is open in a buffer, we could go
   there and highlight the error.  Could we really?  Maybe not, because
   the error was probably generated for the on-disk copy of the .h file,
   whose contents might differ wildly from the buffer's.  Then again, a
   smart backend could consider that.

So maybe your "error" proposal makes sense, maybe it doesn't.  I'd
rather not commit to an API right now that could block evolution.

For now I've added a note to the manual's description of
flymake-make-diagnostic and a protection to flymake--handle-report that
will ignore such diagnostics.  What do you think?

João

bug#34418: 27.0.50; Flymake adds markup to buffers not specified in `flymake-make-diagnostic'

[Lars Ingebrigtsen]

João Távora <joaotavora@gmail.com> writes:

> For now I've added a note to the manual's description of
> flymake-make-diagnostic and a protection to flymake--handle-report that
> will ignore such diagnostics.  What do you think?

This was over a year ago, and there was no response.  Reading the
report, I'm not sure whether there's anything further to be done here,
or whether it can be closed?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34418: 27.0.50; Flymake adds markup to buffers not specified in `flymake-make-diagnostic'

[Philipp]

> Am 02.11.2019 um 17:04 schrieb João Távora <joaotavora@gmail.com>:
> 
> Philipp <p.stephani2@gmail.com> writes:
> 
>> 1. Create two scratch buffers *a* and *b* (e.g. C-x b *a* RET).
>> 
>> 2. Insert some text into the buffers (e.g. "text a" and "text b",
>>   respectively)
>> 
>> 3. Add a trivial Flymake backend to buffer *b*.  E.g., select *b* and
>>   run M-: with the following code:
>> 
>>   (add-hook 'flymake-diagnostic-functions
>>             (lambda (report-fn &rest _args)
>>                    (funcall report-fn (list (with-current-buffer "*a*"
>>                    (flymake-make-diagnostic (current-buffer)
>>                                             (point-min) (point-max)
>>                     :error "message"))))))
>> 
>>   Note that this backend adds a diagnostic for buffer *a*, not *b*.
>> 
>> 4. Enable Flymake mode in buffer *b*.
>> 
>> Buffer *b* will now show a diagnostic, even though it was reported for
>> buffer *a*.  This should either add a diagnostic for buffer *a* or
>> signal an error.
> 
> Hi Phillip,
> 
> Sorry for the very slow turnaround.
> 
> The behaviour for handling the mismatch between the (1) the buffer
> passed to flymake-make-diagnostic (2) the buffer where the report
> function executes is unspecified and it is so by design.
> 
> I've yet to come to a good conclusion on what that behaviour should be,
> and maybe you can help me.  Let's see some plausible real-world scenarios.
> 
> 1. Some uses of Flymake, notably Eglot's via LSP (Language Server
>   Protocol) can possibly take advantage of a good definition of this
>   behaviour, for aggregating the errors reports across a project for
>   example.  So `flymake-make-diagnostic` could be specified to take a
>   BUFFER-OR-FILE, and we could heuristically decide to add the
>   diagnostic to, say, a per-project database.
> 
> 2. In another simpler scenario, checking .c file might issue errors for
>   included .h files, and if that file is open in a buffer, we could go
>   there and highlight the error.  Could we really?  Maybe not, because
>   the error was probably generated for the on-disk copy of the .h file,
>   whose contents might differ wildly from the buffer's.  Then again, a
>   smart backend could consider that.
> 
> So maybe your "error" proposal makes sense, maybe it doesn't.  I'd
> rather not commit to an API right now that could block evolution.

I wouldn't say signaling an error now would block evolution.  I don't think we have a principle like "anything that signals an error now will continue doing so in the future."

> 
> For now I've added a note to the manual's description of
> flymake-make-diagnostic and a protection to flymake--handle-report that
> will ignore such diagnostics.  What do you think?

Adding a note to the manual is definitely a good idea.  I'm not so sure about (silently) ignoring "wrong" diagnostics -- after all, the backend definition then contains a bug, and it's generally a good idea to help developers find and fix bugs by being explicit about them.  So I'd still favor signaling an error, with the potential to change the behavior if we find a good use for such diagnostics.

bug#34418: 27.0.50; Flymake adds markup to buffers not specified in `flymake-make-diagnostic'

[João Távora]

On Mon, Jul 5, 2021 at 5:51 PM Philipp <p.stephani2@gmail.com> wrote:
>
>
>
> > Am 02.11.2019 um 17:04 schrieb João Távora <joaotavora@gmail.com>:
> >
> > Philipp <p.stephani2@gmail.com> writes:
> >
> >> 1. Create two scratch buffers *a* and *b* (e.g. C-x b *a* RET).
> >>
> >> 2. Insert some text into the buffers (e.g. "text a" and "text b",
> >>   respectively)
> >>
> >> 3. Add a trivial Flymake backend to buffer *b*.  E.g., select *b* and
> >>   run M-: with the following code:
> >>
> >>   (add-hook 'flymake-diagnostic-functions
> >>             (lambda (report-fn &rest _args)
> >>                    (funcall report-fn (list (with-current-buffer "*a*"
> >>                    (flymake-make-diagnostic (current-buffer)
> >>                                             (point-min) (point-max)
> >>                     :error "message"))))))
> >>
> >>   Note that this backend adds a diagnostic for buffer *a*, not *b*.
> >>
> >> 4. Enable Flymake mode in buffer *b*.
> >>
> >> Buffer *b* will now show a diagnostic, even though it was reported for
> >> buffer *a*.  This should either add a diagnostic for buffer *a* or
> >> signal an error.
> >
> > Hi Phillip,
> >
> > Sorry for the very slow turnaround.
> >
> > The behaviour for handling the mismatch between the (1) the buffer
> > passed to flymake-make-diagnostic (2) the buffer where the report
> > function executes is unspecified and it is so by design.
> >
> > I've yet to come to a good conclusion on what that behaviour should be,
> > and maybe you can help me.  Let's see some plausible real-world scenarios.
> >
> > 1. Some uses of Flymake, notably Eglot's via LSP (Language Server
> >   Protocol) can possibly take advantage of a good definition of this
> >   behaviour, for aggregating the errors reports across a project for
> >   example.  So `flymake-make-diagnostic` could be specified to take a
> >   BUFFER-OR-FILE, and we could heuristically decide to add the
> >   diagnostic to, say, a per-project database.
> >
> > 2. In another simpler scenario, checking .c file might issue errors for
> >   included .h files, and if that file is open in a buffer, we could go
> >   there and highlight the error.  Could we really?  Maybe not, because
> >   the error was probably generated for the on-disk copy of the .h file,
> >   whose contents might differ wildly from the buffer's.  Then again, a
> >   smart backend could consider that.
> >
> > So maybe your "error" proposal makes sense, maybe it doesn't.  I'd
> > rather not commit to an API right now that could block evolution.
>
> I wouldn't say signaling an error now would block evolution.  I don't think we have a principle like "anything that signals an error now will continue doing so in the future."

And yet, but experience shows that programs rely on externally
observable behaviour again and again, and I have little reason
to believe errors are an exception to that. So unless we're reasonably
sure that the changes we make to a piece of behavior make sense
in the long run, better not do them.

I think there's prior art with negative overlay priorities, for example:
last I checked manual said "please don't use these, as we haven't
decided on what they might mean".  Or something like that.
I don't think an error make sense there, either.

João