Add deprecation and exclusion of checks by Elixir version

[devonestes]

We have some checks that don't behave entirely as expected for some older
versions of Elixir, and we want to deprecate (and eventually remove) thos
checks for those versions. This is the first very quick and very dirty spike
on a way to do that so we can start a discussion around how we want to handle
this process.

This is related to #229 and will hopefully give us a starting point for discussion.

[devonestes]

So, in general, what I have here is a list of checks that we want to deprecate for certain versions of Elixir, and then another list of checks that we want to exclude (or maybe disable is a better name?) for certain versions. We'll need a much more robust way of keeping track of those two lists, but I didn't want to dive too deeply into that for this first run.

When something is deprecated, it prints that it's deprecated, and when it's excluded we remove it from the config as if it were excluded manually by the user. We might also want to have some optional printing to the console so users know that it's happening - otherwise for a new user it might appear to be magically excluding things for no reason. However, having that all the time might get really annoying, which is why I didn't go that route for now.

[rrrene]

This is an interesting approach. I had something different in mind. I have a 4 hour train ride ahead of me, so maybe I'm able to write it up, so we can discuss it using real code 👍

[rrrene]

Here's the quick and dirty solution I came up with on the train: 86c69b1

It is more flexible, since it is not targeting single Elixir versions. But it is lacking the message to the user (that a configured check will not be run).

I think we can find a good solution somewhere in the middle 😁

[devonestes]

@rrrene Yeah, that's gonna be easier to manage in the long run! Would we have two different options for each check - one for elixir_version_deprecate: ">= 1.3.0", elixir_version_exclude: ">= 1.0.0, < 1.3.0" or something like that? I think we're definitely going to want to be able to both deprecate based on version and exclude based on version - although maybe not both at the same time.

[rrrene]

The question here is if we really deprecate checks based on Elixir version? I would have thought that checks are deprecated based on Credo version (e.g. "This check is deprecated and will be removed in future versions of Credo") and excluded based on Elixir version ("This check is excluded for this version of Elixir").

WDYT?

[devonestes]

So, let's imagine I'm on Elixir 1.2, and I upgrade Credo to some new version that excludes a check. I only see "This check is excluded for this version of Elixir", right? I wouldn't see any deprecation of that check first?

I think that could work, but I also imagine that some users might find it confusing. On the upside, though, it could encourage users to upgrade their Elixir versions, which is always good!

[rrrene]

The problem is that i don't see it working any other way. I'm on Elixir 1.2 and upgrade to a newer version of Credo which includes a check that does not work on 1.2 - we can not deprecate stuff in this scenario, right?

Other scenario: we are on Elixir 1.3.0 where a check throws false positives, because :elixir_tokenizer behaves in an unexpected way. We can't deprecate (as opposed to exclude) a check here either, because we do not want people to get the false positives.

That's why I think that deprecation of individual checks is either a true/false thing or tight to a specific future version of Credo (like "this is deprecated now and will be excluded in Credo >= 0.6.0").

[devonestes]

Yeah, I get what you're saying now. You're right, it does need to be done this way.

Well, I can take a crack at working on that a bit this week!

[devonestes]

Ok, I've taken another swing at this based off of the version that @rrrene threw together. The big addition to that version that I made was some UI output for the user so they don't think the check is running because they explicitly included it in their config file but since it's excluded anyway they might think it's running and just not finding any errors. that would be super confusing.

I'm not in love with the output that I have at the moment, but it might be good enough. Below is a screenshot of what it would look like for a user on 1.3.0 as of today.

[rrrene]

FYI: I moved the message to "above the analysis".