Fix behavior of group exclude option (v3)

.vscode/launch.json

@@ -15,6 +15,7 @@
       "args": ["--", "--runInBand", "--watch", "--testTimeout=1000000", "${file}"],
       "sourceMaps": true,
       "outputCapture": "std",
+      // "runtimeVersion": "22",
       "console": "integratedTerminal"
     },
     {

change/beachball-9c18f781-7c73-4b14-adb7-a5fc6930628a.json

@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "Fix behavior of `VersionGroupOptions.exclude` and `ChangelogGroupOptions.exclude`: if a package path **matches** any `exclude` pattern, it's excluded, rather than requiring negation. (To migrate, just remove the leading `!` from your `exclude` patterns.)",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}

change/change-085686e8-22a7-4276-be7f-2335a0f8f8e5.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "type": "major",
+      "comment": "Remove deprecated `ChangelogGroupOptions.masterPackageName` (use `mainPackageName`)",
+      "packageName": "beachball",
+      "email": "elcraig@microsoft.com",
+      "dependentChangeType": "patch"
+    }
+  ]
+}

docs/cli/options.md

@@ -7,7 +7,7 @@ category: doc
 
 # Beachball CLI options
 
-For the latest full list of supported options, see `CliOptions` [in this file](https://github.com/microsoft/beachball/blob/main/src/types/BeachballOptions.ts).
+For the latest full list of supported options, see `CliOptions` [in this file](https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/BeachballOptions.ts).
 
 **Most options can also be specified in the [configuration file](../overview/configuration)**, which is generally preferable as it's easier to read and maintain.
 

docs/concepts/groups.md

@@ -26,27 +26,28 @@ For cases where it's necessary to bump packages together, `beachball` also provi
 
 ### Configuring version groups
 
-Groups can be added to the [configuration file](../overview/configuration). See the [`VersionGroupOptions` source](https://github.com/microsoft/beachball/blob/main/src/types/ChangelogOptions.ts) for full details.
+Groups can be added to the [configuration file](../overview/configuration). See the [`VersionGroupOptions` source](https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/ChangelogOptions.ts) for full details.
 
-| Name                    | Type                         | Description                                                                                                                                             |
-| ----------------------- | ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `name`                  | `string`                     | Name of the version group                                                                                                                               |
-| `include`               | `string \| string[] \| true` | glob pattern(s) for package paths to include (see [notes on globs][1]). If `true`, include all packages except those matching `exclude`.                |
-| `exclude`               | `string \| string[]`         | glob pattern(s) for package paths to exclude (see [notes on globs][1]). This currently must use **negated patterns only** (will be fixed in version 3). |
-| `disallowedChangeTypes` | `ChangeType[] \| null`       | Disallow these change types for the group.                                                                                                              |
+<!-- prettier-ignore -->
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| `name` | `string` | Name of the version group |
+| `include` | `string \| string[] \| true` | glob pattern(s) for package paths to include (see [notes on globs][1]). If `true`, include all packages except those matching `exclude`. |
+| `exclude` | `string \| string[]` | glob pattern(s) for package paths to exclude (see [notes on globs][1]). |
+| `disallowedChangeTypes` | `ChangeType[] \| null` | Disallow these change types for the group. |
 
 Example:
 
-```jsonc
+```json
 {
   "groups": [
     {
       "name": "group name",
       "include": ["packages/groupfoo/*"],
-      "exclude": ["!packages/groupfoo/bar"],
-      "disallowedChangeTypes": ["major"],
-    },
-  ],
+      "exclude": ["packages/groupfoo/bar"],
+      "disallowedChangeTypes": ["major"]
+    }
+  ]
 }
 ```
 
@@ -56,26 +57,27 @@ If you only want to publish or record changes for certain packages, you should u
 
 ## Grouped changelogs
 
-To show changes for multiple packages in one change file, use the `changelog.groups` option. See the [`ChangelogGroupOptions` source](https://github.com/microsoft/beachball/blob/main/src/types/ChangelogOptions.ts) for full details.
+To show changes for multiple packages in one change file, use the `changelog.groups` option. See the [`ChangelogGroupOptions` source](https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/ChangelogOptions.ts) for full details.
 
-| Name                | Type                         | Description                                                                                                                                             |
-| ------------------- | ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `masterPackageName` | `string`                     | The main package which a group of changes bubbles up to.                                                                                                |
-| `include`           | `string \| string[] \| true` | glob pattern(s) for package paths to include (see [notes on globs][1]). If `true`, include all packages except those matching `exclude`.                |
-| `exclude`           | `string \| string[]`         | glob pattern(s) for package paths to exclude (see [notes on globs][1]). This currently must use **negated patterns only** (will be fixed in version 3). |
-| `changelogPath`     | `string`                     | Put the grouped changelog file under this directory. Can be relative to the root, or absolute.                                                          |
+<!-- prettier-ignore -->
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| `mainPackageName` | `string` | The main package which a group of changes bubbles up to. |
+| `include` | `string \| string[] \| true` | glob pattern(s) for package paths to include (see [notes on globs][1]). If `true`, include all packages except those matching `exclude`. |
+| `exclude` | `string \| string[]` | glob pattern(s) for package paths to exclude (see [notes on globs][1]). |
+| `changelogPath` | `string` | Put the grouped changelog file under this directory. Can be relative to the root, or absolute. |
 
-In this example, changelogs for all packages under `packages/*` (except `packages/baz`) are written to a `CHANGELOG.md` at the repo root (`.`), with `foo` as the master package. (To replace `foo`'s usual changelog with a grouped one, you'd specify `changelogPath` as the path to `foo` instead, e.g. `packages/foo`.)
+In this example, changelogs for all packages under `packages/*` (except `packages/baz`) are written to a `CHANGELOG.md` at the repo root (`.`), with `foo` as the main package. (To replace `foo`'s usual changelog with a grouped one, you'd specify `changelogPath` as the path to `foo` instead, e.g. `packages/foo`.)
 
 ```json
 {
   "changelog": {
     "groups": [
       {
-        "masterPackageName": "foo",
+        "mainPackageName": "foo",
         "changelogPath": ".",
         "include": ["packages/*"],
-        "exclude": ["!packages/baz"]
+        "exclude": ["packages/baz"]
       }
     ]
   }

docs/overview/configuration.md

@@ -67,7 +67,7 @@ To change the `disallowedChangeTypes` for package `foo`, you could add the follo
 
 ## Options
 
-For the latest full list of supported options, see `RepoOptions` [in this file](https://github.com/microsoft/beachball/blob/main/src/types/BeachballOptions.ts).
+For the latest full list of supported options, see `RepoOptions` [in this file](https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/BeachballOptions.ts).
 
 "Applies to" indicates where the settings can be specified: repo-level config or package-level config.
 
@@ -105,10 +105,10 @@ For the latest full list of supported options, see `RepoOptions` [in this file](
 | `tag` | `string` | `'latest'` | repo, package | `dist-tag` for npm when published |
 | `transform` | [`TransformOptions`][4] | | repo | Transformations for change files |
 
-[1]: https://github.com/microsoft/beachball/blob/main/src/types/ChangeFilePrompt.ts
-[2]: https://github.com/microsoft/beachball/blob/main/src/types/ChangelogOptions.ts
+[1]: https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/ChangeFilePrompt.ts
+[2]: https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/ChangelogOptions.ts
 [3]: ../concepts/groups#version-groups
-[4]: https://github.com/microsoft/beachball/blob/main/src/types/BeachballOptions.ts
+[4]: https://github.com/microsoft/beachball/blob/main/packages/beachball/src/types/BeachballOptions.ts
 [5]: #specifying-the-target-branch-and-remote
 [6]: #glob-matching
 

docs/overview/v3-migration.md

@@ -8,13 +8,9 @@ category: doc
 
 This page describes how to migrate from beachball v2 to v3.
 
-## Changelog
-
-For the full list of changes between v2 and v3, see the [beachball CHANGELOG.md](https://github.com/microsoft/beachball/blob/main/packages/beachball/CHANGELOG.md).
-
 ## Running the migrate command
 
-beachball v3 includes a `migrate` command that checks your config and logs any updates needed for v3:
+beachball v3 includes a `migrate` command that **checks your config and logs any updates needed** for v3:
 
 ```bash
 beachball migrate
@@ -26,4 +22,20 @@ If your config is already compatible, you will see:
 No config updates are needed for v3.
 ```
 
-Otherwise, the command will list specific config updates that are needed. The command doesn't attempt to make updates directly due to the variety of locations and file types where the config can be specified.
+Otherwise, the command will list specific config updates that are needed. The command does NOT attempt to make updates directly due to the variety of locations and file types where the config can be specified.
+
+## Breaking changes
+
+For the full list of changes between v2 and v3, see the [beachball CHANGELOG.md](https://github.com/microsoft/beachball/blob/main/packages/beachball/CHANGELOG.md).
+
+<!-- TODO: go over full list of major changes before release -->
+
+### Fix group `exclude` negation behavior
+
+Remove the requirement for `groups[*].exclude` and `changelog.groups[*].exclude` patterns to be negated (leading `!`).
+
+To migrate, simply remove the leading `!` from all `exclude` patterns.
+
+### Rename `changelog.groups[*].masterPackageName` to `mainPackageName`
+
+To migrate, find and replace `masterPackageName` to `mainPackageName`.

packages/beachball/src/__functional__/commands/migrate.test.ts

@@ -2,12 +2,23 @@ import { describe, expect, it, beforeAll, afterAll } from '@jest/globals';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { RepositoryFactory } from '../../__fixtures__/repositoryFactory';
 import { migrate } from '../../commands/migrate';
+import type { BeachballOptions } from '../../types/BeachballOptions';
+import { getDefaultOptions } from '../../options/getDefaultOptions';
+import { BeachballError } from '../../types/BeachballError';
+import type { ChangelogGroupOptions } from '../../types/ChangelogOptions';
 
 describe('migrate command', () => {
   const logs = initMockLogs();
 
   let repositoryFactory: RepositoryFactory;
 
+  function migrateWrapper(options: Partial<BeachballOptions>) {
+    return migrate({
+      ...getDefaultOptions(),
+      ...options,
+    });
+  }
+
   beforeAll(() => {
     repositoryFactory = new RepositoryFactory('single');
   });
@@ -17,8 +28,115 @@ describe('migrate command', () => {
   });
 
   it('logs a success message when no config updates are needed', () => {
-    const repo = repositoryFactory.cloneRepository();
-    migrate({ path: repo.rootPath });
-    expect(logs.getMockLines('log')).toMatchInlineSnapshot(`"No config updates are needed for v3."`);
+    migrateWrapper({
+      groups: [{ name: 'test', include: 'packages/test', exclude: ['packages/foo'], disallowedChangeTypes: null }],
+      changelog: {
+        groups: [
+          {
+            mainPackageName: 'test',
+            include: ['packages/test'],
+            exclude: ['packages/bar'],
+            changelogPath: 'packages/test',
+          },
+        ],
+      },
+    });
+    expect(logs.getMockLines('log')).toEqual('No config updates are needed for v3.');
+  });
+
+  it('logs an error for negated groups[*].exclude', () => {
+    const disallowedChangeTypes = null;
+    expect(() =>
+      migrateWrapper({
+        groups: [
+          // the group globs here don't need to make sense; just verify it only checks ! at beginning
+          { name: 'ok', include: true, exclude: 'packages/!(bar)', disallowedChangeTypes },
+          { name: 'badstring', include: true, exclude: '!packages/foo', disallowedChangeTypes },
+          {
+            name: 'badarray',
+            include: true,
+            exclude: ['packages/bar', '!packages/foo', '!packages/baz'],
+            disallowedChangeTypes,
+          },
+        ],
+      })
+    ).toThrow(BeachballError);
+
+    expect(logs.getMockLines('error')).toMatchInlineSnapshot(`
+      "The following updates are needed for v3:
+        • \`groups\`
+          ▪ Group "badstring"
+            ◦ Remove the leading "!" from these \`exclude\` patterns:
+              ▫ !packages/foo
+          ▪ Group "badarray"
+            ◦ Remove the leading "!" from these \`exclude\` patterns:
+              ▫ !packages/foo
+              ▫ !packages/baz"
+    `);
+  });
+
+  it('logs an error for changelog.groups[*].masterPackageName', () => {
+    expect(() =>
+      migrateWrapper({
+        changelog: {
+          groups: [
+            { masterPackageName: 'test1', changelogPath: '', include: true } as unknown as ChangelogGroupOptions,
+            { mainPackageName: 'test2', changelogPath: '', include: true },
+            { masterPackageName: 'test3', changelogPath: '', include: true } as unknown as ChangelogGroupOptions,
+          ],
+        },
+      })
+    ).toThrow(BeachballError);
+
+    expect(logs.getMockLines('error')).toMatchInlineSnapshot(`
+      "The following updates are needed for v3:
+        • \`changelog.groups\`
+          ▪ Group for package "test1"
+            ◦ Rename \`masterPackageName\` to \`mainPackageName\`
+          ▪ Group for package "test3"
+            ◦ Rename \`masterPackageName\` to \`mainPackageName\`"
+    `);
+  });
+
+  it('logs an error for negated changelog.groups[*].exclude and masterPackageName', () => {
+    expect(() =>
+      migrateWrapper({
+        changelog: {
+          groups: [
+            {
+              masterPackageName: 'test',
+              include: true,
+              exclude: ['!packages/bar', '!packages/baz'],
+              changelogPath: '',
+            } as Partial<ChangelogGroupOptions> as ChangelogGroupOptions,
+            {
+              mainPackageName: 'test2',
+              include: true,
+              exclude: '!packages/foo',
+              changelogPath: '',
+            },
+            {
+              mainPackageName: 'test3',
+              include: true,
+              exclude: 'packages/!(bar)',
+              changelogPath: '',
+            },
+          ],
+        },
+      })
+    ).toThrow(BeachballError);
+
+    expect(logs.getMockLines('error')).toMatchInlineSnapshot(`
+      "The following updates are needed for v3:
+        • \`changelog.groups\`
+          ▪ Group for package "test"
+            ◦ Rename \`masterPackageName\` to \`mainPackageName\`
+            ◦ Remove the leading "!" from these \`exclude\` patterns:
+              ▫ !packages/bar
+              ▫ !packages/baz
+          ▪ Group for package "test2"
+            ◦ Remove the leading "!" from these \`exclude\` patterns:
+              ▫ !packages/foo"
+    `);
   });
 });

packages/beachball/src/__tests__/monorepo/getPackageGroups.test.ts

@@ -117,7 +117,7 @@ describe('getPackageGroups', () => {
       },
       repoOptions: {
         groups: [
-          { name: 'group', include: ['packages/*'], exclude: ['!packages/internal'], disallowedChangeTypes: null },
+          { name: 'group', include: ['packages/*'], exclude: ['packages/internal'], disallowedChangeTypes: null },
         ],
       },
     });
@@ -139,7 +139,7 @@ describe('getPackageGroups', () => {
           {
             name: 'group1',
             include: ['packages/**/*'],
