fix(core): Check SDK and tool installation before running format comm…

…and (#204) Closes #179 Closes #202 Co-authored-by: Craigory Coppola <craigorycoppola@gmail.com> Co-authored-by: Ben Callaghan <bcallaghan@selectbankcard.com>
nx-dotnet · Oct 14, 2021 · 3ad6291 · 3ad6291
1 parent 76c9f2b
commit 3ad6291
Show file tree

Hide file tree

Showing 3 changed files with 147 additions and 6 deletions.
diff --git a/packages/core/src/executors/format/executor.spec.ts b/packages/core/src/executors/format/executor.spec.ts
@@ -45,6 +45,9 @@ describe('Format Executor', () => {
       isVerbose: false,
     };
     dotnetClient = new DotNetClient(mockDotnetFactory());
+    (dotnetClient as jest.Mocked<DotNetClient>).printSdkVersion.mockReturnValue(
+      Buffer.from('5.0.402'),
+    );
   });
 
   afterEach(async () => {
@@ -91,4 +94,111 @@ describe('Format Executor', () => {
     ).toHaveBeenCalled();
     expect(res.success).toBeTruthy();
   });
+
+  it('installs dotnet-format if not already installed', async () => {
+    try {
+      const directoryPath = `${root}/apps/my-app`;
+      await fs.mkdir(directoryPath, { recursive: true });
+      await Promise.all([fs.writeFile(`${directoryPath}/1.csproj`, '')]);
+
+      const manifestPath = `${root}/.config`;
+      await fs.mkdir(manifestPath, { recursive: true });
+      await fs.writeFile(`${manifestPath}/dotnet-tools.json`, '{"tools": {}}');
+    } catch (e) {
+      if (assertErrorMessage(e)) console.warn(e.message);
+    }
+
+    const res = await executor(options, context, dotnetClient);
+    expect(
+      (dotnetClient as jest.Mocked<DotNetClient>).installTool,
+    ).toHaveBeenCalled();
+    expect(res.success).toBeTruthy();
+  });
+
+  it('does not install dotnet-format if already installed', async () => {
+    try {
+      const directoryPath = `${root}/apps/my-app`;
+      await fs.mkdir(directoryPath, { recursive: true });
+      await Promise.all([fs.writeFile(`${directoryPath}/1.csproj`, '')]);
+
+      const manifestPath = `${root}/.config`;
+      await fs.mkdir(manifestPath, { recursive: true });
+      await fs.writeFile(
+        `${manifestPath}/dotnet-tools.json`,
+        '{"tools": {"dotnet-format": {"version": "5.1.250801"}}}',
+      );
+    } catch (e) {
+      if (assertErrorMessage(e)) console.warn(e.message);
+    }
+
+    const res = await executor(options, context, dotnetClient);
+    expect(
+      (dotnetClient as jest.Mocked<DotNetClient>).installTool,
+    ).not.toHaveBeenCalled();
+    expect(res.success).toBeTruthy();
+  });
+
+  it('does not install dotnet-format if SDK is 6+', async () => {
+    (dotnetClient as jest.Mocked<DotNetClient>).printSdkVersion.mockReturnValue(
+      Buffer.from('6.0.101'),
+    );
+
+    try {
+      const directoryPath = `${root}/apps/my-app`;
+      await fs.mkdir(directoryPath, { recursive: true });
+      await Promise.all([fs.writeFile(`${directoryPath}/1.csproj`, '')]);
+
+      const manifestPath = `${root}/.config`;
+      await fs.mkdir(manifestPath, { recursive: true });
+      await fs.writeFile(`${manifestPath}/dotnet-tools.json`, '{"tools": {}}');
+    } catch (e) {
+      if (assertErrorMessage(e)) console.warn(e.message);
+    }
+
+    const res = await executor(options, context, dotnetClient);
+    expect(
+      (dotnetClient as jest.Mocked<DotNetClient>).installTool,
+    ).not.toHaveBeenCalled();
+    expect(res.success).toBeTruthy();
+  });
+
+  it('passes the --check option on .NET 5 and earlier', async () => {
+    try {
+      const directoryPath = `${root}/apps/my-app`;
+      await fs.mkdir(directoryPath, { recursive: true });
+      await Promise.all([fs.writeFile(`${directoryPath}/1.csproj`, '')]);
+    } catch (e) {
+      if (assertErrorMessage(e)) console.warn(e.message);
+    }
+
+    const res = await executor(options, context, dotnetClient);
+    expect(res.success).toBeTruthy();
+
+    const formatOptions = (dotnetClient as jest.Mocked<DotNetClient>).format
+      .mock.calls[0][1];
+    const checkFlag = formatOptions?.find((o) => o.flag == 'check');
+    expect(checkFlag?.value).toBeTruthy();
+  });
+
+  it('passes the --verify-no-changes option on .NET 6 and later', async () => {
+    (dotnetClient as jest.Mocked<DotNetClient>).printSdkVersion.mockReturnValue(
+      Buffer.from('6.0.101'),
+    );
+
+    try {
+      const directoryPath = `${root}/apps/my-app`;
+      await fs.mkdir(directoryPath, { recursive: true });
+      await Promise.all([fs.writeFile(`${directoryPath}/1.csproj`, '')]);
+    } catch (e) {
+      if (assertErrorMessage(e)) console.warn(e.message);
+    }
+
+    const res = await executor(options, context, dotnetClient);
+    expect(res.success).toBeTruthy();
+
+    const formatOptions = (dotnetClient as jest.Mocked<DotNetClient>).format
+      .mock.calls[0][1];
+    const checkFlag = formatOptions?.find((o) => o.flag == 'verifyNoChanges');
+    expect(checkFlag?.value).toBeTruthy();
+  });
 });
