bug#69387: 30.0.50; A string shouldn't be both a docstring and a return value

["Mattias Engdegård" <mattias.engdegard@gmail.com>]

> There is no benefit to this misfeature.
> Leaving this alone will maintain a low-level of pain for the
> foreseeable future.  Fixing it may introduce minor breakage (like
> `C-h m` saying there's no docstring) in the short term but these are
> easy to fix and they'll disappear quickly.

Indeed. Eli is right that we need to be careful, but a mechanised scan of our code really shows that this might be a source of bugs that we should consider doing something about. An amusing example:

> (defun semantic-mrub-read-history nil
>   "History of `semantic-mrub-completing-read'.")

... which is then not called but used as a variable by the code.

Or this one:

> ;; Don't alias this to `ignore', as that will cause the resulting
> ;; function to be interactive.
> (defun use-package-normalize/:disabled (_name _keyword _arg)
>   "Do nothing, return nil.")


Return nil my foot.

Would it be productive to warn, and about what? Not for pure lambdas, because there should be nothing wrong with

  (lambda () "string")

and it's extremely unlikely to be intended as a doc string (I found not a single instance).

The overwhelming majority of docstring-value `defun`s are functions that are placeholders, unfinished or unimplemented, sometimes with commented-out code. Often it's clear that the user didn't know how to proceed:

This indicates that maybe we should warn for `defun`; users can easily add a `nil` before or (more commonly) after the string depending on what they mean.

The docstring-result syndrome is particularly common in `cl-defgeneric` because it's not seen as code that will ever be used, so we should probably not warn in that case.

We could expand

 (defun (ARGS) "string") -> (defun (ARGS) "string" "string")

to preserve semantics (and the same for defmacro, cl-defun, etc).