Fix NPE in resolve when called at runtime inside macro body

When `resolve` is called at runtime (e.g. inside a macro body during
code walking), `lookup` may find bindings not set up by the analyzer
(e.g. from `macroexpand-1`'s env merge). Skip the idx machinery
(iden->invoke-idx, outer-idens, aget) when idx is nil, returning
[k v] directly instead.

Author: borkdude

CHANGELOG.md

@@ -19,6 +19,7 @@ SCI is used in [babashka](https://github.com/babashka/babashka),
 - `deftype` now macroexpands to `deftype*`, matching JVM Clojure, enabling code walkers like riddley
 - `case` now macroexpands to JVM-compatible `case*` format, enabling tools like riddley and cloverage to work with SCI
 - Preserve `:tag` metadata in `copy-var`
+- Fix NPE in `resolve` when called at runtime (e.g. inside a macro body during code walking) by skipping analysis-only idx machinery
 - Fix `.method` on class objects (e.g. `(.getDeclaredField String "value")`) routing to static instead of instance method path
 - `macroexpand-1` of `(.method ClassName)` now wraps class targets in `identity`, matching Clojure behavior
 - `macroexpand-1` now accepts an optional env map as first argument, like `cljs.analyzer/macroexpand-1`

src/sci/impl/resolve.cljc

@@ -172,34 +172,40 @@
         (if only-var?
           [k nil]
           (let [idx (or (get (:iden->invoke-idx ctx) v)
-                        (let [oi (:outer-idens ctx)
-                              ob (oi v)]
-                          (update-parents ctx (:closure-bindings ctx) ob)))
-                #?@(:clj [tag (or  (:tag m)
-                                   (some-> k meta :tag))])
-                mutable? (when track-mutable?
-                           (when-let [m (some-> k meta)]
-                             #?(:clj (or (:volatile-mutable m)
-                                         (:unsynchronized-mutable m))
-                                :cljs (or (:mutable m)
-                                          (:volatile-mutable m)))))
-                v (if call? ;; resolve-symbol is already handled in the call case
-                    (mark-resolve-sym k idx)
-                    (let [v (cond-> (if mutable?
-                                      (let [ext-map (second (lookup ctx '__sci_this false))]
-                                        (->Node
-                                          (let [this (sci.impl.types/eval ext-map ctx bindings)
-                                                inner (sci.impl.types/getVal this)]
-                                            (get inner sym))
-                                          nil))
-                                      (->Node
-                                        (aget ^objects bindings idx)
-                                        nil))
-                              #?@(:clj [tag (with-meta
-                                              {:tag tag})])
-                              mutable? (vary-meta assoc :mutable true))]
-                      v))]
-            [k v])))
+                        (when-let [oi (:outer-idens ctx)]
+                          (let [ob (oi v)]
+                            (update-parents ctx (:closure-bindings ctx) ob))))]
+            (if (nil? idx)
+              ;; idx is only set during analysis. When lookup is
+              ;; called at runtime (e.g. resolve inside a macro body),
+              ;; bindings may contain entries not set up by the
+              ;; analyzer, so we skip the idx machinery.
+              [k v]
+              (let [#?@(:clj [tag (or  (:tag m)
+                                       (some-> k meta :tag))])
+                    mutable? (when track-mutable?
+                               (when-let [m (some-> k meta)]
+                                 #?(:clj (or (:volatile-mutable m)
+                                             (:unsynchronized-mutable m))
+                                    :cljs (or (:mutable m)
+                                              (:volatile-mutable m)))))
+                    v (if call? ;; resolve-symbol is already handled in the call case
+                        (mark-resolve-sym k idx)
+                        (let [v (cond-> (if mutable?
+                                          (let [ext-map (second (lookup ctx '__sci_this false))]
+                                            (->Node
+                                              (let [this (sci.impl.types/eval ext-map ctx bindings)
+                                                    inner (sci.impl.types/getVal this)]
+                                                (get inner sym))
+                                              nil))
+                                          (->Node
+                                            (aget ^objects bindings idx)
+                                            nil))
+                                  #?@(:clj [tag (with-meta
+                                                  {:tag tag})])
+                                  mutable? (vary-meta assoc :mutable true))]
+                          v))]
+                [k v])))))
       (when-let [kv (lookup* ctx sym call? only-var?)]
         (when (:check-permissions ctx)
           (check-permission! ctx sym kv))

test/sci/core_test.cljc

@@ -449,7 +449,15 @@
   (is (= [6] (eval* "[(-> 3 inc inc inc)]")))
   (is (= [{3 6}] (eval* "[{(->> 2 inc) (-> 3 inc inc inc)}]")))
   (is (eval* (str `(#(< 10 % 18) 15))))
-  (is (eval* (str `(#(and (int? %) (< 10 % 18)))) 15)))
+  (is (eval* (str `(#(and (int? %) (< 10 % 18)))) 15))
+  #?(:clj
+     (testing "macroexpand-1 with env does not NPE when macro calls resolve on a local"
+       (is (= '(f 1)
+              (tu/eval* "
+(defmacro resolving-macro [form]
+  (resolve (first form))
+  form)
+(macroexpand-1 '{f nil} '(resolving-macro (f 1)))" {}))))))
 
 (deftest permission-test
   (is (tu/eval* "(int? 1)" {:allow '[int?]}))
@@ -1834,4 +1842,4 @@
 (comment
   (eval* 1 '(inc *in*))
   (test-difference "foo" "[10 10]" 0 10)
-  (test-difference "rand" #(rand) 0 10))
+  (test-difference "rand" #(rand) 0 10))