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.