ClojureScript

protocol implementations do not support qualified method names

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: 1.7.145
  • Fix Version/s: 1.7.228
  • Component/s: None
  • Labels:
  • Environment:
    Mac OS X Mavericks. Java: (build 1.8.0_45-b14)

Description

src/proj/one.cljs
=================

(ns proj.one)

(defprotocol Proc
  (meth [this]))

src/proj/two.cljs (correctly works)
=================

(ns proj.two
  (:require [proj.one] :as one))

(def ProcImpl
  (reify one/Proc
    (meth [_] nil))) ;; works but should not work as 'meth' is defined in ns: one.

src/proj/two.cljs (also correctly works)
=================

(ns proj.two
  (:require [proj.one] :as one))

(def ProcImpl
  (reify one/Proc
    (one/meth [_] nil))) ;; does not work but should work per the doc on 'defprotocol'.

Yields error:
"Caused by: clojure.lang.ExceptionInfo: set! target must be a field or a symbol naming a var at line..."

Activity

Hide
Jonathan Leonard added a comment -

Btw, the section noted "incorrectly works" should be changed to "also correctly works" (verified this with Clojure proper locally). [I can't see any way to edit this or I would make the change my self].

Show
Jonathan Leonard added a comment - Btw, the section noted "incorrectly works" should be changed to "also correctly works" (verified this with Clojure proper locally). [I can't see any way to edit this or I would make the change my self].
Hide
Jonathan Leonard added a comment - - edited

Hmm, I think I may have been mistaken when I said the second example should be labeled "also correctly works". Otherwise there wouldn't be a bug here.

The fully qualified method names do not work in ClojureScript but they do in Clojure proper. That is the bug.

Thanks!

Show
Jonathan Leonard added a comment - - edited Hmm, I think I may have been mistaken when I said the second example should be labeled "also correctly works". Otherwise there wouldn't be a bug here. The fully qualified method names do not work in ClojureScript but they do in Clojure proper. That is the bug. Thanks!
Hide
David Nolen added a comment - - edited

A simple fix would be to just call Clojure's name on the protocol method name to drop the namespace component. Some culprits worth considering are protocol-prefix in the cljs/core.cljc macros file.

Show
David Nolen added a comment - - edited A simple fix would be to just call Clojure's name on the protocol method name to drop the namespace component. Some culprits worth considering are protocol-prefix in the cljs/core.cljc macros file.
Hide
Jonathan Leonard added a comment - - edited

@sebastian - are you sure?

```
user=> (defprotocol Proc (meth [this]))
Proc

user=> (def ProcImpl (reify user/Proc (user/meth [_] 997)))
#'user/ProcImpl

user=> (user/meth ProcImpl)
997
```

Show
Jonathan Leonard added a comment - - edited @sebastian - are you sure? ``` user=> (defprotocol Proc (meth [this])) Proc user=> (def ProcImpl (reify user/Proc (user/meth [_] 997))) #'user/ProcImpl user=> (user/meth ProcImpl) 997 ```
Hide
Sebastian Bensusan added a comment -

Jonathan Leonard I took it because we were discussing it yesterday and it seemed like it was up for grabs. If you are considering a solution, please assign it to yourself (if that's possible?). Sorry to step on your toes

Show
Sebastian Bensusan added a comment - Jonathan Leonard I took it because we were discussing it yesterday and it seemed like it was up for grabs. If you are considering a solution, please assign it to yourself (if that's possible?). Sorry to step on your toes
Hide
Jonathan Leonard added a comment -

@sebastian: No, I was responding to your comment (which seems to have since been deleted) that protocol implementations do not accept qualified names. It was just a little output from the REPL showing the contrary.

Show
Jonathan Leonard added a comment - @sebastian: No, I was responding to your comment (which seems to have since been deleted) that protocol implementations do not accept qualified names. It was just a little output from the REPL showing the contrary.
Hide
Sebastian Bensusan added a comment -

Jonathan Leonard that's weird, I made no comments. No problem, I'll handle it then.

Show
Sebastian Bensusan added a comment - Jonathan Leonard that's weird, I made no comments. No problem, I'll handle it then.
Hide
Jonathan Leonard added a comment -

Oh, sorry. I saw it come across as an email but when I went to the site, it wasn't there.

Show
Jonathan Leonard added a comment - Oh, sorry. I saw it come across as an email but when I went to the site, it wasn't there.
Hide
Jonathan Leonard added a comment -

Ahh, I know what happened now. I saw the title of this bug in the content of the email where you changed the assignee and thought that it was your comment. [Parsing those generated emails (esp. on mobile) can be challenging sometimes].

Show
Jonathan Leonard added a comment - Ahh, I know what happened now. I saw the title of this bug in the content of the email where you changed the assignee and thought that it was your comment. [Parsing those generated emails (esp. on mobile) can be challenging sometimes].
Hide
Sebastian Bensusan added a comment -

The attached patch has two commits:

  1. The first commit adds a test and solves the issue by calling name on method names (on two different places, the validation step and the emitting macro). To ignore the namespace part implies that now methods can be provided with the wrong namespace:
    (reify a/Foo (b/foo [_] nil))
    .
  2. The second commit warns if the provided method names don't exist or don't belong to the associated protocols, thus catching the aforementioned incorrect method names.

The patch can be tested in this example repository

Show
Sebastian Bensusan added a comment - The attached patch has two commits:
  1. The first commit adds a test and solves the issue by calling name on method names (on two different places, the validation step and the emitting macro). To ignore the namespace part implies that now methods can be provided with the wrong namespace:
    (reify a/Foo (b/foo [_] nil))
    .
  2. The second commit warns if the provided method names don't exist or don't belong to the associated protocols, thus catching the aforementioned incorrect method names.
The patch can be tested in this example repository

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: