Final polish

Author: stephen-carter-at-sf

src/commands/code-analyzer/config.ts

@@ -8,7 +8,6 @@ import {EnginePluginsFactoryImpl} from '../../lib/factories/EnginePluginsFactory
 import {BundleName, getMessage, getMessages} from '../../lib/messages';
 import {LogEventDisplayer} from '../../lib/listeners/LogEventListener';
 import {RuleSelectionProgressSpinner} from '../../lib/listeners/ProgressEventListener';
-import {AnnotatedConfigModel} from '../../lib/models/ConfigModel';
 import {Displayable, UxDisplay} from '../../lib/Display';
 
 export default class ConfigCommand extends SfCommand<void> implements Displayable {
@@ -67,7 +66,6 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
 			pluginsFactory: new EnginePluginsFactoryImpl(),
 			logEventListeners: [new LogEventDisplayer(uxDisplay)],
 			progressEventListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
-			modelGenerator: AnnotatedConfigModel,
 			actionSummaryViewer: new ConfigActionSummaryViewer(uxDisplay),
 			viewer: new ConfigStyledYamlViewer(uxDisplay)
 		};

src/lib/actions/ConfigAction.ts

@@ -8,12 +8,11 @@ import {createWorkspace} from '../utils/WorkspaceUtil';
 import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener';
 import {ProgressEventListener} from '../listeners/ProgressEventListener';
 import {ConfigActionSummaryViewer} from '../viewers/ActionSummaryViewer';
-import {ConfigModel, ConfigModelConstructor} from '../models/ConfigModel';
+import {AnnotatedConfigModel, ConfigModel} from '../models/ConfigModel';
 
 export type ConfigDependencies = {
 	configFactory: CodeAnalyzerConfigFactory;
 	pluginsFactory: EnginePluginsFactory;
-	modelGenerator: ConfigModelConstructor;
 	logEventListeners: LogEventListener[];
 	progressEventListeners: ProgressEventListener[];
 	writer?: ConfigWriter;
@@ -109,7 +108,7 @@ export class ConfigAction {
 		// We need the Set of all Engines that returned rules for the user's selection on both the Default and User Cores.
 		const relevantEngines: Set<string> = new Set([...userRules.getEngineNames(), ...selectedDefaultRules.getEngineNames()]);
 
-		const configModel: ConfigModel = new this.dependencies.modelGenerator(userConfig, userCore, userRules, allDefaultRules, relevantEngines);
+		const configModel: ConfigModel = new AnnotatedConfigModel(userConfig, userCore, userRules, allDefaultRules, relevantEngines);
 
 		const fileWritten: boolean = this.dependencies.writer
 			? await this.dependencies.writer.write(configModel)

src/lib/models/ConfigModel.ts

@@ -2,7 +2,8 @@ import {dump as yamlDump} from 'js-yaml';
 import {
 	CodeAnalyzer,
 	CodeAnalyzerConfig,
-	ConfigDescription, ConfigFieldDescription,
+	ConfigDescription,
+	ConfigFieldDescription,
 	EngineConfig,
 	Rule,
 	RuleSelection,
@@ -21,13 +22,16 @@ export interface ConfigModel {
 	toFormattedOutput(format: OutputFormat): string;
 }
 
-export type ConfigModelConstructor = new (config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) => ConfigModel;
-
 export class AnnotatedConfigModel implements ConfigModel {
 	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
 	private readonly codeAnalyzer: CodeAnalyzer;
 	private readonly userRules: RuleSelection;
 	private readonly allDefaultRules: RuleSelection;
+
+	// Note that it is important that we calculate the relevant engines list based on (the user rule selection with no
+	// config) plus (user rule selection with user config) since we still want to show the "disable_engine" config value
+	// in the output if a user even if selects an engine that is currently disabled. But we don't want to the engine
+	// configs not associated with the user's rule selection, thus we can't use the engines from allDefaultRules.
 	private readonly  relevantEngines: Set<string>;
 
 	constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
@@ -199,11 +203,11 @@ abstract class YamlFormatter {
 		// assume that the object is not undefined.
 		for (const configField of Object.keys(engineConfigDescriptor.fieldDescriptions)) {
 			const fieldDescription: ConfigFieldDescription = engineConfigDescriptor.fieldDescriptions[configField];
-			const userValue = userEngineConfig[configField] ?? fieldDescription.defaultValue;
+			const resolvedValue = userEngineConfig[configField] ?? fieldDescription.defaultValue;
 			// Add a leading newline to visually break up the property from the previous one.
 			yamlCode += '\n' +
 				indent(this.toYamlComment(fieldDescription.descriptionText), 2) + '\n' +
-				indent(this.toYamlFieldUsingFieldDescription(configField, userValue, fieldDescription), 2) + '\n';
+				indent(this.toYamlFieldUsingFieldDescription(configField, resolvedValue, fieldDescription), 2) + '\n';
 		}
 		return yamlCode.trimEnd();
 	}

test/lib/actions/ConfigAction.test.ts

@@ -8,7 +8,6 @@ import * as EngineApi from "@salesforce/code-analyzer-engine-api";
 import {CodeAnalyzerConfigFactory} from "../../../src/lib/factories/CodeAnalyzerConfigFactory";
 import {EnginePluginsFactory} from '../../../src/lib/factories/EnginePluginsFactory';
 import {ConfigAction, ConfigDependencies, ConfigInput} from '../../../src/lib/actions/ConfigAction';
-import {AnnotatedConfigModel} from '../../../src/lib/models/ConfigModel';
 import {ConfigStyledYamlViewer} from '../../../src/lib/viewers/ConfigViewer';
 import {ConfigActionSummaryViewer} from '../../../src/lib/viewers/ActionSummaryViewer';
 
@@ -37,7 +36,6 @@ describe('ConfigAction tests', () => {
 					progressEventListeners: [],
 					viewer: new ConfigStyledYamlViewer(spyDisplay),
 					configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
-					modelGenerator: AnnotatedConfigModel,
 					actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
 					pluginsFactory: new StubEnginePluginFactory()
 				};
@@ -164,7 +162,6 @@ describe('ConfigAction tests', () => {
 					progressEventListeners: [],
 					viewer: new ConfigStyledYamlViewer(spyDisplay),
 					configFactory: stubConfigFactory,
-					modelGenerator: AnnotatedConfigModel,
 					actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
 					pluginsFactory: new StubEnginePluginFactory()
 				};
@@ -397,7 +394,6 @@ describe('ConfigAction tests', () => {
 				progressEventListeners: [],
 				viewer: new ConfigStyledYamlViewer(spyDisplay),
 				configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
-				modelGenerator: AnnotatedConfigModel,
 				actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
 				pluginsFactory: new StubEnginePluginFactory()
 			};
@@ -446,7 +442,6 @@ describe('ConfigAction tests', () => {
 				progressEventListeners: [],
 				viewer: new ConfigStyledYamlViewer(spyDisplay),
 				configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
-				modelGenerator: AnnotatedConfigModel,
 				actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
 				pluginsFactory: new StubEnginePluginFactory()
 			}