Welcome! Please see the About page for a little more info on how this works.

0 votes
in ClojureScript by
The Closure [Spec For Union Types|https://developers.google.com/closure/compiler/docs/js-for-compiler#unions] states that parentheses are necessary for union type expressions. Trying this ...



(defn foo
  "@param {(IBar|IMap)} x"
  [x]
  ...)

 
Raises a Closure Error :


...ERROR - Bad type annotation. expected closing }
* @param {user.(IBar|user.IMap)}


This is because comp/resolve-types treats the parentheses as a part of the type tokens and incorrect var resolution occurs as a result. In addition, the compiler emits multiple resolved types separated by "|" characters but does not enclose them in parentheses to create a valid union type.

15 Answers

0 votes
by

Comment made by: pkillean

This patch includes:

  • comp/resolve-types now removes parentheses when present and emits them when >1 type is detected. This makes parenthesis use optional and existing code remains unbroken (with the added benefit that it may work now)

  • changes to comp/resolve-type
    1. checks for js globals like document or window which are recognized by closure
    1. allows dot.delimited.forms to pass through so we can use types defined in externs and avoid unnecessary resolution
    1. uses ana/resolve-existing-var with a "unresolved jsdoc type" warning
    1. checks if a resolved var is a protocol and warns otherwise. This is more informative than Closure's standard unrecognized type error

  • a test for comp/resolve-types

0 votes
by

Comment made by: dnolen

Thanks will try to look more closely at this tomorrow.

0 votes
by

Comment made by: dnolen

The patch is getting there, please remove the js-doc-type meta stuff. Just extend the signature of {{resolve-existing-var}} to take an additional parameter - the {{confirm-var-exists}} handler.

0 votes
by

Comment made by: pkillean

CLJS-1627-1.patch:
{{resolve-existing-var}} now has an additional arity that accepts a missing-var handler passed to {{confirm-existing-var}}

0 votes
by

Comment made by: pkillean

This has revealed a problem where {{deftype}} + {{defrecord}} using Object protocols emit resolved names when really they shouldn't. For example : "@implements {cljs.core.async.impl.timers.Object}" --> Bad Type Annotation

Since {{Object}} is a special case simply excluding it from the comments should fix it. Another patch incoming

0 votes
by

Comment made by: pkillean

CLJS-1627-2.patch:
The emit* methods for deftype and defrecord now filter out Object protocols.

This produced an interesting result! With no more bad type annotations, static analysis can now proceed... and it has alot to say. Theres all kinds of info now about arity discrepencies (particularly cljs.core.IndexedSeq), type mismatches, and more. It even includes a type coverage percentage. Lots to parse here but very cool.

0 votes
by

Comment made by: pkillean

CLJS-1627-3.patch:
fix require extern
add type application support for Array & Object
GC likes uppercase for Object & Array, lowercase for string, number.
support for explicit nullable types, variable typed arg
* function type context modifiers {{this}} + {{new}}

-Missing is the GC 'record type'- . It also may be useful to fill out the node externs for common types

0 votes
by

Comment made by: pkillean

CLJS-1627-4.patch:
fix a few problems in last patch
add record type support. Everything (link: https://developers.google.com/closure/compiler/docs/js-for-compiler#types text: here) should be covered

0 votes
by

Comment made by: pkillean

update patch

0 votes
by

Comment made by: mfikes

CLJS-1627-5.patch no longer applies

0 votes
by

Comment made by: pkillean

patch 6:
routes js comments through comp/emit-comment which has been altered to handle inline comments.
Supported tags: param, return, type, implements, typedef, enum, extends, throws, lends, const, this
add macro core/goog-typedef. This lets you name a custom + type refer to it in annotations.

More work is needed to support multi-arity fns, but I think this pretty much unlocks basic static type checking :-)

0 votes
by

Comment made by: mfikes

Hey Patrick, CLJS-1627-6.patch does not apply to master.

0 votes
by

Comment made by: pkillean

shoulda had coffee first

0 votes
by

Comment made by: mfikes

CLJS-1627-7.patch doe not apply to current master

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJS-1627 (reported by pkillean)
...