Resolve :warning-handlers compiler option when symbols

Description

`:warning-handlers` currently takes a function objects, which are not possible to send in when invoking through cljs.main.

To assist build tooling, augment these options to support taking in a symbol that goes through `requiring-resolve` in 1.10. That way, build tooling can refer to functions on the classpath.

CLJS can steal https://github.com/clojure/clojure/blob/master/src/clj/clojure/core/server.clj#L263-L270

I believe :watch-fn and :watch-error-fn do the right thing already, via https://github.com/clojure/clojurescript/blob/47386d7c03e6fc36dc4f0145bd62377802ac1c02/src/main/clojure/cljs/closure.clj#L80-L109

Desired Behaviour

The :warning-handlers compiler option is now able to specify functions via their fully qualified namespaced symbols. For example a --compile-opts with the following cljs.edn:

will find and resolve warning handler functions warning-handler1 and warning-handler2 so long as they exist and are on the classpath.

Analysis

  • As mentioned above, we do have prior work which resolves compiler options: :watch-fn, :watch-error-fn and also reprocess.

  • :warning-handlers is bound to cljs-warning-handlers in several places, but only used from one function: cljs.analyzer/warning.

  • Prior work makes use of private function cljs.closure/sym->var to resolve.

Approach

Move sym->var function from cljs.closure to cljs.util, make it public, and make load-mutex a parameter.
Make use of sym->var from cljs.analyzer/warning to resolve each warning-handler.

Tests

Add tests to cljs.analyzer-api-tests to verify success and symbol mis-configuration paths.

Implementation Notes

  1. Self-hosting ClojureScript is not considered

  2. sym->var throws :compilation phase exceptions, I think this is appropriate for warning-handlers given the choices: https://clojure.org/reference/repl_and_main#_error_printing

  3. sym->var includes the associated compiler option keyword within ex-info's map. But... in cljs.analyzer/warning, we are working with the binding cljs-warning-handlers rather than the compiler option keyword. I assume making reference to :warning-handlers in an error from cljs.analyzer/warning which is really dealing with cljs-warning-handlers is acceptable.

Observations

  1. sym->var and its usage are a bit odd in that they throw when sym ns is not found and throw when sym is not fully qualified but the error of sym not being resolved is quietly ignored. This seems a bit user cruel to me. For this change, I've introduced and used sym->var-checked to throw when sym is not resolved. A separate ticket might consider using sym->var-checked in place of existing calls to sym->var.

  2. Because warning handlers are only resolved in cljs.analyzer/warning the user will not learn of mis-configuration until a warning occurs. This could be considered a pro or con depending on your perspective. Perhaps repeating resolution earlier from a likely path, if only to fail faster, would be helpful. Maybe from cljs.cli/load-edn-opts?

Environment

None

Activity

Show:
Mike Fikes
April 11, 2020, 12:19 AM

CLJS-3074-2.patch conflicts with master

Mike Fikes
May 12, 2019, 2:50 AM

CLJS-3074-2.patch passes CI

Mike Fikes
May 12, 2019, 2:50 AM

CLJS-3074-2.patch added to Patch Tender

Lee Read
May 11, 2019, 11:45 PM

CLJS-3074-2.patch replaces CLJS-3074.patch and addresses Windows CI failure. Test now deals with newline differences.

Lee Read
May 11, 2019, 11:01 PM

Oh ho! Thanks Mike, I shall fix shortly.

Assignee

Lee Read

Reporter

Ghadi Shayban

Labels

None

Approval

None

Patch

Code and Test

Priority

Minor