I think it is possible to argue that the reading of eastwoods config files is somewhat convoluted.
I won't argue hard against you 🙂 Any particular function or functions that lead you to that conclusion?
The fact that the configs are basically edn-data, but still, they’re loaded into the util ns, where the configs call a function which adds each config map into an atom.
It seems like the configs could just be read and then reduced. No need for loading nor the atom.
I believe at least one of the config files takes advantage of the fact that they are executed as code, by performing a loop.
https://github.com/jonase/eastwood/blob/master/resource/eastwood/config/third-party-libs.clj#L1-L4
Yeah, and I guess it opens up a wide variety of applications of Hyrums law :)
https://github.com/jonase/eastwood/blob/master/resource/eastwood/config/third-party-libs.clj#L131
It is a sharp tool, I agree.
Running Eastwood launches the missiles, if require'ing your code launches the missiles.
I tried to put that pretty early in the README, and also in the output of lein eastwood help.
and if there are bugs in Eastwood, it could launch missiles even if require'ing your code doesn't launch missiles 🙂
I'm not saying the config files must remain that way -- just pointing out a consequence of changing it.
I don’t think it’s the first thing to change, since it’s somewhat part of the public api.
It was just that I stumbled upon/over it today as I was trying to do something.
I was in this area:
https://github.com/jonase/eastwood/blob/master/src/eastwood/util.clj#L895
and I tried to pass warning-enable-config-atom as a deref’ed parameter to the enclosing fn. Which did not work at all.
So I grep’ed through src to see if anyone was calling disable-warning, and there was noone 🙂
yep, nothing in src does 🙂
In other news, I’m getting to the point where I’m able to write some tests for the main/setup parts of eastwood.
https://github.com/jonase/eastwood/commit/39ad1a94a96878d6813448c27ac0279f3e21ecf5
Interesting. Yeah, crucible testing is nice for showing what folks see in the wild, but it doesn't have much variety in how those setup things are configured.
Yeah, I remember Mike Fikes had testing for planck much in the same way. Record the output from running the program, and ensure that it didn’t change.
Except for crucible I never really expected to ensure that the output didn't change. Some of the messages change from run to run for some projects, because of JVM hex values (object IDs and/or pointers) changing across runs and being included in the output. Running a decent visual diff tool helps skim through that stuff fairly quickly by hand.
is there a known issue with using a function to generate the second arg to clojure.test/is showing up as a "suspicious test" because eastwood doesn't know the function returns a string?
to follow up, there's a special case for (str ...) (format ...) and some related forms
I know that Eastwood will not warn if there is a literal string constant in that position, but it will warn if it cannot easily tell that the expression results in a string.
I have seen it warn for things that a person can obviously tell it is a string, e.g. (apply str ...) expressions.
the specific case is using a spec helper function to generate the error string if is fails
so I'm wondering if the best approach is (str (special-helper ...)) or type hinting the helper with ^String, or...
That is one of the longer warning messages, giving some details to the reader about why it might be a false positive.
yeah
(str (special-helper ..)) sounds like it would avoid the warning -- worth a quick test to see.
co-worker reports that ^String appeases the linter
Probably worth including in Eastwood's README as a workaround for that case
Since tests should not be performance-critical
yeah - it's more that having a false positive in the linting output reduces the value of the linter, so better to eliminate the warning or turn off that rule
I have seen that linter for Eastwood catch semantically incorrect (is ...) forms with misplaced parens.
right, it should also catch an is that should have had an =
(we might be thinking of the same common error)
I have seen at least 3 or 4 variations of semantically wrong things that it can catch right now. You interested in creating an issue and/or better yet a PR for Eastwood's README suggesting wrapping the 'message' arg in (str ...) to quiet it down?
I'll make a note of that, I might even have time for it in the next day or two
We could potentially even mention it in the warning message itself...
OK, I think you've given me an itch that is so small I can scratch it in 10 mins or less 🙂
He said optimistically.
hah
OK, current verbose warning message is: "'is' form has non-string as second arg (inferred type is %s). The second arg is an optional message to print if the test fails, not a test expression, and will never cause your test to fail unless it throws an exception. If the second arg is an expression that evaluates to a string during test time, and you intended this, then ignore this warning."
Planning to change that to: "'is' form has non-string as second arg (inferred type is %s). The second arg is an optional message to print if the test fails, not a test expression, and will never cause your test to fail unless it throws an exception. If the second arg is an expression that evaluates to a string during test time, and you intended this, you may wrap it in a call to (str ...) so this warning goes away."
hinting the expression with ^String also works
not sure if that's worth the extra verbosity
I'd like not to mention both options in the warning message, long as it is already 🙂
haha, right
and trust that (str ...) wrapping works in more cases, e.g. if the expression is a macro invocation, I don't think the type hint will work then.
oh, yeah - so str is more general
yeah, I believe so. The number of corner cases I have seen regarding type hints ... Thankfully I am forgetting most of that over time.
tangential question - if a function was type hinted, where would I find that in the var metadata?
The var is type hinted? Or the arg vector?
oh, so I couldn't find the hint info from the var?
That is my way of giving myself a few more seconds to try to remember the answer, but I think it may actually depend upon that...
Functions can type hint on the arg vector, and I think even have different ones on different arities, if a function has multiple arities defined.
I have forgotten the advantages/disadvantages of the typehint-the-var vs. typehint-the-arg-vector approaches -- Bronsa probably has it closer to his reachable memory than I do.
that was enough info to find it via trial and error at least
(ins)user=> (defn hm ^java.util.Map [& args] (apply hash-map args))
#'user/hm
(ins)user=> (-> #'hm meta)
{:arglists ([& args]), :line 24, :column 1, :file "NO_SOURCE_PATH", :name hm, :ns #object[clojure.lang.Namespace 0x6e78fcf5 "user"]}
(ins)user=> (-> #'hm meta :arglists meta)
nil
(ins)user=> (-> #'hm meta :arglists first meta)
{:tag java.util.Map}
cool. Most of what I ever learned about Clojure type hints is in Eastwood's readme, the section on the :wrong-tag linter: https://github.com/jonase/eastwood#wrong-tag
It does not attempt to document all places type hints are useful in Clojure, e.g. it doesn't attempt to cover anything about protocols, multi methods, deftype, etc.
those features I have never attempted to learn how type hints work or don't.