Disadvantages with using metadata for tool-specific Clojure code annotation?

[Andy Fingerhut]

I am thinking of using metadata in Clojure source code as a way for Eastwood users [1] to disable/enable classes of lint warnings for pieces of Clojure code smaller than a whole top level form.

I want to find out if there are any issues with this approach that I haven't considered yet, either with Clojure as it is now, or with planned future features, e.g. conditional reader/feature expressions [2].

The primary method I've seen used with other lint tools are comments with specially-defined syntax in them that the lint tool recognizes, but are unlikely to be typed by someone who is not using that tool. [3]

I could use that approach.  However, we do have metadata available for annotating code, and I'd like to use it for this purpose if there are no significant disadvantages.

Example:

    (defn my-fn [a b c]
       (let [x (+ a b)]
          ^{:lint {:disable-linters [ :def-in-def ]}}
          (def bar (* x c))
          (- b c)))

The kind of metadata that Eastwood would pay attention to for this purpose could be restricted to values on a single key, like :lint in the example above.  The extent of the disabling effect would be limited to the form (def bar (* x c)), and only for the type of warning Eastwood calls :def-in-def.  Such a warning anywhere outside of that form would still be reported.  Warnings other than the :def-in-def kind would still be reported inside of that form.

The issues I have thought of so far:

Metadata cannot be applied to any literals except symbols.  That doesn't seem like a big hindrance to me -- one could apply the metadata to the next larger enclosing form/vector/map/set.  There may be nasty examples where the disabling extent would be increased more than one would want, but I haven't thought of one yet.

Eastwood would need to 'claim' at least one key value to affect its behavior, and if I'm not careful that could collide with another tool's use of such a key.  I could use :eastwood/lint to make that very unlikely, but would prefer something a bit shorter.

Any others?

Thanks,
Andy

P.S.: Some other questions you may be thinking of:

This doesn't affect Clojure itself, so why are you asking?  No, it doesn't affect how Clojure is implemented.  I'm not asking because I want to change how Clojure works for this.  I am asking because I want to choose a method that doesn't have already-known problems with it.

Why disable warnings selectively?  Because then you can disable warnings that have been manually verified to be not-really-a-bug, and the next run of the lint tool will produce no warnings.  You can interpret the lack of warnings to mean that the only warnings found were in the few suppressed places, and nowhere else.

Why disable warnings in a section of code smaller than a function?  Because if you introduce a bug that linting can catch outside of those small sections, but inside the same function, it will be reported the next time you lint.

Why not write a lint tool that never produces warnings for correct code?  That is computationally undecidable, if it is even well defined.  Disregarding that, it can be difficult to write linters that avoid false positives without also resulting in more false negatives (i.e. there was a bug, but lint was silent about it).

[1] https://github.com/jonase/eastwood/
[2] http://dev.clojure.org/display/design/Release.Next+Planning
[3] Here are some examples for a C lint tool called splint:
    http://www.splint.org/manual/html/appC.html

[Colin Fleming]

I'm also interested in this, since I'll need something similar for Cursive at some point. IntelliJ already allows inspections to be disabled globally or at various granularities using either annotations or specially formatted comments. This is pretty clearly desirable.

The minimum scope issue may turn out to be a problem, though. Just of the top of my head, IntelliJ (for Java) provides a series of inspections to help with I18N. So any string literal will be marked with a warning, together with associated quick-fix to automatically add it to a properties file and so forth. You can use an annotation to mark strings that for whatever reason shouldn't have this warning - in Java this annotation would go on the relevant variable, parameter or field. In Clojure we wouldn't be able to using metadata, and this inspection is not one you'd be likely to want with a larger scope. I'm not sure whether this is likely to be a problem, and of course there's nothing preventing the use of both metadata and specially formatted comments for situations where you can't use metadata - IntelliJ does this.

Cheers,
Colin

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.
To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

[Ambrose Bonnaire-Sergeant]

I've avoided the metadata approach on arbitrary forms (assuming you're using tools.analyzer).

For wrapping entire forms, I find something like this is much more reliable:

(do :eastwood.core/special-form

    :remove-linters

    {:opt1 v1 :op2 v2}

  body)

Eastwood needs to dispatch on the second :statements node of a :do form if the

first form is :eastwood.core/special-form to pick this annotation up, for example.

You can provide a macro (or write your own) to turn this into

(def foo

 (eastwood {:remove-linter [:def-in-def]}

   (def bar 2)))

No amount of crazy handling of metadata in the (arbitrary) form (def bar 2) can

mess with the outer do form.

I haven't done any deep thinking on the composability of this approach.

Thanks,

Ambrose

[Andy Fingerhut]

Ambrose: Nice suggestion!  It definitely has the advantage of working even where metadata does not.

Colin, I had not used IntelliJ before to see these features you are describing.  I found the link below with a bit of searching to show examples of its features for assisting in I18N.  Is that what you are referring to?

    http://www.jetbrains.com/idea/features/i18n_support.html

