diff --git a/src/commit.ts b/src/commit.ts index bd0d5a800..2d7f0af98 100644 --- a/src/commit.ts +++ b/src/commit.ts @@ -259,7 +259,9 @@ function toConventionalChangelogFormat( if (parent.type === 'token') { parent = ancestors.pop(); let footerText = ''; - const semanticFooter = node.value.toLowerCase() === 'release-as'; + const footerType = node.value.toLowerCase(); + const semanticFooter = + footerType === 'release-as' || footerType === 'revert'; visit( parent, ['type', 'scope', 'breaking-change', 'separator', 'text', 'newline'], @@ -281,7 +283,7 @@ function toConventionalChangelogFormat( ); // Any footers that carry semantic meaning, e.g., Release-As, should // be added to the footer field, for the benefits of post-processing: - if (semanticFooter) { + if (footerType === 'release-as') { let releaseAs = ''; visit(parent, ['text'], (node: parser.Text) => { releaseAs = node.value; @@ -294,14 +296,16 @@ function toConventionalChangelogFormat( if (!headerCommit.footer) headerCommit.footer = ''; headerCommit.footer += `\n${footerText.toLowerCase()}`.trimStart(); } - try { - for (const commit of toConventionalChangelogFormat( - parser.parser(footerText) - )) { - commits.push(commit); + if (!semanticFooter) { + try { + for (const commit of toConventionalChangelogFormat( + parser.parser(footerText) + )) { + commits.push(commit); + } + } catch (err) { + // Footer does not appear to be an additional commit. } - } catch (err) { - // Footer does not appear to be an additional commit. } } } @@ -402,6 +406,91 @@ function splitMessages(message: string): string[] { return [...conventionalCommits, ...messages.slice(1)]; } +const REVERT_TARGET_PATTERNS = [ + /(?:^|\r?\n)This reverts commit ([0-9a-z]+)\.?(?=\r?(?:\n|$))/gi, + /(?:^|\r?\n)Revert:\s*([0-9a-z]+)\.?(?=\r?(?:\n|$))/gi, +]; + +function revertTargets(message: string): string[] { + const targets = new Set(); + for (const pattern of REVERT_TARGET_PATTERNS) { + pattern.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = pattern.exec(message)) !== null) { + targets.add(match[1].toLowerCase()); + } + } + return [...targets]; +} + +/** + * Removes revert pairs whose original and reverting commits both occur in the + * current commit range. Reverts of commits outside the range remain visible, + * since they describe a change relative to the previous release. + */ +function filterRevertedCommits(commits: Commit[]): Commit[] { + const revertTargetByCommit = new Map(); + const revertersByTarget = new Map(); + + for (let commitIndex = 0; commitIndex < commits.length; commitIndex++) { + const targets = revertTargets(commits[commitIndex].message); + if (targets.length === 0) continue; + + let resolvedTarget: number | undefined; + let canResolve = true; + for (const target of targets) { + const matches: number[] = []; + for ( + let candidateIndex = 0; + candidateIndex < commits.length; + candidateIndex++ + ) { + if (candidateIndex === commitIndex) continue; + const candidateSha = commits[candidateIndex].sha.toLowerCase(); + if ( + candidateSha.startsWith(target) || + target.startsWith(candidateSha) + ) { + matches.push(candidateIndex); + } + } + + if (matches.length !== 1) { + canResolve = false; + break; + } + if (resolvedTarget !== undefined && resolvedTarget !== matches[0]) { + canResolve = false; + break; + } + resolvedTarget = matches[0]; + } + + if (!canResolve || resolvedTarget === undefined) continue; + revertTargetByCommit.set(commitIndex, resolvedTarget); + const reverters = revertersByTarget.get(resolvedTarget) ?? []; + reverters.push(commitIndex); + revertersByTarget.set(resolvedTarget, reverters); + } + + const active = new Map(); + const isActive = (commitIndex: number): boolean => { + const cached = active.get(commitIndex); + if (cached !== undefined) return cached; + + const isReverted = (revertersByTarget.get(commitIndex) ?? []).some( + reverterIndex => isActive(reverterIndex) + ); + active.set(commitIndex, !isReverted); + return !isReverted; + }; + + return commits.filter((_, commitIndex) => { + if (!isActive(commitIndex)) return false; + return !revertTargetByCommit.has(commitIndex); + }); +} + /** * Given a list of raw commits, parse and expand into conventional commits. * @@ -417,7 +506,7 @@ export function parseConventionalCommits( ): ConventionalCommit[] { const conventionalCommits: ConventionalCommit[] = []; - for (const commit of commits) { + for (const commit of filterRevertedCommits(commits)) { for (const commitMessage of splitMessages( preprocessCommitMessage(commit) )) { diff --git a/test/changelog-notes/default-changelog-notes.ts b/test/changelog-notes/default-changelog-notes.ts index db10273b3..007a2bf0c 100644 --- a/test/changelog-notes/default-changelog-notes.ts +++ b/test/changelog-notes/default-changelog-notes.ts @@ -374,13 +374,29 @@ describe('DefaultChangelogNotes', () => { expect(notes).to.not.include('Test User'); safeSnapshot(notes); }); - // it('ignores reverted commits', async () => { - // const commits = [buildCommitFromFixture('multiple-messages')]; - // const changelogNotes = new DefaultChangelogNotes(); - // const notes = await changelogNotes.buildNotes(parseConventionalCommits(commits), notesOptions); - // expect(notes).to.is.string; - // safeSnapshot(notes); - // }); + it('ignores reverted commits', async () => { + const commits = parseConventionalCommits([ + { + sha: '1111111111111111111111111111111111111111', + message: 'feat: some feature', + }, + { + sha: '2222222222222222222222222222222222222222', + message: + 'revert: feat: some feature\n\nThis reverts commit 1111111111111111111111111111111111111111.', + }, + { + sha: '3333333333333333333333333333333333333333', + message: 'fix: some bugfix', + }, + ]); + const changelogNotes = new DefaultChangelogNotes(); + + const notes = await changelogNotes.buildNotes(commits, notesOptions); + + expect(notes).to.include('some bugfix'); + expect(notes).to.not.include('some feature'); + }); }); }); describe('pull request compatibility', () => { diff --git a/test/commits.ts b/test/commits.ts index 55ecdeacb..975b006ea 100644 --- a/test/commits.ts +++ b/test/commits.ts @@ -263,25 +263,108 @@ describe('parseConventionalCommits', () => { expect(commit.type).to.eql('chore'); }); - // it('ignores reverted commits', async () => { - // const commits = [ - // {sha: 'sha1', message: 'feat: some feature', files: ['path1/file1.txt']}, - // { - // sha: 'sha2', - // message: 'revert: feat: some feature\nThe reverts commit sha1.\n', - // files: ['path1/file1.rb'], - // }, - // { - // sha: 'sha3', - // message: 'docs: some documentation', - // files: ['path1/file1.java'], - // }, - // ]; - // const conventionalCommits = parseConventionalCommits(commits); - // expect(conventionalCommits).lengthOf(1); - // expect(conventionalCommits[0].type).to.equal('docs'); - // expect(conventionalCommits[0].scope).is.null; - // }); + describe('with reverted commits', () => { + it('ignores a commit and its standard Git revert', async () => { + const commits = [ + { + sha: '1111111111111111111111111111111111111111', + message: 'feat: some feature', + files: ['path1/file1.txt'], + }, + { + sha: '2222222222222222222222222222222222222222', + message: + 'revert: feat: some feature\n\nThis reverts commit 1111111111111111111111111111111111111111.\n', + files: ['path1/file1.txt'], + }, + { + sha: '3333333333333333333333333333333333333333', + message: 'docs: some documentation', + files: ['path1/file1.java'], + }, + ]; + + const conventionalCommits = parseConventionalCommits(commits); + + expect(conventionalCommits).lengthOf(1); + expect(conventionalCommits[0].type).to.equal('docs'); + expect(conventionalCommits[0].scope).is.null; + }); + + it('supports an explicit Revert footer with an abbreviated SHA', async () => { + const commits = [ + { + sha: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + message: 'fix: an unreleased fix', + }, + { + sha: 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + message: 'revert: undo the fix\n\nRevert: aaaaaaa', + }, + ]; + + expect(parseConventionalCommits(commits)).to.be.empty; + }); + + it('keeps a revert when its target is outside the commit range', async () => { + const commits = [ + { + sha: 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + message: + 'revert: undo a released feature\n\nThis reverts commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.', + }, + ]; + + const conventionalCommits = parseConventionalCommits(commits); + + expect(conventionalCommits).lengthOf(1); + expect(conventionalCommits[0].type).to.equal('revert'); + }); + + it('does not match an ambiguous abbreviated SHA', async () => { + const commits = [ + { + sha: 'abcdef0111111111111111111111111111111111', + message: 'feat: first feature', + }, + { + sha: 'abcdef0222222222222222222222222222222222', + message: 'feat: second feature', + }, + { + sha: '3333333333333333333333333333333333333333', + message: 'revert: unclear target\n\nRevert: abcdef0', + }, + ]; + + expect(parseConventionalCommits(commits)).lengthOf(3); + }); + + it('restores a commit when its revert is itself reverted', async () => { + const commits = [ + { + sha: '1111111111111111111111111111111111111111', + message: 'feat: some feature', + }, + { + sha: '2222222222222222222222222222222222222222', + message: + 'revert: feat: some feature\n\nThis reverts commit 1111111111111111111111111111111111111111.', + }, + { + sha: '3333333333333333333333333333333333333333', + message: + 'revert: restore the feature\n\nThis reverts commit 2222222222222222222222222222222222222222.', + }, + ]; + + const conventionalCommits = parseConventionalCommits(commits); + + expect(conventionalCommits).lengthOf(1); + expect(conventionalCommits[0].type).to.equal('feat'); + expect(conventionalCommits[0].bareMessage).to.equal('some feature'); + }); + }); }); function assertHasCommit( diff --git a/test/versioning-strategies/default.ts b/test/versioning-strategies/default.ts index 9b0b7a623..ed0e752d4 100644 --- a/test/versioning-strategies/default.ts +++ b/test/versioning-strategies/default.ts @@ -15,10 +15,34 @@ import {describe, it} from 'mocha'; import {expect} from 'chai'; +import {parseConventionalCommits} from '../../src/commit'; import {DefaultVersioningStrategy} from '../../src/versioning-strategies/default'; import {Version} from '../../src/version'; describe('DefaultVersioningStrategy', () => { + it('does not bump for an unreleased feature that was reverted', async () => { + const commits = parseConventionalCommits([ + { + sha: '1111111111111111111111111111111111111111', + message: 'feat: some feature', + }, + { + sha: '2222222222222222222222222222222222222222', + message: + 'revert: feat: some feature\n\nThis reverts commit 1111111111111111111111111111111111111111.', + }, + { + sha: '3333333333333333333333333333333333333333', + message: 'fix: some bugfix', + }, + ]); + const strategy = new DefaultVersioningStrategy(); + + const newVersion = await strategy.bump(Version.parse('1.2.3'), commits); + + expect(newVersion.toString()).to.equal('1.2.4'); + }); + describe('with breaking change', () => { const commits = [ {