-            exclude: ['!packages/core/*'],
+            exclude: ['packages/core/*'],
             disallowedChangeTypes: null,
           },
         ],

packages/beachball/src/__tests__/monorepo/isPathIncluded.test.ts

@@ -3,34 +3,43 @@ import { isPathIncluded } from '../../monorepo/isPathIncluded';
 
 describe('isPathIncluded', () => {
   it('returns true if path is included (single include path)', () => {
-    expect(isPathIncluded('packages/a', 'packages/*')).toBeTruthy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: 'packages/*' })).toBeTruthy();
   });
 
-  it('returns false if path is excluded (single exclude path)', () => {
-    expect(isPathIncluded('packages/a', 'packages/*', '!packages/a')).toBeFalsy();
+  it('returns false if path is not included, with single include path', () => {
+    expect(isPathIncluded({ relativePath: 'stuff/b', include: 'packages/*' })).toBeFalsy();
+    expect(isPathIncluded({ relativePath: 'packages/b', include: 'packages/!(b)' })).toBeFalsy();
   });
 
-  it('returns true if path is included (multiple include paths)', () => {
-    expect(isPathIncluded('packages/a', ['packages/b', 'packages/a'], ['!packages/b'])).toBeTruthy();
+  it('returns false if path is excluded, with single exclude path', () => {
+    expect(isPathIncluded({ relativePath: 'packages/a', include: 'packages/*', exclude: 'packages/a' })).toBeFalsy();
   });
 
-  it('returns false if path is excluded (multiple exclude paths)', () => {
-    expect(isPathIncluded('packages/a', ['packages/*'], ['!packages/a', '!packages/b'])).toBeFalsy();
+  it('returns true if path is included, with multiple include paths', () => {
+    expect(
+      isPathIncluded({ relativePath: 'packages/a', include: ['packages/b', 'packages/a'], exclude: ['packages/b'] })
+    ).toBeTruthy();
+  });
+
+  it('returns false if path is excluded, with multiple exclude paths', () => {
+    expect(
+      isPathIncluded({ relativePath: 'packages/a', include: ['packages/*'], exclude: ['packages/a'] })
+    ).toBeFalsy();
   });
 
   it('returns true if include is true (no exclude paths)', () => {
-    expect(isPathIncluded('packages/a', true)).toBeTruthy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: true })).toBeTruthy();
   });
 
   it('returns false if include is true and path is excluded', () => {
-    expect(isPathIncluded('packages/a', true, '!packages/a')).toBeFalsy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: true, exclude: 'packages/a' })).toBeFalsy();
   });
 
   it('returns false if include path is empty', () => {
-    expect(isPathIncluded('packages/a', '')).toBeFalsy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: '' })).toBeFalsy();
   });
 
   it('ignores empty exclude path array', () => {
-    expect(isPathIncluded('packages/a', 'packages/*', [])).toBeTruthy();
+    expect(isPathIncluded({ relativePath: 'packages/a', include: 'packages/*', exclude: [] })).toBeTruthy();
   });
 });