Add more tests which use ast_plus_one_deps_strict_deps_unused_deps_error toolchain

[ittaiz]

Description

ast_plus_one_deps_strict_deps_unused_deps_error toolchain has more coverage and first class

This PR moves //scala:ast_plus_one_deps_strict_deps_unused_deps_error toolchain to the prod section since we now promote it as the alternative but leave users up to stitching everything themselves. I also added reference to it in the readme.
Additionally it builds and tests test/... with this toolchain to surface issues like #1029.

Motivation

Better coverage for AST and other combo and better alignment with our approach.

Open issues

1. Should we choose a more concise name for the toolchain? Right now it's honest and complete but I'm not sure we should stick to it (wasn't sure what the abstraction is OTOH).
2. I needed to add quite a few unused_dependency_checker_ignored_targets exclusions to get the build passing. I'd really love for us to understand if they are working as intended, bugs we can fix or bugs we'll have to live with.
   We might be able to merge this PR even with these exclusions but I'm willing to do that only if it's clear who's owning the rest of the inquiry.

[ittaiz]

@Jamie5 I'd love your review here

[ittaiz]

probably obvious but I'll just add that I added the above unused_dependency_checker_ignored_targets only after I saw they are required (that is the build fails with them and without them on different dep combos)

[ittaiz]

linting fails. Pushing a fix. Otherwise green

[Jamie5]

Looks reasonable to me. I may be able to look at the false positives to see what's going on (don't have much context on these rules but hopefully can at least get something useful out of it). Not sure of timeline though, but if there's a tracking thing I can deal with it at some point. As for name maybe exact_deps? Since the goal of ast/+1 is to include only the deps found in the source, and the strict_deps error mode/unused_deps error mode also make we don't have random unneeded deps, nor not including deps which are found in the source. So I guess the high level is having deps exactly match up with the source. (only thing that may not fully is plus-one as opposed to transitive, but eh)

[ittaiz]

Great.
With respect to false positives- I’ll open 3 PRs for each of the issues so you can see them?
With respect to the name- “minimal_source_deps”? “exact_source_deps”? Because the big diff between direct and plus one is that we’re trying to focus about source deps and between transitive and plus one is that we’re trying to be minimal

[Jamie5]

Sounds good re opening PRs (or issues is fine too). Re name exact_source_deps I slightly prefer because we're not quite going for minimal or we wouldn't care about strict_deps, and in the transitive vs plus one, we're trying to be minimal about deps implicitly brought in, not necessarily source deps. But again not a strong opinion here.

[ittaiz]

I also don't have a really strong opinion but I think this discussion has a bit more value in it.
If you feel you've exhausted it feel free to say so :)
I think we're trying to fit a few different dimensions here which makes naming hard :)
WDYT about minimal_direct_source_deps? The reason I feel uneasy about exact is the use-cases of more than one level (i.e. deep inheritance) which means that you will need to bring in a dep you don't need source wise. This is why I feel minimal tries to give a bit more context there.
WDYT?

[ittaiz]

oh and re PRs vs issues- The reason why I'm thinking PRs is so that you can have easy repros. If this isn't valuable then I'll just open issues. LMK.

[Jamie5]

Hmm minimal_direct_source_deps sounds reasonable to me. I guess PRs might be helpful then if it's not much trouble for you.