Skip to content

Commit

Permalink
NEW: @W-16150311@: Add config_folder to CodeAnalyzerConfig and forwar…
Browse files Browse the repository at this point in the history
…d it to engines (#38)
  • Loading branch information
stephen-carter-at-sf authored Jul 2, 2024
1 parent 1f269c1 commit d169678
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 38 deletions.
2 changes: 1 addition & 1 deletion packages/code-analyzer-core/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@salesforce/code-analyzer-core",
"description": "Core Package for the Salesforce Code Analyzer",
"version": "0.6.0",
"version": "0.7.0",
"author": "The Salesforce Code Analyzer Team",
"license": "BSD-3-Clause license",
"homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
Expand Down
5 changes: 1 addition & 4 deletions packages/code-analyzer-core/src/code-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,10 @@ export class CodeAnalyzer {
await Promise.all(promises);
}

// This method should be called from the client with an absolute path to the module if it isn't available globally.
// Basically, clients should call this method after resolving the module using require.resolve. For example:
// codeAnalyzer.dynamicallyAddEnginePlugin(require.resolve('./someRelativePluginModule'));
public async dynamicallyAddEnginePlugin(enginePluginModulePath: string): Promise<void> {
let pluginModule;
try {
enginePluginModulePath = require.resolve(enginePluginModulePath);
enginePluginModulePath = require.resolve(enginePluginModulePath, {paths: [this.config.getConfigFolder()]});
pluginModule = (await import(enginePluginModulePath));
} catch (err) {
throw new Error(getMessage('FailedToDynamicallyLoadModule', enginePluginModulePath, (err as Error).message), {cause: err});
Expand Down
71 changes: 58 additions & 13 deletions packages/code-analyzer-core/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {toAbsolutePath} from "./utils"
import {SeverityLevel} from "./rules";

export const FIELDS = {
CONFIG_FOLDER: 'config_folder',
LOG_FOLDER: 'log_folder',
CUSTOM_ENGINE_PLUGIN_MODULES: 'custom_engine_plugin_modules',
RULES: 'rules',
Expand All @@ -22,13 +23,26 @@ export type RuleOverride = {
}

type TopLevelConfig = {
// The absolute folder path where other paths values in the config may be relative to.
// Default: The location of the config file if supplied and the current working directory otherwise.
config_folder: string

// Folder where the user would like to store log files. May be relative to config_folder.
// Default: The default temporary directory on the user's machine.
log_folder: string

// List of EnginePlugin modules to be dynamically added. Paths may be relative to the config_folder.
custom_engine_plugin_modules: string[]

// Rule override entries of the format rules.<engine_name>.<rule_name>.<property_name> = <override_value>
rules: Record<string, Record<string, RuleOverride>>

// Engine specific configuration entries of the format engines.<engine_name>.<engine_specific_property> = <value>
engines: Record<string, engApi.ConfigObject>
}

const DEFAULT_CONFIG: TopLevelConfig = {
config_folder: process.cwd(),
log_folder: os.tmpdir(),
custom_engine_plugin_modules: [],
rules: {},
Expand All @@ -52,27 +66,29 @@ export class CodeAnalyzerConfig {
const fileExt : string = path.extname(file).toLowerCase();

if (fileExt == '.json') {
return CodeAnalyzerConfig.fromJsonString(fileContents);
return CodeAnalyzerConfig.fromJsonString(fileContents, path.dirname(file));
} else if (fileExt == '.yaml' || fileExt == '.yml') {
return CodeAnalyzerConfig.fromYamlString(fileContents);
return CodeAnalyzerConfig.fromYamlString(fileContents, path.dirname(file));
} else {
throw new Error(getMessage('ConfigFileExtensionUnsupported', file, 'json,yaml,yml'))
}
}

public static fromJsonString(jsonString: string): CodeAnalyzerConfig {
public static fromJsonString(jsonString: string, configFolder?: string): CodeAnalyzerConfig {
const data: object = parseAndValidate(() => JSON.parse(jsonString));
return CodeAnalyzerConfig.fromObject(data);
return CodeAnalyzerConfig.fromObject(data, configFolder);
}

public static fromYamlString(yamlString: string): CodeAnalyzerConfig {
public static fromYamlString(yamlString: string, configFolder?: string): CodeAnalyzerConfig {
const data: object = parseAndValidate(() => yaml.load(yamlString));
return CodeAnalyzerConfig.fromObject(data);
return CodeAnalyzerConfig.fromObject(data, configFolder);
}

public static fromObject(data: object): CodeAnalyzerConfig {
public static fromObject(data: object, configFolder?: string): CodeAnalyzerConfig {
configFolder = extractConfigFolderValue(data, configFolder);
const config: TopLevelConfig = {
log_folder: extractLogFolderValue(data),
config_folder: configFolder,
log_folder: extractLogFolderValue(data, configFolder),
custom_engine_plugin_modules: extractCustomEnginePluginModules(data),
rules: extractRulesValue(data),
engines: extractEnginesValue(data)
Expand All @@ -88,6 +104,10 @@ export class CodeAnalyzerConfig {
return this.config.log_folder;
}

public getConfigFolder(): string {
return this.config.config_folder;
}

public getCustomEnginePluginModules(): string[] {
return this.config.custom_engine_plugin_modules;
}
Expand All @@ -101,18 +121,43 @@ export class CodeAnalyzerConfig {
}

public getEngineConfigFor(engineName: string): engApi.ConfigObject {
return this.config.engines[engineName] || {};
// Each engine should have access to the config folder location and its own engine specific config, so that it
// can resolve any relative paths in its engine specific config with this location. Thus, we always add the
// config folder to the engine specific config.
const configToGiveToEngine: engApi.ConfigObject = this.config.engines[engineName] || {};
configToGiveToEngine.config_folder = this.getConfigFolder();
return configToGiveToEngine;
}
}

function extractLogFolderValue(data: object): string {
function extractConfigFolderValue(data: object, configFolder?: string): string {
if (!(FIELDS.CONFIG_FOLDER in data)) {
return configFolder || DEFAULT_CONFIG.config_folder;
}
configFolder = toAbsolutePath(validateType('string', data[FIELDS.CONFIG_FOLDER], FIELDS.CONFIG_FOLDER));
if (!fs.existsSync(configFolder)) {
throw new Error(getMessage('ConfigValueFolderMustExist', FIELDS.CONFIG_FOLDER, configFolder));
} else if (!fs.statSync(configFolder).isDirectory()) {
throw new Error(getMessage('ConfigValueMustBeFolder', FIELDS.CONFIG_FOLDER, configFolder));
} else if (data[FIELDS.CONFIG_FOLDER] !== configFolder) {
throw new Error(getMessage('ConfigValueMustBeAbsolutePath', FIELDS.CONFIG_FOLDER, configFolder));
}
return configFolder;
}

function extractLogFolderValue(data: object, configFolder: string): string {
if (!(FIELDS.LOG_FOLDER in data)) {
return DEFAULT_CONFIG.log_folder;
}
const logFolder: string = toAbsolutePath(validateType('string', data[FIELDS.LOG_FOLDER], FIELDS.LOG_FOLDER));
const rawLogFolder: string = validateType('string', data[FIELDS.LOG_FOLDER], FIELDS.LOG_FOLDER);
let logFolder: string = toAbsolutePath(rawLogFolder, configFolder); // First assume it is relative to the config folder
if (!fs.existsSync(logFolder)) {
throw new Error(getMessage('ConfigValueFolderMustExist', FIELDS.LOG_FOLDER, logFolder));
} else if (!fs.statSync(logFolder).isDirectory()) {
logFolder = toAbsolutePath(rawLogFolder); // Otherwise just try to resolve without config folder
if (!fs.existsSync(logFolder)) {
throw new Error(getMessage('ConfigValueFolderMustExist', FIELDS.LOG_FOLDER, logFolder));
}
}
if (!fs.statSync(logFolder).isDirectory()) {
throw new Error(getMessage('ConfigValueMustBeFolder', FIELDS.LOG_FOLDER, logFolder));
}
return logFolder;
Expand Down
3 changes: 3 additions & 0 deletions packages/code-analyzer-core/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ const messageCatalog : { [key: string]: string } = {
ConfigValueMustBeFolder:
'The %s configuration value is not a folder: %s',

ConfigValueMustBeAbsolutePath:
'The %s configuration value must be provided as an absolute path location. Please change the value to: %s',

RulePropertyOverridden:
'The %s value of rule "%s" of engine "%s" was overridden according to the specified configuration. The old value of %s was replaced with the new value of %s.',

Expand Down
8 changes: 6 additions & 2 deletions packages/code-analyzer-core/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import path from "node:path";

export function toAbsolutePath(fileOrFolder: string): string {
export function toAbsolutePath(fileOrFolder: string, parentFolder?: string): string {
// Convert slashes to platform specific slashes and then convert to absolute path
return path.resolve(fileOrFolder.replace(/[\\/]/g, path.sep));
fileOrFolder = fileOrFolder.replace(/[\\/]/g, path.sep);
if (!parentFolder) {
return path.resolve(fileOrFolder);
}
return fileOrFolder.startsWith(parentFolder) ? fileOrFolder : path.join(parentFolder, fileOrFolder);
}

export interface Clock {
Expand Down
12 changes: 8 additions & 4 deletions packages/code-analyzer-core/test/add-engines.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {StubEngine1, StubEngine2} from "./stubs";

changeWorkingDirectoryToPackageRoot();

const DEFAULT_CONFIG_FOLDER: string = process.cwd();

describe("Tests for adding engines to Code Analyzer", () => {
let codeAnalyzer: CodeAnalyzer;
let logEvents: LogEvent[];
Expand All @@ -24,14 +26,15 @@ describe("Tests for adding engines to Code Analyzer", () => {
expect(codeAnalyzer.getEngineNames().sort()).toEqual(["stubEngine1","stubEngine2"]);
const stubEngine1: StubEngine1 = stubEnginePlugin.getCreatedEngine('stubEngine1') as StubEngine1;
expect(stubEngine1.getName()).toEqual('stubEngine1');
expect(stubEngine1.config).toEqual({});
expect(stubEngine1.config).toEqual({config_folder: DEFAULT_CONFIG_FOLDER});
const stubEngine2: StubEngine2 = stubEnginePlugin.getCreatedEngine('stubEngine2') as StubEngine2;
expect(stubEngine2.getName()).toEqual('stubEngine2');
expect(stubEngine2.config).toEqual({});
expect(stubEngine2.config).toEqual({config_folder: DEFAULT_CONFIG_FOLDER});
});

it('When adding engine plugin using non-default config then engines are correctly added with engine specific configurations', async () => {
codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.resolve(__dirname, 'test-data', 'sample-config-02.Yml')));
const testDataFolder: string= path.resolve(__dirname, 'test-data');
codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromFile(path.join(testDataFolder, 'sample-config-02.Yml')));

const stubEnginePlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin();
await codeAnalyzer.addEnginePlugin(stubEnginePlugin);
Expand All @@ -40,6 +43,7 @@ describe("Tests for adding engines to Code Analyzer", () => {
const stubEngine1: StubEngine1 = stubEnginePlugin.getCreatedEngine('stubEngine1') as StubEngine1;
expect(stubEngine1.getName()).toEqual('stubEngine1');
expect(stubEngine1.config).toEqual({
config_folder: testDataFolder,
miscSetting1: true,
miscSetting2: {
miscSetting2A: 3,
Expand All @@ -48,7 +52,7 @@ describe("Tests for adding engines to Code Analyzer", () => {
});
const stubEngine2: StubEngine2 = stubEnginePlugin.getCreatedEngine('stubEngine2') as StubEngine2;
expect(stubEngine2.getName()).toEqual('stubEngine2');
expect(stubEngine2.config).toEqual({});
expect(stubEngine2.config).toEqual({config_folder: testDataFolder});
});

it('(Forward Compatibility) When addEnginePlugin receives a plugin with a future api version then cast down to current api version', async () => {
Expand Down
51 changes: 39 additions & 12 deletions packages/code-analyzer-core/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@ import {changeWorkingDirectoryToPackageRoot} from "./test-helpers";

changeWorkingDirectoryToPackageRoot();

const DEFAULT_CONFIG_FOLDER: string = process.cwd();
const TEST_DATA_DIR: string = path.resolve(__dirname, 'test-data');

describe("Tests for creating and accessing configuration values", () => {
it("When constructing config withDefaults then default values are returned", () => {
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults();

expect(conf.getConfigFolder()).toEqual(DEFAULT_CONFIG_FOLDER);
expect(conf.getLogFolder()).toEqual(os.tmpdir());
expect(conf.getCustomEnginePluginModules()).toEqual([]);
expect(conf.getRuleOverridesFor("stubEngine1")).toEqual({});
expect(conf.getEngineConfigFor("stubEngine1")).toEqual({});
expect(conf.getEngineConfigFor("stubEngine1")).toEqual({config_folder: DEFAULT_CONFIG_FOLDER});
expect(conf.getRuleOverridesFor("stubEngine2")).toEqual({});
expect(conf.getEngineConfigFor("stubEngine2")).toEqual({});
expect(conf.getEngineConfigFor("stubEngine2")).toEqual({config_folder: DEFAULT_CONFIG_FOLDER});
});

it("When configuration file does not exist, then throw an error", () => {
Expand All @@ -34,8 +38,9 @@ describe("Tests for creating and accessing configuration values", () => {
});

it("When constructing config from yaml file then values from file are parsed correctly", () => {
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.resolve(__dirname, 'test-data', 'sample-config-01.yaml'));
expect(conf.getLogFolder()).toEqual(path.resolve(__dirname, 'test-data', 'sampleLogFolder'));
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-01.yaml'));
expect(conf.getConfigFolder()).toEqual(TEST_DATA_DIR);
expect(conf.getLogFolder()).toEqual(path.join(TEST_DATA_DIR, 'sampleLogFolder'));
expect(conf.getRuleOverridesFor('stubEngine1')).toEqual({
stub1RuleB: {
severity: SeverityLevel.Critical
Expand All @@ -50,13 +55,13 @@ describe("Tests for creating and accessing configuration values", () => {
tags: ['Security', "SomeNewTag"]
}
});
expect(conf.getEngineConfigFor('stubEngine1')).toEqual({});
expect(conf.getEngineConfigFor('stubEngine2')).toEqual({});
expect(conf.getEngineConfigFor('stubEngine1')).toEqual({config_folder: TEST_DATA_DIR});
expect(conf.getEngineConfigFor('stubEngine2')).toEqual({config_folder: TEST_DATA_DIR});
});

it("When constructing config from file with yml extension then it is parsed as a yaml file", () => {
// Also note that Yml should work just like yml. Case doesn't matter.
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.resolve(__dirname, 'test-data', 'sample-config-02.Yml'));
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-02.Yml'));
expect(conf.getLogFolder()).toEqual(os.tmpdir());
expect(conf.getCustomEnginePluginModules()).toEqual(['dummy_plugin_module_path']);
expect(conf.getRuleOverridesFor('stubEngine1')).toEqual({});
Expand All @@ -66,23 +71,24 @@ describe("Tests for creating and accessing configuration values", () => {
}
});
expect(conf.getEngineConfigFor('stubEngine1')).toEqual({
config_folder: TEST_DATA_DIR,
miscSetting1: true,
miscSetting2: {
miscSetting2A: 3,
miscSetting2B: ["hello", "world"]
}
});
expect(conf.getEngineConfigFor('stubEngine2')).toEqual({});
expect(conf.getEngineConfigFor('stubEngine2')).toEqual({config_folder: TEST_DATA_DIR});
});

it("When constructing config from json file then values from file are parsed correctly", () => {
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.resolve(__dirname, 'test-data', 'sample-config-03.json'));
expect(conf.getLogFolder()).toEqual(path.resolve(__dirname, 'test-data', 'sampleLogFolder'));
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-03.json'));
expect(conf.getLogFolder()).toEqual(path.join(TEST_DATA_DIR, 'sampleLogFolder'));
expect(conf.getCustomEnginePluginModules()).toEqual([]);
expect(conf.getRuleOverridesFor('stubEngine1')).toEqual({});
expect(conf.getRuleOverridesFor('stubEngine2')).toEqual({});
expect(conf.getEngineConfigFor('stubEngine1')).toEqual({});
expect(conf.getEngineConfigFor('stubEngine2')).toEqual({miscSetting: "miscValue"});
expect(conf.getEngineConfigFor('stubEngine1')).toEqual({config_folder: TEST_DATA_DIR});
expect(conf.getEngineConfigFor('stubEngine2')).toEqual({config_folder: TEST_DATA_DIR, miscSetting: "miscValue"});
});

it("When constructing config from invalid yaml string then we throw an error", () => {
Expand Down Expand Up @@ -202,4 +208,25 @@ describe("Tests for creating and accessing configuration values", () => {
expect(() => CodeAnalyzerConfig.fromObject({custom_engine_plugin_modules: 'oops'})).toThrow(
getMessage('ConfigValueNotAValidStringArray','custom_engine_plugin_modules', '"oops"'));
});

it("When supplied config_folder path is a valid absolute path, then we use it", () => {
const configFolderValue: string = path.join(TEST_DATA_DIR, 'sampleWorkspace');
const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromObject({config_folder: configFolderValue});
expect(conf.getConfigFolder()).toEqual(configFolderValue);
});

it("When supplied config_folder path does not exist, then we error", () => {
expect(() => CodeAnalyzerConfig.fromObject({config_folder: path.resolve('doesNotExist')})).toThrow(
getMessage('ConfigValueFolderMustExist','config_folder', path.resolve('doesNotExist')));
});

it("When supplied config_folder path is a file instead of a folder, then we error", () => {
expect(() => CodeAnalyzerConfig.fromObject({config_folder: path.resolve('package.json')})).toThrow(
getMessage('ConfigValueMustBeFolder','config_folder', path.resolve('package.json')));
});

it("When supplied config_folder path is a relative folder, then we error", () => {
expect(() => CodeAnalyzerConfig.fromObject({config_folder: 'test/test-data'})).toThrow(
getMessage('ConfigValueMustBeAbsolutePath','config_folder', path.resolve('test','test-data')));
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
log_folder: test\test-data\sampleLogFolder
log_folder: sampleLogFolder

rules:
stubEngine1:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"log_folder": "test/test-data/sampleLogFolder",
"log_folder": "sampleLogFolder",
"engines": {
"stubEngine2": {
"miscSetting": "miscValue"
Expand Down

0 comments on commit d169678

Please sign in to comment.