Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make lambda tests work with ClojureScript too #19

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

iarenaza
Copy link

@iarenaza iarenaza commented Dec 7, 2021

This pull request fixes #14.

It also fixes some reflection warnings that are produced when using Java 17, and a few other warnings pointed out by clj-kondo.

Finally it also runs the unit tests with Clojure 1.10 (in addition to the previous versions)

When running the ClojureScript tests, doo complains with the following
warning:

  [doo] WARNING: In project.clj :compiler-opts, value for :main is
        quoted. This is deprecated and Doo will stop supporting it. To
        make this warning go away, please change {:main
        'cljstache.runner} to {:main cljstache.runner}.

Just follow the instructions given in the warning and unquote the
value for :main option.
spec-path is only used by the load-spec-tests function, which is only
defined and used when running Clojure code. So make it Clojure only
too.
There is no need to use the .toMatchResult method to get the matching
start and end. Those methods are alreay available in the Matcher
class. They return the exact same values and they don't need to use an
inner class. This access is not allowed when using the babashka
Clojure(Script) interpreter.

Babashka is statically compiled using GraalVM, and GraalVM is fairly
restrictive with access to classes. Unless you specifically list them
in the build configuration, the access is denied. Potentially even it
can completely strip the classes out, if it thinks they are not
used (it performs some static analysis to determine it, but it can
fail depending on how the classes are used).
There is no need to use the .toMatchResult method to get the matching
start and end. Those methods are alreay available in the Matcher
class. They return the exact same values and they don't need to use an
inner class. This access is not allowed when using the babashka
Clojure(Script) interpreter.

Babashka is statically compiled using GraalVM, and GraalVM is fairly
restrictive with access to classes. Unless you specifically list them
in the build configuration, the access is denied. Potentially even it
can completely strip the classes out, if it thinks they are not
used (it performs some static analysis to determine it, but it can
fail depending on how the classes are used).
Develop with Clojure 1.11 and unit test with Clojure 1.7 to 11.1,
using the latest available version for each major version.
It is only used when the library is used as ClojureScript code, but it
was defined for both CLJ and CLJS. When linting the file with
clj-kondo and eastwood as CLJ, they complained that it was unsed.
The previous implementation was flawed, and always used the library
defined version, even if the Clojure core version was availabe.

The check for the `seqable?` symbol didn't qualify it with the Clojure
core namespace, which resulted in the current (library) namespace
symbol being found (the very own `def` that was checking if it was
resolvable). Which in retrospect makes sense, as we were explicitly
excluding `seqable?` when refering Clojure core at the top.

We need to use the fully qualified name of the symbol to check for
Clojure core implementation. And if it is resolvable ("it exists"), we
need to `require` it before trying to use it. Otherwise we end up
using the current namespace symbol again.
Nashon being pretty old (and unmaintaned, even removed from recent
JDKs), it doesn't work with ClojureScript later than 1.10.597. So use
the latest working version.
According t https://clojure.org/reference/java_interop#typehints:

> For function return values, the type hint can be placed before the
> arguments vector:

This fixes clj-kondo warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lambdas for cljs
1 participant