Skip to content

Commit f3cb27f

Browse files
stephen-carter-at-sfstephen-carter-at-sf
committed
Some refactoring
1 parent 3ae4cd6 commit f3cb27f

File tree

4 files changed

+46
-61
lines changed

4 files changed

+46
-61
lines changed

‎src/commands/code-analyzer/config.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ 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';
11+
import {AnnotatedConfigModel} from '../../lib/models/ConfigModel';
1212
import {Displayable, UxDisplay} from '../../lib/Display';
1313

1414
export default class ConfigCommand extends SfCommand<void> implements Displayable {
@@ -62,15 +62,12 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl
6262

6363
protected createDependencies(outputFile?: string): ConfigDependencies {
6464
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-
};
6865
const dependencies: ConfigDependencies = {
6966
configFactory: new CodeAnalyzerConfigFactoryImpl(),
7067
pluginsFactory: new EnginePluginsFactoryImpl(),
7168
logEventListeners: [new LogEventDisplayer(uxDisplay)],
7269
progressEventListeners: [new RuleSelectionProgressSpinner(uxDisplay)],
73-
modelGenerator: modelGeneratorFunction,
70+
modelGenerator: AnnotatedConfigModel,
7471
actionSummaryViewer: new ConfigActionSummaryViewer(uxDisplay),
7572
viewer: new ConfigStyledYamlViewer(uxDisplay)
7673
};

‎src/lib/actions/ConfigAction.ts‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ 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 {ConfigModel, ConfigModelConstructor} from '../models/ConfigModel';
1212

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

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);
112+
const configModel: ConfigModel = new this.dependencies.modelGenerator(userConfig, userCore, userRules, allDefaultRules, relevantEngines);
123113

124114
const fileWritten: boolean = this.dependencies.writer
125115
? await this.dependencies.writer.write(configModel)

‎src/lib/models/ConfigModel.ts‎

Lines changed: 37 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,50 +21,48 @@ export interface ConfigModel {
2121
toFormattedOutput(format: OutputFormat): string;
2222
}
2323

24-
export type ConfigContext = {
25-
config: CodeAnalyzerConfig;
26-
core: CodeAnalyzer;
27-
rules: RuleSelection;
28-
}
29-
30-
export type ConfigModelGeneratorFunction = (relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) => ConfigModel;
24+
export type ConfigModelConstructor = new (config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) => ConfigModel;
3125

3226
export class AnnotatedConfigModel implements ConfigModel {
33-
private readonly relevantEngines: Set<string>;
34-
private readonly userContext: ConfigContext;
35-
private readonly defaultContext: ConfigContext;
36-
37-
private constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
27+
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
28+
private readonly codeAnalyzer: CodeAnalyzer;
29+
private readonly userRules: RuleSelection;
30+
private readonly allDefaultRules: RuleSelection;
31+
private readonly relevantEngines: Set<string>;
32+
33+
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
34+
this.config = config;
35+
this.codeAnalyzer = codeAnalyzer;
36+
this.userRules = userRules;
37+
this.allDefaultRules = allDefaultRules;
3838
this.relevantEngines = relevantEngines;
39-
this.userContext = userContext;
40-
this.defaultContext = defaultContext;
4139
}
4240

4341
toFormattedOutput(format: OutputFormat): string {
4442
// istanbul ignore else: Should be impossible
4543
if (format === OutputFormat.STYLED_YAML) {
46-
return new StyledYamlFormatter(this.relevantEngines, this.userContext, this.defaultContext).toYaml();
44+
return new StyledYamlFormatter(this.config, this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
4745
} else if (format === OutputFormat.RAW_YAML) {
48-
return new PlainYamlFormatter(this.relevantEngines, this.userContext, this.defaultContext).toYaml();
46+
return new PlainYamlFormatter(this.config, this.codeAnalyzer, this.userRules, this.allDefaultRules, this.relevantEngines).toYaml();
4947
} else {
5048
throw new Error(`Unsupported`)
5149
}
5250
}
53-
54-
public static fromSelection(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext): AnnotatedConfigModel {
55-
return new AnnotatedConfigModel(relevantEngines, userContext, defaultContext);
56-
}
5751
}
5852

