tools.analyzer

Restore some data to AST that Eastwood uses for line/col numbers for some linters

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

Warning: Treat this suggested change as the ravings of a hacker who is poking at things without understanding how they work, and isn't even looking to see if there are bad consequences that may result.

This commit to tools.analyzer removed some data from the resulting ASTs that Eastwood was using for printing line/col numbers for some linters: https://github.com/clojure/tools.analyzer/commit/96fff1041bdebf75f3b539c439a41f3c5f496c7c

I did a quick experiment with the latest t.a(.jvm) and Eastwood as of Feb 10 2014, and 'lein test' passes with Eastwood again with the attached patch.

If there are good reasons to avoid applying this patch, I can certainly find another way to get the desired line/col numbers in Eastwood's error messages. However, if you consider keeping this data in the AST harmless, it would be nice to have.

Activity

Hide
Nicola Mometto added a comment -

This change was made for tools.emitter.jvm to generate correctly munged class-names for functions, I'm ok with accepting this patch but I'll need to fix the behaviour on tools.emitter.jvm first then.

Show
Nicola Mometto added a comment - This change was made for tools.emitter.jvm to generate correctly munged class-names for functions, I'm ok with accepting this patch but I'll need to fix the behaviour on tools.emitter.jvm first then.
Hide
Nicola Mometto added a comment -

I actually went completely on another direction with this after some thought: https://github.com/clojure/tools.analyzer/commit/ae2b7ee34a326905b332d6a77cd7229f69626fb9

Now :name is a string and represents the munged name for a fn.
This decision was made to make :name more consistent.

It should be trivial to write a pass for eastwood to restore the previous functionallity though, and if you point me out to where this is needed I'll be glad to do that myself.

Show
Nicola Mometto added a comment - I actually went completely on another direction with this after some thought: https://github.com/clojure/tools.analyzer/commit/ae2b7ee34a326905b332d6a77cd7229f69626fb9 Now :name is a string and represents the munged name for a fn. This decision was made to make :name more consistent. It should be trivial to write a pass for eastwood to restore the previous functionallity though, and if you point me out to where this is needed I'll be glad to do that myself.
Hide
Andy Fingerhut added a comment -

I created a Github issue for Eastwood at the link below with steps to reproduce, and where in the Eastwood source code it is attempting to get line/col numbers but getting nils, when it formerly got line/col numbers with older t.a(.jvm) versions.

https://github.com/jonase/eastwood/issues/58

Show
Andy Fingerhut added a comment - I created a Github issue for Eastwood at the link below with steps to reproduce, and where in the Eastwood source code it is attempting to get line/col numbers but getting nils, when it formerly got line/col numbers with older t.a(.jvm) versions. https://github.com/jonase/eastwood/issues/58

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: