Classes generated by deftype and defrecord don't play nice with .getPackage

Question

Classes generated loaded by DynamicClassLoader return nil for .getPackage. Tools like CIDER and vim-fireplace are relying on this information to implement things like completion hints.

(.getPackage String) ;; => #<Package package java.lang, Java Platform API Specification, version 1.7> (deftype T []) (.getPackage T) ;; => nil

Proposed: During DynamicClassLoader.defineClass(), invoke definePackage() on the class being defined (similar to what URLClassLoader does).

Patch: clj-1550-v4.patch

Screened by: Alex Miller

Answer 1

Comment made by: alexmiller

According to http://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#getPackage() this method returns the package information found by the class loader or null if there is none. Its not clear to me that the current behavior is wrong per the spec. I would need to experiment more to see if this is unusual or not.

Answer 2

Comment made by: bozhidar

A bit of (link: https://github.com/clojure-emacs/cider-nrepl/pull/127 text: background) for the issue. I'm no expert on the topic, but being able to procure all the class information except its package definitely looks strange to me.

Answer 3

Comment made by: hiredman

if you AOT compile(generate a class file on disk for a deftype), getPackage works fine, which suggests to me it is a jvm issue

Answer 4

Comment made by: hiredman

actually, it must just be that dynamicclassloader doesn't define a package for classes it loads

Answer 5

Comment made by: alexmiller

Yep, I believe that's correct.

Answer 6

Comment made by: stu

There is no problem statement here. What is package information needed for?

Answer 7

Comment made by: bozhidar

I've linked the problem above. Basically tools like CIDER and vim-fireplace are relying on this information to implement things like completion hints.
This might not be problem when running your apps, but it's definitely a problem when inspecting their state...

Answer 8

Comment made by: michaelblume

s/Packate/Package

Answer 9

Comment made by: alexmiller

Refreshed patch to apply to current master, attribution retained, no semantic changes. Marked prescreened.

Answer 10

Comment made by: gshayban

We should consider interactions with the JDK9 module system here

Answer 11

Comment made by: alexmiller

off the top of my head, I don't see how this would do anything negative wrt the JDK9 module system

Answer 12

Comment made by: alexmiller

v4 patch rebases to master and retains attribution, no semantic changes.

Answer 13

Comment made by: bozhidar

Alex, is this going to be included in Clojure 1.10?

Answer 14

Comment made by: alexmiller

That's the current path...

Answer 15

_Comment made by: gshayban_

This is a bad patch that slipped through. The motivation linked above shows an nREPL excerpt that attempts to symbolize the package, basically:

(symbol (.getName (.getPackage TheClass))). The original code can derive the package name in the same way the JVM grabs it, from the binary name. (Take the prefix up until the last period.)

The fix also introduces a deprecated call, and more work during classloading (which is critical path).

As an aside, JDK9+ automatically defines a package upon class definition, derived from the binary name:


## JDK 8
➜  clojure git:(f5cfd24d) ✗ docker run --rm -v $HOME/.m2:/m2 openjdk:8-slim java -jar /m2/repository/org/clojure/clojure/1.8.0/clojure-1.8.0.jar -e '(deftype T [])' -e '(or (.getPackage T) :nada)'
user.T
:nada

## JDK 9
➜  clojure git:(f5cfd24d) ✗ docker run --rm -v $HOME/.m2:/m2 openjdk:9-slim java -jar /m2/repository/org/clojure/clojure/1.8.0/clojure-1.8.0.jar -e '(deftype T [])' -e '(or (.getPackage T) :nada)'
user.T
#object[java.lang.Package 0x10993713 "package user"]

## JDK 11
➜  clojure git:(f5cfd24d) ✗ docker run --rm -v $HOME/.m2:/m2 openjdk:11-slim java -jar /m2/repository/org/clojure/clojure/1.8.0/clojure-1.8.0.jar -e '(deftype T [])' -e '(or (.getPackage T) :nada)'
user.T
#object[java.lang.Package 0x6f3c660a "package user"]
➜  clojure git:(f5cfd24d) ✗



I think this should get reverted, and let user space calculate the package from the binary name.