5953
abstract class YamlFormatter {
54+
private readonly config: CodeAnalyzerConfig;
55+
private readonly codeAnalyzer: CodeAnalyzer;
56+
private readonly userRules: RuleSelection;
57+
private readonly allDefaultRules: RuleSelection;
6058
private readonly relevantEngines: Set<string>;
61-
private readonly userContext: ConfigContext;
62-
private readonly defaultContext: ConfigContext;
6359

64-
protected constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
60+
protected constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
61+
this.config = config;
62+
this.codeAnalyzer = codeAnalyzer;
63+
this.userRules = userRules;
64+
this.allDefaultRules = allDefaultRules;
6565
this.relevantEngines = relevantEngines;
66-
this.userContext = userContext;
67-
this.defaultContext = defaultContext;
6866
}
6967

7068
protected abstract toYamlComment(commentText: string): string
@@ -109,20 +107,20 @@ abstract class YamlFormatter {
109107
}
110108

111109
const commentText: string = getMessage(BundleName.ConfigModel, 'template.modified-from', [defaultValueJson]);
112-
resolvedValue = replaceAbsolutePathsWithRelativePathsWherePossible(resolvedValue, this.userContext.config.getConfigRoot() + path.sep);
110+
resolvedValue = replaceAbsolutePathsWithRelativePathsWherePossible(resolvedValue, this.config.getConfigRoot() + path.sep);
113111
return this.toYamlUncheckedFieldWithInlineComment(fieldName, resolvedValue, commentText);
114112
}
115113

