[flutter_plugin_tool] Don't allow NEXT on version bumps

[stuartmorgan-g]

The special "NEXT" entry in a CHANGELOG should never be present in a
commit that bumped the version. This validates that this is true even if
the CHANGELOG would be correct for a non-version-change state, to catch
someone incorrectly resolving a merge conflict by leaving both parts of
the conflict, rather than folding the 'NEXT' entry's list into the new
version's notes.

Fixes flutter/flutter#85584

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
• All existing and new tests are passing.

[gaaclarke]

Looks good other than some niggles about _hasConsistentVersion. It's not a huge deal, let me know what you think.

[gaaclarke]

nit: I'd put versionChanged = false here since setting it above isn't clearly associated with the unchanged branch.

[stuartmorgan-g]

Changed to no initialization of the variable so that the compiler forces setting it in each branch, for better clarity/future-proofing.

[gaaclarke]

Can versionChanged be calculated inside of this function? The first 2 arguments make sense to me, this one isn't clear. It seems like you wouldn't ask "has consistent version?" if you already know the "version hasn't changed".

We should at least update the docstring. Also, based on the name it seems like a pure function but it is printing out stuff.

[stuartmorgan-g]

I changed the name and docstring, so hopefully it's clearer what this is doing now (it's unfortunately hard to do pure functions in these checks a lot of the time, because verbose logging of everything that's happening is a feature in CI checks; failures where you don't get easily and clearly enough detail about why are a pain). I also changed the parameter name to be more specific about what version changed (the pubspec version) since that was, as evidenced by your question, ambiguous.

For an example of why we'd call this even when that hasn't changed: if a PR author forgets to actually update the version in pubspec.yaml but remembers to update CHANGELOG.md as if they had, we want that to fail CI. In that case the parameter here will be false, but the version consistency check will fail.

[gaaclarke]

Should we be calling hasConsistentVersion when _CurrentVersionState.unknown?

[stuartmorgan-g]

In general, my experience has been that unless it's expensive (which isn't the case here) it's much better to have CI checks fail but continue, so that we find more errors per run. It's really frustrating to fix a CI failure, push a PR, and wait for checks to run again, only to discover that there's another error now that could have been surfaced before but wasn't. That's a pattern I've been following in most of the tool overhaul work, and it's already made things better.

In this case, errors in the CHANGELOG are almost entirely unrelated to whether the version actually changed (only the one edge case this test now covers); almost all of the time a failure here will just be due to a mismatch between what's in pubspec.yaml and what's in CHANGELOG.md, so surfacing that (so the PR author can fix it in parallel) even if something weird happens with the version check is useful.

[gaaclarke]

LGTM!