pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.35k stars 640 forks source link

Check for redundant dependencies in BUILD files when inference is in use #13571

Open g-cassie opened 3 years ago

g-cassie commented 3 years ago

Is your feature request related to a problem? Please describe.

When relying on dependency inference, we should be aiming to keep BUILD files as lean as possible. This means we should only specify a dependency in a BUILD file if it is not inferred.

Describe the solution you'd like

Provide a goal (or extend an existing one) to check all BUILD files to see if any dependencies that are explicitly defined would have been inferred. This should account for situations where the BUILD file specifies a target level dependency like src/lib:lib and inference would have included the corresponding file-level targets (e.g. src/lib/myfile.py).

Describe alternatives you've considered

We have looked at using ./pants peek output with a custom script however it does not provide the information needed to distinguish deps that were inferred and deps that were explicitly included. It would also require a fair amount of custom scripting to reconcile target level deps with file level deps.

thejcannon commented 2 years ago

I think this might be easy to do now, with the added "validate dependencies" union

Eric-Arellano commented 2 years ago

@thejcannon I think it is :)

@g-cassie and others, thoughts on what the UX should look like? Do you have to opt-into this feature via an option? How do you force Pants to allow a certain dependency?

g-cassie commented 2 years ago

I don't have strong opinions. Off the top of my head, this seems to me kind of like a lint type behaviour in which case you could think of it as a linting tool specific to BUILD files. Following on that, it might make sense to simply use a # noqa comment if you want to ignore a violation of the rules like you do with flake8.