Does adding a data constructor require a major version bump according to the PVP?

Learn
#1

If I’m versioning my package according to the PVP and I add a new data constructor to a type that is exported, then does the PVP require me to do a MAJOR version bump? Or is only a MINOR version bump allowed?

The PVP says (emphasis mine):

A.B is known as the major version number, and C the minor version number. When a package is updated, the following rules govern how the version number must change relative to the previous version:

  1. Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes or classes were changed, or orphan instances were added or any instances were removed, then the new A.B MUST be greater than the previous A.B. Note that modifying imports or depending on a newer version of another package may cause extra orphan instances to be exported and thus force a major version change.

  2. Non-breaking change. Otherwise, if only new bindings, types, classes, non-orphan instances or modules (but see below) were added to the interface, then A.B MAY remain the same but the new C MUST be greater than the old C. Note that modifying imports or depending on a newer version of another package may cause extra non-orphan instances to be exported and thus force a minor version change.

Does adding a data constructor to an existing type count as changing the definition of a datatype (which would require a major version bump), or adding a new binding (which would require only a minor version bump)?

If anyone is interested in the reason for this question, I’m wondering specifically about the package language-javascript-0.7.1.0, where a new constructor is added to an existing data type: https://github.com/erikd/language-javascript/pull/113. The JSAsyncFunction constructor is added to the JSStatement data type, and the release that contained this change did a minor version bump from 0.7.0.0 to 0.7.1.0.

This came up while I was trying to update PureScript to a more recent LTS release: https://github.com/purescript/purescript/pull/4199#discussion_r744139266

1 Like
#2

Breaking change. E.g. adding a constructor to a sum type causes downstream exhaustive pattern matches to fail to compile.

8 Likes
#3

Non-exhaustive pattern matches is not a compilation error by default in GHC. Only if you compile with -Werror, otherwise it’s just a warning. PVP addresses only compile-time compatibility, i.e. bounds should result in a buildable project (not necessary working correctly in runtime).

However, there’s this clause:

  1. Deprecation. Deprecated entities (via a DEPRECATED pragma) SHOULD be counted as removed for the purposes of upgrading the API, because packages that use -Werror will be broken by the deprecation. In other words the new A.B SHOULD be greater than the previous A.B.

This means that PVP actually cares about -Werror being enabled even though it’s written in vague words.

But even without that clause, I’d still bump up a major version after adding a new constructor. Some functions can pattern match on all constructors and they may throw runtime error with the new version of the package with a new constructor. You may want to change the code to address this problem.

In my vision, minor versions could be upgraded automatically and it should be always safe to upgrade a minor version without any code changes. But upgrading to a newer major version may require some code changes and this action should be explicit.

7 Likes
#4

Hmm, that’s the kind of thing that begs for an udpate (or an FAQ) of the PVP spec. :slight_smile:

5 Likes
#6

The spec doesn’t say this explicitly, but the flowchart does say that changes in behavior of exported functions should lead to a major version bump. I might have this slightly wrong but there’s some care here in MUST vs SHOULD where the MUST clauses are the ones that keep things compiling and the SHOULD clauses are the ones that are additional common practice. I tend to agree that “porting” the clause about behavior changes leading to bumps from the flowchart to the main body of the spec would be a good idea.

#7

Yep. Despite whatever wording details put in PVP or not - it meant in the spirit. What is API breakage & would break software - is an API breakage.

Non-exhaustive constructors is not a norm - it is a bane we must bare. It would’ve been ideal to cover all constructors all the time - but sometimes that is counterproductive. Exhaustive case statements are of course recommended.

Adding a new data constructor - would break both non-exhaustive & exhaustive case statements. It is not a minor update but a breakage, when with proper code downstream compilation would break & downstream maintainer needs to go through the exhaustive case statements & add new constructor to them so the project compiles again.
If patterns are not exhaustive & have a bottom match - meaning of the bottom match gets expanded without control & so every non-exhaustive case statement then is a dice roll of whenever what it does in case of new constructor is right.

For that reason along, for bottom matches - several haskellers (for example 2016 - Jasper Van der Jeugt - Haskell: mistakes I made (and how to fix them) - YouTube) & tooling (GHC, HLS & highlighting) - support naming of bottom matches, as _text - to notate a semantic meaning to bottom match, so when bottom match bug is encountered - developer knows something about what that buttom was made for in the first place.

Releasing a constructor - changes the semantic meaning of the bottoms in non-exhaustive pattern matches.

The releaser knows about it when releases stuff. If the release is marked a minor release - that breaks the downstream logic. It is a lucky longshot gamble if downstream has all cases nonexhaustive & keeps compiling & all case statement logic still works right.
So both ways it is an API breaching change.

It is normal practice to fix those things, package versions get deprecated, minor releases rereleased as major releases & even major releases (as in binary majors are (twice) found to be non-breaking & so also deprecated & made minor releases of the previous release. The hell - my last release was a mess - good that I released a support release for what downstream needs first, before making a big major, because after that I had 2 deprecations, deprecated a major release & so as a result made no major release at all, but because all that was fixed right & what is provided does not break API - nothing is changed for downstreams.

PVP works great. If people have troubles with wording - it may be refactored. But the core idea is to still keep it KISS without detailing.

For example, PVP addendum document, where details are explained. So PVP stays KISS & concise & people share & refer to it, but it also has a link to an addendum - a Wiki of explanations if needed.

2 Likes
#8

As a hopefully equivalent thought experiment: Would you expect a major version bump if base added a new constructor to Maybe? I certainly would.

2 Likes
#9

Wouldn’t that also change API of functions like maybe and then require a bump anyway? Would you expect a major version bump if you add a constructor to a Color sum type?

1 Like
#10

Would you expect a major version bump if you add a constructor to a Color sum type?

If there is (or could be) a reasonable recusor/inductor/whatever-the-correct-name-is on it, yes. I haven not seen a single piece of code pattern matching on — say — Color, but why risk it? A major bump will not break existing programs for sure and will not cause major hassle to people using the new API.

1 Like
#11

A major bump will not break existing programs for sure and will not cause major hassle to people using the new API.

Here’s what most people forget: Every major version bump is effectively a fork/branch of your package. Sadly, most maintainers don’t see it that way and you only get security patches and bugfixes for the latest major version.

That puts people at risk, who use upper bounds and don’t have time to run expensive migrations to new major versions.

So there’s a trade-off. If you follow PVP properly, major version bumps incur a maintenance overhead. If you only maintain the latest major version properly, your users will lose.

3 Likes