Skip to content

Commit f105bfe

Browse files
Merge pull request #1691 from forcedotcom/sc/W-17293079
CHANGE: @W-17293079@: Update dependencies, refactor config command that fixes python_command issue
2 parents be7eb26 + abbf926 commit f105bfe

File tree

11 files changed

+314
-192
lines changed

11 files changed

+314
-192
lines changed

package.json

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",
77
"dependencies": {
88
"@oclif/core": "^3.3.2",
9-
"@salesforce/code-analyzer-core": "0.17.0",
10-
"@salesforce/code-analyzer-engine-api": "0.14.0",
11-
"@salesforce/code-analyzer-eslint-engine": "0.14.0",
12-
"@salesforce/code-analyzer-flowtest-engine": "0.14.1",
13-
"@salesforce/code-analyzer-pmd-engine": "0.14.0",
14-
"@salesforce/code-analyzer-regex-engine": "0.14.0",
15-
"@salesforce/code-analyzer-retirejs-engine": "0.14.0",
9+
"@salesforce/code-analyzer-core": "0.18.0",
10+
"@salesforce/code-analyzer-engine-api": "0.15.0",
11+
"@salesforce/code-analyzer-eslint-engine": "0.15.0",
12+
"@salesforce/code-analyzer-flowtest-engine": "0.15.0",
13+
"@salesforce/code-analyzer-pmd-engine": "0.15.0",
14+
"@salesforce/code-analyzer-regex-engine": "0.15.0",
15+
"@salesforce/code-analyzer-retirejs-engine": "0.15.0",
1616
"@salesforce/core": "^5",
1717
"@salesforce/sf-plugins-core": "^5.0.4",
1818
"@salesforce/ts-types": "^2.0.9",

src/commands/code-analyzer/config.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {EnginePluginsFactoryImpl} from '../../lib/factories/EnginePluginsFactory
88
import {BundleName, getMessage, getMessages} from '../../lib/messages';
99
import {LogEventDisplayer} from '../../lib/listeners/LogEventListener';
1010
import {RuleSelectionProgressSpinner} from '../../lib/listeners/ProgressEventListener';
11-
import {AnnotatedConfigModel, ConfigContext} from '../../lib/models/ConfigModel';
1211
import {Displayable, UxDisplay} from '../../lib/Display';
1312

1413
export default class ConfigCommand extends SfCommand<void> implements Displayable {
@@ -62,15 +61,11 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
6261

6362
protected createDependencies(outputFile?: string): ConfigDependencies {
6463
const uxDisplay: UxDisplay = new UxDisplay(this, this.spinner);
65-
const modelGeneratorFunction = /* istanbul ignore next - Model tested separately */ (relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) => {
66-
return AnnotatedConfigModel.fromSelection(relevantEngines, userContext, defaultContext);
67-
};
6864
const dependencies: ConfigDependencies = {
6965
configFactory: new CodeAnalyzerConfigFactoryImpl(),
7066
pluginsFactory: new EnginePluginsFactoryImpl(),
7167
logEventListeners: [new LogEventDisplayer(uxDisplay)],
7268
progressEventListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
73-
modelGenerator: modelGeneratorFunction,
7469
actionSummaryViewer: new ConfigActionSummaryViewer(uxDisplay),
7570
viewer: new ConfigStyledYamlViewer(uxDisplay)
7671
};

src/lib/actions/ConfigAction.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ import {createWorkspace} from '../utils/WorkspaceUtil';
88
import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener';
99
import {ProgressEventListener} from '../listeners/ProgressEventListener';
1010
import {ConfigActionSummaryViewer} from '../viewers/ActionSummaryViewer';
11-
import {ConfigModel, ConfigModelGeneratorFunction, ConfigContext} from '../models/ConfigModel';
11+
import {AnnotatedConfigModel, ConfigModel} from '../models/ConfigModel';
1212

1313
export type ConfigDependencies = {
1414
configFactory: CodeAnalyzerConfigFactory;
1515
pluginsFactory: EnginePluginsFactory;
16-
modelGenerator: ConfigModelGeneratorFunction;
1716
logEventListeners: LogEventListener[];
1817
progressEventListeners: ProgressEventListener[];
1918
writer?: ConfigWriter;
@@ -109,17 +108,7 @@ export class ConfigAction {
109108
// We need the Set of all Engines that returned rules for the user's selection on both the Default and User Cores.
110109
const relevantEngines: Set<string> = new Set([...userRules.getEngineNames(), ...selectedDefaultRules.getEngineNames()]);
111110

112-
const userConfigContext: ConfigContext = {
113-
config: userConfig,
114-
core: userCore,
115-
rules: userRules
116-
};
117-
const defaultConfigContext: ConfigContext = {
118-
config: defaultConfig,
119-
core: defaultCoreForAllRules,
120-
rules: allDefaultRules
121-
};
122-
const configModel: ConfigModel = this.dependencies.modelGenerator(relevantEngines, userConfigContext, defaultConfigContext);
111+
const configModel: ConfigModel = new AnnotatedConfigModel(userConfig, userCore, userRules, allDefaultRules, relevantEngines);
123112

124113
const fileWritten: boolean = this.dependencies.writer
125114
? await this.dependencies.writer.write(configModel)

src/lib/messages.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ export enum BundleName {
1717
ResultsWriter = 'results-writer',
1818
RunAction = 'run-action',
1919
RunCommand = 'run-command',
20-
RunSummaryViewer = 'run-summary-viewer',
2120
Shared = 'shared',
2221
WorkspaceUtil = 'workspace-util'
2322
}

src/lib/models/ConfigModel.ts

Lines changed: 79 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {
33
CodeAnalyzer,
44
CodeAnalyzerConfig,
55
ConfigDescription,
6+
ConfigFieldDescription,
7+
EngineConfig,
68
Rule,
79
RuleSelection,
810
SeverityLevel
@@ -20,50 +22,51 @@ export interface ConfigModel {
2022
toFormattedOutput(format: OutputFormat): string;
2123
}
2224

23-
export type ConfigContext = {
24-
config: CodeAnalyzerConfig;
25-
core: CodeAnalyzer;
26-
rules: RuleSelection;
27-
}
28-
29-
export type ConfigModelGeneratorFunction = (relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) => ConfigModel;
30-
3125
export class AnnotatedConfigModel implements ConfigModel {
32-
private readonly relevantEngines: Set<string>;
33-
private readonly userContext: ConfigContext;
34-
private readonly defaultContext: ConfigContext;
35-
36-
private constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
26+
private readonly config: CodeAnalyzerConfig; // TODO: It would be nice if we updated the CodeAnalyzer (in our core module) to just return its CodeAnalyzerConfig with a getter so we didn't need to pass it around
27+
private readonly codeAnalyzer: CodeAnalyzer;
28+
private readonly userRules: RuleSelection;
29+
private readonly allDefaultRules: RuleSelection;
30+
31+
// Note that it is important that we calculate the relevant engines list based on (the user rule selection with no
32+
// config) plus (user rule selection with user config) since we still want to show the "disable_engine" config value
33+
// in the output if a user even if selects an engine that is currently disabled. But we don't want to the engine
34+
// configs not associated with the user's rule selection, thus we can't use the engines from allDefaultRules.
35+
private readonly relevantEngines: Set<string>;
36+
37+
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
38+
this.config = config;
39+
this.codeAnalyzer = codeAnalyzer;
40+
this.userRules = userRules;
41+
this.allDefaultRules = allDefaultRules;
3742
this.relevantEngines = relevantEngines;
38-
this.userContext = userContext;
39-
this.defaultContext = defaultContext;
4043
}
4144

4245
toFormattedOutput(format: OutputFormat): string {
4346
// istanbul ignore else: Should be impossible
4447
if (format === OutputFormat.STYLED_YAML) {
45-
return new StyledYamlFormatter(this.relevantEngines, this.userContext, this.defaultContext).toYaml();
48+
return new StyledYamlFormatter(this.config, this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
4649
} else if (format === OutputFormat.RAW_YAML) {
47-
return new PlainYamlFormatter(this.relevantEngines, this.userContext, this.defaultContext).toYaml();
50+
return new PlainYamlFormatter(this.config, this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
4851
} else {
4952
throw new Error(`Unsupported`)
5053
}
5154
}
52-
53-
public static fromSelection(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext): AnnotatedConfigModel {
54-
return new AnnotatedConfigModel(relevantEngines, userContext, defaultContext);
55-
}
5655
}
5756

5857
abstract class YamlFormatter {
58+
private readonly config: CodeAnalyzerConfig;
59+
private readonly codeAnalyzer: CodeAnalyzer;
60+
private readonly userRules: RuleSelection;
61+
private readonly allDefaultRules: RuleSelection;
5962
private readonly relevantEngines: Set<string>;
60-
private readonly userContext: ConfigContext;
61-
private readonly defaultContext: ConfigContext;
6263

63-
protected constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
64+
protected constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
65+
this.config = config;
66+
this.codeAnalyzer = codeAnalyzer;
67+
this.userRules = userRules;
68+
this.allDefaultRules = allDefaultRules;
6469
this.relevantEngines = relevantEngines;
65-
this.userContext = userContext;
66-
this.defaultContext = defaultContext;
6770
}
6871

6972
protected abstract toYamlComment(commentText: string): string
@@ -83,50 +86,65 @@ abstract class YamlFormatter {
8386
return yamlCode.replace(/(\r?\n|$)/, ` ${comment}$1`);
8487
}
8588

86-
private toYamlField(fieldName: string, userValue: unknown, defaultValue: unknown): string {
87-
if (looksLikeAPathValue(userValue) && userValue === defaultValue) {
88-
// We special handle a path value when it is equal to the default value, making it equal null because
89-
// chances are it is a derived file or folder value based on the specific environment that we do not want to
90-
// actually want to hard code since checking in the config to CI/CD system may create a different value
91-
const commentText: string = getMessage(BundleName.ConfigModel, 'template.last-calculated-as', [JSON.stringify(userValue)]);
92-
return this.toYamlUncheckedFieldWithInlineComment(fieldName, null, commentText);
93-
} else if (JSON.stringify(userValue) === JSON.stringify(defaultValue)) {
94-
return this.toYamlUncheckedField(fieldName, userValue);
89+
private toYamlFieldWithFieldDescription(fieldName: string, resolvedValue: unknown, fieldDescription: ConfigFieldDescription): string {
90+
const resolvedValueJson: string = JSON.stringify(resolvedValue);
91+
const defaultValueJson: string = JSON.stringify(fieldDescription.defaultValue);
92+
93+
let yamlField: string;
94+
if (!fieldDescription.wasSuppliedByUser && resolvedValueJson !== defaultValueJson) {
95+
// Whenever the user did not supply the value themselves but the resolved value is different from the
96+
// default value, this means the value was not a "fixed" value but a value "calculated" at runtime.
97+
// Since "calculated" values often depend on the specific environment, we do not want to actually hard code
98+
// this value into the config since checking in the config to CI/CD system may create a different value.
99+
const commentText: string = getMessage(BundleName.ConfigModel, 'template.last-calculated-as', [resolvedValueJson]);
100+
yamlField = this.toYamlUncheckedFieldWithInlineComment(fieldName, fieldDescription.defaultValue, commentText);
101+
} else {
102+
yamlField = this.toYamlField(fieldName, resolvedValue, fieldDescription.defaultValue);
95103
}
96104

97-
userValue = replaceAbsolutePathsWithRelativePathsWherePossible(userValue, this.userContext.config.getConfigRoot() + path.sep);
105+
return this.toYamlComment(fieldDescription.descriptionText) + "\n" + yamlField
106+
}
107+
108+
private toYamlField(fieldName: string, resolvedValue: unknown, defaultValue: unknown): string {
109+
const resolvedValueJson: string = JSON.stringify(resolvedValue);
110+
const defaultValueJson: string = JSON.stringify(defaultValue);
98111

99-
const commentText: string = getMessage(BundleName.ConfigModel, 'template.modified-from', [JSON.stringify(defaultValue)]);
100-
return this.toYamlUncheckedFieldWithInlineComment(fieldName, userValue, commentText);
112+
if (resolvedValueJson === defaultValueJson) {
113+
return this.toYamlUncheckedField(fieldName, resolvedValue);
114+
}
115+
116+
const commentText: string = getMessage(BundleName.ConfigModel, 'template.modified-from', [defaultValueJson]);
117+
resolvedValue = replaceAbsolutePathsWithRelativePathsWherePossible(resolvedValue, this.config.getConfigRoot() + path.sep);
118+
return this.toYamlUncheckedFieldWithInlineComment(fieldName, resolvedValue, commentText);
101119
}
102120

103121
toYaml(): string {
104-
const topLevelDescription: ConfigDescription = CodeAnalyzerConfig.getConfigDescription();
105-
return this.toYamlSectionHeadingComment(topLevelDescription.overview!) + '\n' +
122+
const topLevelDescription: ConfigDescription = this.config.getConfigDescription();
123+
return this.toYamlSectionHeadingComment(topLevelDescription.overview) + '\n' +
106124
'\n' +
107-
this.toYamlComment(topLevelDescription.fieldDescriptions!.config_root) + '\n' +
108-
this.toYamlField('config_root', this.userContext.config.getConfigRoot(), this.defaultContext.config.getConfigRoot()) + '\n' +
125+
this.toYamlFieldWithFieldDescription('config_root', this.config.getConfigRoot(),
126+
topLevelDescription.fieldDescriptions.config_root) + '\n' +
109127
'\n' +
110-
this.toYamlComment(topLevelDescription.fieldDescriptions!.log_folder) + '\n' +
111-
this.toYamlField('log_folder', this.userContext.config.getLogFolder(), this.defaultContext.config.getLogFolder()) + '\n' +
128+
this.toYamlFieldWithFieldDescription('log_folder', this.config.getLogFolder(),
129+
topLevelDescription.fieldDescriptions.log_folder) + '\n' +
112130
'\n' +
113-
this.toYamlComment(topLevelDescription.fieldDescriptions!.rules) + '\n' +
131+
this.toYamlComment(topLevelDescription.fieldDescriptions.rules.descriptionText) + '\n' +
114132
this.toYamlRuleOverrides() + '\n' +
115133
'\n' +
116-
this.toYamlComment(topLevelDescription.fieldDescriptions!.engines) + '\n' +
134+
this.toYamlComment(topLevelDescription.fieldDescriptions.engines.descriptionText) + '\n' +
117135
this.toYamlEngineOverrides() + '\n' +
118136
'\n' +
119137
this.toYamlSectionHeadingComment(getMessage(BundleName.ConfigModel, 'template.common.end-of-config')) + '\n';
120138
}
121139

122140
private toYamlRuleOverrides(): string {
123-
if (this.userContext.rules.getCount() === 0) {
141+
if (this.userRules.getCount() === 0) {
124142
const commentText: string = getMessage(BundleName.ConfigModel, 'template.yaml.no-rules-selected');
125143
return `rules: {} ${this.toYamlComment(commentText)}`;
126144
}
127145

128146
let yamlCode: string = 'rules:\n';
129-
for (const engineName of this.userContext.rules.getEngineNames()) {
147+
for (const engineName of this.userRules.getEngineNames()) {
130148
yamlCode += '\n';
131149
yamlCode += indent(this.toYamlRuleOverridesForEngine(engineName), 2) + '\n';
132150
}
@@ -138,7 +156,7 @@ abstract class YamlFormatter {
138156
[engineName.toUpperCase()]);
139157
let yamlCode: string = this.toYamlSectionHeadingComment(engineConfigHeader) + '\n';
140158
yamlCode += `${engineName}:\n`;
141-
for (const userRule of this.userContext.rules.getRulesFor(engineName)) {
159+
for (const userRule of this.userRules.getRulesFor(engineName)) {
142160
const defaultRule: Rule|null = this.getDefaultRuleFor(engineName, userRule.getName());
143161
yamlCode += indent(this.toYamlRuleOverridesForRule(userRule, defaultRule), 2) + '\n';
144162
}
@@ -147,7 +165,7 @@ abstract class YamlFormatter {
147165

148166
private getDefaultRuleFor(engineName: string, ruleName: string): Rule|null {
149167
try {
150-
return this.defaultContext.rules.getRule(engineName, ruleName);
168+
return this.allDefaultRules.getRule(engineName, ruleName);
151169
} catch (e) {
152170
// istanbul ignore next
153171
return null;
@@ -176,31 +194,28 @@ abstract class YamlFormatter {
176194
}
177195

178196
private toYamlEngineOverridesForEngine(engineName: string): string {
179-
const engineConfigDescriptor = this.userContext.core.getEngineConfigDescription(engineName);
180-
const userEngineConfig = this.userContext.core.getEngineConfig(engineName);
181-
const defaultEngineConfig = this.defaultContext.core.getEngineConfig(engineName);
197+
const engineConfigDescriptor: ConfigDescription = this.codeAnalyzer.getEngineConfigDescription(engineName);
198+
const userEngineConfig: EngineConfig = this.codeAnalyzer.getEngineConfig(engineName);
182199

183200
let yamlCode: string = '\n' +
184-
this.toYamlSectionHeadingComment(engineConfigDescriptor.overview!) + '\n' +
201+
this.toYamlSectionHeadingComment(engineConfigDescriptor.overview) + '\n' +
185202
`${engineName}:\n`;
186203
// By fiat, the field description will always include, at minimum, an entry for "disable_engine", so we can
187204
// assume that the object is not undefined.
188-
for (const configField of Object.keys(engineConfigDescriptor.fieldDescriptions!)) {
189-
const fieldDescription = engineConfigDescriptor.fieldDescriptions![configField];
190-
const defaultValue = defaultEngineConfig[configField] ?? null;
191-
const userValue = userEngineConfig[configField] ?? defaultValue;
205+
for (const configField of Object.keys(engineConfigDescriptor.fieldDescriptions)) {
206+
const fieldDescription: ConfigFieldDescription = engineConfigDescriptor.fieldDescriptions[configField];
207+
const resolvedValue = userEngineConfig[configField] ?? fieldDescription.defaultValue;
192208
// Add a leading newline to visually break up the property from the previous one.
193209
yamlCode += '\n' +
194-
indent(this.toYamlComment(fieldDescription), 2) + '\n' +
195-
indent(this.toYamlField(configField, userValue, defaultValue), 2) + '\n';
210+
indent(this.toYamlFieldWithFieldDescription(configField, resolvedValue, fieldDescription), 2) + '\n';
196211
}
197212
return yamlCode.trimEnd();
198213
}
199214
}
200215

201216
class PlainYamlFormatter extends YamlFormatter {
202-
constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
203-
super(relevantEngines, userContext, defaultContext);
217+
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
218+
super(config, codeAnalyzer, userRules, allDefaultRules, relevantEngines);
204219
}
205220

206221
protected toYamlComment(commentText: string): string {
@@ -209,19 +224,15 @@ class PlainYamlFormatter extends YamlFormatter {
209224
}
210225

211226
class StyledYamlFormatter extends YamlFormatter {
212-
constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
213-
super(relevantEngines, userContext, defaultContext);
227+
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
228+
super(config, codeAnalyzer, userRules, allDefaultRules, relevantEngines);
214229
}
215230

216231
protected toYamlComment(commentText: string): string {
217232
return commentText.replace(/^.*/gm, s => makeGrey(`# ${s}`));
218233
}
219234
}
220235

221-
function looksLikeAPathValue(value: unknown) {
222-
return typeof(value) === 'string' && !value.includes('\n') && value.includes(path.sep);
223-
}
224-
225236
function replaceAbsolutePathsWithRelativePathsWherePossible(value: unknown, parentFolder: string): unknown {
226237
if (typeof value === 'string') {
227238
// Check if the string starts with the parent folder

test/fixtures/comparison-files/lib/actions/ConfigAction.test.ts/override-configurations/StubEngine2_forConfigWithRelativePathScenario.yml.goldfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@
77
disable_engine: false
88

99
# Some description for top_field
10-
top_field: # Modified from: null
10+
top_field: # Modified from: {}
1111
sub_field:
1212
- optional-input-config.yml

0 commit comments

Comments
 (0)