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

prevent throwing null pointer exception #46

Closed
dpsutton opened this issue Mar 31, 2017 · 5 comments
Closed

prevent throwing null pointer exception #46

dpsutton opened this issue Mar 31, 2017 · 5 comments
Labels

Comments

@dpsutton
Copy link
Contributor

Got an issue popping up in CIDER with compliment throwing a null pointer exception by just typing ::/ and the autocomplete functionality throwing:

clojure-emacs/cider#1972

I've created the following test that "tests" it to demonstrate the error programatically and I'm starting to look into the codebase but i'm not too familiar with it.

(fact "keyword reading does not throw an error"
        (do (src/candidates "::/" *ns* nil)
            => (strip-tags (just #{""}))))
@dpsutton
Copy link
Contributor Author

(defn aliased-candidates
  "Returns a list of alias-qualified double-colon keywords (like ::str/foo),
  where alias has to be registered in the given namespace."
  [prefix ns]
  (let [[_ alias prefix] (re-matches #"::([^/]+)/(.*)" prefix)
        alias-ns-name (str (resolve-namespace (symbol alias) ns))]
    (for [[kw _] (keywords-table)
          :when (= (namespace kw) alias-ns-name)
          :when (.startsWith (name kw) prefix)]
      (tagged-candidate (str "::" alias "/" (name kw))))))

Amazingly, it's this function. It has a double colon and a slash but this regex can still fail.

I've tentatively changed it to the following:

(defn aliased-candidates
  "Returns a list of alias-qualified double-colon keywords (like ::str/foo),
  where alias has to be registered in the given namespace."
  [prefix ns]
  (let [[_ alias prefix] (re-matches #"::([^/]+)/(.*)" prefix)]
    (if (and alias prefix)
      (let [alias-ns-name (str (resolve-namespace (symbol alias) ns))]
        (for [[kw _] (keywords-table)
              :when (= (namespace kw) alias-ns-name)
              :when (.startsWith (name kw) prefix)]
          (tagged-candidate (str "::" alias "/" (name kw)))))
      (tagged-candidate ""))))

except i'm not sure how at this point to return no completion candidates. But it's guarding against this regex failing or making the guards going into this (the cond over double-colon? and has-slash? a bit smarter, but that makes those predicates not really match what they say).

@alexander-yakushev
Copy link
Owner

Thanks for reporting! Your fix looks correct. You can just return nil or an empty list for no candidates. So it might look like:

(when-let [[_ alias prefix] (re-matches #"::([^/]+)/(.*)" prefix)]
  (let [alias-ns-name (str (resolve-namespace (symbol alias) ns))]
    (for [[kw _] (keywords-table)
          :when (= (namespace kw) alias-ns-name)
          :when (.startsWith (name kw) prefix)]
      (tagged-candidate (str "::" alias "/" (name kw))))))

Do you wish to create a pull request with this fix?

@dpsutton
Copy link
Contributor Author

I'm at work and won't be able to do it until evening. If it's no big deal for you go ahead, otherwise i'll do it tonight.

About the when-let form, that was my first try and it seemed like the when-let form didn't allow destructuring but maybe i was a little hasty and missed an extra set of [].

@alexander-yakushev
Copy link
Owner

alexander-yakushev commented Mar 31, 2017

No hurry, create it whenever you have time. It's just that since you've discovered the bug and fixed it, it makes more sense that the commit comes from you :).

when-let allows destructuring, but the when part checks only the top-level (undestructured) value. Just something to keep in mind.

@alexander-yakushev
Copy link
Owner

Fixed by #47.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants