Warn about duplicated constraints

[adamchainz]

What's the problem this feature will solve?

Whilst developing #1075 to remove line number annotations, the main feedback was that the annotations are useful for detecting duplicated requirements/constraints in a single file. But that requirements humans to read, and removing the line number annotations is best to reduce diff noise when compiling requirements.

Duplicated requirements should always be combinable, and can point to sources of bad assumptions and thus potential bugs if they're in separated parts of the file.

e.g.

django>=1.11
django<2.0

can be

django>=1.11,<2.0

Describe the solution you'd like

A warning, or maybe a comment at the bottom of the generated file, about duplicated requirements in a single file.

Alternative Solutions

Just don't warn

[AndydeCleyre]

FWIW I'm leaning toward a comment in the generated file rather than just stderr warning, because:

1. It seems a similar case to the unsafe packages (comment) warning: "you really shouldn't be doing this, but you did, and this output file will reflect that in some way."
2. It's harder to ignore when committing/merging, and will be seen by more people than just the one who compiled it.

That said, some thoughts against myself:

1. Deprecate --allow-unsafe option #989 is open; the days of those unsafe comments might be numbered, casting doubt on using that behavior as a positive model.
2. How wrong is it to put them on separate lines? It seems to me "not very," especially if the constraints file is small, and each constraint is there for a different, commented reason. If it's not so wrong, and people know what they want, then the comment might be an annoyance, leading to requests for another flag to bypass that behavior. And adding more flags is IMO something to be resisted.

TBH I preferred all the info in the annotation, I don't mind the diff noise.

I wonder if this makes sense, as a package:

• (Deprecate --allow-unsafe option #989) deprecate --allow-unsafe and make it default (would we need to add an inverse option, or does no one care?)
• lump together the unsafe packages warning and duplicate constraints warning, to be included or not in the comments based on a new flag that applies to all warnings (are there other similar warnings aside from those already?)
• eventually drop --allow-unsafe

This would in the long run be a net-zero change in flag count, and the new --omit-warnings (or whatever) flag would be general purpose enough for similar needs now or down the road.

EDIT: Especially if warnings of different types are lumped together, it would be necessary for the hypothetical --omit-warnings flag to only affect the file output, not the stderr, so that new unexpected warnings still appear at least to the person compiling.

[adamchainz]

How wrong is it to put them on separate lines? It seems to me "not very," especially if the constraints file is small, and each constraint is there for a different, commented reason. If it's not so wrong, and people know what they want, then the comment might be an annoyance, leading to requests for another flag to bypass that behavior. And adding more flags is IMO something to be resisted.

Yeah, I guess it is "not very" bad. The comment leading here, that I should've linked before, was about accidentally parallel-added constraints on mock: #1058 (comment) . Doesn't sound like it caused any real world issues though. At the end of the day if you constrain versions too much, you just won't be able to match any version and the compile will break.

[AndydeCleyre]

Here's a start, FWIW.

[atugushev]

It's not quite a duplication from UX POV, see an example of requirements.in:

# We support Django>=1.11
django>=1.11

# TODO remove this after Django-2.0 support being added
django<2.0