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

Description

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.

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

Environment

None

Activity

Show:
Alex Miller
June 29, 2018, 11:31 PM

That's the current path...

Ghadi Shayban
October 5, 2018, 3:51 AM

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

. 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:

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

Bozhidar Batsov
October 5, 2018, 5:19 AM

> 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.)

I don't completely understand the workaround you're proposing. Can you elaborate on this?

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

If that's working on newer JDKs with no changes needed in the user space and in Clojure, that's fine by me. By now I had lost faith that was going to be fixed in Clojure anyways.

Ghadi Shayban
October 5, 2018, 4:45 PM

https://github.com/clojure-emacs/orchard/blob/db6d8a7be853b52c9986c5af8d009639644bb390/src/orchard/java.clj#L157-L159

Took me a while to track it down, but instead of asking for the package name then grabbing the symbol, do this:

Alex Miller
October 6, 2018, 4:58 AM

Patch reverted

Assignee

Unassigned

Reporter

Bozhidar Batsov

Labels

Approval

Vetted

Patch

Code and Test

Affects versions

Priority

Minor
Configure