Skip to content

defrecord/deftype namespaces not properly required #64

Closed
bostonaholic opened this Issue Jan 28, 2014 · 7 comments

3 participants

@bostonaholic

The namspace of a defrecord/deftype must also be required.

See the following example:

src/my/domain/file_header.clj

(ns my.domain.file-header)

(defprotocol Whatever
  ...)

(defrecord FileHeader [args]
  Whatever
  ...)

my/domain/core.clj

(ns my.domain.core
  (:require [my.domain.file-header :refer :all])
  (:import (my.domain.file_header FileHeader))

But slamhound is forgetting the :require piece. The same goes for deftype.

@guns
Collaborator
guns commented Jan 28, 2014

Hello, thank you for reporting!

Yes, this is an oversight. This escaped my notice because I either always AOT compile my artifacts or the necessary namespace just happens to be required to satisfy other unqualified references.

I'll take a look at this today.

@guns guns added a commit that closed this issue Jan 29, 2014
@guns guns :require matching ns when importing classes created by deftype
Classes created by deftype and defrecord are created on ns evaluation,
therefore that ns must be required before referencing these classes.

deftype and defrecord classes implement IType and IRecord in Clojure
1.3.0+, so we can check the hierarchy to identify them.

There is unfortunately no straightforward mapping of class package names
to Clojure namespaces as deftype uses clojure.core/namespace-munge to
simply change all \- to \_.

Therefore a two step search for the matching ns is made:

    1. Try the simple case where all underscores are converted to dashes
    2. Search for the first namespace that matches the package name with
       dashes and/or underscores.

Closes #64
ebdb831
@guns guns closed this in ebdb831 Jan 29, 2014
@guns
Collaborator
guns commented Jan 29, 2014

Hello,

I believe this fixes the issue. Please re-open if you find any errors.

Thank you!

@bostonaholic

When is the next planned release?

@guns
Collaborator
guns commented Jan 29, 2014

I'll make one today if @technomancy agrees.

@technomancy
Owner

Sounds good to me; thanks @guns.

@guns
Collaborator
guns commented Jan 29, 2014

Okay 1.5.1 is live on clojars! Happy hacking.

@bostonaholic

Awesome! Looks like it's working. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.