Skip to content

Commit

Permalink
CHANGE @W-16438030@ Make file extensions an optional field for regex …
Browse files Browse the repository at this point in the history 
…engine rules (#65)
  • Loading branch information
@ravipanguluri
ravipanguluri authored Aug 8, 2024
1 parent b68a793 commit 04718ce
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 72 deletions.
9 changes: 7 additions & 2 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

81 changes: 41 additions & 40 deletions packages/code-analyzer-regex-engine/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,59 @@
"license": "BSD-3-Clause",
"homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
"repository": {
"type": "git",
"url": "git+https://github.com/forcedotcom/code-analyzer-core.git",
"directory": "packages/code-analyzer-regex-engine"
"type": "git",
"url": "git+https://github.com/forcedotcom/code-analyzer-core.git",
"directory": "packages/code-analyzer-regex-engine"
},
"main": "dist/index.js",
"types": "dist/index.d.ts",
"dependencies": {
"@types/node": "^20.0.0",
"@salesforce/code-analyzer-engine-api": "0.8.0"
"@salesforce/code-analyzer-engine-api": "0.8.0",
"@types/node": "^20.0.0",
"isbinaryfile": "^5.0.2"
},
"devDependencies": {
"@eslint/js": "^8.57.0",
"@types/jest": "^29.0.0",
"eslint": "^8.57.0",
"jest": "^29.0.0",
"rimraf": "*",
"ts-jest": "^29.0.0",
"typescript": "^5.4.5",
"typescript-eslint": "^7.8.0"
"@eslint/js": "^8.57.0",
"@types/jest": "^29.0.0",
"eslint": "^8.57.0",
"jest": "^29.0.0",
"rimraf": "*",
"ts-jest": "^29.0.0",
"typescript": "^5.4.5",
"typescript-eslint": "^7.8.0"
},
"engines": {
"node": ">=20.0.0"
"node": ">=20.0.0"
},
"files": [
"dist",
"LICENSE",
"package.json"
"dist",
"LICENSE",
"package.json"
],
"scripts": {
"build": "tsc --build tsconfig.build.json --verbose",
"test": "jest --coverage",
"lint": "eslint src/**/*.ts",
"package": "npm pack",
"all": "npm run build && npm run test && npm run lint && npm run package",
"clean": "tsc --build tsconfig.build.json --clean",
"postclean": "rimraf dist && rimraf coverage && rimraf ./*.tgz && rimraf vulnerabilities",
"scrub": "npm run clean && rimraf node_modules",
"showcoverage": "open ./coverage/lcov-report/index.html"
"build": "tsc --build tsconfig.build.json --verbose",
"test": "jest --coverage",
"lint": "eslint src/**/*.ts",
"package": "npm pack",
"all": "npm run build && npm run test && npm run lint && npm run package",
"clean": "tsc --build tsconfig.build.json --clean",
"postclean": "rimraf dist && rimraf coverage && rimraf ./*.tgz && rimraf vulnerabilities",
"scrub": "npm run clean && rimraf node_modules",
"showcoverage": "open ./coverage/lcov-report/index.html"
},
"jest": {
"preset": "ts-jest",
"testEnvironment": "node",
"testMatch": [
"**/*.test.ts"
],
"testPathIgnorePatterns": [
"/node_modules/",
"/dist/"
],
"collectCoverageFrom": [
"src/**/*.ts",
"!src/index.ts"
]
"preset": "ts-jest",
"testEnvironment": "node",
"testMatch": [
"**/*.test.ts"
],
"testPathIgnorePatterns": [
"/node_modules/",
"/dist/"
],
"collectCoverageFrom": [
"src/**/*.ts",
"!src/index.ts"
]
}
}
}
9 changes: 5 additions & 4 deletions packages/code-analyzer-regex-engine/src/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ export type RegexRules = {
// The regular expression that triggers a violation when matched against the contents of a file.
regex: RegExp;

/// The extensions of the files that you would like to test the regular expression against.
file_extensions: string[];
// The extensions of the files that you would like to test the regular expression against.
// If not defined, or equal to null, then all text-based files of any file extension will be tested.
file_extensions?: string[];

// A description of the rule's purpose.
description: string;
Expand All @@ -55,15 +56,15 @@ export function validateAndNormalizeConfig(rawConfig: ConfigObject): RegexEngine
const description: string = ruleExtractor.extractRequiredString('description')
const rawRegexString: string = ruleExtractor.extractRequiredString('regex')
const regex: RegExp = validateRegex(rawRegexString, ruleExtractor.getFieldPath('regex'))
const rawFileExtensions: string[] = ruleExtractor.extractRequiredArray('file_extensions',
const rawFileExtensions: string[] | undefined = ruleExtractor.extractArray('file_extensions',
(element, fieldPath) => ValueValidator.validateString(element, fieldPath, FILE_EXT_PATTERN));

customRules[ruleName] = {
regex: regex,
description: description,
violation_message: ruleExtractor.extractString('violation_message',
getDefaultRuleViolationMessage(regex, ruleName, description))!,
file_extensions: normalizeFileExtensions(rawFileExtensions),
...(rawFileExtensions ? { file_extensions: normalizeFileExtensions(rawFileExtensions) } : {}),

// Additional fields to complete the RuleDescription:
name: ruleName,
Expand Down
33 changes: 20 additions & 13 deletions packages/code-analyzer-regex-engine/src/engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ import path from "node:path";
import fs from "node:fs";
import os from "node:os";
import {RegexRules} from "./config";
import {isBinaryFileSync} from "isbinaryfile";

const TEXT_BASED_FILE_EXTS = new Set<string>(
[
'.cjs', '.cls', '.cmp', '.css', '.csv', '.htm', '.html', '.js', '.json', '.jsx', '.md', '.mdt', '.mjs', '.page',
'.rtf', '.trigger', '.ts', '.tsx', '.txt', '.xml', '.xsl', '.xslt', '.yaml', '.yml'
]
)

export class RegexEngine extends Engine {
static readonly NAME = "regex";
Expand All @@ -34,12 +42,12 @@ export class RegexEngine extends Engine {
if (!describeOptions.workspace) {
return Object.values(this.regexRules);
}
const fullFileList: string[] = await describeOptions.workspace.getExpandedFiles();
const fileExtSet: Set<string> = getSetOfFileExtensionsFrom(fullFileList);

const fullFileList: string[] = await describeOptions.workspace.getExpandedFiles();
const ruleDescriptions: RuleDescription[] = [];

for (const regexRule of Object.values(this.regexRules)) {
if (regexRule.file_extensions.some(fileExt => fileExtSet.has(fileExt))) {
if (fullFileList.some(fileName => this.shouldScanFile(fileName, regexRule.name))){
ruleDescriptions.push(regexRule);
}
}
Expand All @@ -62,7 +70,9 @@ export class RegexEngine extends Engine {

private shouldScanFile(fileName: string, ruleName: string): boolean {
const ext: string = path.extname(fileName).toLowerCase();
return this.regexRules[ruleName].file_extensions.includes(ext);
return this.regexRules[ruleName].file_extensions ?
this.regexRules[ruleName].file_extensions.includes(ext) :
TEXT_BASED_FILE_EXTS.has(ext) || isTextFile(fileName);
}

private async scanFile(fileName: string, ruleName: string): Promise<Violation[]> {
Expand Down Expand Up @@ -94,14 +104,6 @@ export class RegexEngine extends Engine {
}
}

function getSetOfFileExtensionsFrom(files: string[]): Set<string> {
const fileExtSet: Set<string> = new Set();
for (const file of files) {
fileExtSet.add(path.extname(file).toLowerCase());
}
return fileExtSet;
}

function getColumnNumber(charIndex: number, newlineIndexes: number[]): number {
const idxOfNextNewline = newlineIndexes.findIndex(el => el >= charIndex);
const idxOfCurrentLine = idxOfNextNewline === -1 ? newlineIndexes.length - 1: idxOfNextNewline - 1;
Expand All @@ -121,10 +123,15 @@ function getLineNumber(charIndex: number, newlineIndexes: number[]): number{
function getNewlineIndices(fileContents: string): number[] {
const newlineRegex: RegExp = new RegExp(os.EOL, "g");
const matches = fileContents.matchAll(newlineRegex);
const newlineIndexes = [0];
const newlineIndexes = [-1];

for (const match of matches) {
newlineIndexes.push(match.index)
}
return newlineIndexes;
}

// TODO: Make this async and cache results to improve performance
function isTextFile(fileName: string): boolean {
return !isBinaryFileSync(fileName);
}
4 changes: 3 additions & 1 deletion packages/code-analyzer-regex-engine/test/end-to-end.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ describe('End to end test', () => {
const ruleDescriptions: RuleDescription[] = await engine.describeRules({workspace: workspace});
const recommendedRuleNames: string[] = ruleDescriptions.filter(rd => rd.tags.includes('Recommended')).map(rd => rd.name);
const engineRunResults: EngineRunResults = await engine.runRules(recommendedRuleNames, {workspace: workspace});

const violationsFromTsFile: Violation[] = engineRunResults.violations.filter(v => path.extname(v.codeLocations[0].file) === '.ts');

expect(violationsFromTsFile).toHaveLength(1);
expect(violationsFromTsFile.map(v => v.ruleName)).toEqual(['NoTodos']);

const violationsFromClsFile: Violation[] = engineRunResults.violations.filter(v => path.extname(v.codeLocations[0].file) === '.cls');

expect(violationsFromClsFile).toHaveLength(1);
expect(violationsFromClsFile.map(v => v.ruleName)).toEqual(['NoTrailingWhitespace']);
});
Expand Down
66 changes: 59 additions & 7 deletions packages/code-analyzer-regex-engine/test/engine.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ const SAMPLE_CUSTOM_RULES: RegexRules = {
severityLevel: SeverityLevel.Moderate,
tags: ['Recommended'],
resourceUrls: []
},
NoHellos: {
regex: /hello/gi,
description: "Detects hellos in project.",
violation_message: "sample violation message",
name: "NoHellos",
type: RuleType.Standard,
severityLevel: SeverityLevel.Moderate,
tags: ['Recommended'],
resourceUrls: []
}
};

Expand All @@ -48,6 +58,15 @@ const EXPECTED_NoTodos_RULE_DESCRIPTION = {
resourceUrls: []
};

const EXPECTED_NoHellos_RULE_DESCRIPTION = {
name: "NoHellos",
severityLevel: SeverityLevel.Moderate,
type: RuleType.Standard,
tags: ["Recommended"],
description: "Detects hellos in project.",
resourceUrls: []
};

describe("Tests for RegexEngine's getName and describeRules methods", () => {
let engine: RegexEngine;
beforeAll(() => {
Expand All @@ -64,34 +83,38 @@ describe("Tests for RegexEngine's getName and describeRules methods", () => {

it('Calling describeRules without workspace, returns all available rules', async () => {
const rulesDescriptions: RuleDescription[] = await engine.describeRules({});
expect(rulesDescriptions).toHaveLength(2);
expect(rulesDescriptions).toHaveLength(3);
expect(rulesDescriptions[0]).toMatchObject(EXPECTED_NoTrailingWhitespace_RULE_DESCRIPTION);
expect(rulesDescriptions[1]).toMatchObject(EXPECTED_NoTodos_RULE_DESCRIPTION);
expect(rulesDescriptions[2]).toMatchObject(EXPECTED_NoHellos_RULE_DESCRIPTION);
});

it("When workspace contains zero applicable files, then describeRules returns no rules", async () => {
const rulesDescriptions: RuleDescription[] = await engine.describeRules({workspace: new Workspace([
path.resolve(__dirname, 'test-data', 'sampleWorkspace', 'dummy2.ts') // .ts files aren't applicable to the rules
path.resolve(__dirname, 'test-data', 'workspaceWithNoTextFiles') //
])});
expect(rulesDescriptions).toHaveLength(0);
});

it("When workspace contains files only applicable to one of the rules, then describeRules only returns that one rule", async () => {
it("When workspace contains files only applicable to only some of the rules, then describeRules only returns those rules", async () => {
const rulesDescriptions: RuleDescription[] = await engine.describeRules({workspace: new Workspace([
path.resolve(__dirname, 'test-data', 'sampleWorkspace', 'dummy3.js')
])});
expect(rulesDescriptions).toHaveLength(1);
expect(rulesDescriptions).toHaveLength(2);
expect(rulesDescriptions[0]).toMatchObject(EXPECTED_NoTodos_RULE_DESCRIPTION);
expect(rulesDescriptions[1]).toMatchObject(EXPECTED_NoHellos_RULE_DESCRIPTION);
});

it("When workspace contains files are applicable to all available rules, then describeRules returns all rules", async () => {
const rulesDescriptions: RuleDescription[] = await engine.describeRules({workspace: new Workspace([
path.resolve(__dirname, 'test-data', 'sampleWorkspace')
])});
expect(rulesDescriptions).toHaveLength(2);
expect(rulesDescriptions).toHaveLength(3);
expect(rulesDescriptions[0]).toMatchObject(EXPECTED_NoTrailingWhitespace_RULE_DESCRIPTION);
expect(rulesDescriptions[1]).toMatchObject(EXPECTED_NoTodos_RULE_DESCRIPTION);
expect(rulesDescriptions[2]).toMatchObject(EXPECTED_NoHellos_RULE_DESCRIPTION)
});

});

describe('Tests for runRules', () => {
Expand All @@ -113,7 +136,7 @@ describe('Tests for runRules', () => {
expect(runResults.violations).toHaveLength(0);
});

it("Ensure runRules when called on a list Apex classes, properly emits violations", async () => {
it("Ensure runRules when called on a list Apex classes, properly emits violations", async () => {
const runOptions: RunOptions = {workspace: new Workspace([
path.resolve(__dirname, "test-data", "apexClassWhitespace")
])};
Expand Down Expand Up @@ -182,7 +205,7 @@ describe('Tests for runRules', () => {
const runOptions: RunOptions = {workspace: new Workspace([
path.resolve(__dirname, "test-data", "sampleWorkspace")
])};
const runResults: EngineRunResults = await engine.runRules(["NoTodos"], runOptions);
const runResults: EngineRunResults = await engine.runRules(["NoTodos", "NoHellos"], runOptions);

const expectedViolations: Violation[] = [
{
Expand Down Expand Up @@ -212,7 +235,36 @@ describe('Tests for runRules', () => {
endColumn: 22
}
]
},
{
ruleName: "NoHellos",
message: "sample violation message",
primaryLocationIndex: 0,
codeLocations: [
{
file: path.resolve(__dirname, "test-data", "sampleWorkspace", "dummy3.js"),
startLine: 3,
startColumn: 29,
endLine: 3,
endColumn: 34
}
]
},
{
ruleName: "NoHellos",
message: "sample violation message",
primaryLocationIndex: 0,
codeLocations: [
{
file: path.resolve(__dirname, "test-data", "sampleWorkspace", "dummy4.txt"),
startLine: 1,
startColumn: 1,
endLine: 1,
endColumn: 6
}
]
}

];

expect(runResults.violations).toHaveLength(expectedViolations.length);
Expand Down
Loading

0 comments on commit 04718ce

Please sign in to comment.