Interest in "Elisp lint"-like tool?

Interest in "Elisp lint"-like tool?

[Daniel Hackney]

Having done random hacking on and reading of some of the elisp libraries
around the web, especially in the various ELPA archives, there is a lack
of consistency across the different packages. They can range from
small-ish things like newline characters to documentation styles to more
significant things like the presence of autoloads, which are especially
significant for packages installed via ELPA.

There are a few tools which check various elements of style or
consistency like `checkdoc', `package', and `byte-compile', but there's
no comprehensive reference and validator for the whole range of
style-like things. I've put together a preliminary list of some of the
checks and warning I think might be useful. A number of these would
require some sort of agreement or decision by the community (indentation
style, for example), and for those, I've left the "check specification"
as a question.

  - short docstring on first line, blank line following
  - document all args
  - specify defcustom format
  - custom group
  - keymap in mode definition
  - can setup purely through autoloads and custom
  - all customs, defuns, and modes documented, especially interactive ones
  - variable and face for text faces, prefer existing face or
    inheritance from an existing face
  - quote symbol references (in docstrings)
  - Standard indent format? What to use?
  - whitespace, tabs and newline policy?
  - Single vs. double space after end of sentence?
  - warn about runtime CL funcs, suggest alternatives?
  - document free variables?
  - should section character be present or not?
  - where do file-local variables go?
  - how should global minor modes be handled?
  - document prefix arguments for interactive commands
  - warn when accessing the prefix argument without it being declared in the
    interactive specification
  - standardized keybinding declaration (defvar vs. let) and
    specification (`kbd' vs "\C")
  - `global-set-key' vs. global minor mode?
  - Don't override reserved keys (unless you are sure)
  - Autoload mode custom variables
  - no * in docstring
  - header and footer comments
  - enable modes by custom
  - use `define-*-mode' rather than raw `defun'
  - proper copyright, license, author, version, etc. headers
  - warn about comments in the header not in a standard section
  - avoid `defadvice'
  - use ";;" for comments
  - "namespace" should match filename
  - warn about deprecated variables and functions
  - `provide' call at end of file
  - semver.org (or "ruby gem") version policy
  - order of headers?
  - no comment ASCII art boxes (e.g. ";;;;;; Section description ;;;;;;")
  - `require's at top (except when not)
  - in-code version number? (e.g. (defvar FOO-version "1.2.3"))
  - warn on `require's without declared package dependency
  - unnecessary `require'
  - complain about too many function arguments?
  - should keyword args be suggested? Encouraged? Discouraged?
  - Check for author email and home page (possibly more?)
  - Document public vs. private vars and functions

Also, I thought there were some interesting/important behaviors which
such a package would require:

  - Magic comment to override style checks (;;;###stop-whining)
  - set strictness level/style group to check
  - Different checks (e.g. deprecated) for different Emacs versions

So you could have a group called `documentation' which would contain
checks that related to comments and docstrings, for example. Then, if
you wanted to run the checks without the documentation checks, you could
remove the `documentation' group from the list of applied checks.

As for a name, I came up with a couple (not particularly attached to any
of them):

  - styEl (style for elisp)
  - elLint
  - elCritic (in the same vain as Perl::Critic)
  - CriticEl
  - StyleEl

Some of these style choices seem fairly uncontroversial (like making
sure `provide' is at the end of the file), and I think it would be a
boon to the community to have some further standardization around best
practices and expected patterns. This becomes especially relevant in
light of the "packagization" of an increasing number of Elisp libraries,
as they do not go through the rigors of making it into Emacs core.

What do you all think?

--
Daniel M. Hackney

Re: Interest in "Elisp lint"-like tool?

[Glenn Morris]

(trivial comments only)

Daniel Hackney wrote:

>   - elLint

A bad name, since elint.el already exists. (See also lisp-mnt.el.)

> Some of these style choices seem fairly uncontroversial (like making
> sure `provide' is at the end of the file)

It is not always possible though (to avoid recursive requires is the
usual reason). 

Re: Interest in "Elisp lint"-like tool?

[Ted Zlatanov]

On Mon, 30 Jan 2012 17:45:57 -0500 Daniel Hackney <dan@haxney.org> wrote: 

DH> Having done random hacking on and reading of some of the elisp libraries
DH> around the web, especially in the various ELPA archives, there is a lack
DH> of consistency across the different packages. They can range from
DH> small-ish things like newline characters to documentation styles to more
DH> significant things like the presence of autoloads, which are especially
DH> significant for packages installed via ELPA.
...
DH> Some of these style choices seem fairly uncontroversial (like making
DH> sure `provide' is at the end of the file), and I think it would be a
DH> boon to the community to have some further standardization around best
DH> practices and expected patterns. This becomes especially relevant in
DH> light of the "packagization" of an increasing number of Elisp libraries,
DH> as they do not go through the rigors of making it into Emacs core.

DH> What do you all think?

I would appreciate such a tool.

Ted

Re: Interest in "Elisp lint"-like tool?

[Daniel Hackney]

Glenn Morris <rgm@gnu.org> wrote:
> (trivial comments only)
>
> Daniel Hackney wrote:
>
>>   - elLint
>
> A bad name, since elint.el already exists. (See also lisp-mnt.el.)

Ah, I wasn't aware. Again, these are merely random ideas off the top of
my head; I'm sure others could come up with better (read: less cheesy
and less already-taken-y) names.

>> Some of these style choices seem fairly uncontroversial (like making
>> sure `provide' is at the end of the file)
>
> It is not always possible though (to avoid recursive requires is the
> usual reason).

Haha, bad choice of "uncontroversial" example, then! ;)

Looking at the list again, I guess none of them are truly
uncontroversial, but many should be reasonably easy to agree on, like
having ";;; filename.el --- What this file does" as the first line or
"no trailing whitespace."

The broader point is that I think it would be useful to have some
guidelines on these sorts of things beyond what currently exists, and
some way to enforce or "gently suggest" adherence to them.

--
Daniel M. Hackney

Re: Interest in "Elisp lint"-like tool?

[Stefan Monnier]

>>> Some of these style choices seem fairly uncontroversial (like making
>>> sure `provide' is at the end of the file)
>> It is not always possible though (to avoid recursive requires is the
>> usual reason).
> Haha, bad choice of "uncontroversial" example, then! ;)

No, not at all.  It's just that for many of the conventions we'd like to
follow, there are cases where it's impractical to follow them.
So, while some may really be enforced and should generate a clear "you
need to fix this", the others should signal a gentler reminder.
I fully agree (and I think Glenn does too) that provide should be at the
end of the file, but I also agree that sometimes it's inconvenient to
do so.

Ideally, for every possible convention-warning (except for those which
should hopefully always be followed such as ";;; file --- description"),
there should be a way to write the code such that the warning is
silenced.  In the worst case, with the use of a comment or some other
explicit annotation.


        Stefan