eastwood

All things realted to eastwood - the Clojure linter
slipset 2018-10-11T10:56:22.000200Z

I think it is possible to argue that the reading of eastwoods config files is somewhat convoluted.

2018-10-11T19:53:35.000100Z

I won't argue hard against you 🙂 Any particular function or functions that lead you to that conclusion?

slipset 2018-10-11T19:59:37.000100Z

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.

slipset 2018-10-11T20:00:56.000100Z

It seems like the configs could just be read and then reduced. No need for loading nor the atom.

2018-10-11T20:02:45.000100Z

I believe at least one of the config files takes advantage of the fact that they are executed as code, by performing a loop.

slipset 2018-10-11T20:04:28.000200Z

Yeah, and I guess it opens up a wide variety of applications of Hyrums law :)

2018-10-11T20:04:57.000100Z

It is a sharp tool, I agree.

2018-10-11T20:05:11.000100Z

Running Eastwood launches the missiles, if require'ing your code launches the missiles.

2018-10-11T20:05:55.000100Z

I tried to put that pretty early in the README, and also in the output of lein eastwood help.

2018-10-11T20:06:36.000100Z

and if there are bugs in Eastwood, it could launch missiles even if require'ing your code doesn't launch missiles 🙂

2018-10-11T20:08:26.000200Z

I'm not saying the config files must remain that way -- just pointing out a consequence of changing it.

slipset 2018-10-11T20:30:12.000100Z

I don’t think it’s the first thing to change, since it’s somewhat part of the public api.

slipset 2018-10-11T20:30:40.000100Z

It was just that I stumbled upon/over it today as I was trying to do something.

slipset 2018-10-11T20:31:53.000100Z

I was in this area:

slipset 2018-10-11T20:32:31.000100Z

and I tried to pass warning-enable-config-atom as a deref’ed parameter to the enclosing fn. Which did not work at all.

slipset 2018-10-11T20:33:10.000100Z

So I grep’ed through src to see if anyone was calling disable-warning, and there was noone 🙂

2018-10-11T20:34:24.000100Z

yep, nothing in src does 🙂

slipset 2018-10-11T20:36:46.000100Z

In other news, I’m getting to the point where I’m able to write some tests for the main/setup parts of eastwood.

2018-10-11T20:46:45.000100Z

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.

slipset 2018-10-11T20:54:39.000100Z

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.

2018-10-11T20:57:15.000100Z

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.

2018-10-11T23:20:10.000100Z

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?

2018-10-11T23:27:54.000100Z

to follow up, there's a special case for (str ...) (format ...) and some related forms

2018-10-11T23:37:04.000100Z

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.

2018-10-11T23:37:34.000100Z

I have seen it warn for things that a person can obviously tell it is a string, e.g. (apply str ...) expressions.

2018-10-11T23:37:35.000100Z

the specific case is using a spec helper function to generate the error string if is fails

2018-10-11T23:38:16.000200Z

so I'm wondering if the best approach is (str (special-helper ...)) or type hinting the helper with ^String, or...

2018-10-11T23:38:19.000100Z

That is one of the longer warning messages, giving some details to the reader about why it might be a false positive.

2018-10-11T23:38:27.000100Z

yeah

2018-10-11T23:38:55.000100Z

(str (special-helper ..)) sounds like it would avoid the warning -- worth a quick test to see.

2018-10-11T23:39:13.000100Z

co-worker reports that ^String appeases the linter

2018-10-11T23:39:17.000100Z

Probably worth including in Eastwood's README as a workaround for that case

2018-10-11T23:39:36.000100Z

Since tests should not be performance-critical

2018-10-11T23:40:18.000100Z

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

2018-10-11T23:40:37.000100Z

I have seen that linter for Eastwood catch semantically incorrect (is ...) forms with misplaced parens.

2018-10-11T23:41:01.000100Z

right, it should also catch an is that should have had an =

2018-10-11T23:41:10.000100Z

(we might be thinking of the same common error)

2018-10-11T23:42:01.000100Z

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?

2018-10-11T23:42:33.000100Z

I'll make a note of that, I might even have time for it in the next day or two

2018-10-11T23:42:55.000100Z

We could potentially even mention it in the warning message itself...

2018-10-11T23:44:33.000100Z

OK, I think you've given me an itch that is so small I can scratch it in 10 mins or less 🙂

2018-10-11T23:44:43.000100Z

He said optimistically.

2018-10-11T23:45:16.000100Z

hah

2018-10-11T23:46:12.000100Z

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."

2018-10-11T23:47:31.000100Z

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."

2018-10-11T23:48:32.000100Z

hinting the expression with ^String also works

2018-10-11T23:48:53.000100Z

not sure if that's worth the extra verbosity

2018-10-11T23:48:55.000100Z

I'd like not to mention both options in the warning message, long as it is already 🙂

2018-10-11T23:49:01.000100Z

haha, right

2018-10-11T23:49:19.000100Z

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.

2018-10-11T23:49:34.000100Z

oh, yeah - so str is more general

2018-10-11T23:50:03.000100Z

yeah, I believe so. The number of corner cases I have seen regarding type hints ... Thankfully I am forgetting most of that over time.

2018-10-11T23:50:44.000100Z

tangential question - if a function was type hinted, where would I find that in the var metadata?

2018-10-11T23:51:19.000200Z

The var is type hinted? Or the arg vector?

2018-10-11T23:51:46.000100Z

oh, so I couldn't find the hint info from the var?

2018-10-11T23:51:54.000100Z

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...

2018-10-11T23:52:37.000100Z

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.

2018-10-11T23:54:06.000100Z

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.

2018-10-11T23:54:55.000100Z

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}

2018-10-11T23:56:05.000100Z

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

2018-10-11T23:56:37.000100Z

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.

2018-10-11T23:56:58.000100Z

those features I have never attempted to learn how type hints work or don't.