116114
toYaml(): string {
117-
const topLevelDescription: ConfigDescription = this.userContext.config.getConfigDescription();
115+
const topLevelDescription: ConfigDescription = this.config.getConfigDescription();
118116
return this.toYamlSectionHeadingComment(topLevelDescription.overview) + '\n' +
119117
'\n' +
120118
this.toYamlComment(topLevelDescription.fieldDescriptions.config_root.descriptionText) + '\n' +
121-
this.toYamlFieldUsingFieldDescription('config_root', this.userContext.config.getConfigRoot(),
119+
this.toYamlFieldUsingFieldDescription('config_root', this.config.getConfigRoot(),
122120
topLevelDescription.fieldDescriptions.config_root) + '\n' +
123121
'\n' +
124122
this.toYamlComment(topLevelDescription.fieldDescriptions.log_folder.descriptionText) + '\n' +
125-
this.toYamlFieldUsingFieldDescription('log_folder', this.userContext.config.getLogFolder(),
123+
this.toYamlFieldUsingFieldDescription('log_folder', this.config.getLogFolder(),
126124
topLevelDescription.fieldDescriptions.log_folder) + '\n' +
127125
'\n' +
128126
this.toYamlComment(topLevelDescription.fieldDescriptions.rules.descriptionText) + '\n' +
@@ -135,13 +133,13 @@ abstract class YamlFormatter {
135133
}
136134

137135
private toYamlRuleOverrides(): string {
138-
if (this.userContext.rules.getCount() === 0) {
136+
if (this.userRules.getCount() === 0) {
139137
const commentText: string = getMessage(BundleName.ConfigModel, 'template.yaml.no-rules-selected');
140138
return `rules: {} ${this.toYamlComment(commentText)}`;
141139
}
142140

143141
let yamlCode: string = 'rules:\n';
144-
for (const engineName of this.userContext.rules.getEngineNames()) {
142+
for (const engineName of this.userRules.getEngineNames()) {
145143
yamlCode += '\n';
146144
yamlCode += indent(this.toYamlRuleOverridesForEngine(engineName), 2) + '\n';
147145
}
@@ -153,7 +151,7 @@ abstract class YamlFormatter {
153151
[engineName.toUpperCase()]);
154152
let yamlCode: string = this.toYamlSectionHeadingComment(engineConfigHeader) + '\n';
155153
yamlCode += `${engineName}:\n`;
156-
for (const userRule of this.userContext.rules.getRulesFor(engineName)) {
154+
for (const userRule of this.userRules.getRulesFor(engineName)) {
157155
const defaultRule: Rule|null = this.getDefaultRuleFor(engineName, userRule.getName());
158156
yamlCode += indent(this.toYamlRuleOverridesForRule(userRule, defaultRule), 2) + '\n';
159157
}
@@ -162,7 +160,7 @@ abstract class YamlFormatter {
162160

163161
private getDefaultRuleFor(engineName: string, ruleName: string): Rule|null {
164162
try {
165-
return this.defaultContext.rules.getRule(engineName, ruleName);
163+
return this.allDefaultRules.getRule(engineName, ruleName);
166164
} catch (e) {
167165
// istanbul ignore next
168166
return null;
@@ -191,8 +189,8 @@ abstract class YamlFormatter {
191189
}
192190

193191
private toYamlEngineOverridesForEngine(engineName: string): string {
194-
const engineConfigDescriptor: ConfigDescription = this.userContext.core.getEngineConfigDescription(engineName);
195-
const userEngineConfig: EngineConfig = this.userContext.core.getEngineConfig(engineName);
192+
const engineConfigDescriptor: ConfigDescription = this.codeAnalyzer.getEngineConfigDescription(engineName);
193+
const userEngineConfig: EngineConfig = this.codeAnalyzer.getEngineConfig(engineName);
196194

197195
let yamlCode: string = '\n' +
198196
this.toYamlSectionHeadingComment(engineConfigDescriptor.overview) + '\n' +
@@ -212,8 +210,8 @@ abstract class YamlFormatter {
212210
}
213211

214212
class PlainYamlFormatter extends YamlFormatter {
215-
constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
216-
super(relevantEngines, userContext, defaultContext);
213+
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
214+
super(config, codeAnalyzer, userRules, allDefaultRules, relevantEngines);
217215
}
218216

219217
protected toYamlComment(commentText: string): string {
@@ -222,8 +220,8 @@ class PlainYamlFormatter extends YamlFormatter {
222220
}
223221

224222
class StyledYamlFormatter extends YamlFormatter {
225-
constructor(relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) {
226-
super(relevantEngines, userContext, defaultContext);
223+
constructor(config: CodeAnalyzerConfig, codeAnalyzer: CodeAnalyzer, userRules: RuleSelection, allDefaultRules: RuleSelection, relevantEngines: Set<string>) {
224+
super(config, codeAnalyzer, userRules, allDefaultRules, relevantEngines);
227225
}
228226

229227
protected toYamlComment(commentText: string): string {

‎test/lib/actions/ConfigAction.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('ConfigAction tests', () => {
3737
progressEventListeners: [],
3838
viewer: new ConfigStyledYamlViewer(spyDisplay),
3939
configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
40-
modelGenerator: AnnotatedConfigModel.fromSelection,
40+
modelGenerator: AnnotatedConfigModel,
4141
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
4242
pluginsFactory: new StubEnginePluginFactory()
4343
};
@@ -164,7 +164,7 @@ describe('ConfigAction tests', () => {
164164
progressEventListeners: [],
165165
viewer: new ConfigStyledYamlViewer(spyDisplay),
166166
configFactory: stubConfigFactory,
167-
modelGenerator: AnnotatedConfigModel.fromSelection,
167+
modelGenerator: AnnotatedConfigModel,
168168
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
169169
pluginsFactory: new StubEnginePluginFactory()
170170
};
@@ -397,7 +397,7 @@ describe('ConfigAction tests', () => {
397397
progressEventListeners: [],
398398
viewer: new ConfigStyledYamlViewer(spyDisplay),
399399
configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
400-
modelGenerator: AnnotatedConfigModel.fromSelection,
400+
modelGenerator: AnnotatedConfigModel,
401401
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
402402
pluginsFactory: new StubEnginePluginFactory()
403403
};
@@ -446,7 +446,7 @@ describe('ConfigAction tests', () => {
446446
progressEventListeners: [],
447447
viewer: new ConfigStyledYamlViewer(spyDisplay),
448448
configFactory: new DefaultStubCodeAnalyzerConfigFactory(),
449-
modelGenerator: AnnotatedConfigModel.fromSelection,
449+
modelGenerator: AnnotatedConfigModel,
450450
actionSummaryViewer: new ConfigActionSummaryViewer(spyDisplay),
451451
pluginsFactory: new StubEnginePluginFactory()
452452
}

0 commit comments

Comments
 (0)