Fix issues flagged by lint rules (#1017)

microsoft · Nov 25, 2024 · 29103cf · 29103cf
1 parent 34e521c
commit 29103cf
Show file tree

Hide file tree

Showing 67 changed files with 257 additions and 221 deletions.
diff --git a/change/beachball-e300f8e4-e881-44c4-b266-4150412232fe.json b/change/beachball-e300f8e4-e881-44c4-b266-4150412232fe.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Fix issues found by lint rules",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/src/__e2e__/bump.test.ts b/src/__e2e__/bump.test.ts
@@ -10,6 +10,7 @@ import { getPackageInfos } from '../monorepo/getPackageInfos';
 import { BeachballOptions, HooksOptions } from '../types/BeachballOptions';
 import type { Repository } from '../__fixtures__/repository';
 import { getDefaultOptions } from '../options/getDefaultOptions';
+import type { PackageJson } from '../types/PackageInfo';
 
 describe('version bumping', () => {
   let repositoryFactory: RepositoryFactory | undefined;
@@ -277,7 +278,7 @@ describe('version bumping', () => {
 
   it('should not bump out-of-scope package even if package has change', async () => {
     repositoryFactory = new RepositoryFactory('monorepo');
-    const monorepo = repositoryFactory.fixture.folders!;
+    const monorepo = repositoryFactory.fixture.folders;
     repo = repositoryFactory.cloneRepository();
 
     const options = getOptions({
@@ -299,7 +300,7 @@ describe('version bumping', () => {
 
   it('should not bump out-of-scope package and its dependencies even if dependency of the package has change', async () => {
     repositoryFactory = new RepositoryFactory('monorepo');
-    const monorepo = repositoryFactory.fixture.folders!;
+    const monorepo = repositoryFactory.fixture.folders;
     repo = repositoryFactory.cloneRepository();
 
     const options = getOptions({
@@ -622,7 +623,7 @@ describe('version bumping', () => {
           expect(version).toBe('1.1.0');
 
           const jsonPath = path.join(packagePath, 'package.json');
-          expect(fs.readJSONSync(jsonPath).version).toBe('1.0.0');
+          expect((fs.readJSONSync(jsonPath) as PackageJson).version).toBe('1.0.0');
         }),
       },
     });
@@ -652,7 +653,7 @@ describe('version bumping', () => {
           expect(version).toBe('1.1.0');
 
           const jsonPath = path.join(packagePath, 'package.json');
-          expect((await fs.readJSON(jsonPath)).version).toBe('1.0.0');
+          expect(((await fs.readJSON(jsonPath)) as PackageJson).version).toBe('1.0.0');
         }),
       },
     });
@@ -677,7 +678,7 @@ describe('version bumping', () => {
       path: repo.rootPath,
       bumpDeps: false,
       hooks: {
-        prebump: async (_packagePath, _name, _version): Promise<void> => {
+        prebump: (): Promise<void> => {
           throw new Error('Foo');
         },
       },
@@ -708,7 +709,7 @@ describe('version bumping', () => {
           expect(version).toBe('1.1.0');
 
           const jsonPath = path.join(packagePath, 'package.json');
-          expect(fs.readJSONSync(jsonPath).version).toBe('1.1.0');
+          expect((fs.readJSONSync(jsonPath) as PackageJson).version).toBe('1.1.0');
         }),
       },
     });
@@ -738,7 +739,7 @@ describe('version bumping', () => {
           expect(version).toBe('1.1.0');
 
           const jsonPath = path.join(packagePath, 'package.json');
-          expect((await fs.readJSON(jsonPath)).version).toBe('1.1.0');
+          expect(((await fs.readJSON(jsonPath)) as PackageJson).version).toBe('1.1.0');
         }),
       },
     });
@@ -762,7 +763,7 @@ describe('version bumping', () => {
     const options = getOptions({
       bumpDeps: false,
       hooks: {
-        postbump: async (_packagePath, _name, _version): Promise<void> => {
+        postbump: (): Promise<void> => {
           throw new Error('Foo');
         },
       },

diff --git a/src/__e2e__/change.test.ts b/src/__e2e__/change.test.ts
@@ -9,7 +9,7 @@ import { BeachballOptions } from '../types/BeachballOptions';
 import { defaultBranchName } from '../__fixtures__/gitDefaults';
 import { MockStdout } from '../__fixtures__/mockStdout';
 import { MockStdin } from '../__fixtures__/mockStdin';
-import { ChangeFileInfo } from '../types/ChangeInfo';
+import type { ChangeFileInfo, ChangeInfoMultiple } from '../types/ChangeInfo';
 import { Repository } from '../__fixtures__/repository';
 import { getDefaultOptions } from '../options/getDefaultOptions';
 
@@ -23,7 +23,7 @@ jest.mock(
     ((questions, options) => {
       questions = Array.isArray(questions) ? questions : [questions];
       questions = questions.map(q => ({ ...q, stdin, stdout }));
-      return (jest.requireActual('prompts') as typeof prompts)(questions, options);
+      return jest.requireActual<typeof prompts>('prompts')(questions, options);
     }) as typeof prompts
 );
 
@@ -35,7 +35,9 @@ jest.mock(
 let mockBeachballOptions: Partial<BeachballOptions> | undefined;
 jest.mock('../options/getDefaultOptions', () => ({
   getDefaultOptions: () => ({
-    ...(jest.requireActual('../options/getDefaultOptions') as any).getDefaultOptions(),
+    ...jest
+      .requireActual<typeof import('../options/getDefaultOptions')>('../options/getDefaultOptions')
+      .getDefaultOptions(),
     ...mockBeachballOptions,
   }),
 }));
@@ -192,7 +194,7 @@ describe('change command', () => {
     repo = repositoryFactory.cloneRepository();
 
     const options = getOptions({
-      package: repositoryFactory.fixture.rootPackage!.name,
+      package: repositoryFactory.fixture.rootPackage.name,
       commit: false,
     });
     const changePromise = change(options);
@@ -240,7 +242,7 @@ describe('change command', () => {
 
     const changeFiles = getChangeFiles(options);
     expect(changeFiles).toHaveLength(2);
-    const changeFileContents = changeFiles.map(changeFile => fs.readJSONSync(changeFile)) as ChangeFileInfo[];
+    const changeFileContents = changeFiles.map(changeFile => fs.readJSONSync(changeFile) as ChangeFileInfo);
     expect(changeFileContents).toContainEqual(
       expect.objectContaining({ comment: 'custom', packageName: 'pkg-1', type: 'minor' })
     );
@@ -276,7 +278,7 @@ describe('change command', () => {
 
     const changeFiles = getChangeFiles(options);
     expect(changeFiles).toHaveLength(1);
-    const contents = fs.readJSONSync(changeFiles[0]);
+    const contents = fs.readJSONSync(changeFiles[0]) as ChangeInfoMultiple;
     expect(contents.changes).toEqual([
       expect.objectContaining({ comment: 'custom', packageName: 'pkg-1', type: 'minor' }),
       expect.objectContaining({ comment: 'commit 2', packageName: 'pkg-2', type: 'patch' }),
@@ -323,7 +325,7 @@ describe('change command', () => {
 
     const changeFiles = getChangeFiles(options);
     expect(changeFiles).toHaveLength(1);
-    const contents = fs.readJSONSync(changeFiles[0]);
+    const contents = fs.readJSONSync(changeFiles[0]) as ChangeInfoMultiple;
     expect(contents.changes).toEqual([
       expect.objectContaining({ packageName: 'pkg-1', type: 'patch', comment: 'commit 2' }),
       expect.objectContaining({ packageName: 'pkg-2', type: 'patch', comment: 'commit 2', custom: 'stuff' }),

diff --git a/src/__e2e__/publishE2E.test.ts b/src/__e2e__/publishE2E.test.ts
@@ -12,6 +12,7 @@ import { publish } from '../commands/publish';
 import { getDefaultOptions } from '../options/getDefaultOptions';
 import { BeachballOptions } from '../types/BeachballOptions';
 import { _mockNpmPublish, initNpmMock } from '../__fixtures__/mockNpm';
+import type { PackageJson } from '../types/PackageInfo';
 
 // Spawning actual npm to run commands against a fake registry is extremely slow, so mock it for
 // this test (packagePublish covers the more complete npm registry scenario).
@@ -109,7 +110,7 @@ describe('publish command (e2e)', () => {
     // Adds a step that injects a race condition
     let fetchCount = 0;
 
-    addGitObserver((args, output) => {
+    addGitObserver(args => {
       if (args[0] === 'fetch') {
         if (fetchCount === 0) {
           const anotherRepo = repositoryFactory!.cloneRepository();
@@ -150,14 +151,14 @@ describe('publish command (e2e)', () => {
     // Adds a step that injects a race condition
     let fetchCount = 0;
 
-    addGitObserver((args, output) => {
+    addGitObserver(args => {
       if (args[0] === 'fetch') {
         if (fetchCount === 0) {
           const anotherRepo = repositoryFactory!.cloneRepository();
           // inject a checkin
           const packageJsonFile = anotherRepo.pathTo('package.json');
-          const contents = fs.readJSONSync(packageJsonFile, 'utf-8');
-          delete contents.dependencies.baz;
+          const contents = fs.readJSONSync(packageJsonFile, 'utf-8') as PackageJson;
+          delete contents.dependencies?.baz;
           anotherRepo.commitChange('package.json', JSON.stringify(contents, null, 2));
           anotherRepo.push();
         }
@@ -182,8 +183,8 @@ describe('publish command (e2e)', () => {
     expect(fetchCount).toBe(2);
 
     const packageJsonFile = repo.pathTo('package.json');
-    const contents = JSON.parse(fs.readFileSync(packageJsonFile, 'utf-8'));
-    expect(contents.dependencies.baz).toBeUndefined();
+    const contents = JSON.parse(fs.readFileSync(packageJsonFile, 'utf-8')) as PackageJson;
+    expect(contents.dependencies?.baz).toBeUndefined();
   });
 
   it('can perform a successful npm publish without bump', async () => {
@@ -241,8 +242,8 @@ describe('publish command (e2e)', () => {
 
     // bump baz => dependent bump bar => dependent bump foo
     generateChangeFiles(['baz'], options);
-    expect(repositoryFactory.fixture.folders!.packages.foo.dependencies!.bar).toBeTruthy();
-    expect(repositoryFactory.fixture.folders!.packages.bar.dependencies!.baz).toBeTruthy();
+    expect(repositoryFactory.fixture.folders.packages.foo.dependencies!.bar).toBeTruthy();
+    expect(repositoryFactory.fixture.folders.packages.bar.dependencies!.baz).toBeTruthy();
     repo.push();
 
     await publish(options);
@@ -329,11 +330,13 @@ describe('publish command (e2e)', () => {
     repositoryFactory = new RepositoryFactory('monorepo');
     repo = repositoryFactory.cloneRepository();
 
+    type ExtraPackageJson = PackageJson & { onPublish?: { main: string } };
+
     const options = getOptions({
       hooks: {
         prepublish: (packagePath: string) => {
           const packageJsonPath = path.join(packagePath, 'package.json');
-          const packageJson = fs.readJSONSync(packageJsonPath);
+          const packageJson = fs.readJSONSync(packageJsonPath) as ExtraPackageJson;
           if (packageJson.onPublish) {
             Object.assign(packageJson, packageJson.onPublish);
             delete packageJson.onPublish;
@@ -352,28 +355,30 @@ describe('publish command (e2e)', () => {
     const show = (await npmShow('foo'))!;
     expect(show.name).toEqual('foo');
     expect(show.main).toEqual('lib/index.js');
-    expect(show.hasOwnProperty('onPublish')).toBeFalsy();
+    expect(show).not.toHaveProperty('onPublish');
 
     repo.checkout(defaultBranchName);
     repo.pull();
 
     // All git results should still have previous information
     expect(repo.getCurrentTags()).toEqual(['foo_v1.1.0']);
-    const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json'));
+    const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json')) as ExtraPackageJson;
     expect(fooPackageJson.main).toBe('src/index.ts');
-    expect(fooPackageJson.onPublish.main).toBe('lib/index.js');
+    expect(fooPackageJson.onPublish?.main).toBe('lib/index.js');
   });
 
   it('should respect postpublish hooks', async () => {
     repositoryFactory = new RepositoryFactory('monorepo');
     repo = repositoryFactory.cloneRepository();
     let notified;
 
+    type ExtraPackageJson = PackageJson & { afterPublish?: { notify: string } };
+
     const options = getOptions({
       hooks: {
         postpublish: packagePath => {
           const packageJsonPath = path.join(packagePath, 'package.json');
-          const packageJson = fs.readJSONSync(packageJsonPath);
+          const packageJson = fs.readJSONSync(packageJsonPath) as ExtraPackageJson;
           if (packageJson.afterPublish) {
             notified = packageJson.afterPublish.notify;
           }
@@ -386,9 +391,9 @@ describe('publish command (e2e)', () => {
 
     await publish(options);
 
-    const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json'));
+    const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json')) as ExtraPackageJson;
     expect(fooPackageJson.main).toBe('src/index.ts');
-    expect(notified).toBe(fooPackageJson.afterPublish.notify);
+    expect(notified).toBe(fooPackageJson.afterPublish?.notify);
   });
 
   it('can perform a successful npm publish without fetch', async () => {
@@ -404,7 +409,7 @@ describe('publish command (e2e)', () => {
 
     let fetchCount = 0;
 
-    addGitObserver((args, output) => {
+    addGitObserver(args => {
       if (args[0] === 'fetch') {
         fetchCount++;
       }
@@ -436,7 +441,7 @@ describe('publish command (e2e)', () => {
 
     let fetchCommand: string = '';
 
-    addGitObserver((args, output) => {
+    addGitObserver(args => {
       if (args[0] === 'fetch') {
         fetchCommand = args.join(' ');
       }
@@ -592,7 +597,8 @@ describe('publish command (e2e)', () => {
 
   it('should respect postpublish hook respecting the concurrency limit when publishing multiple packages concurrently', async () => {
     const packagesToPublish = ['pkg1', 'pkg2', 'pkg3', 'pkg4', 'pkg5', 'pkg6', 'pkg7', 'pkg8', 'pkg9'];
-    const packages: { [packageName: string]: PackageJsonFixture } = {};
+    type ExtraPackageJsonFixture = PackageJsonFixture & { afterPublish?: { notify: string } };
+    const packages: { [packageName: string]: ExtraPackageJsonFixture } = {};
     for (const name of packagesToPublish) {
       packages[name] = {
         version: '1.0.0',
@@ -622,7 +628,7 @@ describe('publish command (e2e)', () => {
           await simulateWait(100);
           const packageName = path.basename(packagePath);
           const packageJsonPath = path.join(packagePath, 'package.json');
-          const packageJson = fs.readJSONSync(packageJsonPath);
+          const packageJson = fs.readJSONSync(packageJsonPath) as ExtraPackageJsonFixture;
           if (packageJson.afterPublish) {
             afterPublishStrings.push({
               packageName,
@@ -644,7 +650,7 @@ describe('publish command (e2e)', () => {
     expect(maxConcurrency).toBe(concurrency);
 
     for (const pkg of packagesToPublish) {
-      const packageJson = fs.readJSONSync(repo.pathTo(`packages/${pkg}/package.json`));
+      const packageJson = fs.readJSONSync(repo.pathTo(`packages/${pkg}/package.json`)) as ExtraPackageJsonFixture;
       if (packageJson.afterPublish) {
         // Verify that all postpublish hooks were called
         expect(afterPublishStrings).toContainEqual({

diff --git a/src/__e2e__/publishGit.test.ts b/src/__e2e__/publishGit.test.ts
@@ -12,6 +12,7 @@ import { BeachballOptions } from '../types/BeachballOptions';
 import { ChangeFileInfo } from '../types/ChangeInfo';
 import { getPackageInfos } from '../monorepo/getPackageInfos';
 import { getDefaultOptions } from '../options/getDefaultOptions';
+import type { PackageJson } from '../types/PackageInfo';
 
 describe('publish command (git)', () => {
   let repositoryFactory: RepositoryFactory;
@@ -61,7 +62,7 @@ describe('publish command (git)', () => {
 
     const newRepo = repositoryFactory.cloneRepository();
 
-    const packageJson = fs.readJSONSync(newRepo.pathTo('package.json'));
+    const packageJson = fs.readJSONSync(newRepo.pathTo('package.json')) as PackageJson;
 
     expect(packageJson.version).toBe('1.1.0');
   });
@@ -91,7 +92,7 @@ describe('publish command (git)', () => {
     const newRepo = repositoryFactory.cloneRepository();
     const changeFiles = getChangeFiles({ ...options1, path: newRepo.rootPath });
     expect(changeFiles).toHaveLength(1);
-    const changeFileContent: ChangeFileInfo = fs.readJSONSync(changeFiles[0]);
+    const changeFileContent = fs.readJSONSync(changeFiles[0]) as ChangeFileInfo;
     expect(changeFileContent.packageName).toBe('foo2');
   });
 });
diff --git a/src/__fixtures__/changelog.ts b/src/__fixtures__/changelog.ts
@@ -38,7 +38,7 @@ export function readChangelogJson(packagePath: string, filename?: string, noClea
     return null;
   }
 
-  const changelog: ChangelogJson = fs.readJSONSync(changelogJsonFile, { encoding: 'utf-8' });
+  const changelog = fs.readJSONSync(changelogJsonFile, { encoding: 'utf-8' }) as ChangelogJson;
   if (noClean) {
     return changelog;
   }
@@ -50,7 +50,7 @@ export function readChangelogJson(packagePath: string, filename?: string, noClea
     }
 
     for (const comments of Object.values(entry.comments)) {
-      for (const comment of comments!) {
+      for (const comment of comments) {
         if (comment.commit) {
           comment.commit = fakeCommit;
         }

diff --git a/src/__fixtures__/gitDefaults.ts b/src/__fixtures__/gitDefaults.ts
@@ -9,7 +9,7 @@ export const defaultRemoteBranchName = 'origin/master';
  * users have configured a different default branch name.
  * @param isRemoteRepo Whether this is the bare repo used as the remote
  */
-export function setDefaultBranchName(cwd: string, isRemoteRepo?: boolean) {
+export function setDefaultBranchName(cwd: string, isRemoteRepo?: boolean): void {
   if (isRemoteRepo) {
     // Change the name of the default branch on the repo used for the remote
     // (for clones, this is unnecessary and can cause problems)
@@ -26,6 +26,6 @@ export function setDefaultBranchName(cwd: string, isRemoteRepo?: boolean) {
  * (This should NOT be used to make full snapshots of git logs, or as the primary means of testing.
  * But it can be useful for allowing backup checks for absence of strings like "warning" or "fatal".)
  */
-export function optsWithLang(opts?: Parameters<typeof git>[1]) {
+export function optsWithLang(opts?: Parameters<typeof git>[1]): Parameters<typeof git>[1] {
   return { ...opts, env: { ...opts?.env, LANG: 'en_US.UTF-8' } };
 }
diff --git a/src/__fixtures__/mockLogs.ts b/src/__fixtures__/mockLogs.ts
@@ -38,8 +38,8 @@ export function initMockLogs(options: MockLogsOptions = {}): MockLogs {
 
   const logs: MockLogs = {
     mocks: {} as MockLogs['mocks'],
-    setOverrideOptions: options => {
-      overrideOptions = options;
+    setOverrideOptions: opt => {
+      overrideOptions = opt;
     },
     clear: () => {
       allLines = [];