Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW @W-17397558@ CodeAnalyzerCore now requires Node v20 or later (1 of 2) #159

Merged
merged 4 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions .github/workflows/verify-pr/validate-changed-package-versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ function main() {

displayList('THE FOLLOWING FILES WERE CHANGED:', changedFiles);

const changedPackages = identifyChangedPackages(changedFiles);
const changedPackages = identifyMeaningfullyChangedPackages(changedFiles);

if (changedPackages.length === 0) {
console.log('No changed packages; no verification needed');
process.exit(0);
}

displayList('THE FOLLOWING PACKAGES HAVE CHANGED FILES:', changedPackages);
displayList('THE FOLLOWING PACKAGES HAVE CHANGED (NON-TEST) FILES:', changedPackages);

const incorrectlyVersionedPackages = identifyIncorrectlyVersionedPackages(changedPackages);

Expand All @@ -38,12 +38,12 @@ function displayList(header, list) {
console.log('');
}

function identifyChangedPackages(changedFiles) {
function identifyMeaningfullyChangedPackages(changedFiles) {
const changedPackages = new Set();

for (const changedFile of changedFiles) {
const changedPackage = convertFileNameToPackageNameIfPossible(changedFile);
if (changedPackage) {
if (changedPackage && !isFileInTestFolder(changedFile)) {
changedPackages.add(changedPackage);
}
}
Expand All @@ -60,6 +60,13 @@ function convertFileNameToPackageNameIfPossible(changedFile) {
}
}

function isFileInTestFolder(changedFile) {
const changedFilePathSegments = path.dirname(changedFile).split('/');
return changedFilePathSegments.length >= 3
&& changedFilePathSegments[0] === 'packages'
&& changedFilePathSegments[2] === 'test';
}

function identifyIncorrectlyVersionedPackages(changedPackages) {
const incorrectlyVersionedPackages = [];
for (const changedPackage of changedPackages) {
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

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

2 changes: 2 additions & 0 deletions packages/code-analyzer-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
"@types/js-yaml": "^4.0.9",
"@types/node": "^20.0.0",
"@types/sarif": "^2.1.7",
"@types/semver": "^7.5.8",
"csv-stringify": "^6.5.2",
"js-yaml": "^4.1.0",
"semver": "^7.6.3",
"xmlbuilder": "^15.1.1"
},
"devDependencies": {
Expand Down
13 changes: 12 additions & 1 deletion packages/code-analyzer-core/src/code-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
RunResultsImpl,
UnexpectedErrorEngineRunResults
} from "./results"
import {SemVer} from 'semver';
import {EngineLogEvent, EngineResultsEvent, EngineRunProgressEvent, Event, EventType, LogLevel} from "./events"
import {getMessage} from "./messages";
import * as engApi from "@salesforce/code-analyzer-engine-api"
Expand Down Expand Up @@ -37,6 +38,8 @@ export type RunOptions = {

export type EngineConfig = engApi.ConfigObject;

const MINIMUM_SUPPORTED_NODE = 20;

export class CodeAnalyzer {
private readonly config: CodeAnalyzerConfig;
private clock: Clock = new RealClock();
Expand All @@ -48,10 +51,18 @@ export class CodeAnalyzer {
private readonly rulesCache: Map<string, RuleImpl[]> = new Map();
private readonly engineRuleDiscoveryProgressAggregator: EngineProgressAggregator = new EngineProgressAggregator();

constructor(config: CodeAnalyzerConfig) {
constructor(config: CodeAnalyzerConfig, version: string = process.version) {
stephen-carter-at-sf marked this conversation as resolved.
Show resolved Hide resolved
this.validateEnvironment(version);
this.config = config;
}

private validateEnvironment(version: string): void {
const semver: SemVer = new SemVer(version);
if (semver.major < MINIMUM_SUPPORTED_NODE) {
throw new Error(getMessage('UnsupportedNodeVersion', MINIMUM_SUPPORTED_NODE, version));
}
stephen-carter-at-sf marked this conversation as resolved.
Show resolved Hide resolved
}

// For testing purposes only
_setClock(clock: Clock) {
this.clock = clock;
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 @@ -39,6 +39,9 @@ const MESSAGE_CATALOG : MessageCatalog = {
GenericEngineConfigOverview:
`%s ENGINE CONFIGURATION`,

UnsupportedNodeVersion:
`Code Analyzer requires Node v%s or later, but is currently running on Node %s. Please upgrade the version of 'node' on your machine.`,

EngineConfigFieldDescription_disable_engine:
`Whether to turn off the '%s' engine so that it is not included when running Code Analyzer commands.`,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,24 @@ import {UndefinedCodeLocation} from "../src/results";

changeWorkingDirectoryToPackageRoot();

describe("Tests for CodeAnalyzer constructor", () => {
it.each([
{version: 'v18.0.0'},
{version: 'v4.0.0'} // 4 is less than 20, but 4 is greater than 2. This is a classic trap for SemVer comparisons.
])("When supplied with a Node Version prior to v20, construction fails. Case: $version", ({version}) => {
// Expect the construction to fail with an error message that mentions v20, the minimum compatible version.
expect(() => new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), version)).toThrow('v20');
});

it.each([
{version: 'v20.0.0'},
{version: 'v21.0.0'},
{version: 'v100.0.0'} // 100 is greater than 20, but 1 is less than 2. This is a classic trap for SemVer comparisons.
])('When supplied with a Node Version of v20 or later, construction succeeds. Case: $version"', ({version}) => {
expect(new CodeAnalyzer(CodeAnalyzerConfig.withDefaults(), version)).toBeInstanceOf(CodeAnalyzer);
});
});

describe("Tests for the run method of CodeAnalyzer", () => {
let sampleRunOptions: RunOptions;
let sampleTimestamp: Date;
Expand Down Expand Up @@ -62,7 +80,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {
const badPathStartPoint: string = path.resolve(__dirname, 'doesNotExist.xml#someMethod');
const runOptions: RunOptions = {
... sampleRunOptions,
pathStartPoints: [path.resolve(__dirname, 'run.test.ts'), badPathStartPoint]
pathStartPoints: [path.resolve(__dirname, 'code-analyzer.test.ts'), badPathStartPoint]
};
await expect(codeAnalyzer.run(selection, runOptions)).rejects.toThrow(
getMessage('PathStartPointFileDoesNotExist', badPathStartPoint, path.resolve(__dirname, 'doesNotExist.xml')));
Expand Down Expand Up @@ -137,14 +155,14 @@ describe("Tests for the run method of CodeAnalyzer", () => {
it("When specifying path start points as files and subfolders, then they are passed to each engine successfully", async () => {
await codeAnalyzer.run(selection, {
workspace: await codeAnalyzer.createWorkspace(['test']),
pathStartPoints: ['test/test-data', 'test/run.test.ts']
pathStartPoints: ['test/test-data', 'test/code-analyzer.test.ts']
});

const expectedEngineRunOptions: engApi.RunOptions = {
workspace: new engApi.Workspace([path.resolve('test')], "FixedId"),
pathStartPoints: [
{ file: path.resolve("test", "test-data") },
{ file: path.resolve("test", "run.test.ts")}
{ file: path.resolve("test", "code-analyzer.test.ts")}
]
};
expect(stubEngine1.runRulesCallHistory).toHaveLength(1);
Expand All @@ -158,7 +176,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {
it("When specifying path start points individual methods, then they are passed to each engine successfully", async () => {
await codeAnalyzer.run(selection, {
workspace: await codeAnalyzer.createWorkspace(['test', 'src/utils.ts', 'src/index.ts']),
pathStartPoints: ['test/run.test.ts#someMethod','test/stubs.ts#method1;method2;method3','src/utils.ts']
pathStartPoints: ['test/code-analyzer.test.ts#someMethod','test/stubs.ts#method1;method2;method3','src/utils.ts']
});

const expectedEngineRunOptions: engApi.RunOptions = {
Expand All @@ -170,7 +188,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {
"FixedId"),
pathStartPoints: [
{
file: path.resolve("test", "run.test.ts"),
file: path.resolve("test", "code-analyzer.test.ts"),
methodName: 'someMethod'
},
{
Expand Down Expand Up @@ -326,7 +344,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {
expect(engine1Violations[1].getMessage()).toEqual('SomeViolationMessage2');
const engine1Violation2CodeLocations: CodeLocation[] = engine1Violations[1].getCodeLocations();
expect(engine1Violation2CodeLocations).toHaveLength(1);
assertCodeLocation(engine1Violation2CodeLocations[0], path.resolve('test', 'run.test.ts'), 21, 7, 25, 4);
assertCodeLocation(engine1Violation2CodeLocations[0], path.resolve('test', 'code-analyzer.test.ts'), 21, 7, 25, 4);
expect(engine1Violations[1].getPrimaryLocationIndex()).toEqual(0);
expect(engine1Violations[1].getResourceUrls()).toEqual([
"https://example.com/stub1RuleC",
Expand Down
4 changes: 2 additions & 2 deletions packages/code-analyzer-core/test/stubs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export function getSampleViolationForStub1RuleC(): engApi.Violation {
message: 'SomeViolationMessage2',
codeLocations: [
{
file: 'test/run.test.ts',
file: 'test/code-analyzer.test.ts',
startLine: 21,
startColumn: 7,
endLine: 25,
Expand All @@ -319,7 +319,7 @@ export function getSampleViolationForStub1RuleE(): engApi.Violation {
message: 'Some Violation that contains\na new line in `it` and "various" \'quotes\'. Also it has <brackets> that may need to be {escaped}.',
codeLocations: [
{
file: 'test/run.test.ts',
file: 'test/code-analyzer.test.ts',
startLine: 56,
startColumn: 4
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"rule","engine","severity","tags","file","startLine","startColumn","endLine","endColumn","message","resources"
"stub1RuleA","stubEngine1",4,"Recommended,CodeStyle","test{{PATHSEP}}config.test.ts",3,6,11,8,"SomeViolationMessage1","https://example.com/stub1RuleA"
"stub1RuleA","stubEngine1",4,"Recommended,CodeStyle","test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts",10,4,11,2,"SomeViolationMessage1","https://example.com/stub1RuleA"
"stub1RuleC","stubEngine1",3,"Recommended,Performance,Custom","test{{PATHSEP}}run.test.ts",21,7,25,4,"SomeViolationMessage2","https://example.com/stub1RuleC,https://example.com/aViolationSpecificUrl1,https://example.com/violationSpecificUrl2"
"stub1RuleE","stubEngine1",3,"Performance","test{{PATHSEP}}run.test.ts",56,4,,,"Some Violation that contains
"stub1RuleC","stubEngine1",3,"Recommended,Performance,Custom","test{{PATHSEP}}code-analyzer.test.ts",21,7,25,4,"SomeViolationMessage2","https://example.com/stub1RuleC,https://example.com/aViolationSpecificUrl1,https://example.com/violationSpecificUrl2"
"stub1RuleE","stubEngine1",3,"Performance","test{{PATHSEP}}code-analyzer.test.ts",56,4,,,"Some Violation that contains
a new line in `it` and ""various"" 'quotes'. Also it has <brackets> that may need to be {escaped}.","https://example.com/stub1RuleE,https://example.com/stub1RuleE_2"
"stub2RuleC","stubEngine2",2,"Recommended,BestPractice","test{{PATHSEP}}stubs.ts",76,8,,,"SomeViolationMessage3",
"stub3RuleA","stubEngine3",3,"Recommended,ErrorProne","test{{PATHSEP}}stubs.ts",90,1,95,10,"SomeViolationMessage4",
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
})();

// ==== START OF VIOLATIONS ====
const data = {"runDir":"{{ESCAPEDRUNDIR}}","violationCounts":{"total":6,"sev1":0,"sev2":1,"sev3":3,"sev4":2,"sev5":0},"versions":{"stubEngine1":"0.0.1","stubEngine2":"0.1.0","stubEngine3":"1.0.0"},"violations":[{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}config.test.ts","startLine":3,"startColumn":6,"endLine":11,"endColumn":8}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts","startLine":10,"startColumn":4,"endLine":11,"endColumn":2}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleC","engine":"stubEngine1","severity":3,"tags":["Recommended","Performance","Custom"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}run.test.ts","startLine":21,"startColumn":7,"endLine":25,"endColumn":4}],"message":"SomeViolationMessage2","resources":["https://example.com/stub1RuleC","https://example.com/aViolationSpecificUrl1","https://example.com/violationSpecificUrl2"]},{"rule":"stub1RuleE","engine":"stubEngine1","severity":3,"tags":["Performance"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}run.test.ts","startLine":56,"startColumn":4}],"message":"Some Violation that contains\na new line in `it` and &quot;various&quot; &#39;quotes&#39;. Also it has &lt;brackets&gt; that may need to be {escaped}.","resources":["https://example.com/stub1RuleE","https://example.com/stub1RuleE_2"]},{"rule":"stub2RuleC","engine":"stubEngine2","severity":2,"tags":["Recommended","BestPractice"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":4,"startColumn":13},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":9,"startColumn":1},{"file":"test{{PATHSEP}}stubs.ts","startLine":76,"startColumn":8}],"message":"SomeViolationMessage3","resources":[]},{"rule":"stub3RuleA","engine":"stubEngine3","severity":3,"tags":["Recommended","ErrorProne"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":20,"startColumn":10,"endLine":22,"endColumn":25,"comment":"Comment at location 1"},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":5,"startColumn":10,"comment":"Comment at location 2"},{"file":"test{{PATHSEP}}stubs.ts","startLine":90,"startColumn":1,"endLine":95,"endColumn":10}],"message":"SomeViolationMessage4","resources":[]}]};
const data = {"runDir":"{{ESCAPEDRUNDIR}}","violationCounts":{"total":6,"sev1":0,"sev2":1,"sev3":3,"sev4":2,"sev5":0},"versions":{"stubEngine1":"0.0.1","stubEngine2":"0.1.0","stubEngine3":"1.0.0"},"violations":[{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}config.test.ts","startLine":3,"startColumn":6,"endLine":11,"endColumn":8}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts","startLine":10,"startColumn":4,"endLine":11,"endColumn":2}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleC","engine":"stubEngine1","severity":3,"tags":["Recommended","Performance","Custom"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}code-analyzer.test.ts","startLine":21,"startColumn":7,"endLine":25,"endColumn":4}],"message":"SomeViolationMessage2","resources":["https://example.com/stub1RuleC","https://example.com/aViolationSpecificUrl1","https://example.com/violationSpecificUrl2"]},{"rule":"stub1RuleE","engine":"stubEngine1","severity":3,"tags":["Performance"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}code-analyzer.test.ts","startLine":56,"startColumn":4}],"message":"Some Violation that contains\na new line in `it` and &quot;various&quot; &#39;quotes&#39;. Also it has &lt;brackets&gt; that may need to be {escaped}.","resources":["https://example.com/stub1RuleE","https://example.com/stub1RuleE_2"]},{"rule":"stub2RuleC","engine":"stubEngine2","severity":2,"tags":["Recommended","BestPractice"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":4,"startColumn":13},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":9,"startColumn":1},{"file":"test{{PATHSEP}}stubs.ts","startLine":76,"startColumn":8}],"message":"SomeViolationMessage3","resources":[]},{"rule":"stub3RuleA","engine":"stubEngine3","severity":3,"tags":["Recommended","ErrorProne"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":20,"startColumn":10,"endLine":22,"endColumn":25,"comment":"Comment at location 1"},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":5,"startColumn":10,"comment":"Comment at location 2"},{"file":"test{{PATHSEP}}stubs.ts","startLine":90,"startColumn":1,"endLine":95,"endColumn":10}],"message":"SomeViolationMessage4","resources":[]}]};
// ==== END OF VIOLATIONS ====

class Model {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"primaryLocationIndex": 0,
"locations": [
{
"file": "test{{PATHSEP}}run.test.ts",
"file": "test{{PATHSEP}}code-analyzer.test.ts",
"startLine": 21,
"startColumn": 7,
"endLine": 25,
Expand All @@ -96,7 +96,7 @@
"primaryLocationIndex": 0,
"locations": [
{
"file": "test{{PATHSEP}}run.test.ts",
"file": "test{{PATHSEP}}code-analyzer.test.ts",
"startLine": 56,
"startColumn": 4
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uri": "test{{ENCODEDPATHSEP}}code-analyzer.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
Expand All @@ -153,7 +153,7 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uri": "test{{ENCODEDPATHSEP}}code-analyzer.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
Expand All @@ -177,7 +177,7 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uri": "test{{ENCODEDPATHSEP}}code-analyzer.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
Expand All @@ -191,7 +191,7 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uri": "test{{ENCODEDPATHSEP}}code-analyzer.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
<primaryLocationIndex>0</primaryLocationIndex>
<locations>
<location>
<file>test{{PATHSEP}}run.test.ts</file>
<file>test{{PATHSEP}}code-analyzer.test.ts</file>
<startLine>21</startLine>
<startColumn>7</startColumn>
<endLine>25</endLine>
Expand All @@ -97,7 +97,7 @@
<primaryLocationIndex>0</primaryLocationIndex>
<locations>
<location>
<file>test{{PATHSEP}}run.test.ts</file>
<file>test{{PATHSEP}}code-analyzer.test.ts</file>
<startLine>56</startLine>
<startColumn>4</startColumn>
</location>
Expand Down
Loading
Loading