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

Don't fail analysis due to unknown tagged literals #32

Closed
holyjak opened this issue Dec 4, 2020 · 3 comments
Closed

Don't fail analysis due to unknown tagged literals #32

holyjak opened this issue Dec 4, 2020 · 3 comments

Comments

@holyjak
Copy link
Contributor

holyjak commented Dec 4, 2020

Currently, if the analyzed source code contains an unknown tagged literal, an exception is thrown and the analysis fails. However, most of the time, we don't need to have a perfect representation of the code, we only care about top-level functions and such so, 99.9% we shouldn't really care about those tagged literals and their values.

Possible solutions

1. Detect and apply data readers from the project

We know where data readers are declared, in data_readers.clj[c] at the top of the classpath, so we could check for that and apply those data readers, f.ex. by rebinding *data-readers* or by supplying the map of data readers to the reader that reads the code (if supported; if we use tools.reader, there is *data-readers*).

Pros: We would see the code that the runtime sees.

Cons: More work on our part.

2. Ignore unknown tagged literals without failing

Alternatively, we could define a fallback data reader that simply replaces everything unknown with nil, that would be I assume good enough for analyzing the code and simple.

From the docs:

If no data reader is found for a tag, the function bound in default-data-reader-fn will be invoked with the tag and value to produce a value. If default-data-reader-fn is nil (the default), a RuntimeException will be thrown.

@martinklepsch martinklepsch transferred this issue from cljdoc/cljdoc Dec 4, 2020
@martinklepsch
Copy link
Member

@holyjak I've moved this issue to the cljdoc-analyzer project which is the standalone project that takes care of analyzing Clojure and ClojureScript sources.

I hunch is that it fails when requiring the namespace.

A good next step would be adding a test to the metagetta module and verify that it fails. It looks like Kaocha is set up for this project in which case I think you should be able to run tests with clj -A:test or clj -A:test --watch (I think). Note that these commands need to be run in the modules/metagetta subdirectory.

I hope this helps. Once we have a failing test case I think we can explore using binding to set default-data-reader-fn.

@holyjak
Copy link
Contributor Author

holyjak commented Feb 8, 2021

I have discovered that the solution is not complete. When, inside the analyzer's main namespace I do the following:

(analyze {:project "com.fulcrologic/fulcro" :version "3.4.16"})

INFO [metagetta.utils] Beware: ns/file com.fulcrologic.fulcro.algorithms.normalized-state includes the unknown tagged literal js, ignoring it and replacing the value with data. This should not influence the analysis unless the value is a top-level public var.{:clojure.main/message
"Execution error at com.fulcrologic.fulcro.algorithms.normalized-state/eval941$loading (normalized_state.cljc:1).\nCan't embed object in code, maybe print-dup not defined: clojure.lang.TaggedLiteral@dc7daa72\n",

The error comes from emitValue in the Clojure Compiler:

https://github.com/clojure/clojure/blob/140ed11e905de46331de705e955c50c0ef79095b/src/jvm/clojure/lang/Compiler.java#L4898

So it seems that we are trying to "embed the TaggedLiteral in code". So replacing unknown tag literals with this is not sufficient. But the tests succeed so I am unsure what is different here. What is also surprising is that I do not see any use of #js in the normalized-state ns. Is the reported location wrong or what? Likely it is in one of the required namespaces?

@holyjak
Copy link
Contributor Author

holyjak commented Feb 8, 2021

the commit above fixes the problem

holyjak pushed a commit that referenced this issue Feb 10, 2021
Fix #32: unknown tagged literals -> valid code
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

No branches or pull requests

2 participants