diff --git a/packages/core/src/executors/format/executor.ts b/packages/core/src/executors/format/executor.ts
@@ -1,4 +1,5 @@
-import { ExecutorContext } from '@nrwl/devkit';
+import { ExecutorContext, readJsonFile } from '@nrwl/devkit';
+import { existsSync } from 'fs';
 
 import {
   DotNetClient,
@@ -14,6 +15,7 @@ import { FormatExecutorSchema } from './schema';
 
 function normalizeOptions(
   options: FormatExecutorSchema,
+  isNet6OrHigher: boolean,
 ): Record<string, string | boolean | undefined> {
   const { diagnostics, include, exclude, check, fix, ...flags } = options;
   return {
@@ -23,7 +25,8 @@ function normalizeOptions(
       : diagnostics,
     include: Array.isArray(include) ? include.join(' ') : include,
     exclude: Array.isArray(exclude) ? exclude.join(' ') : exclude,
-    check: fix ? false : check,
+    check: fix ? false : check && !isNet6OrHigher, // The --check flag is for .NET 5 and older
+    verifyNoChanges: fix ? false : check && isNet6OrHigher, // The --verify-no-changes flag is for .NET 6 and newer
   };
 }
 
@@ -32,17 +35,21 @@ export default async function runExecutor(
   context: ExecutorContext,
   dotnetClient: DotNetClient = new DotNetClient(dotnetFactory()),
 ) {
+  const sdkVersion = dotnetClient.printSdkVersion().toString();
+  const majorVersion = parseInt(sdkVersion.split('.')[0]);
+  const isNet6OrHigher = majorVersion >= 6;
+
   const nxProjectConfiguration = getExecutedProjectConfiguration(context);
   const projectFilePath = await getProjectFileForNxProject(
     nxProjectConfiguration,
   );
 
-  const normalized = normalizeOptions(options);
+  const normalized = normalizeOptions(options, isNet6OrHigher);
 
-  dotnetClient.installTool('dotnet-format');
+  ensureFormatToolInstalled(context, dotnetClient, isNet6OrHigher);
   dotnetClient.format(
     projectFilePath,
-    Object.keys(options).map((x) => ({
+    Object.keys(normalized).map((x) => ({
       flag: x as dotnetFormatFlags,
       value: normalized[x],
     })),
@@ -52,3 +59,25 @@ export default async function runExecutor(
     success: true,
   };
 }
+
+function ensureFormatToolInstalled(
+  context: ExecutorContext,
+  dotnetClient: DotNetClient,
+  isNet6OrHigher: boolean,
+) {
+  if (isNet6OrHigher) {
+    // dotnet-format is already included as part of .NET SDK 6+
+    return;
+  }
+
+  const manifestPath = `${context.cwd}/.config/dotnet-tools.json`;
+  const manifest = existsSync(manifestPath)
+    ? readJsonFile(manifestPath)
+    : undefined;
+  if (manifest?.tools['dotnet-format']) {
+    // dotnet-format is already installed.
+    return;
+  }
+
+  dotnetClient.installTool('dotnet-format');
+}
diff --git a/packages/dotnet/src/lib/models/dotnet-format/dotnet-format-flags.ts b/packages/dotnet/src/lib/models/dotnet-format/dotnet-format-flags.ts
@@ -10,12 +10,14 @@ export type dotnetFormatFlags =
   | 'check'
   | 'report'
   | 'binarylog'
-  | 'verbosity';
+  | 'verbosity'
+  | 'verifyNoChanges';
 // Deliberately excluding the version option, as it doesn't perform any actual formatting.
 
 export const formatKeyMap: Partial<{ [key in dotnetFormatFlags]: string }> = {
   noRestore: 'no-restore',
   fixWhitespace: 'fix-whitespace',
   fixStyle: 'fix-style',
   fixAnalyzers: 'fix-analyzers',
+  verifyNoChanges: 'verify-no-changes',
 };