Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): add workaround for broken .NET format command in v6+ #397

Merged
merged 2 commits into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix(core): add workaround for broken .NET format command in v6+
  • Loading branch information
AgentEnder committed Mar 10, 2022
commit cc2b4b47903d8f82435924e1e59f0b5530cbb10e
Empty file modified .husky/pre-push
100644 → 100755
Empty file.
8 changes: 4 additions & 4 deletions packages/core/src/executors/format/executor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Format Executor', () => {
};
dotnetClient = new DotNetClient(mockDotnetFactory());
(dotnetClient as jest.Mocked<DotNetClient>).getSdkVersion.mockReturnValue(
Buffer.from('5.0.402'),
'5.0.402',
);
});

Expand Down Expand Up @@ -91,7 +91,7 @@ describe('Format Executor', () => {

it('does not install dotnet-format if SDK is 6+', async () => {
(dotnetClient as jest.Mocked<DotNetClient>).getSdkVersion.mockReturnValue(
Buffer.from('6.0.101'),
'6.0.101',
);

jest.spyOn(fs, 'existsSync').mockReturnValue(true);
Expand All @@ -108,7 +108,7 @@ describe('Format Executor', () => {

it('passes the --check option on .NET 5 and earlier', async () => {
(dotnetClient as jest.Mocked<DotNetClient>).getSdkVersion.mockReturnValue(
Buffer.from('5.0.101'),
'5.0.101',
);
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
jest
Expand All @@ -126,7 +126,7 @@ describe('Format Executor', () => {

it('passes the --verify-no-changes option on .NET 6 and later', async () => {
(dotnetClient as jest.Mocked<DotNetClient>).getSdkVersion.mockReturnValue(
Buffer.from('6.0.101'),
'6.0.101',
);

const res = await executor(options, context, dotnetClient);
Expand Down
27 changes: 22 additions & 5 deletions packages/core/src/executors/format/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default async function runExecutor(
context: ExecutorContext,
dotnetClient: DotNetClient = new DotNetClient(dotnetFactory()),
) {
const sdkVersion = dotnetClient.getSdkVersion().toString();
const sdkVersion = dotnetClient.getSdkVersion();
const majorVersion = parseInt(sdkVersion.split('.')[0]);
const isNet6OrHigher = majorVersion >= 6;

Expand All @@ -45,7 +45,7 @@ export default async function runExecutor(

const normalized = normalizeOptions(options, isNet6OrHigher);

ensureFormatToolInstalled(context, dotnetClient, isNet6OrHigher);
ensureFormatToolInstalled(context, dotnetClient, majorVersion);
dotnetClient.format(projectFilePath, normalized);

return {
Expand All @@ -56,9 +56,12 @@ export default async function runExecutor(
function ensureFormatToolInstalled(
context: ExecutorContext,
dotnetClient: DotNetClient,
isNet6OrHigher: boolean,
majorVersion: number,
) {
if (isNet6OrHigher) {
// Currently the built-in .NET Format executor is broken on .NET 6
// Fall back to installing and using the tool directly
// eslint-disable-next-line no-constant-condition
if (false && majorVersion >= 6) {
// dotnet-format is already included as part of .NET SDK 6+
return;
}
Expand All @@ -73,5 +76,19 @@ function ensureFormatToolInstalled(
return;
}

dotnetClient.installTool('dotnet-format');
if (majorVersion == 6) {
dotnetClient.installTool(
'dotnet-format',
'6.*',
'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6/nuget/v3/index.json',
);
} else if (majorVersion == 7) {
dotnetClient.installTool(
'dotnet-format',
'7.*',
'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet7/nuget/v3/index.json',
);
} else {
dotnetClient.installTool('dotnet-format');
}
}
22 changes: 17 additions & 5 deletions packages/dotnet/src/lib/core/dotnet.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,14 @@ export class DotNetClient {
return this.logAndExecute(params);
}

installTool(tool: string): void {
installTool(tool: string, version?: string, source?: string): void {
const cmd = [`tool`, `install`, tool];
if (version) {
cmd.push('--version', version);
}
if (source) {
cmd.push('--add-source', source);
}
return this.logAndExecute(cmd);
}

Expand All @@ -130,8 +136,14 @@ export class DotNetClient {
return this.logAndExecute(cmd);
}

format(project: string, parameters?: dotnetFormatOptions): void {
const params = [`format`, project];
format(
project: string,
parameters?: dotnetFormatOptions,
forceToolUsage?: boolean,
): void {
const params = forceToolUsage
? ['tool', 'run', 'dotnet-format', project]
: [`format`, project];
if (parameters) {
parameters = swapKeysUsingMap(parameters, formatKeyMap);
params.push(...getSpawnParameterArray(parameters));
Expand All @@ -144,8 +156,8 @@ export class DotNetClient {
this.logAndExecute(params);
}

getSdkVersion(): Buffer {
return this.execute(['--version']);
getSdkVersion(): string {
return this.cliCommand.info.version.toString();
}

printSdkVersion(): void {
Expand Down