mirror of
https://github.com/status-im/status-react.git
synced 2025-01-10 19:16:59 +00:00
9ed68ee7d1
It's well known that shadowing core Clojure vars can lead to unexpected bugs. In fact, it's a common source of bugs in other languages too. In the status-mobile repository there are, in total, 562 shadowed vars, ~500 are core vars. Excluding the "old code" we still have 285 offenders. In status-mobile I've already seen two bugs caused by shadowed vars, both with the shadowed var "name". But probably other problems happened in the past, and others will happen in the future if we don't do something about this. This PR is also my response to my frustration trying to review PRs and checking for shadowed vars, humans were not meant for that! In this commit we are enabling ":shadowed-var" to lint certain (not all) core vars as errors (not warnings). In future PRs we can gradually unshadow more vars. For the record, name is shadowed 40 times in the new code and 130 in total, and type is shadowed 93 times in the new code and 124 in total! What about non-core vars, should we allow shadowing? There are ~70 non-core shadowed vars. In my opinion, we should also lint and disallow shadowing non-core vars, since it may cause the same kind of bugs of shadowing core vars. But this decision can be left for another moment/issue, after we have fixed the most prominent problem of shadowing core vars. Which vars are unshadowed in this PR? I fixed 62 errors and unshadowed cljs.core/iter, cljs.core/time, cljs.core/count, cljs.core/key, clojure.core/key. Resources: - [clj-kondo linter: shadowed-var](https://github.com/clj-kondo/clj-kondo/blob/master/doc/linters.md#shadowed-var)