Skip to content

Commit d0acac4

Browse files
NEW: @W-17293079@: Update ConfigDescription objects to include type and default value information
1 parent 4b243f6 commit d0acac4

File tree

14 files changed

+301
-94
lines changed

14 files changed

+301
-94
lines changed

package-lock.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,16 @@ export class CodeAnalyzer {
238238
return;
239239
}
240240

241+
const engineOverrides: EngineOverrides = this.config.getEngineOverridesFor(engineName);
242+
241243
try {
242244
const engineConfigDescription: engApi.ConfigDescription = enginePluginV1.describeEngineConfig(engineName);
243-
normalizeEngineConfigDescription(engineConfigDescription, engineName);
244-
this.engineConfigDescriptions.set(engineName, engineConfigDescription);
245+
const configDescription: ConfigDescription = normalizeEngineConfigDescription(engineConfigDescription, engineName, engineOverrides);
246+
this.engineConfigDescriptions.set(engineName, configDescription);
245247
} catch (err) {
246248
throw new Error(getMessage('PluginErrorWhenCreatingEngine', engineName, (err as Error).message));
247249
}
248250

249-
const engineOverrides: EngineOverrides = this.config.getEngineOverridesFor(engineName);
250251
if (engineOverrides[FIELDS.DISABLE_ENGINE]) {
251252
this.emitLogEvent(LogLevel.Debug, getMessage('EngineDisabled', engineName,
252253
`${FIELDS.ENGINES}.${engineName}.${FIELDS.DISABLE_ENGINE}`))
@@ -524,15 +525,28 @@ function isIntegerBetween(value: number, leftBound: number, rightBound: number):
524525
return value >= leftBound && value <= rightBound && Number.isInteger(value);
525526
}
526527

