Skip to content

Commit e2553dc

Browse files
FIX: @W-18030774@: Update definition of wasSuppliedByUser to treat ex… (#222)
1 parent 6b0bea9 commit e2553dc

File tree

7 files changed

+250
-13
lines changed

7 files changed

+250
-13
lines changed

packages/code-analyzer-core/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@salesforce/code-analyzer-core",
33
"description": "Core Package for the Salesforce Code Analyzer",
4-
"version": "0.24.0",
4+
"version": "0.24.1-SNAPSHOT",
55
"author": "The Salesforce Code Analyzer Team",
66
"license": "BSD-3-Clause",
77
"homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",

packages/code-analyzer-core/src/code-analyzer.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -668,19 +668,26 @@ function toConfigDescription(engineConfigDescription: engApi.ConfigDescription,
668668
descriptionText: getMessage('EngineConfigFieldDescription_disable_engine', engineName),
669669
valueType: "boolean",
670670
defaultValue: false,
671-
wasSuppliedByUser: hasCaseInsensitiveKey(engineOverrides, FIELDS.DISABLE_ENGINE)
671+
wasSuppliedByUser: wasFieldSuppliedByUser(engineOverrides, FIELDS.DISABLE_ENGINE)
672672
}
673673
}
674674
}
675675
for (const fieldName in engineConfigDescription.fieldDescriptions) {
676676
configDescription.fieldDescriptions[fieldName] = {
677677
... engineConfigDescription.fieldDescriptions[fieldName],
678-
wasSuppliedByUser: hasCaseInsensitiveKey(engineOverrides, fieldName)
678+
wasSuppliedByUser: wasFieldSuppliedByUser(engineOverrides, fieldName)
679679
};
680680
}
681681
return configDescription;
682682
}
683683

684-
function hasCaseInsensitiveKey(obj: object, key: string): boolean {
685-
return Object.keys(obj).some(k => k.toLowerCase() === key.toLowerCase());
684+
function wasFieldSuppliedByUser(engineOverrides: EngineOverrides, fieldName: string): boolean {
685+
const correctedFieldName: string | undefined = findCaseInsensitiveKey(engineOverrides, fieldName);
686+
return correctedFieldName !== undefined &&
687+
engineOverrides[correctedFieldName] !== null &&
688+
engineOverrides[correctedFieldName] !== undefined;
689+
}
690+
691+
function findCaseInsensitiveKey(obj: object, key: string): string | undefined {
692+
return Object.keys(obj).find(k => k.toLowerCase() === key.toLowerCase());
686693
}

packages/code-analyzer-core/src/config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as path from 'node:path';
44
import * as os from "node:os";
55
import * as yaml from 'js-yaml';
66
import {getMessage} from "./messages";
7-
import {toAbsolutePath} from "./utils"
7+
import {deepEquals, toAbsolutePath} from "./utils"
88
import {SeverityLevel} from "./rules";
99

1010
// Only exported internally to share across files
@@ -169,13 +169,13 @@ export class CodeAnalyzerConfig {
169169
descriptionText: getMessage('ConfigFieldDescription_rules'),
170170
valueType: 'object',
171171
defaultValue: {},
172-
wasSuppliedByUser: this.config.rules !== DEFAULT_CONFIG.rules
172+
wasSuppliedByUser: !deepEquals(this.config.rules, DEFAULT_CONFIG.rules)
173173
},
174174
engines: {
175175
descriptionText: getMessage('ConfigFieldDescription_engines'),
176176
valueType: 'object',
177177
defaultValue: {},
178-
wasSuppliedByUser: this.config.engines !== DEFAULT_CONFIG.engines
178+
wasSuppliedByUser: !deepEquals(this.config.engines, DEFAULT_CONFIG.engines)
179179
}
180180
}
181181
};

packages/code-analyzer-core/src/utils.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,57 @@ export class EngineProgressAggregator {
5151
}
5252
return sumOfPercentages / Math.max(this.percentagesMap.size, 1);
5353
}
54+
}
55+
56+
// Typescript says that {} === {} is false because it does a shallow comparison. To be more robust in our object
57+
// comparison we often need to do a deep equality check, thus the need for this function.
58+
export function deepEquals(value1: unknown, value2: unknown): boolean {
59+
// Check for strict equality first
60+
if (value1 === value2) {
61+
return true;
62+
}
63+
64+
// If one is null or undefined, return false
65+
if (value1 == null || value2 == null) {
66+
return false;
67+
}
68+
69+
// If both are objects (arrays or plain objects), compare them deeply
70+
if (typeof value1 === 'object' && typeof value2 === 'object') {
71+
// If both are arrays
72+
if (Array.isArray(value1) && Array.isArray(value2)) {
73+
if (value1.length !== value2.length) {
74+
return false;
75+
}
76+
// Compare each element in arrays
77+
for (let i = 0; i < value1.length; i++) {
78+
if (!deepEquals(value1[i], value2[i])) {
79+
return false;
80+
}
81+
}
82+
return true;
83+
}
84+
85+
// If both are objects (not arrays)
86+
if (!Array.isArray(value1) && !Array.isArray(value2)) {
87+
const keys1: string[] = Object.keys(value1);
88+
const keys2: string[] = Object.keys(value2);
89+
90+
// If they have different numbers of keys, they are not equal
91+
if (keys1.length !== keys2.length) {
92+
return false;
93+
}
94+
95+
// Compare each key and its corresponding value
96+
for (const key of keys1) {
97+
if (!keys2.includes(key) || !deepEquals(value1[key as keyof object], value2[key as keyof object])) {
98+
return false;
99+
}
100+
}
101+
return true;
102+
}
103+
}
104+
105+
// For all other types (number, string, boolean, etc.), use strict equality
106+
return false;
54107
}

packages/code-analyzer-core/test/add-engines.test.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import {CodeAnalyzer, CodeAnalyzerConfig, EventType, LogEvent, LogLevel} from "../src";
1+
import {CodeAnalyzer, CodeAnalyzerConfig, ConfigDescription, ConfigFieldDescription, EventType, LogEvent, LogLevel} from "../src";
22
import * as stubs from "./stubs";
33
import {getMessage} from "../src/messages";
44
import {changeWorkingDirectoryToPackageRoot, FixedClock} from "./test-helpers";
55
import path from "node:path";
66
import {StubEngine1, StubEngine2, StubEngine3, ThrowingPlugin2} from "./stubs";
7-
import {ConfigDescription, EnginePluginV1} from "@salesforce/code-analyzer-engine-api";
7+
import * as engApi from "@salesforce/code-analyzer-engine-api";
88

99
changeWorkingDirectoryToPackageRoot();
1010

@@ -121,9 +121,9 @@ describe("Tests for adding engines to Code Analyzer", () => {
121121
})
122122

123123
it.each([
124-
{plugin: new stubs.ThrowingPlugin2() as EnginePluginV1, msg: 'SomeErrorFromDescribeEngineConfig', case: 'describeEngineConfig'},
125-
{plugin: new stubs.ThrowingPlugin3() as EnginePluginV1, msg: 'SomeErrorFromCreateEngineConfig', case: 'createEngineConfig'},
126-
{plugin: new stubs.ThrowingPlugin4() as EnginePluginV1, msg: 'SomeErrorFromCreateEngine', case: 'createEngine'}
124+
{plugin: new stubs.ThrowingPlugin2() as engApi.EnginePluginV1, msg: 'SomeErrorFromDescribeEngineConfig', case: 'describeEngineConfig'},
125+
{plugin: new stubs.ThrowingPlugin3() as engApi.EnginePluginV1, msg: 'SomeErrorFromCreateEngineConfig', case: 'createEngineConfig'},
126+
{plugin: new stubs.ThrowingPlugin4() as engApi.EnginePluginV1, msg: 'SomeErrorFromCreateEngine', case: 'createEngine'}
127127
])('When plugin throws error during $case, then we emit error log line and skip that engine', async ({plugin, msg}) => {
128128
await codeAnalyzer.addEnginePlugin(plugin);
129129

@@ -208,6 +208,21 @@ describe("Tests for adding engines to Code Analyzer", () => {
208208
});
209209
});
210210

