Skip to content

Commit

Permalink
fix: compile error TS2589 - Type instantiation is excessively deep an…
Browse files Browse the repository at this point in the history
…d possibly infinite
  • Loading branch information
mshanemc committed Jun 18, 2024
1 parent 546cb6a commit 6b4aa8a
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 37 deletions.
11 changes: 3 additions & 8 deletions src/sfProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@ import { resolveProjectPath, resolveProjectPathSync, SFDX_PROJECT_JSON } from '.

import { SfError } from './sfError';
import { Messages } from './messages';
import { findUpperCaseKeys } from './util/findUppercaseKeys';
import { ensureNoUppercaseKeys } from './util/findUppercaseKeys';

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/core', 'config');

const coreMessages = Messages.loadMessages('@salesforce/core', 'core');

/** @deprecated. Use PackageDirDependency from @salesforce/schemas */
export type PackageDirDependency = PackageDirDependencySchema;

Expand Down Expand Up @@ -70,6 +68,7 @@ export type ProjectJson = ConfigContents & ProjectJsonSchema;
* **See** [force:project:create](https://developer.salesforce.com/docs/atlas.en-us.sfdx_dev.meta/sfdx_dev/sfdx_dev_ws_create_new.htm)
*/
export class SfProjectJson extends ConfigFile<ConfigFile.Options, ProjectJson> {
/** json properties that are uppercase, or allow uppercase keys inside them */
public static BLOCKLIST = ['packageAliases'];

public static getFileName(): string {
Expand Down Expand Up @@ -338,11 +337,7 @@ export class SfProjectJson extends ConfigFile<ConfigFile.Options, ProjectJson> {
}

private validateKeys(): void {
// Verify that the configObject does not have upper case keys; throw if it does. Must be heads down camel case.
const upperCaseKey = findUpperCaseKeys(this.toObject(), SfProjectJson.BLOCKLIST);
if (upperCaseKey) {
throw coreMessages.createError('invalidJsonCasing', [upperCaseKey, this.getPath()]);
}
ensureNoUppercaseKeys(this.getPath())(SfProjectJson.BLOCKLIST)(this.toObject());
}
}

Expand Down
38 changes: 21 additions & 17 deletions src/util/findUppercaseKeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,25 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { JsonMap, Optional, isJsonMap, asJsonMap, AnyJson } from '@salesforce/ts-types';
import { findKey } from '@salesforce/kit';
import { strictEqual } from 'node:assert/strict';
import { JsonMap, isJsonMap } from '@salesforce/ts-types';
import { Messages } from '../messages';

export const findUpperCaseKeys = (data?: JsonMap, sectionBlocklist: string[] = []): Optional<string> => {
let key: Optional<string>;
findKey(data, (val: AnyJson, k: string) => {
if (/^[A-Z]/.test(k)) {
key = k;
} else if (isJsonMap(val)) {
if (sectionBlocklist.includes(k)) {
return key;
}
key = findUpperCaseKeys(asJsonMap(val));
}
return key;
});
return key;
};
Messages.importMessagesDirectory(__dirname);
const coreMessages = Messages.loadMessages('@salesforce/core', 'core');

/** will throw on any upperCase unless they are present in the allowList. Recursively searches the object, returning valid keys */
export const ensureNoUppercaseKeys =
(path: string) =>
(allowList: string[] = []) =>
(data: JsonMap): string[] => {
const keys = getKeys(data, allowList);
const upperCaseKeys = keys.filter((key) => /^[A-Z]/.test(key)).join(', ');
strictEqual(upperCaseKeys.length, 0, coreMessages.getMessage('invalidJsonCasing', [upperCaseKeys, path]));
return keys;
};

const getKeys = (data: JsonMap, allowList: string[]): string[] =>
Object.entries(data)
.filter(([k]) => !allowList.includes(k))
.flatMap(([key, value]) => (isJsonMap(value) ? [key, ...getKeys(value, allowList)] : [key]));
70 changes: 58 additions & 12 deletions test/unit/util/findUppercaseKeysTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,90 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { expect } from 'chai';
import { findUpperCaseKeys } from '../../../src/util/findUppercaseKeys';
import { expect, assert } from 'chai';
import { JsonMap } from '@salesforce/ts-types';
import { ensureNoUppercaseKeys } from '../../../src/util/findUppercaseKeys';

describe('findUpperCaseKeys', () => {
it('should return the first upper case key', () => {
const fn = ensureNoUppercaseKeys('testPath')();

const messageFromFailure = (obj: JsonMap): string => {
const failMessage = 'should have thrown';
try {
fn(obj);
expect.fail(failMessage);
} catch (e) {
assert(e instanceof Error);
assert(e.message !== failMessage);
return e.message;
}
};

it('should throw on top-level uppercase keys', () => {
const testObj = {
lowercase: true,
UpperCase: false,
nested: { camelCase: true },
};
expect(messageFromFailure(testObj)).to.include('UpperCase');
});

it('should throw with multiple uppercase keys', () => {
const testObj = {
lowercase: true,
UpperCase: false,
nested: { camelCase: true },
AnotherUpperCase: false,
};
const msg = messageFromFailure(testObj);
expect(msg).to.include('UpperCase');
expect(msg).to.include('AnotherUpperCase');
});

it('should throw with multiple uppercase keys when one is nested', () => {
const testObj = {
lowercase: true,
UpperCase: false,
nested: { camelCase: true, AnotherUpperCase: true },
};
expect(findUpperCaseKeys(testObj)).to.equal('UpperCase');
const msg = messageFromFailure(testObj);
expect(msg).to.include('UpperCase');
expect(msg).to.include('AnotherUpperCase');
});

it('should return the first nested upper case key', () => {
it('should throw if nested uppercase keys', () => {
const testObj = {
lowercase: true,
uppercase: false,
nested: { NestedUpperCase: true },
};
expect(findUpperCaseKeys(testObj)).to.equal('NestedUpperCase');
expect(messageFromFailure(testObj)).to.include('NestedUpperCase');
});

it('should return undefined when no upper case key is found', () => {
it('returns all non-blocked keys when no uppercase keys are found', () => {
const testObj = {
lowercase: true,
uppercase: false,
nested: { camelCase: true },
};
expect(findUpperCaseKeys(testObj)).to.be.undefined;
expect(fn(testObj)).to.deep.equal(['lowercase', 'uppercase', 'nested', 'camelCase']);
});

it('should return the first nested upper case key unless blocklisted', () => {
it('allowList can have uppercase keys inside it', () => {
const testObj = {
lowercase: true,
uppercase: false,
nested: { NestedUpperCase: true },
};
expect(findUpperCaseKeys(testObj, ['nested'])).to.equal(undefined);
expect(ensureNoUppercaseKeys('testPath')(['nested'])(testObj)).to.deep.equal(['lowercase', 'uppercase']);
});

it('allowList can have top-level uppercase keys', () => {
const testObj = {
lowercase: true,
TopLevel: false,
};
expect(ensureNoUppercaseKeys('testPath')(['TopLevel'])(testObj)).to.deep.equal(['lowercase']);
});

it('handles keys starting with numbers', () => {
Expand All @@ -51,13 +97,13 @@ describe('findUpperCaseKeys', () => {
Abc: false,
nested: { '2abc': true },
};
expect(findUpperCaseKeys(testObj)).to.equal('Abc');
expect(messageFromFailure(testObj)).to.include('Abc');
});
it('handles keys starting with numbers', () => {
const testObj = {
'1abc': true,
nested: { '2abc': true, Upper: false },
};
expect(findUpperCaseKeys(testObj)).to.equal('Upper');
expect(messageFromFailure(testObj)).to.include('Upper');
});
});

2 comments on commit 6b4aa8a

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: 6b4aa8a Previous: 0e27a6c Ratio
Child logger creation 479630 ops/sec (±1.58%) 475808 ops/sec (±0.77%) 0.99
Logging a string on root logger 780523 ops/sec (±10.58%) 789376 ops/sec (±8.56%) 1.01
Logging an object on root logger 555677 ops/sec (±7.62%) 605255 ops/sec (±6.23%) 1.09
Logging an object with a message on root logger 8371 ops/sec (±203.98%) 6189 ops/sec (±208.49%) 0.74
Logging an object with a redacted prop on root logger 452069 ops/sec (±6.72%) 467744 ops/sec (±7.90%) 1.03
Logging a nested 3-level object on root logger 378472 ops/sec (±6.13%) 363830 ops/sec (±6.87%) 0.96

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - windows-latest

Benchmark suite Current: 6b4aa8a Previous: 0e27a6c Ratio
Child logger creation 324106 ops/sec (±0.82%) 327672 ops/sec (±0.85%) 1.01
Logging a string on root logger 762422 ops/sec (±5.71%) 819206 ops/sec (±5.93%) 1.07
Logging an object on root logger 611410 ops/sec (±9.39%) 626163 ops/sec (±6.87%) 1.02
Logging an object with a message on root logger 5701 ops/sec (±205.87%) 4131 ops/sec (±213.62%) 0.72
Logging an object with a redacted prop on root logger 435558 ops/sec (±16.78%) 455776 ops/sec (±10.85%) 1.05
Logging a nested 3-level object on root logger 334658 ops/sec (±5.02%) 330566 ops/sec (±6.20%) 0.99

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.