527-
function normalizeEngineConfigDescription(engineConfigDescription: ConfigDescription, engineName: string): void {
528-
// Every engine config should have an overview, so if missing, then we add in a generic one
529-
if (!engineConfigDescription.overview) {
530-
engineConfigDescription.overview = getMessage('GenericEngineConfigOverview', engineName.toUpperCase());
528+
function normalizeEngineConfigDescription(engineConfigDescription: engApi.ConfigDescription, engineName: string,
529+
engineOverrides: EngineOverrides): ConfigDescription {
530+
const configDescription: ConfigDescription = {
531+
// Every engine config should have an overview, so if missing, then we add in a generic one
532+
overview: engineConfigDescription.overview ? engineConfigDescription.overview :
533+
getMessage('GenericEngineConfigOverview', engineName.toUpperCase()),
534+
535+
fieldDescriptions: {
536+
// Every engine config should have a disable_engine field which we prefer to be first in the object for display purposes
537+
[FIELDS.DISABLE_ENGINE]: {
538+
descriptionText: getMessage('EngineConfigFieldDescription_disable_engine', engineName),
539+
valueType: "boolean",
540+
defaultValue: false,
541+
wasSuppliedByUser: FIELDS.DISABLE_ENGINE in engineOverrides
542+
}
543+
}
531544
}
532-
533-
// Every engine config should have a disable_engine field which we prefer to be first in the object for display purposes
534-
engineConfigDescription.fieldDescriptions = {
535-
[FIELDS.DISABLE_ENGINE]: getMessage('EngineConfigFieldDescription_disable_engine', engineName),
536-
... engineConfigDescription.fieldDescriptions
545+
for (const fieldName in engineConfigDescription.fieldDescriptions) {
546+
configDescription.fieldDescriptions[fieldName] = {
547+
... engineConfigDescription.fieldDescriptions[fieldName],
548+
wasSuppliedByUser: fieldName in engineOverrides
549+
};
537550
}
551+
return configDescription;
538552
}

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

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import * as yaml from 'js-yaml';
66
import {getMessage} from "./messages";
77
import {toAbsolutePath} from "./utils"
88
import {SeverityLevel} from "./rules";
9-
import {getMessageFromCatalog, SHARED_MESSAGE_CATALOG, ValueValidator} from "@salesforce/code-analyzer-engine-api";
109

1110
export const FIELDS = {
1211
CONFIG_ROOT: 'config_root',
@@ -27,15 +26,6 @@ export type RuleOverride = {
2726
severity?: SeverityLevel
2827
tags?: string[]
2928
}
30-
export const TOP_LEVEL_CONFIG_DESCRIPTION: ConfigDescription = {
31-
overview: getMessage('ConfigOverview'),
32-
fieldDescriptions: {
33-
config_root: getMessage('ConfigFieldDescription_config_root'),
34-
log_folder: getMessage('ConfigFieldDescription_log_folder'),
35-
rules: getMessage('ConfigFieldDescription_rules'),
36-
engines: getMessage('ConfigFieldDescription_engines'),
37-
}
38-
}
3929

4030
type TopLevelConfig = {
4131
config_root: string
@@ -53,7 +43,20 @@ export const DEFAULT_CONFIG: TopLevelConfig = {
5343
custom_engine_plugin_modules: [], // INTERNAL USE ONLY
5444
};
5545

56-
export type ConfigDescription = engApi.ConfigDescription;
46+
export type ConfigDescription = {
47+
// A brief overview of this specific configuration object. It is recommended to include a link to documentation when possible.
48+
overview: string
49+
50+
// Description objects for the primary fields in the configuration
51+
fieldDescriptions: Record<string, ConfigFieldDescription>
52+
}
53+
54+
export type ConfigFieldDescription = engApi.ConfigFieldDescription & {
55+
// Whether or not the user has supplied a value for the field in their configuration file
56+
// Note: Unlike the Engine API's ConfigFieldDescription, core has the ability to determine if the user supplied
57+
// the field value (as we create a CodeAnalyzerConfig object), which is why "wasSuppliedByUser" is here only.
58+
wasSuppliedByUser: boolean
59+
}
5760

5861
export class CodeAnalyzerConfig {
5962
private readonly config: TopLevelConfig;
@@ -99,16 +102,44 @@ export class CodeAnalyzerConfig {
99102
config_root: configRoot,
100103
log_folder: configExtractor.extractFolder(FIELDS.LOG_FOLDER, DEFAULT_CONFIG.log_folder)!,
101104
custom_engine_plugin_modules: configExtractor.extractArray(FIELDS.CUSTOM_ENGINE_PLUGIN_MODULES,
102-
ValueValidator.validateString,
105+
engApi.ValueValidator.validateString,
103106
DEFAULT_CONFIG.custom_engine_plugin_modules)!,
104107
rules: extractRulesValue(configExtractor),
105108
engines: extractEnginesValue(configExtractor)
106109
}
107110
return new CodeAnalyzerConfig(config);
108111
}
109112

110-
public static getConfigDescription(): ConfigDescription {
111-
return TOP_LEVEL_CONFIG_DESCRIPTION;
113+
public getConfigDescription(): ConfigDescription {
114+
return {
115+
overview: getMessage('ConfigOverview'),
116+
fieldDescriptions: {
117+
config_root: {
118+
descriptionText: getMessage('ConfigFieldDescription_config_root'),
119+
valueType: 'string',
120+
defaultValue: null, // Using null for doc and since it indicates that the value is calculated based on the environment
121+
wasSuppliedByUser: this.config.config_root !== DEFAULT_CONFIG.config_root
122+
},
123+
log_folder: {
124+
descriptionText: getMessage('ConfigFieldDescription_log_folder'),
125+
valueType: 'string',
126+
defaultValue: null, // Using null for doc and since it indicates that the value is calculated based on the environment
127+
wasSuppliedByUser: this.config.log_folder !== DEFAULT_CONFIG.log_folder
128+
},
129+
rules: {
130+
descriptionText: getMessage('ConfigFieldDescription_rules'),
131+
valueType: 'object',
132+
defaultValue: {},
133+
wasSuppliedByUser: this.config.rules !== DEFAULT_CONFIG.rules
134+
},
135+
engines: {
136+
descriptionText: getMessage('ConfigFieldDescription_engines'),
137+
valueType: 'object',
138+
defaultValue: {},
139+
wasSuppliedByUser: this.config.engines !== DEFAULT_CONFIG.engines
140+
}
141+
}
142+
};
112143
}
113144

114145
private constructor(config: TopLevelConfig) {
@@ -160,7 +191,7 @@ function extractRuleOverridesFrom(engineRuleOverridesExtractor: engApi.ConfigVal
160191
function extractRuleOverrideFrom(ruleOverrideExtractor: engApi.ConfigValueExtractor): RuleOverride {
161192
const engSeverity: engApi.SeverityLevel | undefined = ruleOverrideExtractor.extractSeverityLevel(FIELDS.SEVERITY);
162193
return {
163-
tags: ruleOverrideExtractor.extractArray(FIELDS.TAGS, ValueValidator.validateString),
194+
tags: ruleOverrideExtractor.extractArray(FIELDS.TAGS, engApi.ValueValidator.validateString),
164195
severity: engSeverity === undefined ? undefined : engSeverity as SeverityLevel
165196
}
166197
}
@@ -194,17 +225,19 @@ function parseAndValidate(parseFcn: () => unknown): object {
194225
function validateAbsoluteFolder(value: unknown, fieldPath: string): string {
195226
const folderValue: string = validateAbsolutePath(value, fieldPath);
196227
if (!fs.statSync(folderValue).isDirectory()) {
197-
throw new Error(getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigFolderValueMustNotBeFile', fieldPath, folderValue));
228+
throw new Error(engApi.getMessageFromCatalog(engApi.SHARED_MESSAGE_CATALOG,
229+
'ConfigFolderValueMustNotBeFile', fieldPath, folderValue));
198230
}
199231
return folderValue;
200232
}
201233

202234
function validateAbsolutePath(value: unknown, fieldPath: string): string {
203-
const pathValue: string = ValueValidator.validateString(value, fieldPath);
235+
const pathValue: string = engApi.ValueValidator.validateString(value, fieldPath);
204236
if (pathValue !== toAbsolutePath(pathValue)) {
205237
throw new Error(getMessage('ConfigPathValueMustBeAbsolute', fieldPath, pathValue, toAbsolutePath(pathValue)));
206238
} else if (!fs.existsSync(pathValue)) {
207-
throw new Error(getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigPathValueDoesNotExist', fieldPath, pathValue));
239+
throw new Error(engApi.getMessageFromCatalog(engApi.SHARED_MESSAGE_CATALOG,
240+
'ConfigPathValueDoesNotExist', fieldPath, pathValue));
208241
}
209242
return pathValue;
210243
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
export {
22
CodeAnalyzerConfig,
33
ConfigDescription,
4+
ConfigFieldDescription,
45
EngineOverrides,
56
RuleOverrides,
67
RuleOverride

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

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,16 +185,31 @@ describe("Tests for adding engines to Code Analyzer", () => {
185185
expect(engineConfigDescription1).toEqual({
186186
overview: "OverviewForStub1",
187187
fieldDescriptions: {
188-
disable_engine: getMessage('EngineConfigFieldDescription_disable_engine', 'stubEngine1'),
189-
miscSetting1: "someDescriptionFor_miscSetting1"
188+
disable_engine: {
189+
descriptionText: getMessage('EngineConfigFieldDescription_disable_engine', 'stubEngine1'),
190+
valueType: "boolean",
191+
defaultValue: false,
192+
wasSuppliedByUser: false
193+
},
194+
misc_value: {
195+
descriptionText: "someDescriptionFor_misc_value",
196+
valueType: "number",
197+
defaultValue: 1987,
198+
wasSuppliedByUser: false
199+
}
190200
}
191201
});
192202
const engineConfigDescription2: ConfigDescription = codeAnalyzer.getEngineConfigDescription('stubEngine2');
193203
expect(engineConfigDescription2).toEqual({
194204
overview: getMessage('GenericEngineConfigOverview', 'STUBENGINE2'),
195205
fieldDescriptions: {
196-
disable_engine: getMessage('EngineConfigFieldDescription_disable_engine', 'stubEngine2')
197-
}
206+
disable_engine: {
207+
descriptionText: getMessage('EngineConfigFieldDescription_disable_engine', 'stubEngine2'),
208+
valueType: "boolean",
209+
defaultValue: false,
210+
wasSuppliedByUser: false
211+
}
212+
},
198213
});
199214
});
200215

@@ -220,8 +235,18 @@ describe("Tests for adding engines to Code Analyzer", () => {
220235
expect(codeAnalyzer.getEngineConfigDescription('stubEngine1')).toEqual({
221236
overview: "OverviewForStub1",
222237
fieldDescriptions: {
223-
disable_engine: getMessage('EngineConfigFieldDescription_disable_engine', 'stubEngine1'),
224-
miscSetting1: "someDescriptionFor_miscSetting1"
238+
disable_engine: {
239+
descriptionText: getMessage('EngineConfigFieldDescription_disable_engine', 'stubEngine1'),
240+
valueType: "boolean",
241+
defaultValue: false,
242+
wasSuppliedByUser: true
243+
},
244+
misc_value: {
245+
descriptionText: "someDescriptionFor_misc_value",
246+
valueType: "number",
247+
defaultValue: 1987,
248+
wasSuppliedByUser: true
249+
}
225250
}
226251
});
227252
});

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

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import {CodeAnalyzerConfig, SeverityLevel} from "../src";
22
import * as os from "node:os";
33
import * as path from "node:path";
4-
import {ConfigDescription, getMessageFromCatalog, SHARED_MESSAGE_CATALOG} from "@salesforce/code-analyzer-engine-api";
4+
import {getMessageFromCatalog, SHARED_MESSAGE_CATALOG} from "@salesforce/code-analyzer-engine-api";
55
import {getMessage} from "../src/messages";
66
import {changeWorkingDirectoryToPackageRoot} from "./test-helpers";
7-
import {TOP_LEVEL_CONFIG_DESCRIPTION, DEFAULT_CONFIG} from "../src/config";
7+
import {ConfigDescription, DEFAULT_CONFIG} from "../src/config";
88

99
changeWorkingDirectoryToPackageRoot();
1010

@@ -273,8 +273,45 @@ describe("Tests for creating and accessing configuration values", () => {
273273
'engines.stubEngine1.disable_engine', 'boolean', 'number'));
274274
});
275275

276-
it("When getConfigDescription is called, then it returns our expected description object", () => {
277-
const configDescription: ConfigDescription = CodeAnalyzerConfig.getConfigDescription();
278-
expect(configDescription).toEqual(TOP_LEVEL_CONFIG_DESCRIPTION);
276+
it("When getConfigDescription is called from default config, then it returns our expected description object", () => {
277+
const configDescription: ConfigDescription = CodeAnalyzerConfig.withDefaults().getConfigDescription();
278+
expect(configDescription.overview).toEqual(getMessage('ConfigOverview'));
279+
expect(configDescription.fieldDescriptions).toEqual({
280+
config_root: {
281+
descriptionText: getMessage('ConfigFieldDescription_config_root'),
282+
valueType: 'string',
283+
defaultValue: null,
284+
wasSuppliedByUser: false
285+
},
286+
log_folder: {
287+
descriptionText: getMessage('ConfigFieldDescription_log_folder'),
288+
valueType: 'string',
289+
defaultValue: null,
290+
wasSuppliedByUser: false
291+
},
292+
rules: {
293+
descriptionText: getMessage('ConfigFieldDescription_rules'),
294+
valueType: 'object',
295+
defaultValue: {},
296+
wasSuppliedByUser: false
297+
},
298+
engines: {
299+
descriptionText: getMessage('ConfigFieldDescription_engines'),
300+
valueType: 'object',
301+
defaultValue: {},
302+
wasSuppliedByUser: false
303+
}
304+
});
305+
});
306+
307+
it("When getConfigDescription is called from modified config, then it correctly sets wasSuppliedByUser fields", () => {
308+
const configDescription: ConfigDescription = CodeAnalyzerConfig.fromObject(
309+
{config_root: __dirname, rules: {"someEngine": {"abc": {severity: SeverityLevel.High}}}}
310+
).getConfigDescription();
311+
expect(configDescription.overview).toEqual(getMessage('ConfigOverview'));
312+
expect(configDescription.fieldDescriptions.config_root.wasSuppliedByUser).toEqual(true);
313+
expect(configDescription.fieldDescriptions.log_folder.wasSuppliedByUser).toEqual(false);
314+
expect(configDescription.fieldDescriptions.rules.wasSuppliedByUser).toEqual(true);
315+
expect(configDescription.fieldDescriptions.engines.wasSuppliedByUser).toEqual(false);
279316
});
280317
});

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ export class StubEnginePlugin extends engApi.EnginePluginV1 {
2626
return {
2727
overview: 'OverviewForStub1',
2828
fieldDescriptions: {
29-
miscSetting1: "someDescriptionFor_miscSetting1"
29+
misc_value: {
30+
descriptionText: "someDescriptionFor_misc_value",
31+
valueType: "number",
32+
defaultValue: 1987
33+
}
3034
}
3135
}
3236
}

packages/code-analyzer-engine-api/src/config.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,26 @@ export type ConfigDescription = {
1818
// A brief overview of the configuration. It is recommended to include a link to documentation when possible.
1919
overview?: string
2020

21-
// Description messages of the top-level fields in the configuration
22-
fieldDescriptions?: Record<string, string>
21+
// Description objects for the primary fields in the configuration
22+
fieldDescriptions?: Record<string, ConfigFieldDescription>
23+
}
24+
25+
export type ConfigFieldDescription = {
26+
// Text that describes the field to be documented
27+
descriptionText: string
28+
29+
// The type for the value associated with this field (as a string).
30+
// Note that this as a string instead of an enum in order to give flexibility to describe the type of the field as
31+
// we want it to show up in our public docs, but it is recommended to return one of the following when possible:
32+
// 'string', 'number', 'boolean', 'object', 'array'
33+
valueType: string
34+
35+
// The default value that is to be documented. Use null if you do not have a fixed default value.
36+
// Note that this value may not necessarily be the same that could be auto calculated if the user doesn't provide
37+
// the value in their configuration file. For example, we may want to report null as the default value for a
38+
// java_command field whenever the user doesn't explicitly provide one even though it might automatically
39+
// get calculated at runtime to be some java command found in their environment.
40+
defaultValue: ConfigValue
2341
}
2442

2543
export class ConfigValueExtractor {

packages/code-analyzer-engine-api/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
export {
22
ConfigDescription,
3+
ConfigFieldDescription,
34
ConfigObject,
45
ConfigValue,
56
ConfigValueExtractor,

0 commit comments

Comments
 (0)