ClojureScript

Convert analyzer to conform to tools.analyzer's spec

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.7.145
  • Fix Version/s: 1.9.671
  • Component/s: None
  • Labels:
    None

Description

tools.analyzer has a handy common AST format for map-based analysis results. We should use this format in the ClojureScript analyzer so tooling that already accepts this format can work seamlessly with ClojureScript.

Work in progress: https://github.com/frenchy64/clojurescript/pull/7

Order of work:

  1. Patch 1: CLJS-2260
    • :const
      • rename :constant op to :const
      • add :val entry
  2. Patch 2: CLJS-2788
    • :the-var
      • rename :var-special op to :the-var
    • :deftype
      • rename :deftype* op to :deftype
    • :defrecord
      • rename :defrecord* op to :defrecord
    • :with-meta
      • rename :meta op to :with-meta
  3. Patch 3: CLJS-2789
    • :def
      • add :ns entry
    • :throw
      • rename :throw entry to :exception
    • :try
      • rename :try entry to :body
    • :letfn
      • rename :expr entry to :body
    • :let/:loop
      • rename :expr entry to :body
    • :invoke
      • rename :f to :fn
  4. Patch 4: CLJS-2797
    • :fn-method
      • rename :method op to :fn-method
      • rename :expr entry to :body
      • rename :max-fixed-arity to :fixed-arity
      • rename :variadic to :variadic?
    • :host-field/:host-call
      • split :dot op into :host-field/:host-call
    • :js-object/:js-array
      • split :js-value op into :js-object/:js-array
    • :case
      • rename :case* op to :case
      • rename :v to :test
      • add :case-node op for grouping :case-test and :case-then ops
  5. Patch 5: CLJS-2800
    • :new
      • rename :ctor to :class
    • :children
      • move to tools.analyzer :children format
        • :children are vectors of keyword keys
        • ensure all sequence children are vectors
      • replace :children calls with a compatible function from AST -> children
  6. Patch 6: CLJS-2801
    • :quote
      • add :quote op
        • Note: This moves a lot more logic into handling :const in the emitter. We rename emit-constant -> emit-constant* and turn emit-constant into a function that emits metadata for each form before delegating to emit-constant*.
        • Note: Some emit* defmethods are factored out into higher-order function helpers (:{map,list,vector,set} -> emit-{map,list,vector,set}) and are now called from both emit and emit-constant.
        • Note: analyze-const now registers constants, but throws away the analysis results.
    • :list
      • remove :list op
        • Notes: subsumed by :quote'd :const. analyze-list still used to register constants.
    • Note: Introduced regression fixed by CLJS-2807
  7. Patch 7: CLJS-2803
    • :record-value
      • remove :record-value op, replace with :const
      • add unit tests for record literals
  8. Patch 8: CLJS-2257
    • :var/:binding/:local/:js-var
      • desugar dotted symbols
      • split :var op into :var/:binding/:local/:js-var
      • emit :local op if :js-shadowed-by-local
      • change :local to be #{:fn :letfn :let :arg ...}
    • argument validation in 'var parsing
    • :body? entries for synthetic `do` blocks
    • Unit tests
      • add them all at the end
    • AST format documentation
      • modify from tools.analyzer's

Extra stuff:

  • :type field in :const ops
  • :try/:catch
    • convert to tools.analyzer.js style (:try has :catches, a vector of :catch nodes)
  • :form of :binding should be :name
  • rename :t to :name in :deftype/:defrecord
  • namespace-qualify CLJS-specific ops

Activity

Hide
Ambrose Bonnaire-Sergeant added a comment -

In the body of this issue I have proposed a sequence of potential patches such that each patch will be easily screenable.

Can I have some feedback on this? Should some of these steps be combined/split?

Show
Ambrose Bonnaire-Sergeant added a comment - In the body of this issue I have proposed a sequence of potential patches such that each patch will be easily screenable. Can I have some feedback on this? Should some of these steps be combined/split?
Hide
Benedek Fazekas added a comment -

Recording my findings while looking at https://github.com/clojure-emacs/refactor-nrepl/issues/195 in case somebody finds it useful.

Based on the above mentioned issue I think CLJS-2051 is related to this ticket. I also found that

  • :raw-forms containing the stages of macro expansion including the original form is not part of CLJS ASTs
  • :op = :binding nodes in CLJS ASTs seem to be missing :children entry so walking the AST using the :children references cut short at :binding nodes
Show
Benedek Fazekas added a comment - Recording my findings while looking at https://github.com/clojure-emacs/refactor-nrepl/issues/195 in case somebody finds it useful. Based on the above mentioned issue I think CLJS-2051 is related to this ticket. I also found that
  • :raw-forms containing the stages of macro expansion including the original form is not part of CLJS ASTs
  • :op = :binding nodes in CLJS ASTs seem to be missing :children entry so walking the AST using the :children references cut short at :binding nodes

People

Vote (11)
Watch (5)

Dates

  • Created:
    Updated: