Report errors on missing param list or return type of methods in gen-class and gen-interface

Question

The following are invalid and should produce errors when invoked on gen-class or gen-interface:

(gen-interface :name clj1419.IFail :methods [[myMethod java.lang.String]]) ;; no params, throws error (gen-interface :name clj1419.IFail :methods [[myMethod []]]) ;; no return type (gen-interface :name clj1419.IFail :methods [[myMethod]]) ;; no params or return type

The first example throws an error. The second and third do not but will generate an invalid class, verify with:

(.getMethods clj1419.IFail) ClassNotFoundException java.lang. java.net.URLClassLoader$1.run (URLClassLoader.java:366)

Add checks to prevent these errors.

Answer 1

Comment made by: nathan7

I've implemented both fixes, and attached them as patches.

Answer 2

Comment made by: nathan7

I'd argue that the behaviour of asm-type is at fault here (it can output an invalid type name when passed a nil argument), so I prefer that fix over the purely symptomatic generate-interface fix.

Answer 3

Comment made by: jafingerhut

Nathan, were you planning on submitting a signed Clojure Contributor's Agreement, or already have? Details here if you have not: http://clojure.org/contributing

Patches from non-contributors cannot be committed to Clojure.

Note: I cannot promise you that one of your patches will be accepted into Clojure if you sign a CA -- only that it will not if you do not sign one.

Answer 4

Comment made by: alexmiller

Please add an example of how this happens and the current error.

Answer 5

_Comment made by: nathan7_

Andy — Yep, I've read up on that. My CA will be underway to Rich soon. (filled in, signed, in an envelope, just need to await the arrival of those bloody international stamps…)

Alex Miller — Tahdah!

A demonstration of the issue, both attached and as a gist: https://gist.github.com/nathan7/3a7e3a09e458f1354cbb

Answer 6

Comment made by: nathan7

and here's log of the compiler crash that results (also added to the gist now)

Answer 7

Comment made by: nathan7

Whoops, both of my patches were rather broken due to a misunderstanding on my side.
I forgot entirely that asm-type takes a symbol, not a string.
Modifying asm-type was definitely a bad idea, that check just looks whether it should defer to prim->class.
Adding nil to prim->class would work (and I've attached my patch for that too), but it's starting to look rather inelegant compared to just patching gen-interface.
(on a side note: I'm having a lot of fun exploring the Clojure codebase! thanks for that, humans!)

Answer 8

Comment made by: alexmiller

My reading of the docstring of gen-interface is that method declarations must specify a parameter list and a valid return type. I would expect all of these to be invalid:

(gen-interface :name clj1419.IFail :methods [[fail nil]]) (gen-interface :name clj1419.IFail :methods [[fail [] nil]]) (gen-interface :name clj1419.IFail :methods [[fail []]])

"nil" is not a valid type - you can use "void" for this purpose and this works fine:

(gen-interface :name clj1419.IFail :methods [[fail [] void]])

If this ticket is (as the title states) a request to allow omitting the return type or using "nil" as a return type, then I think the answer is no. If the ticket is a request to improve the error reporting of the failure cases above, then I think we can consider that but it will be very low priority.

Answer 9

Comment made by: nathan7

The code seems to suggest otherwise though, seeing the explicit extra branch for pclasses being nil.
As much as I like PL trivia, I haven't run into void in Clojure anywhere else yet, and I'm surprised to see it here.
Maintaining the principle of least surprise seems preferable to pedantry about whether nil is a type: (= "nil" (str (type (.methodReturningVoid obj)))

Answer 10

Comment made by: alexmiller

The two places to look for words to rely on are docstrings and the http://clojure.org/documentation pages. Implementation details are just that.

"nil" is not a type. "void" is a documented type identifier indicating the absence of a return value - http://clojure.org/java_interop#Java Interop-Aliases

Answer 11

Comment made by: nathan7

Okay. Better error-checking in asm-type then?

Answer 12

Comment made by: alexmiller

I have updated the title and description based on my understanding of what this ticket should be, which is enhanced error-checking on the method specs for gen-class and gen-interface. I'm not sure if that's in asm-type or somewhere earlier.

Answer 13

Comment made by: alexmiller

As of 1.10, these all throw errors now:

`
user=> (gen-interface :name clj1419.IFail :methods [[myMethod java.lang.String]])
Syntax error macroexpanding gen-interface at (REPL:1:1).
Don't know how to create ISeq from: clojure.lang.Symbol

user=> (gen-interface :name clj1419.IFail :methods [[myMethod []]])
Unexpected error (ClassFormatError) macroexpanding gen-interface at (REPL:1:1).
Class name contains illegal character in descriptor in class file clj1419/IFail

user=> (gen-interface :name clj1419.IFail :methods [[myMethod]])
Unexpected error (ClassFormatError) macroexpanding gen-interface at (REPL:1:1).
Class name contains illegal character in descriptor in class file clj1419/IFail
`

Answer 14

Reference: https://clojure.atlassian.net/browse/CLJ-1419 (reported by nathan7)