Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception caused by namespaced keyword #106

Closed
grammati opened this issue Jul 31, 2015 · 7 comments
Closed

Exception caused by namespaced keyword #106

grammati opened this issue Jul 31, 2015 · 7 comments

Comments

@grammati
Copy link

Having a namespaced keyword anywhere in a project causes rename-symbol (called via clj-refactor.el) to fail with an exception. Eg:

RuntimeException: Invalid token: ::bar/thing

Minimal project to reproduce: https://github.com/grammati/refactor-nrepl-ns-kw-bug

The root cause seems to be this behavior of the clojure reader:

(read-string "::foo/bar")  => exception
@grammati
Copy link
Author

@expez
Copy link
Member

expez commented Aug 1, 2015

@Bronsa Is this something you can work around in tools.analyzer? Perhaps we are using it wrong?

@Bronsa
Copy link

Bronsa commented Aug 1, 2015

@expez both tools.analyzer and tools.reader fully support namespaced keywords assuming the correct ns context is set (meaning ns is bound to the current namespace, aliases and required libs are declared), which would require evaluation of the ns form.
To work around this you could bind tools.reader's *alias-map* to identity, which would make tools.reader read ::foo/bar as :foo/bar.

@expez
Copy link
Member

expez commented Aug 1, 2015

assuming the correct ns context set [...]

@Bronsa Isn't that the job of analyze-ns?

Here's the code we use to build the AST:

(defn- build-ast
  [ns aliases]
  (when (ns-on-cp? ns)
    (binding [ana/macroexpand-1 noop-macroexpand-1
              *file* (-> ns ajutils/ns-resource ajutils/source-path)
              reader/*data-readers* *data-readers*]
      (assoc-in (aj/analyze-ns ns) [0 :alias-info] aliases))))

I still need to add *alias-map* identity to the binding vector?

@Bronsa
Copy link

Bronsa commented Aug 1, 2015

@expez yes, if you use t.a.jvm's analyze-ns it should work out of the box, I'll try to read your code tomorrow to see why it's failing

@expez
Copy link
Member

expez commented Aug 1, 2015

@Bronsa after looking more closely at this, it turns out the bug is in the part of our code which uses the clj reader while looking for macros.

Sorry about the noise.

@expez expez closed this as completed in b66e387 Aug 1, 2015
@expez
Copy link
Member

expez commented Aug 1, 2015

@grammati thanks for the report! I've pushed a new snapshot to clojars, which should work on your project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants