Clojurians Log v2

Not sure if this is “in the queue” or has been suggested before, but I just ran into an interesting bug that I think clj-kondo could help with. When using a JDBC transaction (i.e. (jdbc/with-db-transaction [tx @db-] …)), I accidentally used the @db- for some of the queries within the scope of the transaction instead of the tx. Seems like clj-kondo could flag this. Not sure if it is specific only to the JDBC transaction macro or if this would be more generally applicable across other macros that introduce scope.

I suspect the intended way to do this would be to add a hook to that macro which adds new findings if it spots that you're using the same form as you did to bind the transaction anywhere in the code.

And if this hook is useful, then contribute it back to the original project. Which if this is for next.jdbc might be possible. If it's maintained by cognitect as a part of contrib though, then it might be worthwhile to add such a thing to the hooks that are exposed by clj-kondo.

could you point me to any docs for doing this implementation work? I would be happy to take a stab at it.

@mafcocinco the jdbc syntax is supported as a built-in feature, so if you use db instead of tx and db would be unused, then that would be reported. I'm not sure what @db- means

The docs for how to export them are here: https://github.com/clj-kondo/clj-kondo/blob/master/doc/config.md#exporting-and-importing-configuration The docs for how to work on the macro hook and emit custom findings is here: https://github.com/clj-kondo/clj-kondo/blob/master/doc/hooks.md https://github.com/clj-kondo/config/tree/master/resources/clj-kondo.exports/clj-kondo Here's where you'd need to put a new folder if you contribute it back to kondo.

next.jdbc is not a contrib / cognitect project

Yeah, the jdbc syntax works in my version of clj-kondo. I think it is a slightly different bug though. Suppose I have this transaction: (jdbc/with-db-transaction [my-tx my-db] …). So I’m using my-db to create the my-tx binding. In the scope of the transaction, I should be using my-tx for interactions with the DB, running queries, etc. The bug I had was that for one of the queries, I used my-db, which was in scope but caused the transaction to fail because of mutations that were applied to the transaction but not the source db. It occurred to me that if one is using the source DB connection instead of the transaction it has been bound to in the scope of with-db-transaction that that should be flagged as a potential bug.

ah right, so the right hand side local should not be used anymore in that case. makes sense. what do you think @seancorfield? are there exceptions to this rule?

There may be exceptions to that, yes, since TX are per-connection. They’re edge cases but it definitely not “always wrong” to use the non-TX version of the connectable.

So then would that be appropriate to provide as a warning, and then allow users to add ignores in those cases?

Or is the philosophy of kondo to not warn in cases like these where there is a valid usecase?

It can be a configurable opt-in rule

Yeah, I think it’s a reasonable warning to add to clj-kondo (for c.j.j and next.jdbc’s equivalent) but occasionally folks might need to turn it off for a specific form.

and even if you enable the rule, you can ignore on a case by case basis using #_:clj-kondo/ignore

so, feel free to make an issue for this with some proposals, etc and we can take it from there