I saw a mention at this page for using a @NonNI annotation in Java source code to mark strings as not needing internationalization:

    http://www.jetbrains.com/idea/features/annotation_java.html#internationalization

Do you have links to any examples of IntelliJ using specially formatted comments?  I didn't see any in a few minutes of searching.

Thanks,
Andy

[Colin Fleming]

Sure, I'm in the middle of a travel odyssey right now trying to get to EuroClojure, when I get a moment I'll dig up some examples.

Cheers,
Colin

[Colin Fleming]

Hi Andy,

Here's an example of what I'm talking about. Sorry for the image dumps, but it's hard to show without them. Suppose I have a simple class:

Then I insert a method call to "greeting" inside "greet":

​IntelliJ performs a data flow analysis and determines that the dereference may result in an NPE. I can then choose to disable this inspection just for this statement, or for the method, class, or globally, as well as a choice of quick fixes:

​In this case, since this deref happens as part of a local variable declaration where an annotation is valid, I get the choice of whether to use a comment or not:

​If I elect not to use the comment, it will use an annotation:

​If the site of the inspection were not a valid site for an annotation (in a normal statement, for example), it would not give me the option and it would always use a comment.

Let me know if you have more questions or would like more examples, this is one of my favourite parts of IntelliJ - it's really amazing what having good static analysis live in the editor does for your code. Most of this is generic infrastructure, so I'll be able to do the same for Clojure code with Cursive very soon.

Cheers,
Colin

On 25 June 2014 15:58, Andy Fingerhut <andy.fi...@gmail.com> wrote:

[Colin Fleming]

Oh, and for the I18N stuff, there's a video here: http://www.jetbrains.com/idea/training/demos/i18n.html. This uses an ancient version of IntelliJ, but you get the idea of how the annotations work. It's a little slicker and everything looks nicer in recent versions, but I believe it works in a similar way.

Cheers,

Colin

[Andy Fingerhut]

Ambrose (or anyone who knows or figures it out): In what ways would this 'do' approach change the emitted Java bytecode of Clojure code that used it?

As long as all arguments to do except the body were compile-time constants, would the emitted bytecode differ only in ways that the JVM's JIT compiler should be able to optimize away?  If so, how do we know that?

I'll post answers if I learn them, but was hoping someone else might already know, or be willing to dig into it and find out.

Andy

[Andy Fingerhut]

I haven't checked how the 'do' approach would affect the emitted byte code, if at all, but I believe I have an alternate method that seems like it should have no affect on the byte code at all, and yet lets tools based on tools.analyzer see the annotations.

(defmacro lint-ann [opts form]

  form)

;; Admittedly not a warning that most people should want to disable

(lint-ann {:disable-linters [:misplaced-docstrings]}

  (defn foo [x]

    "Bad doc string placement"

    (* x x)))

tools.analyzer returns ASTs with a :raw-forms key since about a month ago, which contains all forms through the steps of macro expansion, up until the final one that is actually analyzed.  The first of these for a (lint-ann ...) form would contain (lint-ann ...), enabling an analysis tool to detect it an examine its contents, but the final form would have no trace of lint-ann or opts, thus having 0 affect on the compiled code (corrections welcome on that last assertion if I've missed something).

Andy

[Ambrose Bonnaire-Sergeant]

Cool idea. I'd be interested to know the relative performance of this approach; do you have

to walk the :raw-forms key on every node?

FWIW I set a dynamic var in core.typed while checking, and only emit extra annotations

in the macroexpansion if the var is set.

Thanks,

Ambrose

Thanks,

Ambrose

[Andy Fingerhut]

On Wed, Jul 9, 2014 at 9:01 AM, Ambrose Bonnaire-Sergeant <abonnair...@gmail.com> wrote:

Cool idea. I'd be interested to know the relative performance of this approach; do you have

to walk the :raw-forms key on every node?

I haven't implemented the idea yet, but yes, I believe it would be necessary to look at :raw-forms on every AST node.  However, IIRC, there is no :raw-forms key at all for AST nodes that are literals, for example, and even for code forms it is nil whenever no macroexpansion occurs on the form producing the AST.  If macroexpand-1 takes N calls to produce the final version, :raw-forms is a list of N-1 forms (all but the last one, which goes on the :form key in the AST node).  The common case is no :raw-forms key, or a value of nil.

Also, I believe the idea of checking for the annotations created in this way would only need to examine the first symbol of these :raw-forms, to see if it was of the expected kind, e.g. something like eastwood/lint-ann in my case.  If that first symbol matches, then you need to examine the options.  If it is anything else, ignore the rest of the form.
 

FWIW I set a dynamic var in core.typed while checking, and only emit extra annotations

in the macroexpansion if the var is set.

I am not familiar with everything you are doing there, but it appears that this extra annotation is only for certain selected Clojure macros like defprotocol, deftype, defrecord, etc?

If so, are you suggesting that such an approach might be extended to a situation where the goal is to let developers add annotations on arbitrary forms (including literals like integers, strings, etc.), and have no effect on the emitted byte code?  I'm interested in learning another way to achieve that, for my own curiosity's sake.

Thanks,
Andy