diff --git a/dev-infra/commit-message/config.ts b/dev-infra/commit-message/config.ts index 2d9739cecf..9183e0c9ba 100644 --- a/dev-infra/commit-message/config.ts +++ b/dev-infra/commit-message/config.ts @@ -12,7 +12,6 @@ export interface CommitMessageConfig { maxLineLength: number; minBodyLength: number; minBodyLengthTypeExcludes?: string[]; - types: string[]; scopes: string[]; } @@ -30,3 +29,46 @@ export function getCommitMessageConfig() { assertNoErrors(errors); return config as Required; } + +/** Scope requirement level to be set for each commit type. */ +export enum ScopeRequirement { + Required, + Optional, + Forbidden, +} + +/** A commit type */ +export interface CommitType { + scope: ScopeRequirement; +} + +/** The valid commit types for Angular commit messages. */ +export const COMMIT_TYPES: {[key: string]: CommitType} = { + build: { + scope: ScopeRequirement.Forbidden, + }, + ci: { + scope: ScopeRequirement.Forbidden, + }, + docs: { + scope: ScopeRequirement.Optional, + }, + feat: { + scope: ScopeRequirement.Required, + }, + fix: { + scope: ScopeRequirement.Required, + }, + perf: { + scope: ScopeRequirement.Required, + }, + refactor: { + scope: ScopeRequirement.Required, + }, + release: { + scope: ScopeRequirement.Forbidden, + }, + test: { + scope: ScopeRequirement.Required, + }, +}; diff --git a/dev-infra/commit-message/validate.spec.ts b/dev-infra/commit-message/validate.spec.ts index edc0bab802..238a7909a9 100644 --- a/dev-infra/commit-message/validate.spec.ts +++ b/dev-infra/commit-message/validate.spec.ts @@ -18,13 +18,6 @@ const config: {commitMessage: CommitMessageConfig} = { commitMessage: { maxLineLength: 120, minBodyLength: 0, - types: [ - 'feat', - 'fix', - 'refactor', - 'release', - 'style', - ], scopes: [ 'common', 'compiler', @@ -33,7 +26,7 @@ const config: {commitMessage: CommitMessageConfig} = { ] } }; -const TYPES = config.commitMessage.types.join(', '); +const TYPES = Object.keys(validateConfig.COMMIT_TYPES).join(', '); const SCOPES = config.commitMessage.scopes.join(', '); const INVALID = false; const VALID = true; @@ -47,7 +40,8 @@ describe('validate-commit-message.js', () => { lastError = ''; spyOn(console, 'error').and.callFake((msg: string) => lastError = msg); - spyOn(validateConfig, 'getCommitMessageConfig').and.returnValue(config); + spyOn(validateConfig, 'getCommitMessageConfig') + .and.returnValue(config as ReturnType); }); describe('validateMessage()', () => { @@ -55,16 +49,16 @@ describe('validate-commit-message.js', () => { expect(validateCommitMessage('feat(packaging): something')).toBe(VALID); expect(lastError).toBe(''); - expect(validateCommitMessage('release(packaging): something')).toBe(VALID); + expect(validateCommitMessage('fix(packaging): something')).toBe(VALID); expect(lastError).toBe(''); - expect(validateCommitMessage('fixup! release(packaging): something')).toBe(VALID); + expect(validateCommitMessage('fixup! fix(packaging): something')).toBe(VALID); expect(lastError).toBe(''); - expect(validateCommitMessage('squash! release(packaging): something')).toBe(VALID); + expect(validateCommitMessage('squash! fix(packaging): something')).toBe(VALID); expect(lastError).toBe(''); - expect(validateCommitMessage('Revert: "release(packaging): something"')).toBe(VALID); + expect(validateCommitMessage('Revert: "fix(packaging): something"')).toBe(VALID); expect(lastError).toBe(''); }); @@ -110,8 +104,8 @@ describe('validate-commit-message.js', () => { expect(validateCommitMessage('feat(bah): something')).toBe(INVALID); expect(lastError).toContain(errorMessageFor('bah', 'feat(bah): something')); - expect(validateCommitMessage('style(webworker): something')).toBe(INVALID); - expect(lastError).toContain(errorMessageFor('webworker', 'style(webworker): something')); + expect(validateCommitMessage('fix(webworker): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('webworker', 'fix(webworker): something')); expect(validateCommitMessage('refactor(security): something')).toBe(INVALID); expect(lastError).toContain(errorMessageFor('security', 'refactor(security): something')); @@ -119,12 +113,12 @@ describe('validate-commit-message.js', () => { expect(validateCommitMessage('refactor(docs): something')).toBe(INVALID); expect(lastError).toContain(errorMessageFor('docs', 'refactor(docs): something')); - expect(validateCommitMessage('release(angular): something')).toBe(INVALID); - expect(lastError).toContain(errorMessageFor('angular', 'release(angular): something')); + expect(validateCommitMessage('feat(angular): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('angular', 'feat(angular): something')); }); it('should allow empty scope', () => { - expect(validateCommitMessage('fix: blablabla')).toBe(VALID); + expect(validateCommitMessage('build: blablabla')).toBe(VALID); expect(lastError).toBe(''); }); @@ -243,7 +237,6 @@ describe('validate-commit-message.js', () => { maxLineLength: 120, minBodyLength: 30, minBodyLengthTypeExcludes: ['docs'], - types: ['fix', 'docs'], scopes: ['core'] } }; diff --git a/dev-infra/commit-message/validate.ts b/dev-infra/commit-message/validate.ts index fdc2f006c4..1c10ada900 100644 --- a/dev-infra/commit-message/validate.ts +++ b/dev-infra/commit-message/validate.ts @@ -7,7 +7,7 @@ */ import {error} from '../utils/console'; -import {getCommitMessageConfig} from './config'; +import {COMMIT_TYPES, getCommitMessageConfig, ScopeRequirement} from './config'; import {parseCommitMessage} from './parse'; /** Options for commit message validation. */ @@ -86,8 +86,26 @@ export function validateCommitMessage( return false; } - if (!config.types.includes(commit.type)) { - printError(`'${commit.type}' is not an allowed type.\n => TYPES: ${config.types.join(', ')}`); + + + if (COMMIT_TYPES[commit.type] === undefined) { + printError(`'${commit.type}' is not an allowed type.\n => TYPES: ${ + Object.keys(COMMIT_TYPES).join(', ')}`); + return false; + } + + /** The scope requirement level for the provided type of the commit message. */ + const scopeRequirementForType = COMMIT_TYPES[commit.type].scope; + + if (scopeRequirementForType === ScopeRequirement.Forbidden && commit.scope) { + printError(`Scopes are forbidden for commits with type '${commit.type}', but a scope of '${ + commit.scope}' was provided.`); + return false; + } + + if (scopeRequirementForType === ScopeRequirement.Required && !commit.scope) { + printError( + `Scopes are required for commits with type '${commit.type}', but no scope was provided.`); return false; }