211+
it('If an engine configuration value is overridden with a null value, then getEngineConfigDescription should be treated as if it has not been overridden', async () => {
212+
codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromObject({
213+
engines: {
214+
stubEngine1: {
215+
misc_value: null
216+
}
217+
}
218+
}));
219+
await codeAnalyzer.addEnginePlugin(new stubs.StubEnginePlugin());
220+
221+
const engineConfigDescription: ConfigDescription = codeAnalyzer.getEngineConfigDescription('stubEngine1');
222+
const miscValueFieldDesc: ConfigFieldDescription = engineConfigDescription.fieldDescriptions['misc_value'];
223+
expect(miscValueFieldDesc.wasSuppliedByUser).toEqual(false);
224+
});
225+
211226
it('If engine has not been added, then getEngineConfig and getEngineConfigDescription should error', async () => {
212227
expect(() => codeAnalyzer.getEngineConfig('stubEngine1')).toThrow(
213228
getMessage('FailedToGetEngineConfig', 'stubEngine1'));

packages/code-analyzer-core/test/config.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,4 +329,12 @@ describe("Tests for creating and accessing configuration values", () => {
329329
expect(configDescription.fieldDescriptions.rules.wasSuppliedByUser).toEqual(true);
330330
expect(configDescription.fieldDescriptions.engines.wasSuppliedByUser).toEqual(false);
331331
});
332+
333+
it ("When getConfigDescription is called from modified config that contains null, then null is treated as if the config field was not supplied", () => {
334+
const configDescription: ConfigDescription = CodeAnalyzerConfig.fromObject(
335+
{rules: null, engines: null}
336+
).getConfigDescription();
337+
expect(configDescription.fieldDescriptions.rules.wasSuppliedByUser).toEqual(false);
338+
expect(configDescription.fieldDescriptions.engines.wasSuppliedByUser).toEqual(false);
339+
});
332340
});
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import {deepEquals} from "../src/utils";
2+
3+
describe('deepEquals function tests', () => {
4+
describe('primitive type comparison tests', () => {
5+
it('should return true for equal numbers', () => {
6+
expect(deepEquals(5, 5)).toBe(true);
7+
});
8+
9+
it('should return false for different numbers', () => {
10+
expect(deepEquals(5, 10)).toBe(false);
11+
});
12+
13+
it('should return true for equal strings', () => {
14+
expect(deepEquals('hello', 'hello')).toBe(true);
15+
});
16+
17+
it('should return false for different strings', () => {
18+
expect(deepEquals('hello', 'world')).toBe(false);
19+
});
20+
21+
it('should return true for equal booleans', () => {
22+
expect(deepEquals(true, true)).toBe(true);
23+
});
24+
25+
it('should return false for different booleans', () => {
26+
expect(deepEquals(true, false)).toBe(false);
27+
});
28+
29+
it('should return false for number vs string', () => {
30+
expect(deepEquals(5, '5')).toBe(false);
31+
});
32+
33+
it('should return false for null vs undefined', () => {
34+
expect(deepEquals(null, undefined)).toBe(false);
35+
});
36+
37+
it('should return true for null vs null', () => {
38+
expect(deepEquals(null, null)).toBe(true);
39+
});
40+
41+
it('should return true for undefined vs undefined', () => {
42+
expect(deepEquals(undefined, undefined)).toBe(true);
43+
});
44+
45+
it('should return false for NaN vs NaN since NaN is not supposed to equal itself', () => {
46+
expect(deepEquals(NaN, NaN)).toBe(false);
47+
});
48+
});
49+
50+
describe('object comparison tests', () => {
51+
it('should return true for equal objects', () => {
52+
const obj1 = { a: 1, b: { c: 2 } };
53+
const obj2 = { a: 1, b: { c: 2 } };
54+
expect(deepEquals(obj1, obj2)).toBe(true);
55+
});
56+
57+
it('should return false for objects with different properties', () => {
58+
const obj1 = { a: 1, b: { c: 2 } };
59+
const obj2 = { a: 1, b: { c: 3 } };
60+
expect(deepEquals(obj1, obj2)).toBe(false);
61+
});
62+
63+
it('should return false for objects with different number of properties', () => {
64+
const obj1 = { a: 1, b: { c: 2 } };
65+
const obj2 = { a: 1 };
66+
expect(deepEquals(obj1, obj2)).toBe(false);
67+
});
68+
69+
it('should return false for objects with different nested structures', () => {
70+
const obj1 = { a: { b: { c: 2 } } };
71+
const obj2 = { a: { b: { d: 3 } } };
72+
expect(deepEquals(obj1, obj2)).toBe(false);
73+
});
74+
75+
// Arrays comparison
76+
it('should return true for equal arrays', () => {
77+
const arr1 = [1, 2, 3];
78+
const arr2 = [1, 2, 3];
79+
expect(deepEquals(arr1, arr2)).toBe(true);
80+
});
81+
82+
it('should return false for arrays with different lengths', () => {
83+
const arr1 = [1, 2, 3];
84+
const arr2 = [1, 2];
85+
expect(deepEquals(arr1, arr2)).toBe(false);
86+
});
87+
88+
it('should return false for arrays with different elements', () => {
89+
const arr1 = [1, 2, 3];
90+
const arr2 = [1, 2, 4];
91+
expect(deepEquals(arr1, arr2)).toBe(false);
92+
});
93+
94+
it('should return true for empty arrays', () => {
95+
const arr1: unknown[] = [];
96+
const arr2: unknown[] = [];
97+
expect(deepEquals(arr1, arr2)).toBe(true);
98+
});
99+
});
100+
101+
describe('mixed array and object comparison tests', () => {
102+
it('should return false for object vs array', () => {
103+
const obj = {a: 1};
104+
const arr = [1];
105+
expect(deepEquals(obj, arr)).toBe(false);
106+
});
107+
108+
it('should return false for array vs string', () => {
109+
const arr = [1, 2];
110+
const str = '12';
111+
expect(deepEquals(arr, str)).toBe(false);
112+
});
113+
114+
// Edge cases
115+
it('should return true for empty objects', () => {
116+
expect(deepEquals({}, {})).toBe(true);
117+
});
118+
119+
it('should return true for empty arrays', () => {
120+
expect(deepEquals([], [])).toBe(true);
121+
});
122+
123+
it('should return false for objects and null', () => {
124+
const obj = {a: 1};
125+
expect(deepEquals(obj, null)).toBe(false);
126+
});
127+
128+
it('should return false for arrays and null', () => {
129+
const arr = [1];
130+
expect(deepEquals(arr, null)).toBe(false);
131+
});
132+
133+
it('should return false for functions', () => {
134+
const fn1 = () => {
135+
};
136+
const fn2 = () => {
137+
};
138+
expect(deepEquals(fn1, fn2)).toBe(false);
139+
});
140+
141+
// Nested arrays and objects
142+
it('should return true for deeply nested equal objects', () => {
143+
const obj1 = {a: {b: {c: 3}}};
144+
const obj2 = {a: {b: {c: 3}}};
145+
expect(deepEquals(obj1, obj2)).toBe(true);
146+
});
147+
148+
it('should return false for deeply nested different objects', () => {
149+
const obj1 = {a: {b: {c: 3}}};
150+
const obj2 = {a: {b: {c: 4}}};
151+
expect(deepEquals(obj1, obj2)).toBe(false);
152+
});
153+
});
154+
});

0 commit comments

Comments
 (0)